From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=55850 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OdSu3-0007oi-31 for qemu-devel@nongnu.org; Mon, 26 Jul 2010 14:59:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OdSu1-0002cB-Mn for qemu-devel@nongnu.org; Mon, 26 Jul 2010 14:59:54 -0400 Received: from mail-gw0-f45.google.com ([74.125.83.45]:40237) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OdSu1-0002by-KE for qemu-devel@nongnu.org; Mon, 26 Jul 2010 14:59:53 -0400 Received: by gwb11 with SMTP id 11so202471gwb.4 for ; Mon, 26 Jul 2010 11:59:52 -0700 (PDT) Message-ID: <4C4DDB25.90000@codemonkey.ws> Date: Mon, 26 Jul 2010 13:59:49 -0500 From: Anthony Liguori 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> <4C4DBDCC.8090408@redhat.com> In-Reply-To: <4C4DBDCC.8090408@redhat.com> 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: Avi Kivity Cc: Bruce Rogers , Markus Armbruster , qemu-devel@nongnu.org On 07/26/2010 11:54 AM, Avi Kivity wrote: >>> >>> 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? What if another tool is parsing -help output? Is what we are supporting just what libvirt expects there to be or what any tool out there expects there to be? > 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? It's not. Our help output is unreadable. The (artificial) restrictions we're putting ourselves with respect to the help output prevents it from being improved. >> >> Version is entirely reliable for detecting whether -drive supports >> cache. > > It's not a reliable interface for detecting features in the face of > backports. Backports are such a special case. Honestly, we're talking about RHEL and it's trivially easy for libvirt to just special case RHEL. >>>> 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. We're losing the ability to make *any* change to our help system by encouraging it to be used in this fashion. > 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. That's simply not the case. Please, provide an actual example where version is not reliable and backports aren't trivially easy to detect. Regards, Anthony Liguori