[Qemu-devel] [PATCH] Revive -version 'QEMU PC Emulator...'

[Qemu-devel] [PATCH] Revive -version 'QEMU PC Emulator...'

[Cole Robinson]

Commit f75ca1ae205f24dae296c82d534c37746f87232f changed the version
string from:

QEMU PC Emulator version x.yy.z

to

QEMU Emulator version x.yy.z

libvirt is overly sensitive to the format of this string, and barfs when
trying to parse qemu -help output. While libvirt should certainly be more
robust here, changing the output format of -version for cosmetic reasons
doesn't seem like the best idea, so let's revert the change and add a
comment explaining the issue.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 vl.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index 85bcc84..8592949 100644
--- a/vl.c
+++ b/vl.c
@@ -2012,7 +2012,9 @@ static void main_loop(void)
 
 static void version(void)
 {
-    printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
+    /* Change with care, at least libvirt parses this string and is
+     * historically sensitive to its format */
+    printf("QEMU PC emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
 }
 
 static void help(int exitcode)
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Jes Sorensen]

On 05/12/10 22:29, Cole Robinson wrote:
> Commit f75ca1ae205f24dae296c82d534c37746f87232f changed the version
> string from:
> 
> QEMU PC Emulator version x.yy.z
> 
> to
> 
> QEMU Emulator version x.yy.z
> 
> libvirt is overly sensitive to the format of this string, and barfs when
> trying to parse qemu -help output. While libvirt should certainly be more
> robust here, changing the output format of -version for cosmetic reasons
> doesn't seem like the best idea, so let's revert the change and add a
> comment explaining the issue.

Rather than this, I would prefer a -version argument that just returns
the current QEMU version string.

IMHO it's not a good approach to do static string matching.

Cheers,
Jes

[Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Cole Robinson]

On 05/12/2010 04:38 PM, Jes Sorensen wrote:
> On 05/12/10 22:29, Cole Robinson wrote:
>> Commit f75ca1ae205f24dae296c82d534c37746f87232f changed the version
>> string from:
>>
>> QEMU PC Emulator version x.yy.z
>>
>> to
>>
>> QEMU Emulator version x.yy.z
>>
>> libvirt is overly sensitive to the format of this string, and barfs when
>> trying to parse qemu -help output. While libvirt should certainly be more
>> robust here, changing the output format of -version for cosmetic reasons
>> doesn't seem like the best idea, so let's revert the change and add a
>> comment explaining the issue.
> 
> Rather than this, I would prefer a -version argument that just returns
> the current QEMU version string.
> 
> IMHO it's not a good approach to do static string matching.
> 

I agree libvirt's method is a crappy approach. Adding a proper -version
argument is certainly the way forward, but doesn't help users with
existing libvirt installations that want to use latest qemu. This is the
type of issue that libvirt devs will be fielding for months. Ideally i'd
like the order to be:

1) Apply this patch
2) Add a proper -version argument, maybe named -version_num
3) libvirt patched to use new version argument (and robustify legacy
version parsing)
4) Some reasonable amount of time from now (6 months, a year?), edit the
current -version string at will

I'd be willing to do 2 and 3 if people agree.

Thanks,
Cole

[Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Jes Sorensen]

On 05/12/10 22:48, Cole Robinson wrote:
> I agree libvirt's method is a crappy approach. Adding a proper -version
> argument is certainly the way forward, but doesn't help users with
> existing libvirt installations that want to use latest qemu. This is the
> type of issue that libvirt devs will be fielding for months. Ideally i'd
> like the order to be:
> 
> 1) Apply this patch
> 2) Add a proper -version argument, maybe named -version_num
> 3) libvirt patched to use new version argument (and robustify legacy
> version parsing)
> 4) Some reasonable amount of time from now (6 months, a year?), edit the
> current -version string at will
> 
> I'd be willing to do 2 and 3 if people agree.

Hi Cole,

I think rather than 1, it would be better to add a patch to libvirt to
catch both formats. I know Chris Lalancette already cooked up a patch
for this. Combined with the 2) patch I just posted, and 3) I think that
should take care of the problems.

