Patchwork vl: add -libvirt-caps option for libvirt to stop parsing help output

login
register
mail settings
Submitter Anthony Liguori
Date July 25, 2012, 7:47 p.m.
Message ID <1343245677-20904-1-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/173254/
State New
Headers show

Comments

Anthony Liguori - July 25, 2012, 7:47 p.m.
The help output is going to change dramatically for 0.13.  We've spent too long
waiting for a perfect solution to capabilities handling and the end loser is
the direct consumer of QEMU because the help output is awful.

I will not apply any patches that change help output until 0.13 development
opens up.  This should give libvirt adequate time to implement support for
dealing with this new option.

This capabilities set comes directly from libvirt's source code so it's entirely
adequate for libvirt's purposes.  We can still explore more sophisticated
approaches that are more general purpose but the help output will be changing.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.objs   |    2 +
 libvirt-caps.c  |  166 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 libvirt-caps.h  |   19 ++++++
 qemu-options.hx |    4 +
 vl.c            |    5 ++
 5 files changed, 196 insertions(+), 0 deletions(-)
 create mode 100644 libvirt-caps.c
 create mode 100644 libvirt-caps.h
Eric Blake - July 25, 2012, 10:12 p.m.
[adding libvirt]

On 07/25/2012 01:47 PM, Anthony Liguori wrote:
> The help output is going to change dramatically for 0.13.  We've spent too long

0.13?  How long have you been sitting on this commit? :)

> waiting for a perfect solution to capabilities handling and the end loser is
> the direct consumer of QEMU because the help output is awful.
> 
> I will not apply any patches that change help output until 0.13 development
> opens up.  This should give libvirt adequate time to implement support for
> dealing with this new option.
> 
> This capabilities set comes directly from libvirt's source code so it's entirely
> adequate for libvirt's purposes.  We can still explore more sophisticated
> approaches that are more general purpose but the help output will be changing.

We've gone a long way with things like newer libvirt requiring QMP, and
being able to use query-commands and even 'qemu -device ?' to figure out
which devices are present, which is already more reliable than parsing
-help output.  There may still be a few things to fix (for example, I
already pointed out that libvirt 0.9.13 is bogus for refusing to even
attempt 'qemu -device,?' unless it guesses from '-help' output that it
will work; it would be better to just attempt it in the first place and
deal with the fallout), but it should be easy to fix in time for libvirt
0.10.0, and certainly coordinate things so that new enough libvirt comes
out close to the time of qemu 1.2.

> +++ b/libvirt-caps.c
> @@ -0,0 +1,166 @@
> +/*
> + * Libvirt capabilities
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Anthony Liguori   <aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "libvirt-caps.h"
> +#include "qemu-common.h"
> +
> +/* Make sure this stays in sync with libvirt/src/qemu/qemu_capabilities.c */

This is the part I'm most worried about - it says that any time libvirt
decides to flag a new capability that it cares about within qemu, then
both libvirt and qemu have to be upgraded in lockstep to coordinate the
use of that capability.  The problem is that sometimes libvirt doesn't
care about a capability until after qemu has already been released (for
example, we didn't realize the power of fd: migration until several
releases after qemu had first implemented it, at which point when we
added the capability bit, we were able to retroactively detect it for
several qemu versions by parsing -help).

