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

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

[Bruce Rogers]

Libvirt parses qemu help output to determine qemu features. In particular
 it probes for the following: "cache=writethrough|writeback|none". The
 addition of the unsafe cache mode was inserted within this string, as
 opposed to being added to the end, which impacted libvirt's probe.
 Unbreak libvirt by keeping the existing cache modes intact and add
 unsafe to the end.

This problem only manifests itself if a caching mode is explicitly
specified in the libvirt xml, in which case older syntax for caching is
passed to qemu, which it  no longer understands.

Signed-off-by: Bruce Rogers <brogers@novell.com>

---
 qemu-options.hx |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 0d7dd90..9ecc54e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -118,7 +118,7 @@ ETEXI
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
-    "       [,cache=writethrough|writeback|unsafe|none][,format=f]\n"
+    "       [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
     "       [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off]\n"
     "                use 'file' as a drive image\n", QEMU_ARCH_ALL)
-- 
1.6.2.4

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

[Anthony Liguori]

On 07/21/2010 03:32 PM, Bruce Rogers wrote:
> Libvirt parses qemu help output to determine qemu features. In particular
>   it probes for the following: "cache=writethrough|writeback|none". The
>   addition of the unsafe cache mode was inserted within this string, as
>   opposed to being added to the end, which impacted libvirt's probe.
>   Unbreak libvirt by keeping the existing cache modes intact and add
>   unsafe to the end.
>
> This problem only manifests itself if a caching mode is explicitly
> specified in the libvirt xml, in which case older syntax for caching is
> passed to qemu, which it  no longer understands.
>
> Signed-off-by: Bruce Rogers<brogers@novell.com>
>    

Errr, libvirt is still doing this?

This comes up frequently and it's a real PITA.  Help text is not a 
feature probing interface.  This is a libvirt bug and it needs to be 
fixed in libvirt.

Regards,

Anthony Liguori

> ---
>   qemu-options.hx |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0d7dd90..9ecc54e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -118,7 +118,7 @@ ETEXI
>   DEF("drive", HAS_ARG, QEMU_OPTION_drive,
>       "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
>       "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
> -    "       [,cache=writethrough|writeback|unsafe|none][,format=f]\n"
> +    "       [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
>       "       [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
>       "       [,readonly=on|off]\n"
>       "                use 'file' as a drive image\n", QEMU_ARCH_ALL)
>    

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

[Daniel P. Berrange]

On Wed, Jul 21, 2010 at 03:55:28PM -0500, Anthony Liguori wrote:
> On 07/21/2010 03:32 PM, Bruce Rogers wrote:
> >Libvirt parses qemu help output to determine qemu features. In particular
> >  it probes for the following: "cache=writethrough|writeback|none". The
> >  addition of the unsafe cache mode was inserted within this string, as
> >  opposed to being added to the end, which impacted libvirt's probe.
> >  Unbreak libvirt by keeping the existing cache modes intact and add
> >  unsafe to the end.
> >
> >This problem only manifests itself if a caching mode is explicitly
> >specified in the libvirt xml, in which case older syntax for caching is
> >passed to qemu, which it  no longer understands.
> >
> >Signed-off-by: Bruce Rogers<brogers@novell.com>
> >   
> 
> Errr, libvirt is still doing this?
>
> This comes up frequently and it's a real PITA.  Help text is not a 
> feature probing interface.  This is a libvirt bug and it needs to be 
> fixed in libvirt

Of course, there is no viable alternative yet. The QMP patches I
sent a few weeks back now to add ability to query capabilities were
intended to provide the neccessary support in QEMU for us to use
instead of -help. Given that QMP isn't going to be declared stable 
in 0.13 though, it looks like we'll be continuing to use -help 
until the 0.14 release of QEMU.


Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

[Anthony Liguori]

On 07/21/2010 04:32 PM, Daniel P. Berrange wrote:
> On Wed, Jul 21, 2010 at 03:55:28PM -0500, Anthony Liguori wrote:
>    
>> On 07/21/2010 03:32 PM, Bruce Rogers wrote:
>>      
>>> Libvirt parses qemu help output to determine qemu features. In particular
>>>   it probes for the following: "cache=writethrough|writeback|none". The
>>>   addition of the unsafe cache mode was inserted within this string, as
>>>   opposed to being added to the end, which impacted libvirt's probe.
>>>   Unbreak libvirt by keeping the existing cache modes intact and add
>>>   unsafe to the end.
>>>
>>> This problem only manifests itself if a caching mode is explicitly
>>> specified in the libvirt xml, in which case older syntax for caching is
>>> passed to qemu, which it  no longer understands.
>>>
>>> Signed-off-by: Bruce Rogers<brogers@novell.com>
>>>
>>>        
>> Errr, libvirt is still doing this?
>>
>> This comes up frequently and it's a real PITA.  Help text is not a
>> feature probing interface.  This is a libvirt bug and it needs to be
>> fixed in libvirt
>>      
> Of course, there is no viable alternative yet.

Yes there is.  Use the version number.

Regards,

Anthony Liguori

>   The QMP patches I
> sent a few weeks back now to add ability to query capabilities were
> intended to provide the neccessary support in QEMU for us to use
> instead of -help. Given that QMP isn't going to be declared stable
> in 0.13 though, it looks like we'll be continuing to use -help
> until the 0.14 release of QEMU.
>
>
> Regards,
> Daniel
>    

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

