Re: [Qemu-devel] [PATCH] move 'unsafe' to end of caching modes in help

[Avi Kivity <avi@redhat.com>]

  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