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

[Anthony Liguori]

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