[Daniel P. Berrange]

On Wed, Jul 21, 2010 at 04:45:46PM -0500, Anthony Liguori wrote:
> On 07/21/2010 04:32 PM, Daniel P. Berrange wrote:
> >On Wed, Jul 21, 2010 at 03:55:28PM -0500, Anthony Liguori wrote:
> >   
> >>On 07/21/2010 03:32 PM, Bruce Rogers wrote:
> >>     
> >>>Libvirt parses qemu help output to determine qemu features. In particular
> >>>  it probes for the following: "cache=writethrough|writeback|none". The
> >>>  addition of the unsafe cache mode was inserted within this string, as
> >>>  opposed to being added to the end, which impacted libvirt's probe.
> >>>  Unbreak libvirt by keeping the existing cache modes intact and add
> >>>  unsafe to the end.
> >>>
> >>>This problem only manifests itself if a caching mode is explicitly
> >>>specified in the libvirt xml, in which case older syntax for caching is
> >>>passed to qemu, which it  no longer understands.
> >>>
> >>>Signed-off-by: Bruce Rogers<brogers@novell.com>
> >>>
> >>>       
> >>Errr, libvirt is still doing this?
> >>
> >>This comes up frequently and it's a real PITA.  Help text is not a
> >>feature probing interface.  This is a libvirt bug and it needs to be
> >>fixed in libvirt
> >>     
> >Of course, there is no viable alternative yet.
> 
> Yes there is.  Use the version number.

The version number is not suitable, because features can be removed at
compile time and/or added via patch backports. The only reliable way is
to query the QEMU binary to ask it what it actually supports.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

[Anthony Liguori]

On 07/21/2010 04:58 PM, Daniel P. Berrange wrote:
>> Yes there is.  Use the version number.
>>      
> The version number is not suitable, because features can be removed at
> compile time and/or

I don't see any features that libvirt would need to know about that are 
disabled at compile time that aren't disabled by platform features (i.e. 
being on a Linux vs. Windows host).

>   added via patch backports.

If a distro backports a feature, it should change the QEMU version 
string.  If it doesn't, that's a distro problem.

>   The only reliable way is
> to query the QEMU binary to ask it what it actually supports.
>    

And you do that by asking QEMU what it's version number is.  If the 
version number isn't reliable because a distro backported features 
without changing it, then the distro is broken.  It's no different than 
if we had a capabilities system and a distro added a feature without 
adjusting the advertises capabilities.

Regards,

Anthony Liguori

> Regards,
> Daniel
>    

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

[Jamie Lokier]

Anthony Liguori wrote:
> On 07/21/2010 04:58 PM, Daniel P. Berrange wrote:
> >>Yes there is.  Use the version number.
> >>     
> >The version number is not suitable, because features can be removed at
> >compile time and/or
> 
> I don't see any features that libvirt would need to know about that are 
> disabled at compile time that aren't disabled by platform features (i.e. 
> being on a Linux vs. Windows host).
> 
> >  added via patch backports.
> 
> If a distro backports a feature, it should change the QEMU version 
> string.  If it doesn't, that's a distro problem.

To what version?  It can't use the newer version if it only backports
a subset of features; it would have to use a distro-specific version
number or a version string that somehow encodes feature independent of
the version number itself, by some agreed libvirt standard.  Which
isn't far off advertising features in the help string :-)

-- Jamie

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

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 07/21/2010 04:58 PM, Daniel P. Berrange wrote:
>>> Yes there is.  Use the version number.
>>>      
>> The version number is not suitable, because features can be removed at
>> compile time and/or
>
> I don't see any features that libvirt would need to know about that
> are disabled at compile time that aren't disabled by platform features
> (i.e. being on a Linux vs. Windows host).
>
>>   added via patch backports.
>
> If a distro backports a feature, it should change the QEMU version
> string.  If it doesn't, that's a distro problem.
>
>>   The only reliable way is
>> to query the QEMU binary to ask it what it actually supports.
>>    
>
> And you do that by asking QEMU what it's version number is.  If the
> version number isn't reliable because a distro backported features
> without changing it, then the distro is broken.  It's no different
> than if we had a capabilities system and a distro added a feature
> without adjusting the advertises capabilities.

No, because a version number isn't a capabilities system.

With a capabilities system, a vendor can add vendor-specific
capabilities.  Should be necessary only as a last resort, because
upstream already describes itself reasonably well, so most of the time
you can just backport the upstream capability.

With a version, all a vendor can do is graft their their own "capability
system" into the version string, say encoded as vendor version.
Guaranteed to be incompatible with anyone else's "capabilitity system".
Only the vendor's tools will understand it, Which means users can't use
anything else.  Avoiding such vendor lock in is one of the core values
of the free software world.

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

[Daniel P. Berrange]

On Wed, Jul 21, 2010 at 06:39:32PM -0500, Anthony Liguori wrote:
> On 07/21/2010 04:58 PM, Daniel P. Berrange wrote:
> >>Yes there is.  Use the version number.
> >>     
> >The version number is not suitable, because features can be removed at
> >compile time and/or
> 
> I don't see any features that libvirt would need to know about that are 
> disabled at compile time that aren't disabled by platform features (i.e. 
> being on a Linux vs. Windows host)

SDL. vhost-net.

> >  added via patch backports.
> 
> If a distro backports a feature, it should change the QEMU version 
> string.  If it doesn't, that's a distro problem.

