[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [Qemu-devel] fix/re-do query-command-line-options



Amos Kong <akong redhat com> writes:

> Hi QEMU/Libvirt list,
>
> When I worked on query-command-line-options, I first used some marcos [1] to
> generate two config & option tables. This will cover all the options,
> but it returns a string, it's difficult for libvirt to parse and use
> it.
>
> Finally I got a suggestion to read info from new interface (QemuOpts),
> We add opts info to vm_config_groups[], query-command-line-options
> will visit the array. It doesn't conver all the options, but the
> latest options are covered, so this implementation is acceptable.
>
> Problem:
> * QemuOpts was designed just for options with parameter, some new option
>   without parameters is lost in query output (eg: -enable-fips)

Yes, and that's an issue not just for query-command-line-options, but
also for -writeconfig / -readconfig.

> * block drive uses three QemuOpts, it's legacy issue.

Code goes into contortions to extend the old option without breaking it.

Part of the contortions is your commit 968854c, which adds special-case
code to make the -drive's parameters visible in
query-command-line-options again, after commit 0006383 made them
invisible.

More on the invisible parameters problem below.

> * QemuOpts of some options aren't updated, it might be difficult to
>   updated when we add some new parameters

What do you have in mind here?

> * other

Here's one: QemuOpts *sucks* :)

Apart from its general suckage, which has been discussed at some length
elsewhere, it sucks as a schema, because it's not nearly expressive
enough.

With QemuOpts, we can basically enumerate acceptable keys and how their
values should be parsed, plus a few extras like an implied key.

On value parsing, all we have is "on"/"off", uint64_t in decimal,
uint64_t in fancy size syntax, and "anything".  This is both more and
less than a QAPI schema.

It's more, because it specifies both type (e.g. uint64_t) and external
syntax (e.g. decimal).

It's less, because it has fewer basic types, and no composite types.

On acceptable keys, all we can do is a list fixed at compile-time.

We can't express discriminated unions, such as when key "type" has value
"tap", then keys ... are acceptable, when it has value "bridge", then
keys ... are acceptable, and so forth.  We routinely do that in QAPI
schemata.

We can't collect acceptable keys at run-time.  For instance, -device
accepts general keys "id", "bus", and a discriminated union of device
model properties, with discriminator "driver".  But the available device
models and their properties are only known at runtime.

When we need something like that (and we need it often), we give up and
specify "any key=value accepted", leaving the actual checking to the
code using the option.  All command line introspection can see then is
"the option exists, and it takes a key=value,... argument".  Which
borders on useless.

Options that have this issue now: -acpitable, -smbios, -netdev, -net,
-device, -object.  Note that several of these are fairly new, which
makes me expect more of them.  -drive isn't on this list only because
your commit 968854c got it off the list after commit 0006383 got it on
it.

QAPI schema can't yet solve this problem, either.  That's why it doesn't
cover -device / device_add.  Since all QMP commands need to be covered,
and QMP surely needs a command to plug devices, the problem will have to
be solved there eventually.

> We discussed to reimplement this command, but it seems DEF maroc is the
> only point to cover all the options, all the options are described in
> qemu-options.hx
>
> I'm considering to reuse the DEF marocs to generate a table, try to
> return the crude info if QemuOpts doesn't cover it.
> Or maintain a split array (like vm_config_groups[]), it only contains
> the option without parameter (option name & help info).

I think the data you can usefully collect with this approach is
approximately the data getopt_long()[*] gets: list of named command line
options, and whether they take an argument.

You can use this data to fill in options not covered by QemuOpts.  This
is a definite improvement.

It still falls short of fully solving the command line introspection
problem.

However, I'm not into rejecting imperfect incremental improvements we
can have now in favor of perfect solutions we can maybe have some day.
Go right ahead with your incremental improvement!

> If you touched some problem of the query-command-line-options, welcome
> to reply it, I will try to satisfy your requests.
>
>
> Thanks, Amos
>
> [1] http://www.redhat.com/archives/libvir-list/2013-January/msg01656.html

[*] Which we don't use, because we prefer our command line
idiosyncratic.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]