| Message ID | 1350502556-4885-3-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 17.10.2012 21:35, schrieb Luiz Capitulino: > If set returns a copy of the parameter list used by the block driver > to create the new image. > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > block.c | 7 ++++++- > block.h | 3 ++- > blockdev.c | 2 +- > qemu-img.c | 2 +- > 4 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/block.c b/block.c > index e95f613..254a5c2 100644 > --- a/block.c > +++ b/block.c > @@ -4294,7 +4294,8 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie) > > int bdrv_img_create(const char *filename, const char *fmt, > const char *base_filename, const char *base_fmt, > - char *options, uint64_t img_size, int flags) > + char *options, uint64_t img_size, int flags, > + QEMUOptionParameter **param_list) > { > QEMUOptionParameter *param = NULL, *create_options = NULL; > QEMUOptionParameter *backing_fmt, *backing_file, *size; > @@ -4430,6 +4431,10 @@ int bdrv_img_create(const char *filename, const char *fmt, > } > > out: > + if (param_list && ret == 0) { > + *param_list = append_option_parameters(NULL, param); > + } If you put this above the out: label, the ret == 0 check wouldn't be necessary. Kevin
On Thu, 18 Oct 2012 13:57:45 +0200 Kevin Wolf <kwolf@redhat.com> wrote: > Am 17.10.2012 21:35, schrieb Luiz Capitulino: > > If set returns a copy of the parameter list used by the block driver > > to create the new image. > > > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > > --- > > block.c | 7 ++++++- > > block.h | 3 ++- > > blockdev.c | 2 +- > > qemu-img.c | 2 +- > > 4 files changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/block.c b/block.c > > index e95f613..254a5c2 100644 > > --- a/block.c > > +++ b/block.c > > @@ -4294,7 +4294,8 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie) > > > > int bdrv_img_create(const char *filename, const char *fmt, > > const char *base_filename, const char *base_fmt, > > - char *options, uint64_t img_size, int flags) > > + char *options, uint64_t img_size, int flags, > > + QEMUOptionParameter **param_list) > > { > > QEMUOptionParameter *param = NULL, *create_options = NULL; > > QEMUOptionParameter *backing_fmt, *backing_file, *size; > > @@ -4430,6 +4431,10 @@ int bdrv_img_create(const char *filename, const char *fmt, > > } > > > > out: > > + if (param_list && ret == 0) { > > + *param_list = append_option_parameters(NULL, param); > > + } > > If you put this above the out: label, the ret == 0 check wouldn't be > necessary. I'll maintain it it there but will drop the ret check, so that we're able to return the options on error too and thus keep the "Formatting" message the way it's today.
On Thu, 18 Oct 2012 10:33:30 -0300 Luiz Capitulino <lcapitulino@redhat.com> wrote: > On Thu, 18 Oct 2012 13:57:45 +0200 > Kevin Wolf <kwolf@redhat.com> wrote: > > > Am 17.10.2012 21:35, schrieb Luiz Capitulino: > > > If set returns a copy of the parameter list used by the block driver > > > to create the new image. > > > > > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > > > --- > > > block.c | 7 ++++++- > > > block.h | 3 ++- > > > blockdev.c | 2 +- > > > qemu-img.c | 2 +- > > > 4 files changed, 10 insertions(+), 4 deletions(-) > > > > > > diff --git a/block.c b/block.c > > > index e95f613..254a5c2 100644 > > > --- a/block.c > > > +++ b/block.c > > > @@ -4294,7 +4294,8 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie) > > > > > > int bdrv_img_create(const char *filename, const char *fmt, > > > const char *base_filename, const char *base_fmt, > > > - char *options, uint64_t img_size, int flags) > > > + char *options, uint64_t img_size, int flags, > > > + QEMUOptionParameter **param_list) > > > { > > > QEMUOptionParameter *param = NULL, *create_options = NULL; > > > QEMUOptionParameter *backing_fmt, *backing_file, *size; > > > @@ -4430,6 +4431,10 @@ int bdrv_img_create(const char *filename, const char *fmt, > > > } > > > > > > out: > > > + if (param_list && ret == 0) { > > > + *param_list = append_option_parameters(NULL, param); > > > + } > > > > If you put this above the out: label, the ret == 0 check wouldn't be > > necessary. > > I'll maintain it it there but will drop the ret check, so that we're > able to return the options on error too and thus keep the "Formatting" > message the way it's today. Actually my idea didn't work. I can think of the following options: 1. Change it to "Formatted" as you suggested. Cons: will only be printed after the image is created 2. Print "Formatting 'foo', 'qcow2' " before the image is created and the options afterwards. Cons: weird? 3. Don't change this. Cons: will keep spitting stuff to stdout while in QMP I think we should do 2.
Am 18.10.2012 19:18, schrieb Luiz Capitulino: > On Thu, 18 Oct 2012 10:33:30 -0300 > Luiz Capitulino <lcapitulino@redhat.com> wrote: > >> On Thu, 18 Oct 2012 13:57:45 +0200 >> Kevin Wolf <kwolf@redhat.com> wrote: >> >>> Am 17.10.2012 21:35, schrieb Luiz Capitulino: >>>> If set returns a copy of the parameter list used by the block driver >>>> to create the new image. >>>> >>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> >>>> --- >>>> block.c | 7 ++++++- >>>> block.h | 3 ++- >>>> blockdev.c | 2 +- >>>> qemu-img.c | 2 +- >>>> 4 files changed, 10 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/block.c b/block.c >>>> index e95f613..254a5c2 100644 >>>> --- a/block.c >>>> +++ b/block.c >>>> @@ -4294,7 +4294,8 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie) >>>> >>>> int bdrv_img_create(const char *filename, const char *fmt, >>>> const char *base_filename, const char *base_fmt, >>>> - char *options, uint64_t img_size, int flags) >>>> + char *options, uint64_t img_size, int flags, >>>> + QEMUOptionParameter **param_list) >>>> { >>>> QEMUOptionParameter *param = NULL, *create_options = NULL; >>>> QEMUOptionParameter *backing_fmt, *backing_file, *size; >>>> @@ -4430,6 +4431,10 @@ int bdrv_img_create(const char *filename, const char *fmt, >>>> } >>>> >>>> out: >>>> + if (param_list && ret == 0) { >>>> + *param_list = append_option_parameters(NULL, param); >>>> + } >>> >>> If you put this above the out: label, the ret == 0 check wouldn't be >>> necessary. >> >> I'll maintain it it there but will drop the ret check, so that we're >> able to return the options on error too and thus keep the "Formatting" >> message the way it's today. > > Actually my idea didn't work. I can think of the following options: > > 1. Change it to "Formatted" as you suggested. Cons: will only be > printed after the image is created > > 2. Print "Formatting 'foo', 'qcow2' " before the image is created > and the options afterwards. Cons: weird? > > 3. Don't change this. Cons: will keep spitting stuff to stdout while in > QMP > > I think we should do 2. How about leaving the code printing the message where it is, but putting an if (!cur_mon) around it? Kevin
On Fri, 19 Oct 2012 11:13:39 +0200 Kevin Wolf <kwolf@redhat.com> wrote: > Am 18.10.2012 19:18, schrieb Luiz Capitulino: > > On Thu, 18 Oct 2012 10:33:30 -0300 > > Luiz Capitulino <lcapitulino@redhat.com> wrote: > > > >> On Thu, 18 Oct 2012 13:57:45 +0200 > >> Kevin Wolf <kwolf@redhat.com> wrote: > >> > >>> Am 17.10.2012 21:35, schrieb Luiz Capitulino: > >>>> If set returns a copy of the parameter list used by the block driver > >>>> to create the new image. > >>>> > >>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > >>>> --- > >>>> block.c | 7 ++++++- > >>>> block.h | 3 ++- > >>>> blockdev.c | 2 +- > >>>> qemu-img.c | 2 +- > >>>> 4 files changed, 10 insertions(+), 4 deletions(-) > >>>> > >>>> diff --git a/block.c b/block.c > >>>> index e95f613..254a5c2 100644 > >>>> --- a/block.c > >>>> +++ b/block.c > >>>> @@ -4294,7 +4294,8 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie) > >>>> > >>>> int bdrv_img_create(const char *filename, const char *fmt, > >>>> const char *base_filename, const char *base_fmt, > >>>> - char *options, uint64_t img_size, int flags) > >>>> + char *options, uint64_t img_size, int flags, > >>>> + QEMUOptionParameter **param_list) > >>>> { > >>>> QEMUOptionParameter *param = NULL, *create_options = NULL; > >>>> QEMUOptionParameter *backing_fmt, *backing_file, *size; > >>>> @@ -4430,6 +4431,10 @@ int bdrv_img_create(const char *filename, const char *fmt, > >>>> } > >>>> > >>>> out: > >>>> + if (param_list && ret == 0) { > >>>> + *param_list = append_option_parameters(NULL, param); > >>>> + } > >>> > >>> If you put this above the out: label, the ret == 0 check wouldn't be > >>> necessary. > >> > >> I'll maintain it it there but will drop the ret check, so that we're > >> able to return the options on error too and thus keep the "Formatting" > >> message the way it's today. > > > > Actually my idea didn't work. I can think of the following options: > > > > 1. Change it to "Formatted" as you suggested. Cons: will only be > > printed after the image is created > > > > 2. Print "Formatting 'foo', 'qcow2' " before the image is created > > and the options afterwards. Cons: weird? > > > > 3. Don't change this. Cons: will keep spitting stuff to stdout while in > > QMP > > > > I think we should do 2. > > How about leaving the code printing the message where it is, but putting > an if (!cur_mon) around it? So ugly that I prefer to drop this patch from my series and revisit this later :)
diff --git a/block.c b/block.c index e95f613..254a5c2 100644 --- a/block.c +++ b/block.c @@ -4294,7 +4294,8 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie) int bdrv_img_create(const char *filename, const char *fmt, const char *base_filename, const char *base_fmt, - char *options, uint64_t img_size, int flags) + char *options, uint64_t img_size, int flags, + QEMUOptionParameter **param_list) { QEMUOptionParameter *param = NULL, *create_options = NULL; QEMUOptionParameter *backing_fmt, *backing_file, *size; @@ -4430,6 +4431,10 @@ int bdrv_img_create(const char *filename, const char *fmt, } out: + if (param_list && ret == 0) { + *param_list = append_option_parameters(NULL, param); + } + free_option_parameters(create_options); free_option_parameters(param); diff --git a/block.h b/block.h index e2d89d7..55d13e6 100644 --- a/block.h +++ b/block.h @@ -341,7 +341,8 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf, int bdrv_img_create(const char *filename, const char *fmt, const char *base_filename, const char *base_fmt, - char *options, uint64_t img_size, int flags); + char *options, uint64_t img_size, int flags, + QEMUOptionParameter **param_list); void bdrv_set_buffer_alignment(BlockDriverState *bs, int align); void *qemu_blockalign(BlockDriverState *bs, size_t size); diff --git a/blockdev.c b/blockdev.c index 99828ad..9dddefd 100644 --- a/blockdev.c +++ b/blockdev.c @@ -783,7 +783,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp) ret = bdrv_img_create(new_image_file, format, states->old_bs->filename, states->old_bs->drv->format_name, - NULL, -1, flags); + NULL, -1, flags, NULL); if (ret) { error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file); goto delete_and_fail; diff --git a/qemu-img.c b/qemu-img.c index f17f187..b841012 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -362,7 +362,7 @@ static int img_create(int argc, char **argv) } ret = bdrv_img_create(filename, fmt, base_filename, base_fmt, - options, img_size, BDRV_O_FLAGS); + options, img_size, BDRV_O_FLAGS, NULL); out: if (ret) { return 1;
If set returns a copy of the parameter list used by the block driver to create the new image. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- block.c | 7 ++++++- block.h | 3 ++- blockdev.c | 2 +- qemu-img.c | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-)