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

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



[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.

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

Attachment: signature.asc
Description: OpenPGP digital signature


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