From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=52791 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OdQwt-0001zX-95 for qemu-devel@nongnu.org; Mon, 26 Jul 2010 12:54:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OdQwr-0001VI-MF for qemu-devel@nongnu.org; Mon, 26 Jul 2010 12:54:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:25852) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OdQwr-0001UQ-EC for qemu-devel@nongnu.org; Mon, 26 Jul 2010 12:54:41 -0400 Message-ID: <4C4DBDCC.8090408@redhat.com> Date: Mon, 26 Jul 2010 19:54:36 +0300 From: Avi Kivity MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] move 'unsafe' to end of caching modes in help References: <4C4704FC020000480009AB6E@sinclair.provo.novell.com> <4C475EC0.2000805@codemonkey.ws> <20100721213238.GB28871@redhat.com> <4C476A8A.6000707@codemonkey.ws> <20100721215833.GC28871@redhat.com> <4C478534.2020106@codemonkey.ws> <20100722084225.GA1524@redhat.com> <4C485383.8020904@codemonkey.ws> <4C4DAF94.1040300@codemonkey.ws> <4C4DB74F.7090507@redhat.com> <4C4DBA71.1000808@codemonkey.ws> In-Reply-To: <4C4DBA71.1000808@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Bruce Rogers , Markus Armbruster , qemu-devel@nongnu.org On 07/26/2010 07:40 PM, Anthony Liguori wrote: > On 07/26/2010 11:26 AM, Avi Kivity wrote: >>> I'm a practical guy, and I don't see that it's a huge burden for >>> libvirt to detect downstreams and build a feature matrix based on >>> versions. If someone demonstrates that it's infeasible, I'll >>> happily reconsider. >> >> >> It generates a dependency. If the downstream backports feature A in >> version V, then a new version of libvirt needs to be issued which has >> (A, V) in its feature matrix. >> >> On the other hand, capability reporting, even if suckily implemented >> via -help, doesn't need new versions of libvirt. > > I agree with you 100% but -help is not a capability reporting system. Right. We don't have a capability reporting system, so libvirt made do with what they have. >> >> Older versions of libvirt aren't a problem, they simply don't know >> about cache=unsafe. > > Let's be clear what's happening here. QEMU produces: > > " [,cache=writethrough|writeback|unsafe|none][,format=f]\n" > > > Which is completely reasonable from a readability perspective. > Libvirt does: > > > qemu_conf.c: if (strstr(help, > "cache=writethrough|writeback|none")) > > > To detect whether QEMU supports cache in -drive. The proposed patch > makes QEMU produce: > > " [,cache=writethrough|writeback|none|unsafe][,format=f]\n" > > So that their strstr() call still works. > > If libvirt is going to parse -help output, they should do a better job > at it. I can't expect QEMU developers to have detailed knowledge of > how libvirt parses the help output to ensure that we don't break their > code. Correct. libvirt could have done much better parsing. qemu developers are not familiar with libvirt code. But is there a problem in accepting the patch that rearranges the output? As far as I can tell, it's just as good for a user, and better for libvirt, so there are no drawbacks to accepting the patch? >> >>> The help output is *not* a supported interface. >> >> There is no supported, usable interface for this. > > Version is entirely reliable for detecting whether -drive supports cache. It's not a reliable interface for detecting features in the face of backports. >>> There are very simple changes libvirt can and should make. The fix >>> to this "problem" belongs in libvirt, no QEMU. >> >> libvirt can't make retroactive changes. Sure, it can issue an >> update, but if we can help them avoid it by changing the order of the >> help text, I don't see why we can't do that. > > Normally, I agree, but we've taken a lot of these over a long period > of time. The result is that libvirt hasn't gotten better at solving > this problem. Again, the vast majority of the detection that libvirt > does could be done reliably and easily via version with just a few > simple exceptions. I don't see what we gain by not doing this. If you want libvirt to do the right thing, provide a proper capabilities interface. Using the version has its downsides as much as the help text. -- error compiling committee.c: too many arguments to function