[Qemu-devel] [PATCH] Introduce a -libvirt-caps flag as a stop-gap

[Qemu-devel] [PATCH] Introduce a -libvirt-caps flag as a stop-gap

[Anthony Liguori]

Today libvirt parses -help output to attempt to enumerate capabilities.  This
is very broken and has led to multiple failures.  Since libvirt is an important
management interface to QEMU, we need to do a better job giving them the ability
to detect what a QEMU executable supports.  Right now, we keep fixing up help
output to appease it's parsing code but this is undesirable.

The Right Solution is to introduce a robust capabilities advertisement that
enumerates every feature we have.  As with most Right Solutions, we don't have
mergable code today and it's unclear that we'll get there by the next release.

This patch introduces an incremental solution of just spitting out the handful
of capabilities libvirt is probing for today.  This interface will need to
remain forever but can stop being updated once we have a Right Solution.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/qemu-options.hx b/qemu-options.hx
index 40cee70..a618914 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2235,7 +2235,26 @@ Normally QEMU loads a configuration file from @var{sysconfdir}/qemu.conf and
 option will prevent QEMU from loading these configuration files at startup.
 ETEXI
 
+DEF("libvirt-caps", 0, QEMU_OPTION_libvirt_caps,
+    "-libvirt-caps   output libvirt-specific capabilities\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -libvirt-caps
+@findex -libvirt-caps
+Output a common separate list of capabilities that this version of QEMU
+supports and exit.  This interface specifically exists for libvirt's use as an
+intermediate solution until we support a full capabilities system.  One this
+capabilities system exist, this option's output should never change.
+
+The format out this command is:
+
+version: VERSION
+package: PACKAGE
+caps: CAP1[,CAP2...]
+ETEXI
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
 ETEXI
+
diff --git a/vl.c b/vl.c
index ba6ee11..8fe354d 100644
--- a/vl.c
+++ b/vl.c
@@ -2616,6 +2616,14 @@ int main(int argc, char **argv, char **envp)
                     fclose(fp);
                     break;
                 }
+            case QEMU_OPTION_libvirt_caps:
+                printf("version: " QEMU_VERSION "\n"
+                       "package: " QEMU_PKGVERSION "\n"
+                       "caps: name,enable-kvm,no-reboot,uuid,xen-domid,drive"
+                       ",cache-v2,format,vga,serial,mem-path,chardev,balloon"
+                       ",device,rtc,netdev,sdl,topology\n");
+                exit(0);
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }
-- 
1.7.0.4

[Qemu-devel] Re: [PATCH] Introduce a -libvirt-caps flag as a stop-gap

[Daniel P. Berrange]

On Tue, Jul 27, 2010 at 10:55:03AM -0500, Anthony Liguori wrote:
> Today libvirt parses -help output to attempt to enumerate capabilities.  This
> is very broken and has led to multiple failures.  Since libvirt is an important
> management interface to QEMU, we need to do a better job giving them the ability
> to detect what a QEMU executable supports.  Right now, we keep fixing up help
> output to appease it's parsing code but this is undesirable.
> 
> The Right Solution is to introduce a robust capabilities advertisement that
> enumerates every feature we have.  As with most Right Solutions, we don't have
> mergable code today and it's unclear that we'll get there by the next release.
> 
> This patch introduces an incremental solution of just spitting out the handful
> of capabilities libvirt is probing for today.  This interface will need to
> remain forever but can stop being updated once we have a Right Solution.

This isn't really workable because it only encodes the subset of things
that libvirt currently looks for. If someone comes along with a libvirt
patch for a new features that is already supported by QEMU, but isn't
in this simple output, we're stuck. Adding a one-off special case for
the 0.13 release that we know will be obsolete in 0.14, and obviously
can't be used in qemu < 0.12 is not really a worthwhile use of time.
libvirt has to keep supporting help parsing indefinitely for <= 0.12
releases & expects to support a new extensible & flexible approach for
qemu >= 0.14. Adding a special case that both libvirt & qemu have to
support indefinitely for 0.13 is not really very nice.

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] Introduce a -libvirt-caps flag as a stop-gap