Cheers,
Jes

[Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Cole Robinson]

On 05/13/2010 04:35 AM, Jes Sorensen wrote:
> On 05/12/10 22:48, Cole Robinson wrote:
>> I agree libvirt's method is a crappy approach. Adding a proper -version
>> argument is certainly the way forward, but doesn't help users with
>> existing libvirt installations that want to use latest qemu. This is the
>> type of issue that libvirt devs will be fielding for months. Ideally i'd
>> like the order to be:
>>
>> 1) Apply this patch
>> 2) Add a proper -version argument, maybe named -version_num
>> 3) libvirt patched to use new version argument (and robustify legacy
>> version parsing)
>> 4) Some reasonable amount of time from now (6 months, a year?), edit the
>> current -version string at will
>>
>> I'd be willing to do 2 and 3 if people agree.
> 
> Hi Cole,
> 
> I think rather than 1, it would be better to add a patch to libvirt to
> catch both formats. I know Chris Lalancette already cooked up a patch
> for this. Combined with the 2) patch I just posted, and 3) I think that
> should take care of the problems.
> 

It doesn't solve the problem for existing libvirt installations. It's
not uncommon for users to track just the latest kvm releases without
upgrading libvirt: any future qemu or kvm release will break every
version of libvirt that exists today. Given that unfortunate case, I
still recommend reverting the 'PC' change at least for long enough for a
few fixed libvirt releases to make it into the wild.

- Cole

[Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Jes Sorensen]

On 05/13/10 15:04, Cole Robinson wrote:
> On 05/13/2010 04:35 AM, Jes Sorensen wrote:
>> On 05/12/10 22:48, Cole Robinson wrote:
>> I think rather than 1, it would be better to add a patch to libvirt to
>> catch both formats. I know Chris Lalancette already cooked up a patch
>> for this. Combined with the 2) patch I just posted, and 3) I think that
>> should take care of the problems.
> 
> It doesn't solve the problem for existing libvirt installations. It's
> not uncommon for users to track just the latest kvm releases without
> upgrading libvirt: any future qemu or kvm release will break every
> version of libvirt that exists today. Given that unfortunate case, I
> still recommend reverting the 'PC' change at least for long enough for a
> few fixed libvirt releases to make it into the wild.

But that is no different from what we have today. Users who update their
qemu and see issues with libvirt can also be asked to update libvirt. I
have already had several cases where I needed to do that anyway.

I don't like reverting a change like this, just to schedule it to be
reapplied again later.

Cheers,
Jes

Re: [Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Daniel P. Berrange]

On Thu, May 13, 2010 at 03:07:49PM +0200, Jes Sorensen wrote:
> On 05/13/10 15:04, Cole Robinson wrote:
> > On 05/13/2010 04:35 AM, Jes Sorensen wrote:
> >> On 05/12/10 22:48, Cole Robinson wrote:
> >> I think rather than 1, it would be better to add a patch to libvirt to
> >> catch both formats. I know Chris Lalancette already cooked up a patch
> >> for this. Combined with the 2) patch I just posted, and 3) I think that
> >> should take care of the problems.
> > 
> > It doesn't solve the problem for existing libvirt installations. It's
> > not uncommon for users to track just the latest kvm releases without
> > upgrading libvirt: any future qemu or kvm release will break every
> > version of libvirt that exists today. Given that unfortunate case, I
> > still recommend reverting the 'PC' change at least for long enough for a
> > few fixed libvirt releases to make it into the wild.
> 
> But that is no different from what we have today. Users who update their
> qemu and see issues with libvirt can also be asked to update libvirt. I
> have already had several cases where I needed to do that anyway.

The general policy of QEMU has been to try and avoid  known breakage of
existing apps unless unavoidable. This change introduced 100% guarenteed
breakage of every single deployment that exists today, for the sake of
removing 2 characters from a string. I really don't think this is a good
cost/benefit tradeoff & agree with Cole that I'd like to see this reverted
for the 0.13 release, and re-considered in a later release once we've had
a chance to get a preventative fix out for libvirt using -version-string
or equivalent.

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] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Luiz Capitulino]

On Thu, 13 May 2010 14:21:02 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Thu, May 13, 2010 at 03:07:49PM +0200, Jes Sorensen wrote:
> > On 05/13/10 15:04, Cole Robinson wrote:
> > > On 05/13/2010 04:35 AM, Jes Sorensen wrote:
> > >> On 05/12/10 22:48, Cole Robinson wrote:
> > >> I think rather than 1, it would be better to add a patch to libvirt to
> > >> catch both formats. I know Chris Lalancette already cooked up a patch
> > >> for this. Combined with the 2) patch I just posted, and 3) I think that
> > >> should take care of the problems.
> > > 
> > > It doesn't solve the problem for existing libvirt installations. It's
> > > not uncommon for users to track just the latest kvm releases without
> > > upgrading libvirt: any future qemu or kvm release will break every
> > > version of libvirt that exists today. Given that unfortunate case, I
> > > still recommend reverting the 'PC' change at least for long enough for a
> > > few fixed libvirt releases to make it into the wild.
> > 
> > But that is no different from what we have today. Users who update their
> > qemu and see issues with libvirt can also be asked to update libvirt. I
> > have already had several cases where I needed to do that anyway.
> 
> The general policy of QEMU has been to try and avoid  known breakage of
> existing apps unless unavoidable. This change introduced 100% guarenteed
> breakage of every single deployment that exists today, for the sake of
> removing 2 characters from a string. I really don't think this is a good
> cost/benefit tradeoff & agree with Cole that I'd like to see this reverted
> for the 0.13 release, and re-considered in a later release once we've had
> a chance to get a preventative fix out for libvirt using -version-string
> or equivalent.

 Agreed, we should improve client's life, not contribute with additional
headaches.

 Regarding Cole's suggestion, instead of adding a new -version parameter
you could connect to QMP, get this information from it and re-run QEMU,
not sure if it's worth the trouble though.

Re: [Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Paul Brook]

> > But that is no different from what we have today. Users who update their
> > qemu and see issues with libvirt can also be asked to update libvirt. I
> > have already had several cases where I needed to do that anyway.
> 
> The general policy of QEMU has been to try and avoid  known breakage of
> existing apps unless unavoidable. This change introduced 100% guarenteed
> breakage of every single deployment that exists today, for the sake of
> removing 2 characters from a string. I really don't think this is a good
> cost/benefit tradeoff & agree with Cole that I'd like to see this reverted
> for the 0.13 release, and re-considered in a later release once we've had
> a chance to get a preventative fix out for libvirt using -version-string
> or equivalent.

For a stable release I'd agree, we shouldn't be breaking anything.
If combined with a version version bump I care a lot less.

Paul

Re: [Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Blue Swirl]

On 5/13/10, Paul Brook <paul@codesourcery.com> wrote:
> > > But that is no different from what we have today. Users who update their
>  > > qemu and see issues with libvirt can also be asked to update libvirt. I
>  > > have already had several cases where I needed to do that anyway.
>  >
>  > The general policy of QEMU has been to try and avoid  known breakage of
>  > existing apps unless unavoidable. This change introduced 100% guarenteed
>  > breakage of every single deployment that exists today, for the sake of
>  > removing 2 characters from a string. I really don't think this is a good
>  > cost/benefit tradeoff & agree with Cole that I'd like to see this reverted
>  > for the 0.13 release, and re-considered in a later release once we've had
>  > a chance to get a preventative fix out for libvirt using -version-string
>  > or equivalent.
>
>
> For a stable release I'd agree, we shouldn't be breaking anything.
>  If combined with a version version bump I care a lot less.

Fully agree.

I think even better line would be 'QEMU System Emulator..'.

Re: [Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Stuart Brady]

On Thu, May 13, 2010 at 10:20:39PM +0300, Blue Swirl wrote:
> Fully agree.
> 
> I think even better line would be 'QEMU System Emulator..'.

Silly idea perhaps, but why not include the target name, e.g. 'QEMU i386
System Emulator'?  It only seems reasonable to me for the different
binaries to produce different output!

Cheers,
-- 
Stuart Brady

[Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Anthony Liguori]

On 05/13/2010 08:07 AM, Jes Sorensen wrote:
> On 05/13/10 15:04, Cole Robinson wrote:
>    
>> On 05/13/2010 04:35 AM, Jes Sorensen wrote:
>>      
>>> On 05/12/10 22:48, Cole Robinson wrote:
>>> I think rather than 1, it would be better to add a patch to libvirt to
>>> catch both formats. I know Chris Lalancette already cooked up a patch
>>> for this. Combined with the 2) patch I just posted, and 3) I think that
>>> should take care of the problems.
>>>        
>> It doesn't solve the problem for existing libvirt installations. It's
>> not uncommon for users to track just the latest kvm releases without
>> upgrading libvirt: any future qemu or kvm release will break every
>> version of libvirt that exists today. Given that unfortunate case, I
>> still recommend reverting the 'PC' change at least for long enough for a
>> few fixed libvirt releases to make it into the wild.
>>      
> But that is no different from what we have today. Users who update their
> qemu and see issues with libvirt can also be asked to update libvirt. I
> have already had several cases where I needed to do that anyway.
>
> I don't like reverting a change like this, just to schedule it to be
> reapplied again later.
>    

The problem is, we've been down this road many times before.  Every 
time, the libvirt folks say they just need this one change for 
compatibility and keep on using the help output.  I understand that 
that's the easiest thing to do so we need to make it easier for them to 
do the right thing (which probably means making it harder to do the 
wrong thing :-)).

So I'm willing to apply this patch but only as part of a larger series 
that eliminates the need for libvirt to parse help output.  The 
important question is then, what information is libvirt getting from the 
help output today.

Regards,

Anthony Liguori

> Cheers,
> Jes
>
>    

Re: [Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Daniel P. Berrange]

On Fri, May 14, 2010 at 08:24:09AM -0500, Anthony Liguori wrote:
> On 05/13/2010 08:07 AM, Jes Sorensen wrote:
> >On 05/13/10 15:04, Cole Robinson wrote:
> >   
> >>On 05/13/2010 04:35 AM, Jes Sorensen wrote:
> >>     
> >>>On 05/12/10 22:48, Cole Robinson wrote:
> >>>I think rather than 1, it would be better to add a patch to libvirt to
> >>>catch both formats. I know Chris Lalancette already cooked up a patch
> >>>for this. Combined with the 2) patch I just posted, and 3) I think that
> >>>should take care of the problems.
> >>>       
> >>It doesn't solve the problem for existing libvirt installations. It's
> >>not uncommon for users to track just the latest kvm releases without
> >>upgrading libvirt: any future qemu or kvm release will break every
> >>version of libvirt that exists today. Given that unfortunate case, I
> >>still recommend reverting the 'PC' change at least for long enough for a
> >>few fixed libvirt releases to make it into the wild.
> >>     
> >But that is no different from what we have today. Users who update their
> >qemu and see issues with libvirt can also be asked to update libvirt. I
> >have already had several cases where I needed to do that anyway.
> >
> >I don't like reverting a change like this, just to schedule it to be
> >reapplied again later.
> >   
> 
> The problem is, we've been down this road many times before.  Every 
> time, the libvirt folks say they just need this one change for 
> compatibility and keep on using the help output.  I understand that 
> that's the easiest thing to do so we need to make it easier for them to 
> do the right thing (which probably means making it harder to do the 
> wrong thing :-)).
> 
> So I'm willing to apply this patch but only as part of a larger series 
> that eliminates the need for libvirt to parse help output.  The 
> important question is then, what information is libvirt getting from the 
> help output today.

The short answer is we extract:

 - QEMU version number
 - KVM version number (if present)
 - Probe for specific named arguments (-drive, -chardev, -device, 
   and many many more)
 - Probe for named flags (eg cache= option to -drive which appeared
   after the initial -drive arg did)

