Re: [Qemu-devel] [PATCH V15 1/6] add def_value_str in QemuOptDesc struct and rewrite qemu_opts_print

[Dongxu Wang <wdongxu@linux.vnet.ibm.com>]

On 2013/5/31 5:22, Eric Blake wrote:
> On 05/30/2013 03:55 AM, Dongxu Wang wrote:
>> From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
>>
>> qemu_opts_print has no user now, so can re-write the function safely.
>>
>> qemu_opts_print will be used while using "qemu-img create", it will
>> produce the same output as previous code.
>>
>> The behavior of this function has changed:
>>
>> 1. Print every possible option, whether a value has been set or not.
>> 2. Option descriptors may provide a default value.
>> 3. Print to stdout instead of stderr.
>>
>> Previously the behavior was to print every option that has been set.
>> Options that have not been set would be skipped.
>
> Up to here is fine.
>
>>
>> v13->v14:
>> 1) fix memory leak.
>> 2) make opt_set do not accpet null value argument.
>>
>> v12->v13
>> 1) re-write commit message.
>>
>> v11->v12
>> 1) make def_value_str become the real default value string in opt_set
>> function.
>>
>> v10->v11:
>> 1) print all values that have actually been assigned while accept-any
>> cases.
>>
>> v7->v8:
>> 1) print "elements => accept any params" while opts_accepts_any() ==
>> true.
>> 2) since def_print_str is the default value if an option isn't set,
>> so rename it to def_value_str.
>
> However, this chunk should be moved to appear after the '---' separator;
> it's useful during review, but does not need to be part of qemu.git.
>
Okay.
>>
>> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
>> Signed-off-by: Dongxu Wang <wdongxu@linux.vnet.ibm.com>
>
> And this looks fishy, having two S-o-B with slightly different spellings
> of your name.  Pick one and stick with it.
>
Ah.. sorry, my mistake..
>> ---
>
> Here is where patch changelogs go.  For more hints, see
> http://wiki.qemu.org/Contribute/SubmitAPatch
>
>>   include/qemu/option.h |  3 ++-
>>   util/qemu-option.c    | 32 ++++++++++++++++++++++++++------
>>   2 files changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/qemu/option.h b/include/qemu/option.h
>> index bdb6d21..b928ab0 100644
>> --- a/include/qemu/option.h
>> +++ b/include/qemu/option.h
>> @@ -96,6 +96,7 @@ typedef struct QemuOptDesc {
>>       const char *name;
>>       enum QemuOptType type;
>>       const char *help;
>> +    const char *def_value_str;
>>   } QemuOptDesc;
>
> Now that we have the 'query-command-line-options' QMP command, I think
> it's worth expanding that command to expose the default value of an
> option, when one is given.  It's probably content for a separate patch
> (util/qemu-config.c:query_option_descs() and qapi-schema.json), but
> still belongs as part of this series.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>