[Anthony Liguori]

On 07/27/2010 11:09 AM, Daniel P. Berrange wrote:
> On Tue, Jul 27, 2010 at 10:55:03AM -0500, Anthony Liguori wrote:
>    
>> Today libvirt parses -help output to attempt to enumerate capabilities.  This
>> is very broken and has led to multiple failures.  Since libvirt is an important
>> management interface to QEMU, we need to do a better job giving them the ability
>> to detect what a QEMU executable supports.  Right now, we keep fixing up help
>> output to appease it's parsing code but this is undesirable.
>>
>> The Right Solution is to introduce a robust capabilities advertisement that
>> enumerates every feature we have.  As with most Right Solutions, we don't have
>> mergable code today and it's unclear that we'll get there by the next release.
>>
>> This patch introduces an incremental solution of just spitting out the handful
>> of capabilities libvirt is probing for today.  This interface will need to
>> remain forever but can stop being updated once we have a Right Solution.
>>      
> This isn't really workable because it only encodes the subset of things
> that libvirt currently looks for. If someone comes along with a libvirt
> patch for a new features that is already supported by QEMU, but isn't
> in this simple output, we're stuck.

Yup.  You'll need to decide up front if you want to probe for a feature 
when it's introduced and have something added to capabilities.

This is simple though.  A few weeks before 0.14 is released, go through 
the change log, and anything that looks interesting, add a cap flag.

>   Adding a one-off special case for
> the 0.13 release that we know will be obsolete in 0.14

IIF capabilities gets merged by 0.14.  I'd certainly like it to, but I'd 
prefer to hedge my bets.

Here are the possible things we can do:

1) merge -libvirt-caps as an intermediate solution, stop caring about 
-help changes, when full caps are introduced, stop updating -libvirt-caps

2) don't merge -libvirt-caps, stop caring about -help changes, put 
everything on getting full caps merged by 0.14

3) don't merge -libvirt-caps, care about making -help changes, use -help 
as the caps mechanism until full caps get merged

We can't do (3).  I'm going to revert the -help changes for 0.13 so that 
old versions of libvirt work but not for master.

(2) makes me pretty uncomfortable because it implies either (a) delay 
0.14 until full caps are ready (b) ship 0.14 such that libvirt is 
totally broken.

(1) isn't ideal, I'll freely admit, but it's a workable intermediate 
solution.

Regards,

Anthony Liguori

> , and obviously
> can't be used in qemu<  0.12 is not really a worthwhile use of time.
> libvirt has to keep supporting help parsing indefinitely for<= 0.12
> releases&  expects to support a new extensible&  flexible approach for
> qemu>= 0.14. Adding a special case that both libvirt&  qemu have to
> support indefinitely for 0.13 is not really very nice.
>
> Daniel
>    

Re: [Qemu-devel] Re: [PATCH] Introduce a -libvirt-caps flag as a stop-gap

[Daniel P. Berrange]

On Tue, Jul 27, 2010 at 11:38:04AM -0500, Anthony Liguori wrote:
> On 07/27/2010 11:09 AM, Daniel P. Berrange wrote:
> >On Tue, Jul 27, 2010 at 10:55:03AM -0500, Anthony Liguori wrote:
> >   
> >>Today libvirt parses -help output to attempt to enumerate capabilities.  
> >>This
> >>is very broken and has led to multiple failures.  Since libvirt is an 
> >>important
> >>management interface to QEMU, we need to do a better job giving them the 
> >>ability
> >>to detect what a QEMU executable supports.  Right now, we keep fixing up 
> >>help
> >>output to appease it's parsing code but this is undesirable.
> >>
> >>The Right Solution is to introduce a robust capabilities advertisement 
> >>that
> >>enumerates every feature we have.  As with most Right Solutions, we don't 
> >>have
> >>mergable code today and it's unclear that we'll get there by the next 
> >>release.
> >>
> >>This patch introduces an incremental solution of just spitting out the 
> >>handful
> >>of capabilities libvirt is probing for today.  This interface will need to
> >>remain forever but can stop being updated once we have a Right Solution.
> >>     
> >This isn't really workable because it only encodes the subset of things
> >that libvirt currently looks for. If someone comes along with a libvirt
> >patch for a new features that is already supported by QEMU, but isn't
> >in this simple output, we're stuck.
> 
> Yup.  You'll need to decide up front if you want to probe for a feature 
> when it's introduced and have something added to capabilities.
> 
> This is simple though.  A few weeks before 0.14 is released, go through 
> the change log, and anything that looks interesting, add a cap flag.

