Re: [Qemu-devel] [PATCH] vl: add -libvirt-caps option for libvirt to stop parsing help output

[Anthony Liguori]

Eric Blake <address@hidden> 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   <address@hidden>
>> + *
>> + * 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   <address@hidden>
>> + *
>> + * 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   address@hidden    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org