Things we use the QEMU/KVM version number for:

 - Track changes in syntax of an existing arg. eg -vnc changed
   syntax between 0.8.x and 0.9.0
 - Determine if we can use VNET_HDR flag
 - Which migration protocols are available (tcp, unix, exec, fd)
 - Whether we can use JSON mode monitor yet
 - Override use of -netdev - even though its detected in -help
   output for 0.12, we can't use it since there was no equivalent
   netdev_add command for hotplug. So we have to blacklist it till
   0.13

In the ideal world we'd never do anything based off version number,
everything would have some clear functional capability that could
be queried. We'd then only need to use version to blacklist features
we detect, but can't use for some reason.

If you want the gory details, the key method is qemudComputeCmdFlag()
in source code at line 1124:

  http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_conf.c;h=5fa8c0aa1867e1f67153d72c5b37ba09157c968e;hb=HEAD

This is where we detect each feature we care about in -help output


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] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Anthony Liguori]

On 05/14/2010 08:54 AM, Daniel P. Berrange wrote:
> On Fri, May 14, 2010 at 08:24:09AM -0500, Anthony Liguori wrote:
>    
>> On 05/13/2010 08:07 AM, Jes Sorensen wrote:
>>      
>>> On 05/13/10 15:04, Cole Robinson wrote:
>>>
>>>        
>>>> On 05/13/2010 04:35 AM, Jes Sorensen wrote:
>>>>
>>>>          
>>>>> On 05/12/10 22:48, Cole Robinson wrote:
>>>>> I think rather than 1, it would be better to add a patch to libvirt to
>>>>> catch both formats. I know Chris Lalancette already cooked up a patch
>>>>> for this. Combined with the 2) patch I just posted, and 3) I think that
>>>>> should take care of the problems.
>>>>>
>>>>>            
>>>> It doesn't solve the problem for existing libvirt installations. It's
>>>> not uncommon for users to track just the latest kvm releases without
>>>> upgrading libvirt: any future qemu or kvm release will break every
>>>> version of libvirt that exists today. Given that unfortunate case, I
>>>> still recommend reverting the 'PC' change at least for long enough for a
>>>> few fixed libvirt releases to make it into the wild.
>>>>
>>>>          
>>> But that is no different from what we have today. Users who update their
>>> qemu and see issues with libvirt can also be asked to update libvirt. I
>>> have already had several cases where I needed to do that anyway.
>>>
>>> I don't like reverting a change like this, just to schedule it to be
>>> reapplied again later.
>>>
>>>        
>> The problem is, we've been down this road many times before.  Every
>> time, the libvirt folks say they just need this one change for
>> compatibility and keep on using the help output.  I understand that
>> that's the easiest thing to do so we need to make it easier for them to
>> do the right thing (which probably means making it harder to do the
>> wrong thing :-)).
>>
>> So I'm willing to apply this patch but only as part of a larger series
>> that eliminates the need for libvirt to parse help output.  The
>> important question is then, what information is libvirt getting from the
>> help output today.
>>      
> The short answer is we extract:
>
>   - QEMU version number
>   - KVM version number (if present)
>   - Probe for specific named arguments (-drive, -chardev, -device,
>     and many many more)
>    

This can be tied to version number, no?

>   - Probe for named flags (eg cache= option to -drive which appeared
>     after the initial -drive arg did)
>    

Again, this ought to be able to be tied to version number, no?

Regards,

Anthony Liguori

> Things we use the QEMU/KVM version number for:
>
>   - Track changes in syntax of an existing arg. eg -vnc changed
>     syntax between 0.8.x and 0.9.0
>   - Determine if we can use VNET_HDR flag
>   - Which migration protocols are available (tcp, unix, exec, fd)
>   - Whether we can use JSON mode monitor yet
>   - Override use of -netdev - even though its detected in -help
>     output for 0.12, we can't use it since there was no equivalent
>     netdev_add command for hotplug. So we have to blacklist it till
>     0.13
>
> In the ideal world we'd never do anything based off version number,
> everything would have some clear functional capability that could
> be queried. We'd then only need to use version to blacklist features
> we detect, but can't use for some reason.
>
> If you want the gory details, the key method is qemudComputeCmdFlag()
> in source code at line 1124:
>
>    http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_conf.c;h=5fa8c0aa1867e1f67153d72c5b37ba09157c968e;hb=HEAD
>
> This is where we detect each feature we care about in -help output
>
>
> Regards,
> Daniel
>    

Re: [Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Daniel P. Berrange]

On Fri, May 14, 2010 at 09:22:55AM -0500, Anthony Liguori wrote:
> On 05/14/2010 08:54 AM, Daniel P. Berrange wrote:
> >On Fri, May 14, 2010 at 08:24:09AM -0500, Anthony Liguori wrote:
> >   
> >>On 05/13/2010 08:07 AM, Jes Sorensen wrote:
> >>     
> >>>On 05/13/10 15:04, Cole Robinson wrote:
> >>>
> >>>       
> >>>>On 05/13/2010 04:35 AM, Jes Sorensen wrote:
> >>>>
> >>>>         
> >>>>>On 05/12/10 22:48, Cole Robinson wrote:
> >>>>>I think rather than 1, it would be better to add a patch to libvirt to
> >>>>>catch both formats. I know Chris Lalancette already cooked up a patch
> >>>>>for this. Combined with the 2) patch I just posted, and 3) I think that
> >>>>>should take care of the problems.
> >>>>>
> >>>>>           
> >>>>It doesn't solve the problem for existing libvirt installations. It's
> >>>>not uncommon for users to track just the latest kvm releases without
> >>>>upgrading libvirt: any future qemu or kvm release will break every
> >>>>version of libvirt that exists today. Given that unfortunate case, I
> >>>>still recommend reverting the 'PC' change at least for long enough for a
> >>>>few fixed libvirt releases to make it into the wild.
> >>>>
> >>>>         
> >>>But that is no different from what we have today. Users who update their
> >>>qemu and see issues with libvirt can also be asked to update libvirt. I
> >>>have already had several cases where I needed to do that anyway.
> >>>
> >>>I don't like reverting a change like this, just to schedule it to be
> >>>reapplied again later.
> >>>
> >>>       
> >>The problem is, we've been down this road many times before.  Every
> >>time, the libvirt folks say they just need this one change for
> >>compatibility and keep on using the help output.  I understand that
> >>that's the easiest thing to do so we need to make it easier for them to
> >>do the right thing (which probably means making it harder to do the
> >>wrong thing :-)).
> >>
> >>So I'm willing to apply this patch but only as part of a larger series
> >>that eliminates the need for libvirt to parse help output.  The
> >>important question is then, what information is libvirt getting from the
> >>help output today.
> >>     
> >The short answer is we extract:
> >
> >  - QEMU version number
> >  - KVM version number (if present)
> >  - Probe for specific named arguments (-drive, -chardev, -device,
> >    and many many more)
> >   
> 
> This can be tied to version number, no?
> 
> >  - Probe for named flags (eg cache= option to -drive which appeared
> >    after the initial -drive arg did)
> >   
> 
> Again, this ought to be able to be tied to version number, no?

It is preferable to query the explicit capability wanted, because
version numbers are useless when distros backport features, and
when kvm is using kvm-XXX numbering that's completely unrelated
to the qemu-X.Y.Z numbering. Version numbers are the last resort
if there's no alternative way to determine the capability


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] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Anthony Liguori]

On 05/14/2010 09:42 AM, Daniel P. Berrange wrote:
>
> It is preferable to query the explicit capability wanted, because
> version numbers are useless when distros backport features,

Unless distros add their own release number to the version information 
and libvirt learns about the features they add.

I think that's the sanest approach.  Just because a distro backports a 
feature doesn't mean that it behaves like the upstream version.  libvirt 
really needs to treat distro packages as separate entities from upstream 
IMHO.

Regards,

Anthony Liguori

>   and
> when kvm is using kvm-XXX numbering that's completely unrelated
> to the qemu-X.Y.Z numbering. Version numbers are the last resort
> if there's no alternative way to determine the capability
>
>
> Daniel
>    

Re: [Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Daniel P. Berrange]

On Fri, May 14, 2010 at 09:52:53AM -0500, Anthony Liguori wrote:
> On 05/14/2010 09:42 AM, Daniel P. Berrange wrote:
> >
> >It is preferable to query the explicit capability wanted, because
> >version numbers are useless when distros backport features,
> 
> Unless distros add their own release number to the version information 
> and libvirt learns about the features they add.

And features removed / compiled out. eg SDL removed, es1370 sound driver
removed, etc, etc. 

Version numbers are a really bad way to determine availability of
functionality. You end up having to build a giant matrix of features
vs version numbers, which will be outdated the moment its created.

If you want feature X, you need to be able to check for prescense of 
feature X, not infer it from another piece of information that is
only loosely related.

eg, if you want to use a kernel filesystem, you don't perform a check
a kernel version number, because its inevitably wrong. You check 
/proc/filesystems to see if the filesystem is supported.

> I think that's the sanest approach.  Just because a distro backports a 
> feature doesn't mean that it behaves like the upstream version.  libvirt 
> really needs to treat distro packages as separate entities from upstream 
> IMHO.

If a distro changes the semantics of a upstream feature everyone looses
no matter which way you look at it.

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] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Anthony Liguori]

On 05/14/2010 10:02 AM, Daniel P. Berrange wrote:
> On Fri, May 14, 2010 at 09:52:53AM -0500, Anthony Liguori wrote:
>    
>> On 05/14/2010 09:42 AM, Daniel P. Berrange wrote:
>>      
>>> It is preferable to query the explicit capability wanted, because
>>> version numbers are useless when distros backport features,
>>>        
>> Unless distros add their own release number to the version information
>> and libvirt learns about the features they add.
>>      
> And features removed / compiled out. eg SDL removed, es1370 sound driver
> removed, etc, etc.
>
> Version numbers are a really bad way to determine availability of
> functionality. You end up having to build a giant matrix of features
> vs version numbers, which will be outdated the moment its created.
>    

You need version numbers to understand behavior.  If you also want to 
know compile settings, we need the equivalent of /proc/config (a simple 
-build-config command could output our build config).

> If you want feature X, you need to be able to check for prescense of
> feature X, not infer it from another piece of information that is
> only loosely related.
>    

You assume that feature X is idempotent across multiple downstreams.  
That's not a realistic assumption IMHO.

> eg, if you want to use a kernel filesystem, you don't perform a check
> a kernel version number, because its inevitably wrong. You check
> /proc/filesystems to see if the filesystem is supported.
>    

And we have this today with QMP.  It provides a mechanism to query 
capabilities.  Fundamentally, this is a bootstrap problem and the best 
way to bootstrap is with a version number.

>> I think that's the sanest approach.  Just because a distro backports a
>> feature doesn't mean that it behaves like the upstream version.  libvirt
>> really needs to treat distro packages as separate entities from upstream
>> IMHO.
>>      
> If a distro changes the semantics of a upstream feature everyone looses
> no matter which way you look at it.
>    

It's a reality so we have to deal with it.

Regards,

Anthony Liguori

> Daniel
>    

[Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Anthony Liguori]

On 05/12/2010 03:48 PM, Cole Robinson wrote:
> On 05/12/2010 04:38 PM, Jes Sorensen wrote:
>    
>> On 05/12/10 22:29, Cole Robinson wrote:
>>      
>>> Commit f75ca1ae205f24dae296c82d534c37746f87232f changed the version
>>> string from:
>>>
>>> QEMU PC Emulator version x.yy.z
>>>
>>> to
>>>
>>> QEMU Emulator version x.yy.z
>>>
>>> libvirt is overly sensitive to the format of this string, and barfs when
>>> trying to parse qemu -help output. While libvirt should certainly be more
>>> robust here, changing the output format of -version for cosmetic reasons
>>> doesn't seem like the best idea, so let's revert the change and add a
>>> comment explaining the issue.
>>>        
>> Rather than this, I would prefer a -version argument that just returns
>> the current QEMU version string.
>>
>> IMHO it's not a good approach to do static string matching.
>>
>>      
> I agree libvirt's method is a crappy approach. Adding a proper -version
> argument is certainly the way forward, but doesn't help users with
> existing libvirt installations that want to use latest qemu. This is the
> type of issue that libvirt devs will be fielding for months. Ideally i'd
> like the order to be:
>
> 1) Apply this patch
> 2) Add a proper -version argument, maybe named -version_num
> 3) libvirt patched to use new version argument (and robustify legacy
> version parsing)
> 4) Some reasonable amount of time from now (6 months, a year?), edit the
> current -version string at will
>
> I'd be willing to do 2 and 3 if people agree.
>    

I'd like to see exactly what information libvirt needs so that we can 
agree on the proper interface to obtain it.  My concern is that we'd 
have the same problem with the version string.

This topic has come up many times in the past.  We don't support the 
format of help output and libvirt shouldn't be using it.  That's been 
expressed many, many times in the past and libvirt has not stopped using it.

I think applying this patch is okay but only as part of a larger series 
that fixes the problem properly.

Regards,

Anthony Liguori

> Thanks,
> Cole
>
>    

Re: [Qemu-devel] Re: [PATCH] Revive -version 'QEMU PC Emulator...'

[Daniel P. Berrange]

On Fri, May 14, 2010 at 08:17:50AM -0500, Anthony Liguori wrote:
> On 05/12/2010 03:48 PM, Cole Robinson wrote:
> >On 05/12/2010 04:38 PM, Jes Sorensen wrote:
> >   
> >>On 05/12/10 22:29, Cole Robinson wrote:
> >>     
> >>>Commit f75ca1ae205f24dae296c82d534c37746f87232f changed the version
> >>>string from:
> >>>
> >>>QEMU PC Emulator version x.yy.z
> >>>
> >>>to
> >>>
> >>>QEMU Emulator version x.yy.z
> >>>
> >>>libvirt is overly sensitive to the format of this string, and barfs when
> >>>trying to parse qemu -help output. While libvirt should certainly be more
> >>>robust here, changing the output format of -version for cosmetic reasons
> >>>doesn't seem like the best idea, so let's revert the change and add a
> >>>comment explaining the issue.
> >>>       
> >>Rather than this, I would prefer a -version argument that just returns
> >>the current QEMU version string.
> >>
> >>IMHO it's not a good approach to do static string matching.
> >>
> >>     
> >I agree libvirt's method is a crappy approach. Adding a proper -version
> >argument is certainly the way forward, but doesn't help users with
> >existing libvirt installations that want to use latest qemu. This is the
> >type of issue that libvirt devs will be fielding for months. Ideally i'd
> >like the order to be:
> >
> >1) Apply this patch
> >2) Add a proper -version argument, maybe named -version_num
> >3) libvirt patched to use new version argument (and robustify legacy
> >version parsing)
> >4) Some reasonable amount of time from now (6 months, a year?), edit the
> >current -version string at will
> >
> >I'd be willing to do 2 and 3 if people agree.
> >   
> 
> I'd like to see exactly what information libvirt needs so that we can 
> agree on the proper interface to obtain it.  My concern is that we'd 
> have the same problem with the version string.
> 
> This topic has come up many times in the past.  We don't support the 
> format of help output and libvirt shouldn't be using it.  That's been 
> expressed many, many times in the past and libvirt has not stopped using it.

We'd love to use something else, but since the 'something else' doesn't
yet exist we've had no choice but to continue groking the help output. 
The discussion about a formally structure -query-version command is a step
in the right direction here. A similar option for querying supported command
line args would remove the need to rely on -help output.

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