That doesn't work for features which already exist in QEMU which are
not yet supported in libvirt. eg consider QEMU 0.13 is released, and
then we want to add a new feature to libvirt a month later. We can't
simply add something extra to the capabilities because QEMU is already
released at this point. There is a large amount of stuff that falls
into this category.

> >  Adding a one-off special case for
> >the 0.13 release that we know will be obsolete in 0.14
> 
> IIF capabilities gets merged by 0.14.  I'd certainly like it to, but I'd 
> prefer to hedge my bets.
> 
> Here are the possible things we can do:
> 
> 1) merge -libvirt-caps as an intermediate solution, stop caring about 
> -help changes, when full caps are introduced, stop updating -libvirt-caps
> 
> 2) don't merge -libvirt-caps, stop caring about -help changes, put 
> everything on getting full caps merged by 0.14
> 
> 3) don't merge -libvirt-caps, care about making -help changes, use -help 
> as the caps mechanism until full caps get merged
> 
> We can't do (3).  I'm going to revert the -help changes for 0.13 so that 
> old versions of libvirt work but not for master.
> 
> (2) makes me pretty uncomfortable because it implies either (a) delay 
> 0.14 until full caps are ready (b) ship 0.14 such that libvirt is 
> totally broken.
> 
> (1) isn't ideal, I'll freely admit, but it's a workable intermediate 
> solution.

It offers significantly less information that the existing -help
data, so I don't think it is workable as a replacement. We'd get
into the bad situation where we could support a feature in 0.12
but not in 0.13, unless we went back to using -help output again.

If we're going for a short term hack, then how about a combination
of 

http://www.mail-archive.com/qemu-devel@nongnu.org/msg34944.html
http://www.mail-archive.com/qemu-devel@nongnu.org/msg34960.html

so that we have the same level of information as '-help' but in
a more stable & machine friendly format. 

As we add further patches for capabilities, we'll migrate 
away from the 'query-help' data and into the other capabilities
commands "query-netdevtypes" 'query-config' etc.

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] Introduce a -libvirt-caps flag as a stop-gap

[Anthony Liguori]

On 07/27/2010 12:00 PM, Daniel P. Berrange wrote:
>> Yup.  You'll need to decide up front if you want to probe for a feature
>> when it's introduced and have something added to capabilities.
>>
>> This is simple though.  A few weeks before 0.14 is released, go through
>> the change log, and anything that looks interesting, add a cap flag.
>>      
> That doesn't work for features which already exist in QEMU which are
> not yet supported in libvirt. eg consider QEMU 0.13 is released, and
> then we want to add a new feature to libvirt a month later.

Right.  So sit down and look at the 0.13 changelog and if there's any 
features that you think you might want to support at some point in time, 
add a capability.

>   We can't
> simply add something extra to the capabilities because QEMU is already
> released at this point. There is a large amount of stuff that falls
> into this category.
>    

It doesn't have to be perfect, it just has to be good enough.

>>>   Adding a one-off special case for
>>> the 0.13 release that we know will be obsolete in 0.14
>>>        
>> IIF capabilities gets merged by 0.14.  I'd certainly like it to, but I'd
>> prefer to hedge my bets.
>>
>> Here are the possible things we can do:
>>
>> 1) merge -libvirt-caps as an intermediate solution, stop caring about
>> -help changes, when full caps are introduced, stop updating -libvirt-caps
>>
>> 2) don't merge -libvirt-caps, stop caring about -help changes, put
>> everything on getting full caps merged by 0.14
>>
>> 3) don't merge -libvirt-caps, care about making -help changes, use -help
>> as the caps mechanism until full caps get merged
>>
>> We can't do (3).  I'm going to revert the -help changes for 0.13 so that
>> old versions of libvirt work but not for master.
>>
>> (2) makes me pretty uncomfortable because it implies either (a) delay
>> 0.14 until full caps are ready (b) ship 0.14 such that libvirt is
>> totally broken.
>>
>> (1) isn't ideal, I'll freely admit, but it's a workable intermediate
>> solution.
>>      
> It offers significantly less information that the existing -help
> data, so I don't think it is workable as a replacement. We'd get
> into the bad situation where we could support a feature in 0.12
> but not in 0.13, unless we went back to using -help output again.
>
> If we're going for a short term hack, then how about a combination
> of
>
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg34944.html
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg34960.html
>    

Would have failed in exactly the same way that the current -help parsing 
fails.  The description of an argument in the help text is not a 
capabilities string.

Regards,

Anthony Liguori

> so that we have the same level of information as '-help' but in
> a more stable&  machine friendly format.
>
> As we add further patches for capabilities, we'll migrate
> away from the 'query-help' data and into the other capabilities
> commands "query-netdevtypes" 'query-config' etc.
>
> Daniel
>    

Re: [Qemu-devel] Re: [PATCH] Introduce a -libvirt-caps flag as a stop-gap

[Daniel P. Berrange]

On Tue, Jul 27, 2010 at 12:20:55PM -0500, Anthony Liguori wrote:
> On 07/27/2010 12:00 PM, Daniel P. Berrange wrote:
> >>Yup.  You'll need to decide up front if you want to probe for a feature
> >>when it's introduced and have something added to capabilities.
> >>
> >>This is simple though.  A few weeks before 0.14 is released, go through
> >>the change log, and anything that looks interesting, add a cap flag.
> >>     
> >That doesn't work for features which already exist in QEMU which are
> >not yet supported in libvirt. eg consider QEMU 0.13 is released, and
> >then we want to add a new feature to libvirt a month later.
> 
> Right.  So sit down and look at the 0.13 changelog and if there's any 
> features that you think you might want to support at some point in time, 
> add a capability.

Not really practical - pretty much anything is a candidate because
we want to support as many of QEMU features as possible.

> >It offers significantly less information that the existing -help
> >data, so I don't think it is workable as a replacement. We'd get
> >into the bad situation where we could support a feature in 0.12
> >but not in 0.13, unless we went back to using -help output again.
> >
> >If we're going for a short term hack, then how about a combination
> >of
> >
> >http://www.mail-archive.com/qemu-devel@nongnu.org/msg34944.html
> >http://www.mail-archive.com/qemu-devel@nongnu.org/msg34960.html
> >   
> 
> Would have failed in exactly the same way that the current -help parsing 
> fails.  The description of an argument in the help text is not a 
> capabilities string.

This particular case of cache modes might have failed, but in the
broader picture it would have been more reliable. The vasty 
majority of the time, we only check whether a particular named
argument exists. This patch would make it very reliable todo so
because you could simply extract a single named field from the
JSON doc. The cases where we looked at the parameter options 
would have been improved a little, but still be potentially fragile.
Checking for version number would also be improved. So overall 
while obviously not perfect, it would be significantly better than
the solution today and using an approach that isn't a complete
throwaway solution.

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] Introduce a -libvirt-caps flag as a stop-gap

[Anthony Liguori]

