Re: [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx

[Eric Blake]

On 03/11/2014 03:04 AM, Markus Armbruster wrote:

>>      '-cdrom filename' could easily be re-written [in a
>> future qemu version] to use QemuOpts with an implied parameter name
>> (we've done that elsewhere, such as for '-machine').

> 
> Incompatible change for funny filenames: -cdrom you,break=me.

Oh.  And probably tangentially related to the bug I just reported where
'-drive id=a,file=funny,,id=1' works, but swapping to '-drive
file=funny,,id=1,id=a' fails.

> 
> Besides breaking funny filenames, we'd also buy ourselves some stupid
> -readconfig / -writeconfig trouble.  Let me explain.
> 
> -cdrom F is effectively sugar for "-drive media=cdrom,index=2,file=FF",
> where FF is F with comma doubled.
> 
> -writeconfig writes out desugared QemuOpts.  Therefore, "-cdrom r7.iso"
> gets written as
> 
>     [drive]
>       media = "cdrom"
>       index = "2"
>       file = "r7.iso"
> 
> which -readconfig can read.

So it sounds like what we REALLY want is a way to tell, for every
command line option, what that command will convert to in config
notation.  'drive' is output as-is, because it is (currently) a
first-class option, while 'cdrom' is sugar.  Mentioning that '-cdrom' is
valid on the command line is good (especially since if we DO have
introspection on what sugar is available, we can remove the sugar in the
future and libvirt can still query whether it should use the sugar form
or the preferred form when controlling a new qemu); but also mentioning
that '-cdrom' is sugar and what the preferred form is would also be good
(libvirt can use the information to use preferred forms instead of sugar).

> 
> If we convert -cdrom to QemuOpts, it gets written out like this:
> 
>     [cdrom]
>        file = "r7.iso"
> 
> If we continue to desugar it, it'll *also* get written out as before.
> Either we *delete* the sugared QemuOpts to avoid duplication, or we
> *stop* desugaring.  The latter breaks -readconfig of existing
> configuration files, and complicates the code reading configuration from
> QemuOpts.
> 
> I don't think any of the old non-QemuOpts options that have become sugar
> for newer, more flexible QemuOpts options should be converted to
> QemuOpts.

Fair enough.  So that means that for every option that exists as sugar
for some other option, the introspection should include enough details
as to give the end user a hint as to what the sugar will expand to.

> 
>> So your idea of a tri-state (QemuOpts, no argument, or other argument)
>> doesn't add anything - any option that takes "other argument" could be
>> converted to take QemuOpts, and from the command line, we can't tell the
>> difference from whether something was implemented by QemuOpts, only by
>> whether we have introspection on what the argument consists of.
> 
> I doubt we can convert all existing options to QemuOpts without breaking
> backward compatibility and complicating the code.

Point taken.

>> We already know 'query-command-line-options' is not a full introspection
>> yet.  So far, libvirt has managed to get by on partial information (in
>> fact, the whole hack for special-casing -drive to merge multiple lists
>> together was precisely to avoid a regression with at least providing the
>> partial information that libvirt was actually using).  Documenting that
>> QemuOpts information may be incomplete may be nice, but shouldn't hold
>> up the initial purpose of this patch which is to document non-QemuOpts
>> options.  And knowing that an option takes unspecified arguments is
>> still better than not knowing about the option at all.
> 
> If all we want is a quick fix for "I can't see whether -frobnicate is
> supported", then let's add a command to dump qemu_options[], and leave
> query-command-line-options broken as designed.
> 
> But if we want proper command line introspection, then let's do it
> properly: no quick hacks, no half-truths.

The current 'query-command-line-options' is misnamed, it is more like
'query-config-options'.  Maybe we want that functionality - but if so,
we should name it correctly.  Meanwhile, libvirt DOES want a way to
query whether -frobnicate is supported, but as we already lack it, not
having it in 2.0 won't make a difference, so I agree with delaying this
series until 2.1 when we can think more about getting it right rather
than fast.

>> {"option":"M", "parameters":[], "config-name":"machine",
>>  "argument": true},
>> {"option":"machine", "parameters":[
>>   {"name": "firmware", "help": "firmware image", "type": "string"},
>>   {"name": "type", "implied-name": true, "help": "emulated machine",
>> "type": "string"}, ...]},
>>
>> to make it a bit more obvious that '-M str' and '-machine str' are both
>> shorthands for the preferred '-machine type=str', and that the same
>> effect is reached via a config file that has a [machine] section.
> 
> Use case for the introspection into the desugaring of -M?
> 
> Can't cover less trivial desugarings, like -cdrom.

Use case for any desugaring is to learn what the preferred form is, and
to know that a preferred form exists (in case there are other existing
command-line arguments that we convert in the future by adding a new
preferred QemuOpts form).  But if we express desugaring at all, then we
need to fully express it - so we need a schema that shows exactly how
cdrom is converted into drive (including both the implied media=cdrom
and index=2 arguments, as well as the named file=FF conversion).

>>
>> Yes.  We've identified at least 3 exceptions now (acpitable, boot, smp),
>> and exposing those exceptions in the introspection is a good idea, to
>> make us quit adding new ones.
> 
> It'll make us quit adding new ones only if we can come up with a test
> that breaks when we add new ones :)

Too true.

>>> Do we care?
>>>
>>>> This is a bug fix patch, so let's shoot to get it into 2.0.
>>>
>>> Yes.

So far, detecting boolean options like -enable-fips has never worked, so
that is out of scope for 2.0.  Likewise, the bug between the command
line -acpitable and the config file [acpi] not having matching names has
been buggy since the query-command-line-options was introduced and
hasn't been tripping up libvirt yet, so again rushing a fix may not be
the best.

> 
> Is better command line introspection in 2.0 worth the risk that comes
> with softening up the hard freeze?

At this point, probably not.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature