Re: [Qemu-devel] [PATCH 2/8] block: bdrv_img_create(): add param_list argument

[Kevin Wolf <kwolf@redhat.com>]

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