This puts you in the position of having to maintain an ever changing
giant compatability table between version numbers and features, which
just results in madness.
 
> >  The only reliable way is
> >to query the QEMU binary to ask it what it actually supports.
> >   
> 
> And you do that by asking QEMU what it's version number is.  If the 
> version number isn't reliable because a distro backported features 
> without changing it, then the distro is broken.  It's no different than 
> if we had a capabilities system and a distro added a feature without 
> adjusting the advertises capabilities.

Using version numbers simply isn't scalable / sustainable in the
long term. Imagine trying to maintain a list of kernel version
numbers where 'ext4' was available, instead of just looking at
/proc/filesystems for the declared capabilities of the kernel
No sane app would do this kind of thing with version numbers
because they will always be outdated and always wrong if the 
user decides to compile out certain features. The only thing
version numbers are good for is in bug reporting to indicate
the build being used to the vendor.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

[Anthony Liguori]

On 07/22/2010 03:42 AM, Daniel P. Berrange wrote:
> On Wed, Jul 21, 2010 at 06:39:32PM -0500, Anthony Liguori wrote:
>    
>> On 07/21/2010 04:58 PM, Daniel P. Berrange wrote:
>>      
>>>> Yes there is.  Use the version number.
>>>>
>>>>          
>>> The version number is not suitable, because features can be removed at
>>> compile time and/or
>>>        
>> I don't see any features that libvirt would need to know about that are
>> disabled at compile time that aren't disabled by platform features (i.e.
>> being on a Linux vs. Windows host)
>>      
> SDL.

libvirt executes qemu from a daemon, how does SDL come into play?  Why 
do you need to probe whether it's supported anyway?  If a user requests 
SDL (I assume through /session) if it fails it fails.  Doesn't seem like 
a huge loss to me comparatively speaking.

>   vhost-net.
>    

vhost-net is unconditionally displayed in the help output regardless of 
whether the binary supports it.

You cannot assume that qemu supports vhost-net just because it says 
something in the help output.

>>>   added via patch backports.
>>>        
>> If a distro backports a feature, it should change the QEMU version
>> string.  If it doesn't, that's a distro problem.
>>      
> This puts you in the position of having to maintain an ever changing
> giant compatability table between version numbers and features, which
> just results in madness.
>    

Or working with QEMU to have a better solution.

We've been complaining for a long time about parsing help output and 
we've made changes as "temporary" stop gaps for libvirt too many times 
in the past.

This problem needs to get fixed properly.

>>>   The only reliable way is
>>> to query the QEMU binary to ask it what it actually supports.
>>>
>>>        
>> And you do that by asking QEMU what it's version number is.  If the
>> version number isn't reliable because a distro backported features
>> without changing it, then the distro is broken.  It's no different than
>> if we had a capabilities system and a distro added a feature without
>> adjusting the advertises capabilities.
>>      
> Using version numbers simply isn't scalable / sustainable in the
> long term.

And parsing help output is?

I'm not claiming that version numbers is a long term solution.  But it's 
a better interim solution than parsing help output.

It's time to fix this problem properly.  There's been plenty of feedback 
that relying on help output isn't acceptable.

Regards,

Anthony Liguori

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

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 07/22/2010 03:42 AM, Daniel P. Berrange wrote:
>> On Wed, Jul 21, 2010 at 06:39:32PM -0500, Anthony Liguori wrote:
[...]
>>> If a distro backports a feature, it should change the QEMU version
>>> string.  If it doesn't, that's a distro problem.
>>>      
>> This puts you in the position of having to maintain an ever changing
>> giant compatability table between version numbers and features, which
>> just results in madness.
>>    
>
> Or working with QEMU to have a better solution.
>
> We've been complaining for a long time about parsing help output and
> we've made changes as "temporary" stop gaps for libvirt too many times
> in the past.
>
> This problem needs to get fixed properly.

You almost sound like Dan refused to consider anything but parsing help
output, like it were Dan's fault that QEMU still doesn't provide a
usable interface for querying its capabilities, and like the way to get
it was to put more pressure on Dan.

That would be unfair.  Dan posted patches to fix this problem properly,
which puts the ball squarely in our court.  Could we please refrain from
gratuitously fscking up libvirt just a bit longer, until we finish
improving and merging Dan's patches?

[...]

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

[Anthony Liguori]

On 07/23/2010 03:00 AM, Markus Armbruster wrote:
> Anthony Liguori<anthony@codemonkey.ws>  writes:
>
>    
>> On 07/22/2010 03:42 AM, Daniel P. Berrange wrote:
>>      
>>> On Wed, Jul 21, 2010 at 06:39:32PM -0500, Anthony Liguori wrote:
>>>        
> [...]
>    
>>>> If a distro backports a feature, it should change the QEMU version
>>>> string.  If it doesn't, that's a distro problem.
>>>>
>>>>          
>>> This puts you in the position of having to maintain an ever changing
>>> giant compatability table between version numbers and features, which
>>> just results in madness.
>>>
>>>        
>> Or working with QEMU to have a better solution.
>>
>> We've been complaining for a long time about parsing help output and
>> we've made changes as "temporary" stop gaps for libvirt too many times
>> in the past.
>>
>> This problem needs to get fixed properly.
>>      
> You almost sound like Dan refused to consider anything but parsing help
> output, like it were Dan's fault that QEMU still doesn't provide a
> usable interface for querying its capabilities, and like the way to get
> it was to put more pressure on Dan.
>    

I'm not blaming anyone.  As I see it, we have a supported interface for 
determining which interfaces are supported by a given version of 
QEMU--the version number.  If the version number is not reliable because 
downstreams backport features in an undetectable way, that is not our 
(upstream) problem.

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.

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

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

The help output is *not* a supported interface.  There are very simple 
changes libvirt can and should make.  The fix to this "problem" belongs 
in libvirt, no QEMU.

Regards,

Anthony Liguori

> [...]
>    

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

[Avi Kivity]

  On 07/26/2010 06:53 PM, Anthony Liguori wrote:
>> You almost sound like Dan refused to consider anything but parsing help
>> output, like it were Dan's fault that QEMU still doesn't provide a
>> usable interface for querying its capabilities, and like the way to get
>> it was to put more pressure on Dan.
>
> I'm not blaming anyone.  As I see it, we have a supported interface 
> for determining which interfaces are supported by a given version of 
> QEMU--the version number.  If the version number is not reliable 
> because downstreams backport features in an undetectable way, that is 
> not our (upstream) problem.
>
> 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.

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

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

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

-- 
error compiling committee.c: too many arguments to function

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

[Avi Kivity]

  On 07/26/2010 07:26 PM, Avi Kivity wrote:
>
>> The help output is *not* a supported interface. 
>
> There is no supported, usable interface for this.

Well actually, libvirt could probe this by starting qemu with cache=x 
for various x and seeing if it breaks.  But the milk has already been 
spilled.

-- 
error compiling committee.c: too many arguments to function

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

[Anthony Liguori]

On 07/26/2010 11:29 AM, Avi Kivity wrote:
>  On 07/26/2010 07:26 PM, Avi Kivity wrote:
>>
>>> The help output is *not* a supported interface. 
>>
>> There is no supported, usable interface for this.
>
> Well actually, libvirt could probe this by starting qemu with cache=x 
> for various x and seeing if it breaks.  But the milk has already been 
> spilled.

So we're stuck with supporting the help output as an interface 
forever?   Even with a capabilities system, what about old versions of 
libvirt?

At some point in time, old versions of libvirt are going to stop working 
with new versions of QEMU because the help output changes.  If libvirt 
switches to something more reliable (like versioning), then the gap is 
closed until there is a capabilities system.

There will be a breakage though and we shouldn't pretend that taking 
this patch does anything other than delay that breakage.

Regards,

Anthony Liguori

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

[Avi Kivity]

  On 07/26/2010 10:03 PM, Anthony Liguori wrote:
> On 07/26/2010 11:29 AM, Avi Kivity wrote:
>>  On 07/26/2010 07:26 PM, Avi Kivity wrote:
>>>
>>>> The help output is *not* a supported interface. 
>>>
>>> There is no supported, usable interface for this.
>>
>> Well actually, libvirt could probe this by starting qemu with cache=x 
>> for various x and seeing if it breaks.  But the milk has already been 
>> spilled.
>
> So we're stuck with supporting the help output as an interface 
> forever?   Even with a capabilities system, what about old versions of 
> libvirt?

No, the rate of crap generation seems to increase all the time, we have 
to get rid of it eventually.  If we provide a replacement, give a 
warning, wait some months for libvirt^Wusers to catch up, and throw away 
the deprecated feature, we can remove any feature we like.  This is 
still a fast moving field and users to upgrade.  Just not every week.

>
> At some point in time, old versions of libvirt are going to stop 
> working with new versions of QEMU because the help output changes.  If 
> libvirt switches to something more reliable (like versioning), then 
> the gap is closed until there is a capabilities system.
>
> There will be a breakage though and we shouldn't pretend that taking 
> this patch does anything other than delay that breakage.

There's a difference between
- planned breakage with a warning ahead of time
- unplanned breakage
- unplanned breakage with unwillingness to fix

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

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

[Juan Quintela]

Avi Kivity <avi@redhat.com> wrote:
>  On 07/26/2010 10:03 PM, Anthony Liguori wrote:
>> On 07/26/2010 11:29 AM, Avi Kivity wrote:
>>>  On 07/26/2010 07:26 PM, Avi Kivity wrote:
>>>>
>>>>> The help output is *not* a supported interface. 
>>>>
>>>> There is no supported, usable interface for this.
>>>
>>> Well actually, libvirt could probe this by starting qemu with
>>> cache=x for various x and seeing if it breaks.  But the milk has
>>> already been spilled.
>>
>> So we're stuck with supporting the help output as an interface
>> forever?   Even with a capabilities system, what about old versions
>> of libvirt?
>
> No, the rate of crap generation seems to increase all the time, we
> have to get rid of it eventually.  If we provide a replacement, give a
> warning, wait some months for libvirt^Wusers to catch up, and throw
> away the deprecated feature, we can remove any feature we like.  This
> is still a fast moving field and users to upgrade.  Just not every
> week.
>
>>
>> At some point in time, old versions of libvirt are going to stop
>> working with new versions of QEMU because the help output changes.
>> If libvirt switches to something more reliable (like versioning),
>> then the gap is closed until there is a capabilities system.
>>
>> There will be a breakage though and we shouldn't pretend that taking
>> this patch does anything other than delay that breakage.
>
> There's a difference between
> - planned breakage with a warning ahead of time
> - unplanned breakage
> - unplanned breakage with unwillingness to fix

Fully agree here.  We have lots of (mis)features that we want to
remove.  But one thing is just remove them, and another to remove
something before we create a valid alternative.

I am all for:

docs/deprecated-features.txt

And having there things that will be removed and when.

Later, Juan.

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

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

[Avi Kivity]

  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

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

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

[Avi Kivity]

  On 07/26/2010 09:59 PM, Anthony Liguori wrote:
>>> 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? 

Then we have an even bigger problem.  As for now, we have a manageable 
problem that this patch solves.  Let's apply this workaround to fix the 
one real problem we know about, and work towards documented capability 
reporting to make sure this doesn't hit us in the future when we have 
more users.

> Is what we are supporting just what libvirt expects there to be or 
> what any tool out there expects there to be?

We should try to support all users, prioritized by the number of end 
users they represent.  If this patch broke some other large user we'd be 
in a bind.  But likely this isn't the case so we aren't.

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

Are you saying


     "       [,cache=writethrough|writeback|unsafe|none][,format=f]\n"

is more readable than

     "       [,cache=writethrough|writeback|none|unsafe][,format=f]\n"

?  if so I disagree.  If you say our help output is in general 
unreadable, then this has nothing to do with this patch.  Implementing a 
capabilities system is more important than improving help output 
readability, let's to this first and then rewrite the help output in 
Shakespearean glory.

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

As it happens the patch came from a Novell employee, so it isn't about 
RHEL.  I applaud your choice of enterprise operating systems, however.

Regardless, outside of Windows users qemu will mostly be consumed via 
distribution branches, with different levels of backport happiness.  We 
should recognize that and work with it, not against it.

>>
>> 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 we could point libvirt towards a better way of doing things (not a 
better regexp, which could just break under different circumstances; a 
supported interface) I'd agree.  Go RTFM chapter-this-or-that and don't 
bother me no more.  But we can't.

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


t=0 starting point, cache=unsafe is unknown
t=1 qemu upstream adds cache=unknown
t=2 libvirt adds support for cache=unsafe, releases
t=3 evil distro backports cache=unsafe, releases qemu-kvm-1.2.3.4
t=4 user tries libvirt from t=2 with qemu from t=3, cache=unsafe not 
detected

version numbers force a libvirt update every time a feature is backported.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

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

[Anthony Liguori]

On 07/26/2010 02:19 PM, Avi Kivity wrote:
>> Is what we are supporting just what libvirt expects there to be or 
>> what any tool out there expects there to be?
>
> We should try to support all users, prioritized by the number of end 
> users they represent.  If this patch broke some other large user we'd 
> be in a bind.  But likely this isn't the case so we aren't.

As I've said, I'm pragmatic and that's why I've argued for these changes 
in the past.  But libvirt should have changed a long time ago to using 
something more reliable (like version).

>> 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.
>
> Are you saying
>
>
>     "       [,cache=writethrough|writeback|unsafe|none][,format=f]\n"
>
> is more readable than
>
>     "       [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
>
> ?  if so I disagree.

Absolutely.  But I meant in the general sense.

>> Backports are such a special case.  Honestly, we're talking about 
>> RHEL and it's trivially easy for libvirt to just special case RHEL.
>
>
> As it happens the patch came from a Novell employee, so it isn't about 
> RHEL.

SLES doesn't carry many backported features.  The reason this hit Bruce 
is that libvirt is detecting using help because of RHEL.

>   I applaud your choice of enterprise operating systems, however.
>
> Regardless, outside of Windows users qemu will mostly be consumed via 
> distribution branches, with different levels of backport happiness.  
> We should recognize that and work with it, not against it.

And it's trivially easy for libvirt to deal with this.  They simply have 
to do: `cat /etc/redhat-release` or `cat /etc/suse-release` and adjust 
version logic accordingly.

It's an easy change for libvirt to make, and the problem goes away for 
the future.  If such a change was made, I'd be inclined to take a patch 
like this now to make up for the difference.

But as I said, we've been accommodating libvirt's use of help for a long 
time now.  We shouldn't let perfect (omnipotent capabilities system) 
stand in the way of good (version/feature matrix).

>>>
>>> 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 we could point libvirt towards a better way of doing things (not a 
> better regexp, which could just break under different circumstances; a 
> supported interface) I'd agree.  Go RTFM chapter-this-or-that and 
> don't bother me no more.  But we can't.

Again, give me an example of where versioning isn't simple and robust 
that applies to things as they stand today and I'm happy to agree with you.

>
>>> 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.
>
>
> t=0 starting point, cache=unsafe is unknown
> t=1 qemu upstream adds cache=unknown
> t=2 libvirt adds support for cache=unsafe, releases
> t=3 evil distro backports cache=unsafe, releases qemu-kvm-1.2.3.4
> t=4 user tries libvirt from t=2 with qemu from t=3, cache=unsafe not 
> detected
>
> version numbers force a libvirt update every time a feature is 
> backported.

That's a bogus scenario because said evil distro would also enhance 
libvirt to detect the feature properly.

Regards,

Anthony Liguori

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

[Avi Kivity]

  On 07/26/2010 10:35 PM, Anthony Liguori wrote:
>
>>
>>>> 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.
>>
>>
>> t=0 starting point, cache=unsafe is unknown
>> t=1 qemu upstream adds cache=unknown
>> t=2 libvirt adds support for cache=unsafe, releases
>> t=3 evil distro backports cache=unsafe, releases qemu-kvm-1.2.3.4
>> t=4 user tries libvirt from t=2 with qemu from t=3, cache=unsafe not 
>> detected
>>
>> version numbers force a libvirt update every time a feature is 
>> backported.
>
> That's a bogus scenario because said evil distro would also enhance 
> libvirt to detect the feature properly.

That means a new libvirt release is needed.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

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

[Anthony Liguori]

On 07/26/2010 02:43 PM, Avi Kivity wrote:
>  On 07/26/2010 10:35 PM, Anthony Liguori wrote:
>> That's a bogus scenario because said evil distro would also enhance 
>> libvirt to detect the feature properly.
>
> That means a new libvirt release is needed.

Which if you're a distro and you backport a KVM feature, you're probably 
doing anyway to support that feature in libvirt.

Regards,

Anthony Liguori

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

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 07/26/2010 02:19 PM, Avi Kivity wrote:
>>> Is what we are supporting just what libvirt expects there to be or
>>> what any tool out there expects there to be?
>>
>> We should try to support all users, prioritized by the number of end
>> users they represent.  If this patch broke some other large user
>> we'd be in a bind.  But likely this isn't the case so we aren't.
>
> As I've said, I'm pragmatic and that's why I've argued for these
> changes in the past.  But libvirt should have changed a long time ago
> to using something more reliable (like version).

You want pragmatic?  I can give you pragmatic!  We apply the trivial
patch that helps libvirt and hurts nobody, and save our breath & typing
for designing and implementing a capability system.

[...]

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

[Jes Sorensen]

On 07/27/10 10:11, Markus Armbruster wrote:
> Anthony Liguori <anthony@codemonkey.ws> writes:
>> On 07/26/2010 02:19 PM, Avi Kivity wrote:
>>> We should try to support all users, prioritized by the number of end
>>> users they represent.  If this patch broke some other large user
>>> we'd be in a bind.  But likely this isn't the case so we aren't.
>>
>> As I've said, I'm pragmatic and that's why I've argued for these
>> changes in the past.  But libvirt should have changed a long time ago
>> to using something more reliable (like version).
> 
> You want pragmatic?  I can give you pragmatic!  We apply the trivial
> patch that helps libvirt and hurts nobody, and save our breath & typing
> for designing and implementing a capability system.

To be honest, this is exactly the same problem we had when the output
from -version changed and libvirt broke because it did static string
parsing instead of doing it properly. Back then the output of -version
was changed back to accommodate libvirt, but I am not aware that libvirt
went ahead and fixed the real problem in the mean time.

While I don't see this specific change being problematic, I don't like
the trend of hacking things to accommodate a specific library or
application, when the group relying on the feature really should start
providing the code for the real solution.

Just my $0.02

Jes

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

[Cole Robinson]

On 07/27/2010 05:47 AM, Jes Sorensen wrote:
> On 07/27/10 10:11, Markus Armbruster wrote:
>> Anthony Liguori <anthony@codemonkey.ws> writes:
>>> On 07/26/2010 02:19 PM, Avi Kivity wrote:
>>>> We should try to support all users, prioritized by the number of end
>>>> users they represent.  If this patch broke some other large user
>>>> we'd be in a bind.  But likely this isn't the case so we aren't.
>>>
>>> As I've said, I'm pragmatic and that's why I've argued for these
>>> changes in the past.  But libvirt should have changed a long time ago
>>> to using something more reliable (like version).
>>
>> You want pragmatic?  I can give you pragmatic!  We apply the trivial
>> patch that helps libvirt and hurts nobody, and save our breath & typing
>> for designing and implementing a capability system.
> 
> To be honest, this is exactly the same problem we had when the output
> from -version changed and libvirt broke because it did static string
> parsing instead of doing it properly. Back then the output of -version
> was changed back to accommodate libvirt, but I am not aware that libvirt
> went ahead and fixed the real problem in the mean time.
> 

The output of -version was not changed back, the revert was rejected.
(Meaning QEMU has no stable interface for determining version info.
Which is confusing, considering that the version string is now being
recommended as the interim capability reporting system.)

I also want to point out that the version string change is far worse
than the cache= issue: in this case, we won't set a user specified cache
value. In the version string case, nearly every existing libvirt
deployment is not going to be able to use qemu-0.13. Whoops! :(

No argument that these are libvirt parsing bugs, but it would be a good
faith gesture to revert these utterly trivial qemu changes until there's
a proper way forward (or some reasonable amount of time has passed).

> While I don't see this specific change being problematic, I don't like
> the trend of hacking things to accommodate a specific library or
> application, when the group relying on the feature really should start
> providing the code for the real solution.
> 

There is no trend. From a quick look of the logs, I couldn't find a
single commit attributed to appeasing libvirt that wasn't a qemu bug (or
related to QMP which is ongoing dev).

Also, the affected group (well, danpb really) _is_ supplying code for
the real solution, he made a post last month:

http://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00921.html

There also was a previous attempt at a -capabilites option by markmc a
couple years back, but it fell by the wayside:

http://lists.gnu.org/archive/html/qemu-devel/2008-11/msg00767.html

- Cole

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

[Anthony Liguori]