> +
> +static const char *supported_caps[] = {
> +//    "kqemu",
> +    "vnc-colon",
> +    "no-reboot",
> +    "drive",
> +    "drive-boot",
> +
...
> +    "drive-iotune",
> +    "system_wakeup",
> +    "scsi-disk.channel",
> +    "scsi-block",
> +    "transaction",
> +
> +    NULL

Already incomplete.  For example, libvirt has recently added
block-job-sync, block-job-async, scsi-cd, and several others.
Thankfully, these days most of the new feature bits being added in
libvirt's qemu_capabilities.h are related to things that were probed
from 'qemu -device ?' or from QMP 'query-commands', and not from
something additionally scraped from '-help' for the first time.  I had
to go back to libvirt commit 63b4243 (May 15) to find a capability that
libvirt learned by scraping -help output (namely, support for
-no-user-config).  But yet this is one of the important config bits that
libvirt really needs to know, and not one that can easily be learned
from machine-parseable '-device ?'

I'm worried that an interface like this will let libvirt know about the
existing features that libvirt is trying to use, but will hurt the
ability of adding new feature detection in to libvirt (basically, a
newer libvirt wouldn't be able to retroactively take advantage of an
older feature already present in older qemu without parsing some
back-channel, at which point what good was having this libvirt-specific
channel?).

There's also another problem if we decide to keep this file synced
across projects:

> +++ b/libvirt-caps.h
> @@ -0,0 +1,19 @@
> +/*
> + * Libvirt capabilities
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Anthony Liguori   <aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.

Libvirt is LGPLv2+.  You can obviously upgrade to GPLv2+ as part of
importing the code from libvirt to qemu; but if you decide that a new
feature is worth adding a bit to this file, then libvirt can't do the
reverse sync unless you relax the license of this file.
Anthony Liguori - July 25, 2012, 11:10 p.m.
Eric Blake <eblake@redhat.com> writes:

> [adding libvirt]
>
> On 07/25/2012 01:47 PM, Anthony Liguori wrote:
>> The help output is going to change dramatically for 0.13.  We've spent too long
>
> 0.13?  How long have you been sitting on this commit? :)

Yeah, I meant 1.3 :-)  It's been a long day...

>> waiting for a perfect solution to capabilities handling and the end loser is
>> the direct consumer of QEMU because the help output is awful.
>> 
>> I will not apply any patches that change help output until 0.13 development
>> opens up.  This should give libvirt adequate time to implement support for
>> dealing with this new option.
>> 
>> This capabilities set comes directly from libvirt's source code so it's entirely
>> adequate for libvirt's purposes.  We can still explore more sophisticated
>> approaches that are more general purpose but the help output will be changing.
>
> We've gone a long way with things like newer libvirt requiring QMP, and
> being able to use query-commands and even 'qemu -device ?' to figure out
> which devices are present, which is already more reliable than parsing
> -help output.  There may still be a few things to fix (for example, I
> already pointed out that libvirt 0.9.13 is bogus for refusing to even
> attempt 'qemu -device,?' unless it guesses from '-help' output that it
> will work; it would be better to just attempt it in the first place and
> deal with the fallout), but it should be easy to fix in time for libvirt
> 0.10.0, and certainly coordinate things so that new enough libvirt comes
> out close to the time of qemu 1.2.
>
>> +++ b/libvirt-caps.c
>> @@ -0,0 +1,166 @@
>> +/*
>> + * Libvirt capabilities
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + *  Anthony Liguori   <aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "libvirt-caps.h"
>> +#include "qemu-common.h"
>> +
>> +/* Make sure this stays in sync with libvirt/src/qemu/qemu_capabilities.c */
>
> This is the part I'm most worried about - it says that any time libvirt
> decides to flag a new capability that it cares about within qemu, then
> both libvirt and qemu have to be upgraded in lockstep to coordinate the
> use of that capability.

This is QEMU's problem--not libvirt.  We shouldn't add new flags or
command line options without adding a capability proactively.  It
shouldn't be necessary for ya'll to always keep up.

> The problem is that sometimes libvirt doesn't
> care about a capability until after qemu has already been released (for
> example, we didn't realize the power of fd: migration until several
> releases after qemu had first implemented it, at which point when we
> added the capability bit, we were able to retroactively detect it for
> several qemu versions by parsing -help).

Again, we'd add capabilities whenever new stuff got added.

Yes, it would be better if this was all automatic but since I can't even
touch option parsing today I can't begin to untangle the mess.

So I'll admit this is not a perfect solution, but it's good enough that
we can start making real progress.

>> +
>> +static const char *supported_caps[] = {
>> +//    "kqemu",
>> +    "vnc-colon",
>> +    "no-reboot",
>> +    "drive",
>> +    "drive-boot",
>> +
> ...
>> +    "drive-iotune",
>> +    "system_wakeup",
>> +    "scsi-disk.channel",
>> +    "scsi-block",
>> +    "transaction",
>> +
>> +    NULL
>
> Already incomplete.  For example, libvirt has recently added
> block-job-sync, block-job-async, scsi-cd, and several others.

Yeah, my libvirt tree is old.  I'll resync.

> Thankfully, these days most of the new feature bits being added in
> libvirt's qemu_capabilities.h are related to things that were probed
> from 'qemu -device ?' or from QMP 'query-commands', and not from
> something additionally scraped from '-help' for the first time.  I had
> to go back to libvirt commit 63b4243 (May 15) to find a capability that
> libvirt learned by scraping -help output (namely, support for
> -no-user-config).  But yet this is one of the important config bits that
> libvirt really needs to know, and not one that can easily be learned
> from machine-parseable '-device ?'

I'd be happy to spend a little more time and trim the list specifically
to command line options.

> I'm worried that an interface like this will let libvirt know about the
> existing features that libvirt is trying to use, but will hurt the
> ability of adding new feature detection in to libvirt (basically, a
> newer libvirt wouldn't be able to retroactively take advantage of an
> older feature already present in older qemu without parsing some
> back-channel, at which point what good was having this libvirt-specific
> channel?).

See above.

> There's also another problem if we decide to keep this file synced
> across projects:
>
>> +++ b/libvirt-caps.h
>> @@ -0,0 +1,19 @@
>> +/*
>> + * Libvirt capabilities
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + *  Anthony Liguori   <aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>
> Libvirt is LGPLv2+.  You can obviously upgrade to GPLv2+ as part of
> importing the code from libvirt to qemu; but if you decide that a new
> feature is worth adding a bit to this file, then libvirt can't do the
> reverse sync unless you relax the license of this file.

My original thinking was that a list of strings (that's really a set of
enums) is not copyrightable which is why I didn't bother carrying over a
copyright notice.

I'm equally happy to pull in the libvirt copyright notice and switch my
license to LGPLv2+.

Regards,

Anthony Liguori

>
> -- 
> Eric Blake   eblake@redhat.com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
Daniel P. Berrange - July 26, 2012, 12:47 p.m.
On Wed, Jul 25, 2012 at 02:47:57PM -0500, Anthony Liguori wrote:
> The help output is going to change dramatically for 0.13.  We've spent too long
> waiting for a perfect solution to capabilities handling and the end loser is
> the direct consumer of QEMU because the help output is awful.
> 
> I will not apply any patches that change help output until 0.13 development
> opens up.  This should give libvirt adequate time to implement support for
> dealing with this new option.

I completely agree with this. We have spent far too long making do with
"help output" and its about time we finish with this once & for all.
I'm assuming you mean the 1.3 release here. If so I'll agree that it
is an acceptable plan to change help output at the start of the 1.3
release.

This gives us enough time to agree on what todo to support apps needing
to query QEMU.

> This capabilities set comes directly from libvirt's source code so it's entirely
> adequate for libvirt's purposes.  We can still explore more sophisticated
> approaches that are more general purpose but the help output will be changing.

While I appreciate what you're trying todo here, I think this would be a
serious mistake on many counts, and even be incorrect in some ways.

 - It ignores the needs of other apps using QEMU. In particular Richard
   Jones has frequently requested a way to detect QEMU capabilities
   to satisfy libguestfs. I think it is unreasonable to expect libguestfs
   to use the libvirt capabilities described here. Likewise other apps

 - This is just a point in time snapshot of what libvirt currently uses
   from QEMU. If, for example, someone provided a patch to libvirt to
   support the bluetooth devices we'd be stuck, because although QEMU
   already supports bluetooth, this is not expressed in any of the
   caps libvirt already has.

 - Not all of this information actually comes from the help output.
   A bunch of it is stuff we detect from '-device ?' and
   '-device name,?'. Although, (AFAIR) no one has objections to use
   parsing the -device output because it is much better defined &
   stable than -help, I think we could use some improvement to make
   the parsing 100% long term maintainable by QEMU/apps. Similarly
   we do  '-cpu ?' and '-machine ?'.  Some of the caps are set based
   on the machine architecture, or QEMU version.

 - Some of the caps are set based on what libvirt is actually
   able to handle from a command line generation POV, so having
   QEMU report these unconditionally is misleading/wrong. It is
   not about what QEMU supports, it is about what libvirt is able
   to cope with.

 - In the future some of the caps may be describing supported
   monitor commands, detected via 'query-commands' QMP cmd.

 - Users of libvirt are asking us to expose information about
   what QEMU supports, in particular wrt to devices, but also
   other aspects of configuration.



A long time back I proposed a '-capabilities' command line argument
for querying QEMU.

  https://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00921.html

There was a long discussion about this & many aspects of it were
disliked. In retrospect I agree with many of the comments, and
am glad we didn't adopt this. I think, however, there is a merit
in trying something vaguely related again, but with some key
differences. Basically I'd sum up my new idea as "just use QMP".


 * No new command line arguments like -capabilities

 * libvirt invokes something like

     $QEMUBINARY -qmp CHARDEV -nodefault -nodefconfig -nographics

 * libvirt then runs a number of  QMP commands to find out
   what it needs to know. I'd expect the following existing
   commands would be used

     - query-version             - already supported
     - query-commands            - already supported
     - query-events              - already supported
     - query-kvm                 - already supported
     - qom-{list,list-types,get} - already supported
     - query-spice/vnc           - already supported

   And add the following new commands

     - query-devices             - new, -device ?, and/or -device NAME,? data in QMP
     - query-machines            - new, -M ? in QMP
     - query-cpu-types           - new, -cpu ? in QMP

The above would take care of probably 50% of the current libvirt
capabilities probing, including a portion of the -help stuff. Then
there is all the rest of the crap we detect from the -help. We could
just take the view, that "as of 1.2", we assume everything we previously
detected is just available by default, and thus don't need to probe
it.  For stuff that is QOM based, I expect we'll be able to detect new
features in the future using the qom-XXX monitor commands. For stuff
that is non-qdev, and non-qom, libvirt can just do a plain version
number check, unless we decide there is specific info worth exposing
via other new 'query-XXX' monitor commands.

So in summary, as of 1.2 QEMU, libvirt would

  - Remove all use of -help, and other -XXXX command line args
    for detecting capabilities

  - Use -qmp and issue commands above to detect whatever it
    can

  - Any other existing capabilities are just "enable by default"
    for QEMU >= 1.2

  - Detect future stuff via existing monitor commands, otherwise
    just do it by QEMU version number.


So in terms of QEMU work, all I'm asking is to be allowed to
implement the query-devices, query-machines & query-cpu-types
QMP monitor commands. I'll happily do this work myself, if it
brings an end to the -help madness.

Regards,
Daniel
Daniel P. Berrange - July 26, 2012, 1:07 p.m.
On Thu, Jul 26, 2012 at 01:47:23PM +0100, Daniel P. Berrange wrote:
> On Wed, Jul 25, 2012 at 02:47:57PM -0500, Anthony Liguori wrote:
> > The help output is going to change dramatically for 0.13.  We've spent too long
> > waiting for a perfect solution to capabilities handling and the end loser is
> > the direct consumer of QEMU because the help output is awful.
> > 
> > I will not apply any patches that change help output until 0.13 development
> > opens up.  This should give libvirt adequate time to implement support for
> > dealing with this new option.
> 
> I completely agree with this. We have spent far too long making do with
> "help output" and its about time we finish with this once & for all.
> I'm assuming you mean the 1.3 release here. If so I'll agree that it
> is an acceptable plan to change help output at the start of the 1.3
> release.
> 
> This gives us enough time to agree on what todo to support apps needing
> to query QEMU.
> 
> > This capabilities set comes directly from libvirt's source code so it's entirely
> > adequate for libvirt's purposes.  We can still explore more sophisticated
> > approaches that are more general purpose but the help output will be changing.
> 
> While I appreciate what you're trying todo here, I think this would be a
> serious mistake on many counts, and even be incorrect in some ways.
> 
>  - It ignores the needs of other apps using QEMU. In particular Richard
>    Jones has frequently requested a way to detect QEMU capabilities
>    to satisfy libguestfs. I think it is unreasonable to expect libguestfs
>    to use the libvirt capabilities described here. Likewise other apps
> 
>  - This is just a point in time snapshot of what libvirt currently uses
>    from QEMU. If, for example, someone provided a patch to libvirt to
>    support the bluetooth devices we'd be stuck, because although QEMU
>    already supports bluetooth, this is not expressed in any of the
>    caps libvirt already has.
> 
>  - Not all of this information actually comes from the help output.
>    A bunch of it is stuff we detect from '-device ?' and
>    '-device name,?'. Although, (AFAIR) no one has objections to use
>    parsing the -device output because it is much better defined &
>    stable than -help, I think we could use some improvement to make
>    the parsing 100% long term maintainable by QEMU/apps. Similarly
>    we do  '-cpu ?' and '-machine ?'.  Some of the caps are set based
>    on the machine architecture, or QEMU version.
> 
>  - Some of the caps are set based on what libvirt is actually
>    able to handle from a command line generation POV, so having
>    QEMU report these unconditionally is misleading/wrong. It is
>    not about what QEMU supports, it is about what libvirt is able
>    to cope with.
> 
>  - In the future some of the caps may be describing supported
>    monitor commands, detected via 'query-commands' QMP cmd.
> 
>  - Users of libvirt are asking us to expose information about
>    what QEMU supports, in particular wrt to devices, but also
>    other aspects of configuration.
> 
> 
> 
> A long time back I proposed a '-capabilities' command line argument
> for querying QEMU.
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00921.html
> 
> There was a long discussion about this & many aspects of it were
> disliked. In retrospect I agree with many of the comments, and
> am glad we didn't adopt this. I think, however, there is a merit
> in trying something vaguely related again, but with some key
> differences. Basically I'd sum up my new idea as "just use QMP".
> 
> 
>  * No new command line arguments like -capabilities
> 
>  * libvirt invokes something like
> 
>      $QEMUBINARY -qmp CHARDEV -nodefault -nodefconfig -nographics
> 
>  * libvirt then runs a number of  QMP commands to find out
>    what it needs to know. I'd expect the following existing
>    commands would be used
> 
>      - query-version             - already supported
>      - query-commands            - already supported
>      - query-events              - already supported
>      - query-kvm                 - already supported
>      - qom-{list,list-types,get} - already supported
>      - query-spice/vnc           - already supported
> 
>    And add the following new commands
> 
>      - query-devices             - new, -device ?, and/or -device NAME,? data in QMP
>      - query-machines            - new, -M ? in QMP
>      - query-cpu-types           - new, -cpu ? in QMP
> 
> The above would take care of probably 50% of the current libvirt
> capabilities probing, including a portion of the -help stuff. Then
> there is all the rest of the crap we detect from the -help. We could
> just take the view, that "as of 1.2", we assume everything we previously
> detected is just available by default, and thus don't need to probe
> it.  For stuff that is QOM based, I expect we'll be able to detect new
> features in the future using the qom-XXX monitor commands. For stuff
> that is non-qdev, and non-qom, libvirt can just do a plain version
> number check, unless we decide there is specific info worth exposing
> via other new 'query-XXX' monitor commands.
> 
> So in summary, as of 1.2 QEMU, libvirt would
> 
>   - Remove all use of -help, and other -XXXX command line args
>     for detecting capabilities
> 
>   - Use -qmp and issue commands above to detect whatever it
>     can
> 
>   - Any other existing capabilities are just "enable by default"
>     for QEMU >= 1.2
> 
>   - Detect future stuff via existing monitor commands, otherwise
>     just do it by QEMU version number.
> 
> 
> So in terms of QEMU work, all I'm asking is to be allowed to
> implement the query-devices, query-machines & query-cpu-types
> QMP monitor commands. I'll happily do this work myself, if it
> brings an end to the -help madness.

Oh, if it is possible to get this data via QOM commands already,
or it can be easily made to work with QOM commands, then obviously
I wouldn't ask for these new query-XXX commands.

Regards,
Daniel
Anthony Liguori - July 26, 2012, 2:10 p.m.
"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Thu, Jul 26, 2012 at 01:47:23PM +0100, Daniel P. Berrange wrote:
>> On Wed, Jul 25, 2012 at 02:47:57PM -0500, Anthony Liguori wrote:
>> > The help output is going to change dramatically for 0.13.  We've spent too long
>> > waiting for a perfect solution to capabilities handling and the end loser is
>> > the direct consumer of QEMU because the help output is awful.
>> > 
>> > I will not apply any patches that change help output until 0.13 development
>> > opens up.  This should give libvirt adequate time to implement support for
>> > dealing with this new option.
>> 
>> I completely agree with this. We have spent far too long making do with
>> "help output" and its about time we finish with this once & for all.
>> I'm assuming you mean the 1.3 release here. If so I'll agree that it
>> is an acceptable plan to change help output at the start of the 1.3
>> release.
>> 
>> This gives us enough time to agree on what todo to support apps needing
>> to query QEMU.
>> 
>> > This capabilities set comes directly from libvirt's source code so it's entirely
>> > adequate for libvirt's purposes.  We can still explore more sophisticated
>> > approaches that are more general purpose but the help output will be changing.
>> 
>> While I appreciate what you're trying todo here, I think this would be a
>> serious mistake on many counts, and even be incorrect in some ways.
>> 
>>  - It ignores the needs of other apps using QEMU. In particular Richard
>>    Jones has frequently requested a way to detect QEMU capabilities
>>    to satisfy libguestfs. I think it is unreasonable to expect libguestfs
>>    to use the libvirt capabilities described here. Likewise other apps
>> 
>>  - This is just a point in time snapshot of what libvirt currently uses
>>    from QEMU. If, for example, someone provided a patch to libvirt to
>>    support the bluetooth devices we'd be stuck, because although QEMU
>>    already supports bluetooth, this is not expressed in any of the
>>    caps libvirt already has.
>> 
>>  - Not all of this information actually comes from the help output.
>>    A bunch of it is stuff we detect from '-device ?' and
>>    '-device name,?'. Although, (AFAIR) no one has objections to use
>>    parsing the -device output because it is much better defined &
>>    stable than -help, I think we could use some improvement to make
>>    the parsing 100% long term maintainable by QEMU/apps. Similarly
>>    we do  '-cpu ?' and '-machine ?'.  Some of the caps are set based
>>    on the machine architecture, or QEMU version.
>> 
>>  - Some of the caps are set based on what libvirt is actually
>>    able to handle from a command line generation POV, so having
>>    QEMU report these unconditionally is misleading/wrong. It is
>>    not about what QEMU supports, it is about what libvirt is able
>>    to cope with.
>> 
>>  - In the future some of the caps may be describing supported
>>    monitor commands, detected via 'query-commands' QMP cmd.
>> 
>>  - Users of libvirt are asking us to expose information about
>>    what QEMU supports, in particular wrt to devices, but also
>>    other aspects of configuration.
>> 
>> 
>> 
>> A long time back I proposed a '-capabilities' command line argument
>> for querying QEMU.
>> 
>>   https://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00921.html
>> 
>> There was a long discussion about this & many aspects of it were
>> disliked. In retrospect I agree with many of the comments, and
>> am glad we didn't adopt this. I think, however, there is a merit
>> in trying something vaguely related again, but with some key
>> differences. Basically I'd sum up my new idea as "just use QMP".
>> 
>> 
>>  * No new command line arguments like -capabilities
>> 
>>  * libvirt invokes something like
>> 
>>      $QEMUBINARY -qmp CHARDEV -nodefault -nodefconfig -nographics
>> 
>>  * libvirt then runs a number of  QMP commands to find out
>>    what it needs to know. I'd expect the following existing
>>    commands would be used
>> 
>>      - query-version             - already supported
>>      - query-commands            - already supported
>>      - query-events              - already supported
>>      - query-kvm                 - already supported
>>      - qom-{list,list-types,get} - already supported
>>      - query-spice/vnc           - already supported
>> 
>>    And add the following new commands
>> 
>>      - query-devices             - new, -device ?, and/or -device NAME,? data in QMP
>>      - query-machines            - new, -M ? in QMP
>>      - query-cpu-types           - new, -cpu ? in QMP
>> 
>> The above would take care of probably 50% of the current libvirt
>> capabilities probing, including a portion of the -help stuff. Then
>> there is all the rest of the crap we detect from the -help. We could
>> just take the view, that "as of 1.2", we assume everything we previously
>> detected is just available by default, and thus don't need to probe
>> it.  For stuff that is QOM based, I expect we'll be able to detect new
>> features in the future using the qom-XXX monitor commands. For stuff
>> that is non-qdev, and non-qom, libvirt can just do a plain version
>> number check, unless we decide there is specific info worth exposing
>> via other new 'query-XXX' monitor commands.
>> 
>> So in summary, as of 1.2 QEMU, libvirt would
>> 
>>   - Remove all use of -help, and other -XXXX command line args
>>     for detecting capabilities
>> 
>>   - Use -qmp and issue commands above to detect whatever it
>>     can
>> 
>>   - Any other existing capabilities are just "enable by default"
>>     for QEMU >= 1.2
>> 
>>   - Detect future stuff via existing monitor commands, otherwise
>>     just do it by QEMU version number.
>> 
>> 
>> So in terms of QEMU work, all I'm asking is to be allowed to
>> implement the query-devices, query-machines & query-cpu-types
>> QMP monitor commands. I'll happily do this work myself, if it
>> brings an end to the -help madness.
>
> Oh, if it is possible to get this data via QOM commands already,
> or it can be easily made to work with QOM commands, then obviously
> I wouldn't ask for these new query-XXX commands.

Yup.

You could do:

qom-list-types implements=TYPE_DEVICE

And that will give you the various types.  We'll need to add a:

device-list-properties typename=FOO

I've got a patch locally for that that I'm testing right now.  Paolo and
I never came to an agreement on how to do this generically for Objects
but I'm happy with a device-specific interface for the short term.

Regards,

Anthony Liguori

>
> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
Markus Armbruster - July 27, 2012, 11:23 a.m.
"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Wed, Jul 25, 2012 at 02:47:57PM -0500, Anthony Liguori wrote:
>> The help output is going to change dramatically for 0.13.  We've
>> spent too long
>> waiting for a perfect solution to capabilities handling and the end loser is
>> the direct consumer of QEMU because the help output is awful.
>> 
>> I will not apply any patches that change help output until 0.13 development
>> opens up.  This should give libvirt adequate time to implement support for
>> dealing with this new option.
>
> I completely agree with this. We have spent far too long making do with
> "help output" and its about time we finish with this once & for all.
> I'm assuming you mean the 1.3 release here. If so I'll agree that it
> is an acceptable plan to change help output at the start of the 1.3
> release.
>
> This gives us enough time to agree on what todo to support apps needing
> to query QEMU.
>
>> This capabilities set comes directly from libvirt's source code so
>> it's entirely
>> adequate for libvirt's purposes.  We can still explore more sophisticated
>> approaches that are more general purpose but the help output will be changing.
>
> While I appreciate what you're trying todo here, I think this would be a
> serious mistake on many counts, and even be incorrect in some ways.
>
>  - It ignores the needs of other apps using QEMU. In particular Richard
>    Jones has frequently requested a way to detect QEMU capabilities
>    to satisfy libguestfs. I think it is unreasonable to expect libguestfs
>    to use the libvirt capabilities described here. Likewise other apps
>
>  - This is just a point in time snapshot of what libvirt currently uses
>    from QEMU. If, for example, someone provided a patch to libvirt to
>    support the bluetooth devices we'd be stuck, because although QEMU
>    already supports bluetooth, this is not expressed in any of the
>    caps libvirt already has.
>
>  - Not all of this information actually comes from the help output.
>    A bunch of it is stuff we detect from '-device ?' and
>    '-device name,?'. Although, (AFAIR) no one has objections to use
>    parsing the -device output because it is much better defined &
>    stable than -help, I think we could use some improvement to make
>    the parsing 100% long term maintainable by QEMU/apps. Similarly
>    we do  '-cpu ?' and '-machine ?'.  Some of the caps are set based
>    on the machine architecture, or QEMU version.
>
>  - Some of the caps are set based on what libvirt is actually
>    able to handle from a command line generation POV, so having
>    QEMU report these unconditionally is misleading/wrong. It is
>    not about what QEMU supports, it is about what libvirt is able
>    to cope with.
>
>  - In the future some of the caps may be describing supported
>    monitor commands, detected via 'query-commands' QMP cmd.
>
>  - Users of libvirt are asking us to expose information about
>    what QEMU supports, in particular wrt to devices, but also
>    other aspects of configuration.
>
>
>
> A long time back I proposed a '-capabilities' command line argument
> for querying QEMU.
>
>   https://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00921.html
>
> There was a long discussion about this & many aspects of it were
> disliked. In retrospect I agree with many of the comments, and
> am glad we didn't adopt this. I think, however, there is a merit
> in trying something vaguely related again, but with some key
> differences. Basically I'd sum up my new idea as "just use QMP".
>
>
>  * No new command line arguments like -capabilities
>
>  * libvirt invokes something like
>
>      $QEMUBINARY -qmp CHARDEV -nodefault -nodefconfig -nographics
>
>  * libvirt then runs a number of  QMP commands to find out
>    what it needs to know. I'd expect the following existing
>    commands would be used
>
>      - query-version             - already supported
>      - query-commands            - already supported
>      - query-events              - already supported
>      - query-kvm                 - already supported
>      - qom-{list,list-types,get} - already supported
>      - query-spice/vnc           - already supported
>
>    And add the following new commands
>
>      - query-devices             - new, -device ?, and/or -device NAME,? data in QMP
>      - query-machines            - new, -M ? in QMP
>      - query-cpu-types           - new, -cpu ? in QMP
>
> The above would take care of probably 50% of the current libvirt
> capabilities probing, including a portion of the -help stuff. Then
> there is all the rest of the crap we detect from the -help. We could
> just take the view, that "as of 1.2", we assume everything we previously
> detected is just available by default, and thus don't need to probe
> it.  For stuff that is QOM based, I expect we'll be able to detect new
> features in the future using the qom-XXX monitor commands. For stuff
> that is non-qdev, and non-qom, libvirt can just do a plain version
> number check, unless we decide there is specific info worth exposing
> via other new 'query-XXX' monitor commands.

Assuming version X (according to query-version) supports no less than
upstream version X sounds fair.

If a command line option has a corresponding QMP command, probing QMP
for the feature suffices.  For instance, query-devices gives you devices
for -device.

I'm afraid there could still be an occasional need for probing the
command line.  A simple, machine-readable command line self-description
could satisfy it.  Something like:

- query-cmdline-options: JSON representation of qemu_options[], with
  unnecessary detail elided.  Basically option name and whether it takes
  an argument.

For options with a QemuOpts argument, we may want to add argument
self-description.  Basically its QemuOptsList, with unnecessary detail
elided.  Non-QemuOpts arguments don't get that.  New structured option
arguments should use QemuOpts anyway.

Some users might prefer to get this via a command line option rather
than a QMP command.  They should ask for it.

> So in summary, as of 1.2 QEMU, libvirt would
>
>   - Remove all use of -help, and other -XXXX command line args
>     for detecting capabilities
>
>   - Use -qmp and issue commands above to detect whatever it
>     can
>
>   - Any other existing capabilities are just "enable by default"
>     for QEMU >= 1.2
>
>   - Detect future stuff via existing monitor commands, otherwise
>     just do it by QEMU version number.
>
>
> So in terms of QEMU work, all I'm asking is to be allowed to
> implement the query-devices, query-machines & query-cpu-types
> QMP monitor commands. I'll happily do this work myself, if it
> brings an end to the -help madness.

Sounds like a plan!
Daniel P. Berrange - July 27, 2012, 11:49 a.m.
On Fri, Jul 27, 2012 at 01:23:03PM +0200, Markus Armbruster wrote:
> 
> Assuming version X (according to query-version) supports no less than
> upstream version X sounds fair.
> 
> If a command line option has a corresponding QMP command, probing QMP
> for the feature suffices.  For instance, query-devices gives you devices
> for -device.
> 
> I'm afraid there could still be an occasional need for probing the
> command line.  A simple, machine-readable command line self-description
> could satisfy it.  Something like:
> 
> - query-cmdline-options: JSON representation of qemu_options[], with
>   unnecessary detail elided.  Basically option name and whether it takes
>   an argument.
> 
> For options with a QemuOpts argument, we may want to add argument
> self-description.  Basically its QemuOptsList, with unnecessary detail
> elided.  Non-QemuOpts arguments don't get that.  New structured option
> arguments should use QemuOpts anyway.
> 
> Some users might prefer to get this via a command line option rather
> than a QMP command.  They should ask for it.

In my original proposal from 2 years back, I actually exposed a number
of QMP query-XXX commands via a -capabilities XXXX command line args.
On revisiting it though, I think that since we'll want to ask for
info on many different aspects, it is easier just to use QMP directly
rather than string together various command line args upfront, or
invoke QEMU multiple times.

Daniel
Markus Armbruster - July 27, 2012, 12:19 p.m.
"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Fri, Jul 27, 2012 at 01:23:03PM +0200, Markus Armbruster wrote:
>> 
>> Assuming version X (according to query-version) supports no less than
>> upstream version X sounds fair.
>> 
>> If a command line option has a corresponding QMP command, probing QMP
>> for the feature suffices.  For instance, query-devices gives you devices
>> for -device.
>> 
>> I'm afraid there could still be an occasional need for probing the
>> command line.  A simple, machine-readable command line self-description
>> could satisfy it.  Something like:
>> 
>> - query-cmdline-options: JSON representation of qemu_options[], with
>>   unnecessary detail elided.  Basically option name and whether it takes
>>   an argument.
>> 
>> For options with a QemuOpts argument, we may want to add argument
>> self-description.  Basically its QemuOptsList, with unnecessary detail
>> elided.  Non-QemuOpts arguments don't get that.  New structured option
>> arguments should use QemuOpts anyway.
>> 
>> Some users might prefer to get this via a command line option rather
>> than a QMP command.  They should ask for it.
>
> In my original proposal from 2 years back, I actually exposed a number
> of QMP query-XXX commands via a -capabilities XXXX command line args.
> On revisiting it though, I think that since we'll want to ask for
> info on many different aspects, it is easier just to use QMP directly
> rather than string together various command line args upfront, or
> invoke QEMU multiple times.

I read that as "libvirt isn't asking for it".  That's fine.  Whoever
wants it needs to ask.
Paolo Bonzini - July 27, 2012, 3:04 p.m.
Il 26/07/2012 16:10, Anthony Liguori ha scritto:
> Yup.
> 
> You could do:
> 
> qom-list-types implements=TYPE_DEVICE
> 
> And that will give you the various types.  We'll need to add a:
> 
> device-list-properties typename=FOO
> 
> I've got a patch locally for that that I'm testing right now.  Paolo and
> I never came to an agreement on how to do this generically for Objects
> but I'm happy with a device-specific interface for the short term.

I don't like the idea of using object_new, but it obviously works at
least for now, so go ahead and call it qom-type-list-properties.

Paolo
Anthony Liguori - July 27, 2012, 3:27 p.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 26/07/2012 16:10, Anthony Liguori ha scritto:
>> Yup.
>> 
>> You could do:
>> 
>> qom-list-types implements=TYPE_DEVICE
>> 
>> And that will give you the various types.  We'll need to add a:
>> 
>> device-list-properties typename=FOO
>> 
>> I've got a patch locally for that that I'm testing right now.  Paolo and
>> I never came to an agreement on how to do this generically for Objects
>> but I'm happy with a device-specific interface for the short term.
>
> I don't like the idea of using object_new, but it obviously works at
> least for now, so go ahead and call it qom-type-list-properties.

My patch doesn't use object_new() to do it.  It uses static properties.
I'm going to try to push static properties to object.  If I can do that
before 1.2, I'll rename this function to qom-type-list-properties.

Regards,

Anthony Liguori

>
> Paolo
Daniel P. Berrange - July 27, 2012, 4:35 p.m.
On Fri, Jul 27, 2012 at 01:23:03PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
> > The above would take care of probably 50% of the current libvirt
> > capabilities probing, including a portion of the -help stuff. Then
> > there is all the rest of the crap we detect from the -help. We could
> > just take the view, that "as of 1.2", we assume everything we previously
> > detected is just available by default, and thus don't need to probe
> > it.  For stuff that is QOM based, I expect we'll be able to detect new
> > features in the future using the qom-XXX monitor commands. For stuff
> > that is non-qdev, and non-qom, libvirt can just do a plain version
> > number check, unless we decide there is specific info worth exposing
> > via other new 'query-XXX' monitor commands.
> 
> Assuming version X (according to query-version) supports no less than
> upstream version X sounds fair.
> 
> If a command line option has a corresponding QMP command, probing QMP
> for the feature suffices.  For instance, query-devices gives you devices
> for -device.
> 
> I'm afraid there could still be an occasional need for probing the
> command line.  A simple, machine-readable command line self-description
> could satisfy it.  Something like:
> 
> - query-cmdline-options: JSON representation of qemu_options[], with
>   unnecessary detail elided.  Basically option name and whether it takes
>   an argument.
> 
> For options with a QemuOpts argument, we may want to add argument
> self-description.  Basically its QemuOptsList, with unnecessary detail
> elided.  Non-QemuOpts arguments don't get that.  New structured option
> arguments should use QemuOpts anyway.

I think you are quite possibly correct, but for the sake of enabling
us to move forward without more arguments, my inclination is to ignore
this just now :-) Lets get the new commands Anthony posted merged, and
port libvirt to use this new approach. Once that's all done, we can
evaluate whether there is a any more information we ought to provide
which might require a query-cmdline-options, or something else more
targetted (eg query-chardev-options or something)


Daniel
Markus Armbruster - July 27, 2012, 5:17 p.m.
"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Fri, Jul 27, 2012 at 01:23:03PM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange" <berrange@redhat.com> writes:
>> > The above would take care of probably 50% of the current libvirt
>> > capabilities probing, including a portion of the -help stuff. Then
>> > there is all the rest of the crap we detect from the -help. We could
>> > just take the view, that "as of 1.2", we assume everything we previously
>> > detected is just available by default, and thus don't need to probe
>> > it.  For stuff that is QOM based, I expect we'll be able to detect new
>> > features in the future using the qom-XXX monitor commands. For stuff
>> > that is non-qdev, and non-qom, libvirt can just do a plain version
>> > number check, unless we decide there is specific info worth exposing
>> > via other new 'query-XXX' monitor commands.
>> 
>> Assuming version X (according to query-version) supports no less than
>> upstream version X sounds fair.
>> 
>> If a command line option has a corresponding QMP command, probing QMP
>> for the feature suffices.  For instance, query-devices gives you devices
>> for -device.
>> 
>> I'm afraid there could still be an occasional need for probing the
>> command line.  A simple, machine-readable command line self-description
>> could satisfy it.  Something like:
>> 
>> - query-cmdline-options: JSON representation of qemu_options[], with
>>   unnecessary detail elided.  Basically option name and whether it takes
>>   an argument.
>> 
>> For options with a QemuOpts argument, we may want to add argument
>> self-description.  Basically its QemuOptsList, with unnecessary detail
>> elided.  Non-QemuOpts arguments don't get that.  New structured option
>> arguments should use QemuOpts anyway.
>
> I think you are quite possibly correct, but for the sake of enabling
> us to move forward without more arguments, my inclination is to ignore
> this just now :-) Lets get the new commands Anthony posted merged, and
> port libvirt to use this new approach. Once that's all done, we can
> evaluate whether there is a any more information we ought to provide
> which might require a query-cmdline-options, or something else more
> targetted (eg query-chardev-options or something)

I certainly don't mean to slow down the move forward.  And requiring a
real use case before accepting a feature makes sense anyway.

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 5ebbcfa..0495a88 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -95,6 +95,8 @@  common-obj-y += qemu-timer.o qemu-timer-common.o
 
 common-obj-$(CONFIG_SLIRP) += slirp/
 
+common-obj-y += libvirt-caps.o
+
 ######################################################################
 # libuser
 
diff --git a/libvirt-caps.c b/libvirt-caps.c
new file mode 100644
index 0000000..6d2b74d
--- /dev/null
+++ b/libvirt-caps.c
@@ -0,0 +1,166 @@ 
+/*
+ * Libvirt capabilities
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "libvirt-caps.h"
+#include "qemu-common.h"
+
+/* Make sure this stays in sync with libvirt/src/qemu/qemu_capabilities.c */
+
+static const char *supported_caps[] = {
+//    "kqemu",
+    "vnc-colon",
+    "no-reboot",
+    "drive",
+    "drive-boot",
+
+    "name",
+    "uuid",
+    "domid",
+    "vnet-hdr",
+//    "migrate-kvm-stdio",
+
+    "migrate-qemu-tcp",
+    "migrate-qemu-exec",
+    "drive-cache-v2",
+    "kvm",
+    "drive-format",
+
+    "vga",
+    "0.10",
+    "pci-device",
+    "mem-path",
+    "drive-serial",
+
+#if defined(CONFIG_XEN)
+    "xen-domid",
+#endif
+    "migrate-qemu-unix",
+    "chardev",
+    "enable-kvm",
+    "monitor-json",
+
+    "balloon",
+    "device",
+    "sdl",
+    "smp-topology",
+    "netdev",
+
+    "rtc",
+    "vhost-net",
+    "rtc-td-hack",
+    "no-hpet",
+//    "no-kvm-pit",
+
+//    "tdf",
+//    "pci-configfd",
+    "nodefconfig",
+    "boot-menu",
+//    "enable-kqemu",
+
+    "fsdev",
+    "nesting",
+    "name-process",
+    "drive-readonly",
+    "smbios-type",
+
+    "vga-qxl",
+    "spice",
+    "vga-none",
+    "migrate-qemu-fd",
+    "boot-index",
+
+    "hda-duplex",
+    "drive-aio",
+    "pci-multibus",
+    "pci-bootindex",
+    "ccid-emulated",
+
+    "ccid-passthru",
+#if defined(CONFIG_SPICE)
+    "chardev-spicevmc",
+    "device-spicevmc",
+#endif
+    "virtio-tx-alg",
+#if defined(CONFIG_SPICE)
+    "device-qxl-vga",
+#endif
+
+    "pci-multifunction",
+    "virtio-blk-pci.ioeventfd",
+    "sga",
+    "virtio-blk-pci.event_idx",
+    "virtio-net-pci.event_idx",
+
+    "cache-directsync",
+    "piix3-usb-uhci",
+    "piix4-usb-uhci",
+    "usb-ehci",
+    "ich9-usb-ehci1",
+
+    "vt82c686b-usb-uhci",
+    "pci-ohci",
+    "usb-redir",
+    "usb-hub",
+    "no-shutdown",
+
+    "cache-unsafe",
+    "rombar",
+    "ich9-ahci",
+    "no-acpi",
+    "fsdev-readonly",
+
+    "virtio-blk-pci.scsi",
+    "blk-sg-io",
+    "drive-copy-on-read",
+    "cpu-host",
+    "fsdev-writeout",
+
+    "drive-iotune",
+    "system_wakeup",
+    "scsi-disk.channel",
+    "scsi-block",
+    "transaction",
+
+    NULL
+};
+
+void qemu_print_capabilities(void)
+{
+    int i;
+    bool new_line = true;
+    int width = 0;
+
+    printf("[\n");
+    for (i = 0; supported_caps[i]; i++) {
+    repeat:
+        if (new_line) {
+            new_line = false;
+            printf("    ");
+            width += 4;
+        } else {
+            printf(", ");
+            width += 2;
+        }
+
+        width += strlen(supported_caps[i]) + 2;
+        if (width >= 78) {
+            printf("\n");
+            new_line = true;
+            width = 0;
+            goto repeat;
+        }
+
+        printf("\"%s\"", supported_caps[i]);
+    }
+    printf("\n]\n");
+}
diff --git a/libvirt-caps.h b/libvirt-caps.h
new file mode 100644
index 0000000..ad89d5b
--- /dev/null
+++ b/libvirt-caps.h
@@ -0,0 +1,19 @@ 
+/*
+ * Libvirt capabilities
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef LIBVIRT_CAPS_H
+#define LIBVIRT_CAPS_H
+
+void qemu_print_capabilities(void);
+
+#endif
diff --git a/qemu-options.hx b/qemu-options.hx
index dc68e15..6f74afa 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2783,6 +2783,10 @@  DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
     "-qtest-log LOG  specify tracing options\n",
     QEMU_ARCH_ALL)
 
+DEF("libvirt-caps", 0, QEMU_OPTION_libvirt_caps,
+    "-libvirt-caps   dump out a JSON list of capabilities\n",
+    QEMU_ARCH_ALL)
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
diff --git a/vl.c b/vl.c
index 54e36ed..24c7596 100644
--- a/vl.c
+++ b/vl.c
@@ -161,6 +161,7 @@  int main(int argc, char **argv)
 #include "arch_init.h"
 
 #include "ui/qemu-spice.h"
+#include "libvirt-caps.h"
 
 //#define DEBUG_NET
 //#define DEBUG_SLIRP
@@ -3198,6 +3199,10 @@  int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_qtest_log:
                 qtest_log = optarg;
                 break;
+            case QEMU_OPTION_libvirt_caps:
+                qemu_print_capabilities();
+                exit(0);
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }