Re: [Qemu-devel] [PATCH] monitor: introduce query-config-schema command

[Osier Yang <jyang@redhat.com>]

On 25/04/13 11:52, Amos Kong wrote:
> On Wed, Apr 24, 2013 at 01:39:08PM -0600, Eric Blake wrote:
>> On 04/24/2013 12:20 PM, Luiz Capitulino wrote:
>>> On Thu, 25 Apr 2013 01:33:24 +0800
>>> Amos Kong <akong@redhat.com> wrote:
>>>
>>>> Libvirt has no way to probe if an option or property is supported,
>>>> This patch introdues a new qmp command to query configuration schema
>>>> information. hmp command isn't added because it's not needed.
>>>>
>>>> V2: fix jaso schema and comments (Eric)
> ...
>
>>>> +#
>>>> +# @flag: If no value is given, the flag is set to 1. Otherwise the value must
>>>> +#        be "on" (set to 1) or "off" (set to 0)
>>> Let's call this 'boolean', because it's what it is. Also, I suggest
>>> 'Accepts "on" or "off"' as the help text.
>> I'm fine with calling the enum value 'boolean' even where the C code
>> called it 'flag'.  As long as we have a documented name that describes
>> the semantics of what the parameter will take, libvirt should be able to
>> cope.
>>
>> One other concern - you document that if a flag parameter is omitted,
>> then it defaults to 1.  Is that really true?
>
> I'm wrong. If it's omitted in cmdline, we will give it a default value.
>
> example:
>     enable_mlock = qemu_opt_get_bool(opts, "mlock", true);
>
> another example:
>     -boot strict=on
>
>     bool boot_strict; (false by default)
>
>     strict boot is disabled by default, type of strict parameter is 'QEMU_OPT_STRING'
>     the logical default parameter is "off".

I didn't look through all the threads, not sure if it's already 
clarified, but
is the "flags" indicates whether the option is enabled or disabled by 
default?

If so, is it possible to have another "name" instead of "flag"? Or please
add documentation to tell more about the meaning.

>
> This kind of default info is only added in help descriptions right
> now, we can add a new item 'default_value' to option.h:QemuOptDesc &
> qapi-schema.json:CommandLineParameterInfo in future?

Sounds good to add it in the struct instead and dump as a JSON parameter.

>
> I guess the default value is useful for libvirt.

Yeah, sure, it's nice if libvirt could known if the device is enabled or 
disabled
by default.

Osier