On 07/27/2010 07:30 AM, Cole Robinson wrote:
> On 07/27/2010 05:47 AM, Jes Sorensen wrote:
>    
>> On 07/27/10 10:11, Markus Armbruster wrote:
>>      
>>> Anthony Liguori<anthony@codemonkey.ws>  writes:
>>>        
>>>> On 07/26/2010 02:19 PM, Avi Kivity wrote:
>>>>          
>>>>> We should try to support all users, prioritized by the number of end
>>>>> users they represent.  If this patch broke some other large user
>>>>> we'd be in a bind.  But likely this isn't the case so we aren't.
>>>>>            
>>>> As I've said, I'm pragmatic and that's why I've argued for these
>>>> changes in the past.  But libvirt should have changed a long time ago
>>>> to using something more reliable (like version).
>>>>          
>>> You want pragmatic?  I can give you pragmatic!  We apply the trivial
>>> patch that helps libvirt and hurts nobody, and save our breath&  typing
>>> for designing and implementing a capability system.
>>>        
>> To be honest, this is exactly the same problem we had when the output
>> from -version changed and libvirt broke because it did static string
>> parsing instead of doing it properly. Back then the output of -version
>> was changed back to accommodate libvirt, but I am not aware that libvirt
>> went ahead and fixed the real problem in the mean time.
>>
>>      
> The output of -version was not changed back, the revert was rejected.
> (Meaning QEMU has no stable interface for determining version info.
>    

Actually, we do.  'info version' in the monitor returns just the version.

Additionally, -version on the command line spits out just a single 
version string.

The trouble libvirt has is that it's parsing the help output and needs 
to use a string to identify which line is the version (due to the way 
it's parsing the output).

Notice a theme here?

Regards,

Anthony Liguori

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

[Cole Robinson]

On 07/27/2010 08:50 AM, Anthony Liguori wrote:
> On 07/27/2010 07:30 AM, Cole Robinson wrote:
>> On 07/27/2010 05:47 AM, Jes Sorensen wrote:
>>    
>>> On 07/27/10 10:11, Markus Armbruster wrote:
>>>      
>>>> Anthony Liguori<anthony@codemonkey.ws>  writes:
>>>>        
>>>>> On 07/26/2010 02:19 PM, Avi Kivity wrote:
>>>>>          
>>>>>> We should try to support all users, prioritized by the number of end
>>>>>> users they represent.  If this patch broke some other large user
>>>>>> we'd be in a bind.  But likely this isn't the case so we aren't.
>>>>>>            
>>>>> As I've said, I'm pragmatic and that's why I've argued for these
>>>>> changes in the past.  But libvirt should have changed a long time ago
>>>>> to using something more reliable (like version).
>>>>>          
>>>> You want pragmatic?  I can give you pragmatic!  We apply the trivial
>>>> patch that helps libvirt and hurts nobody, and save our breath&  typing
>>>> for designing and implementing a capability system.
>>>>        
>>> To be honest, this is exactly the same problem we had when the output
>>> from -version changed and libvirt broke because it did static string
>>> parsing instead of doing it properly. Back then the output of -version
>>> was changed back to accommodate libvirt, but I am not aware that libvirt
>>> went ahead and fixed the real problem in the mean time.
>>>
>>>      
>> The output of -version was not changed back, the revert was rejected.
>> (Meaning QEMU has no stable interface for determining version info.
>>    
> 
> Actually, we do.  'info version' in the monitor returns just the version.
> 

My bad, I didn't know about that. Then again, having to start up a qemu
instance and connect to the monitor just to get a bare version string
seems overkill.

> Additionally, -version on the command line spits out just a single 
> version string.
> 

I wouldn't really call that 'stable', figuring that the output was
changed recently.

> The trouble libvirt has is that it's parsing the help output and needs 
> to use a string to identify which line is the version (due to the way 
> it's parsing the output).
> 
> Notice a theme here?
> 

A few: libvirt's -help parsing is fragile. Libvirt is using an
unsupported/unstable capabilities system, though it works acceptably in
practice.

Some others: Libvirt is a significant qemu consumer and provides value
to the qemu project. qemu lacks a supported discoverable capabilities
interface.

And some facts: qemu 0.13 will not work with 95% of existing libvirt
deployments. The 2 requested qemu reverts will have approx. 0 functional
impact on plain qemu users.

Can we evaluate these all together? Really, what's the harm in reverting
these changes for 0.13, reapplying after the release is cut, and making
a commitment to get some capabilities interface into 0.14? (and no
libvirt appeasing -help/-version patches in the 0.14 cycle)

- Cole

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

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 07/26/2010 02:19 PM, Avi Kivity wrote:
[...]
>> Regardless, outside of Windows users qemu will mostly be consumed
>> via distribution branches, with different levels of backport
>> happiness.  We should recognize that and work with it, not against
>> it.
>
> And it's trivially easy for libvirt to deal with this.  They simply
> have to do: `cat /etc/redhat-release` or `cat /etc/suse-release` and
> adjust version logic accordingly.

It's always trivially easy for *another* project to do the work.

> It's an easy change for libvirt to make, and the problem goes away for
> the future.  If such a change was made, I'd be inclined to take a
> patch like this now to make up for the difference.
>
> But as I said, we've been accommodating libvirt's use of help for a
> long time now.  We shouldn't let perfect (omnipotent capabilities
> system) stand in the way of good (version/feature matrix).

We've declared our intention to provide a decent capability system (I
said decent, not perfect).  If I know anything about libvirt developers,
they'll *jump* at the chance to replace their existing code by a
capability system, because they consider their existing code messy and
brittle.

But now we tell them to replace it by a different method first, or else.
A method that is at least as messy & brittle in their judgement.  Which
is based on a few person-decades of experience trying various methods.
A method that is to be thrown out again when the capabilities system is
ready, i.e. about the time its inevitable wrinkles got ironed out.

Not going to happen.  All this little feud is going to accomplish is to
hurt users.

[...]

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

[Richard W.M. Jones]

On Tue, Jul 27, 2010 at 10:26:10AM +0200, Markus Armbruster wrote:
> Anthony Liguori <anthony@codemonkey.ws> writes:
> 
> > On 07/26/2010 02:19 PM, Avi Kivity wrote:
> [...]
> >> Regardless, outside of Windows users qemu will mostly be consumed
> >> via distribution branches, with different levels of backport
> >> happiness.  We should recognize that and work with it, not against
> >> it.
> >
> > And it's trivially easy for libvirt to deal with this.  They simply
> > have to do: `cat /etc/redhat-release` or `cat /etc/suse-release` and
> > adjust version logic accordingly.
> 
> It's always trivially easy for *another* project to do the work.
> 
> > It's an easy change for libvirt to make, and the problem goes away for
> > the future.  If such a change was made, I'd be inclined to take a
> > patch like this now to make up for the difference.
> >
> > But as I said, we've been accommodating libvirt's use of help for a
> > long time now.  We shouldn't let perfect (omnipotent capabilities
> > system) stand in the way of good (version/feature matrix).
> 
> We've declared our intention to provide a decent capability system (I
> said decent, not perfect).  If I know anything about libvirt developers,
> they'll *jump* at the chance to replace their existing code by a
> capability system, because they consider their existing code messy and
> brittle.