On 07/28/2010 04:53 AM, Daniel P. Berrange wrote:
> On Tue, Jul 27, 2010 at 12:20:55PM -0500, Anthony Liguori wrote:
>    
>> On 07/27/2010 12:00 PM, Daniel P. Berrange wrote:
>>      
>>>> Yup.  You'll need to decide up front if you want to probe for a feature
>>>> when it's introduced and have something added to capabilities.
>>>>
>>>> This is simple though.  A few weeks before 0.14 is released, go through
>>>> the change log, and anything that looks interesting, add a cap flag.
>>>>
>>>>          
>>> That doesn't work for features which already exist in QEMU which are
>>> not yet supported in libvirt. eg consider QEMU 0.13 is released, and
>>> then we want to add a new feature to libvirt a month later.
>>>        
>> Right.  So sit down and look at the 0.13 changelog and if there's any
>> features that you think you might want to support at some point in time,
>> add a capability.
>>      
> Not really practical - pretty much anything is a candidate because
> we want to support as many of QEMU features as possible.
>
>    
>>> It offers significantly less information that the existing -help
>>> data, so I don't think it is workable as a replacement. We'd get
>>> into the bad situation where we could support a feature in 0.12
>>> but not in 0.13, unless we went back to using -help output again.
>>>
>>> If we're going for a short term hack, then how about a combination
>>> of
>>>
>>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg34944.html
>>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg34960.html
>>>
>>>        
>> Would have failed in exactly the same way that the current -help parsing
>> fails.  The description of an argument in the help text is not a
>> capabilities string.
>>      
> This particular case of cache modes might have failed, but in the
> broader picture it would have been more reliable. The vasty
> majority of the time, we only check whether a particular named
> argument exists. This patch would make it very reliable todo so
> because you could simply extract a single named field from the
> JSON doc. The cases where we looked at the parameter options
> would have been improved a little, but still be potentially fragile.
> Checking for version number would also be improved. So overall
> while obviously not perfect, it would be significantly better than
> the solution today and using an approach that isn't a complete
> throwaway solution.
>    

I'd be willing to spit out the option names minus the help 
descriptions.  The option names are part of a supported interface so 
there's no harm in exposing an interface for that.

But I think libvirt really needs option names + indication when we've 
added parameters to an option, right?

Regards,

Anthony Liguori

> Regards,
> Daniel
>    

Re: [Qemu-devel] Re: [PATCH] Introduce a -libvirt-caps flag as a stop-gap

[Chris Wright]

* Anthony Liguori (anthony@codemonkey.ws) wrote:
> Here are the possible things we can do:
> 
> 1) merge -libvirt-caps as an intermediate solution, stop caring
> about -help changes, when full caps are introduced, stop updating
> -libvirt-caps
> 
> 2) don't merge -libvirt-caps, stop caring about -help changes, put
> everything on getting full caps merged by 0.14
> 
> 3) don't merge -libvirt-caps, care about making -help changes, use
> -help as the caps mechanism until full caps get merged

3.5) same as 3) + add test case to qemu to test that -help parser from
libvirt isn't busted.

> We can't do (3).  I'm going to revert the -help changes for 0.13 so
> that old versions of libvirt work but not for master.

I suspect that if the breakage is seen, it'd be easy to fix.  Adding new
help items won't be the problem, just the subtle changes to the existing
output.  Suck?  Yes.  Workable until full caps?  Think so.

thanks,
-chris

Re: [Qemu-devel] Re: [PATCH] Introduce a -libvirt-caps flag as a stop-gap

[Avi Kivity]

  On 07/27/2010 07:38 PM, Anthony Liguori wrote:
> I'm going to revert the -help changes for 0.13 so that old versions of 
> libvirt work but not for master.

What is the goal here?   Make qemu.git explicitly be unusable via libvirt?

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

Re: [Qemu-devel] [PATCH] Introduce a -libvirt-caps flag as a stop-gap

[Amit Shah]

On (Tue) Jul 27 2010 [10:55:03], Anthony Liguori wrote:
> +            case QEMU_OPTION_libvirt_caps:
> +                printf("version: " QEMU_VERSION "\n"
> +                       "package: " QEMU_PKGVERSION "\n"
> +                       "caps: name,enable-kvm,no-reboot,uuid,xen-domid,drive"
> +                       ",cache-v2,format,vga,serial,mem-path,chardev,balloon"
> +                       ",device,rtc,netdev,sdl,topology\n");
> +                exit(0);
> +                break;

This can't work; people have to remember not only to update
documentation but also this case here to ensure libvirt works fine.

There are some other options that might work:
- making such a list by taking the savevm section name and version
  number for each device
- The parameters supported by devices registered with qdev (and their
  defaults)
- etc.

But basically this means libvirt will have to do more work now and also
add support for capabilities later. We can instead just keep 0.13 as it
is and move to capabilities for 0.14.

		Amit