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

[Anthony Liguori]

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.

> > > That would be unfair.  Dan posted patches to fix this problem properly,
> > > which puts the ball squarely in our court.
> >
> > The patches are a good step, but they don't solve the problem.  There 
> > will always be older versions of libvirt and older versions of QEMU 
> > so what's libvirt going to do with those?
>
> Is there a released version of qemu which breaks libvirt?
>
> 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.


> > >    Could we please refrain from
> > > gratuitously fscking up libvirt just a bit longer, until we finish
> > > improving and merging Dan's patches?
> >
> > Even without doing version based feature detection, libvirt could 
> > just do a better job of parsing the help output.
>
> True.  However, we don't provide a sane interface for detecting qemu 
> capabilties, so there's no need to act surprised when users don't 
> implement perfect work arounds.
>
> > 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.

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

Regards,

Anthony Liguori