I'd just like to AOL /me too here:

libguestfs suffers all the same problems parsing help.  Using
version is a silly suggestion IMHO.  We'd like a capabilities
interface, even if it's not perfect.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

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

[Kevin Wolf]

Am 26.07.2010 17:53, schrieb Anthony Liguori:
> 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.

As a practical guy you should see that we're just about to make a
release that doesn't work well with libvirt in this respect. Taking this
patch won't fix the real problem, but it works around the implementation
libvirt uses today and fixes the immediate problem.

No doubt that libvirt needs to change their way of detecting features,
but there's no way for them to release a new version which uses a
different detection mechanism and which will be widespread by the 0.13
release (unless you plan to delay it another couple of months).

We shouldn't start playing politics at the cost of our users. At least
not in release versions.

Kevin

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

[Luiz Capitulino]

On Wed, 21 Jul 2010 15:55:28 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 07/21/2010 03:32 PM, Bruce Rogers wrote:
> > Libvirt parses qemu help output to determine qemu features. In particular
> >   it probes for the following: "cache=writethrough|writeback|none". The
> >   addition of the unsafe cache mode was inserted within this string, as
> >   opposed to being added to the end, which impacted libvirt's probe.
> >   Unbreak libvirt by keeping the existing cache modes intact and add
> >   unsafe to the end.
> >
> > This problem only manifests itself if a caching mode is explicitly
> > specified in the libvirt xml, in which case older syntax for caching is
> > passed to qemu, which it  no longer understands.
> >
> > Signed-off-by: Bruce Rogers<brogers@novell.com>
> >    
> 
> Errr, libvirt is still doing this?
> 
> This comes up frequently and it's a real PITA.  Help text is not a 
> feature probing interface.  This is a libvirt bug and it needs to be 
> fixed in libvirt.

IMHO, we should try to not break it as much as we can while QMP
isn't stable yet, just like we do for the user monitor.

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

[Daniel P. Berrange]

On Wed, Jul 21, 2010 at 02:32:28PM -0600, Bruce Rogers wrote:
> Libvirt parses qemu help output to determine qemu features. In particular
>  it probes for the following: "cache=writethrough|writeback|none". The
>  addition of the unsafe cache mode was inserted within this string, as
>  opposed to being added to the end, which impacted libvirt's probe.
>  Unbreak libvirt by keeping the existing cache modes intact and add
>  unsafe to the end.

We have inverted the check we made so that instead of doing a positive
check for the new syntax, we do a negative check for the old syntax,
avoiding this problem. Of couse any existing deployed livirt will still
be broken with latest QEMU, but the previous QEMU version string change
mean that is already the case for any libvirt < 0.8.2

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

[Bruce Rogers]

 >>> On 7/27/2010 at 04:26 AM, "Daniel P. Berrange" <berrange@redhat.com> wrote: 
> On Wed, Jul 21, 2010 at 02:32:28PM -0600, Bruce Rogers wrote:
>> Libvirt parses qemu help output to determine qemu features. In particular
>>  it probes for the following: "cache=writethrough|writeback|none". The
>>  addition of the unsafe cache mode was inserted within this string, as
>>  opposed to being added to the end, which impacted libvirt's probe.
>>  Unbreak libvirt by keeping the existing cache modes intact and add
>>  unsafe to the end.
> 
> We have inverted the check we made so that instead of doing a positive
> check for the new syntax, we do a negative check for the old syntax,
> avoiding this problem. Of couse any existing deployed livirt will still
> be broken with latest QEMU, but the previous QEMU version string change
> mean that is already the case for any libvirt < 0.8.2
> 
> Daniel


This seems like a quite reasonable resolution to me.

Thanks!

Bruce