From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43526) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SuAjT-0001A1-EU for qemu-devel@nongnu.org; Wed, 25 Jul 2012 19:11:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SuAjR-0007F6-NW for qemu-devel@nongnu.org; Wed, 25 Jul 2012 19:11:07 -0400 Received: from e37.co.us.ibm.com ([32.97.110.158]:46847) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SuAjR-0007Ew-FT for qemu-devel@nongnu.org; Wed, 25 Jul 2012 19:11:05 -0400 Received: from /spool/local by e37.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 25 Jul 2012 17:11:04 -0600 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 9B8E53E40040 for ; Wed, 25 Jul 2012 23:10:55 +0000 (WET) Received: from d03av05.boulder.ibm.com (d03av05.boulder.ibm.com [9.17.195.85]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q6PNAtLe220088 for ; Wed, 25 Jul 2012 17:10:55 -0600 Received: from d03av05.boulder.ibm.com (loopback [127.0.0.1]) by d03av05.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q6PNAtqW014993 for ; Wed, 25 Jul 2012 17:10:55 -0600 From: Anthony Liguori In-Reply-To: <50106F47.8020600@redhat.com> References: <1343245677-20904-1-git-send-email-aliguori@us.ibm.com> <50106F47.8020600@redhat.com> Date: Wed, 25 Jul 2012 18:10:51 -0500 Message-ID: <87hasvsah0.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH] vl: add -libvirt-caps option for libvirt to stop parsing help output List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: "libvir-list@redhat.com" , Peter Maydell , qemu-devel@nongnu.org, Markus Armbruster Eric Blake 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 >> + * >> + * 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 >> + * >> + * 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