[Qemu-devel] KVM call minutes for 2013-04-23

[Qemu-devel] KVM call minutes for 2013-04-23

[Juan Quintela]

* 1.5 pending patches (paolo)
  anthony thinks nothing big is outstanding
  rdma: not probably for this release,  too big change on migration
  cpu-hotplug: andreas expect to get it for 1.5


* What can libvirt expect in 1.5 for introspection of command-line support?

  command extensions?  libvirt want then
* What are the rules for adding optional parameters to existing QMP
  commands?  Would it help if we had introspection of QMP commands?

  what are the options that each command support.

  command line could work for 1.5
    if we got patches on the next 2 days we can get it.
  rest of introspection need 1.6
    it is "challenging"
    we are interesting into feature introspection
    and comand extensions?
    one command to return the schema?
  if we change a command,  how we change the interface without
  changing the c-api?

  we can change "drive_mirror" to use a new command to see if there
  are the new features.

  if we have a stable c-api we can do test cases that work. 


Eric will complete this with his undrestanding from libvirt point of
view.

Later,  Juan.

Re: [Qemu-devel] KVM call minutes for 2013-04-23

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 4657 bytes --]

On 04/23/2013 08:45 AM, Juan Quintela wrote:
> 
> * 1.5 pending patches (paolo)
>   anthony thinks nothing big is outstanding
>   rdma: not probably for this release,  too big change on migration
>   cpu-hotplug: andreas expect to get it for 1.5
> 
> 
> * What can libvirt expect in 1.5 for introspection of command-line support?
>   command extensions?  libvirt want then
> * What are the rules for adding optional parameters to existing QMP
>   commands?  Would it help if we had introspection of QMP commands?
>   what are the options that each command support.
> 
>   command line could work for 1.5
>     if we got patches on the next 2 days we can get it.

Goal is to provide a QMP command that provides JSON representation of
command line options; I will help review whatever is posted to make sure
we like the interface.  Anthony agreed the implementation should be
relatively straightforward and okay to add after soft freeze (but must
be before hard freeze).  Libvirt has some code that would like to make
use of the new command-line introspection; Osier will probably be the
first libvirt developer taking advantage of it - if we can swing it,
we'd like libvirt 1.0.5 to use the new command (libvirt freezes this
weekend for a May 2 release).

>   rest of introspection need 1.6
>     it is "challenging"
>     we are interesting into feature introspection
>     and comand extensions?
>     one command to return the schema?

Anthony was okay with the idea of a full JSON introspection of all QMP
commands, but it is probably too big to squeeze into 1.5 timeframe.
Furthermore, while the command will be useful, we should always be
thinking about API - having to parse through JSON to see if a feature is
present is not always the nicest interface; when adding a new feature,
consider improving an existing query-* or adding a counterpart new
query-* command that makes it much easier to tell if a feature is
available, without having to resort to a QMP introspection.

>   if we change a command,  how we change the interface without
>   changing the c-api?

c-api is not yet a strong consideration (but see [1] below).  Also,
there may be ways to design a C api that is robust to extensions (but
that means designing it into the QMP up front when adding a new
command); there has been some list traffic on this thought.

More importantly, adding an optional parameter to an existing command is
not okay unless something else is also available to tell whether the
feature is usable - QMP introspection would solve this, but is not
necessarily the most elegant way.  For now, while adding QMP
introspection is a good idea, we still want case-by-case review of any
command extensions.

> 
>   we can change "drive_mirror" to use a new command to see if there
>   are the new features.

drive-mirror changed in 1.4 to add optional buf-size parameter; right
now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
granularity) because there is no introspection and no query-* command
that witnesses that the feature is present.  Idea was that we need to
add a new query-drive-mirror-capabilities (name subject to bikeshedding)
command into 1.5 that would let libvirt know that buf-size/granularity
is usable (done right, it would also prevent the situation of buf-size
being a write-only interface where it is set when starting the mirror
but can not be queried later to see what size is in use).

Unclear whether anyone was signing up to tackle the addition of a query
command counterpart for drive-mirror in time for 1.5.

> 
>   if we have a stable c-api we can do test cases that work. 

Having such a testsuite would make a stable C API more important.

> 
> Eric will complete this with his undrestanding from libvirt point of
> view.

Also under discussion: the existing QMP 'screendump' command is not
ideal (not extensible, doesn't allow fd passing, hard-coded output
format).  This was used as an example command that should not be
extended until we have appropriate feature detection in place; probably
easier to add a new QMP command than to add parameters to the existing
one.  At any rate, we're late enough that 'screendump' probably won't be
improved in 1.5, so we have the full 1.6 cycle to get it right.

Not on the phone call, but a recent mail that is related to the topic -
feature detection of whether dump-guest-memory supports paging:
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg04613.html

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] KVM call minutes for 2013-04-23

[Stefan Hajnoczi]

On Tue, Apr 23, 2013 at 10:06:41AM -0600, Eric Blake wrote:
> On 04/23/2013 08:45 AM, Juan Quintela wrote:
> >   we can change "drive_mirror" to use a new command to see if there
> >   are the new features.
> 
> drive-mirror changed in 1.4 to add optional buf-size parameter; right
> now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
> granularity) because there is no introspection and no query-* command
> that witnesses that the feature is present.  Idea was that we need to
> add a new query-drive-mirror-capabilities (name subject to bikeshedding)
> command into 1.5 that would let libvirt know that buf-size/granularity
> is usable (done right, it would also prevent the situation of buf-size
> being a write-only interface where it is set when starting the mirror
> but can not be queried later to see what size is in use).
> 
> Unclear whether anyone was signing up to tackle the addition of a query
> command counterpart for drive-mirror in time for 1.5.

Seems like the trivial solution is a query-command-capabilities QMP
command.

  query-command-capabilities drive-mirror
  => ['buf-size']

It should only be a few lines of code and can be used for other commands
that add optional parameters in the future.  In other words:

typedef struct mon_cmd_t {
    ...
    const char **capabilities; /* drive-mirror uses ["buf-size", NULL] */
};

> > 
> >   if we have a stable c-api we can do test cases that work. 
> 
> Having such a testsuite would make a stable C API more important.

Writing tests in Python has been productive, see qemu-iotests 041 and
friends.  The tests spawn QEMU guests and use QMP to interact:

  result = self.vm.qmp('query-block')
  self.assert_qmp(result, 'return[0]/inserted/file', target_img)

Using this XPath-style syntax it's very easy to access the JSON.

QEMU users tend not to use C, except libvirt.  Even libvirt implements
the QMP protocol dynamically and can handle optional arguments well.

I don't think a static C API makes sense when we have an extensible JSON
protocol.  Let's use the extensibility to our advantage.

Stefan

Re: [Qemu-devel] KVM call minutes for 2013-04-23

[Luiz Capitulino]

On Wed, 24 Apr 2013 10:03:21 +0200
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Tue, Apr 23, 2013 at 10:06:41AM -0600, Eric Blake wrote:
> > On 04/23/2013 08:45 AM, Juan Quintela wrote:
> > >   we can change "drive_mirror" to use a new command to see if there
> > >   are the new features.
> > 
> > drive-mirror changed in 1.4 to add optional buf-size parameter; right
> > now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
> > granularity) because there is no introspection and no query-* command
> > that witnesses that the feature is present.  Idea was that we need to
> > add a new query-drive-mirror-capabilities (name subject to bikeshedding)
> > command into 1.5 that would let libvirt know that buf-size/granularity
> > is usable (done right, it would also prevent the situation of buf-size
> > being a write-only interface where it is set when starting the mirror
> > but can not be queried later to see what size is in use).
> > 
> > Unclear whether anyone was signing up to tackle the addition of a query
> > command counterpart for drive-mirror in time for 1.5.
> 
> Seems like the trivial solution is a query-command-capabilities QMP
> command.
> 
>   query-command-capabilities drive-mirror
>   => ['buf-size']
> 
> It should only be a few lines of code and can be used for other commands
> that add optional parameters in the future.  In other words:

IMO, a separate command is better because we'll return CommandNotFound error
if the command doesn't exist. If we add query-command-capabilities we'd need
a new error class, otherwise a client won't be able to tell if the command
passed as argument exists.

Besides, separate commands tend to be simpler; and we already have
query-migrate-capabilities anyway.

The only disadvantage is some duplication and an increase in the number of
commands, but I don't think this is avoidable.

> typedef struct mon_cmd_t {
>     ...
>     const char **capabilities; /* drive-mirror uses ["buf-size", NULL] */
> };
> 
> > > 
> > >   if we have a stable c-api we can do test cases that work. 
> > 
> > Having such a testsuite would make a stable C API more important.
> 
> Writing tests in Python has been productive, see qemu-iotests 041 and
> friends.  The tests spawn QEMU guests and use QMP to interact:

Good to know.

>   result = self.vm.qmp('query-block')
>   self.assert_qmp(result, 'return[0]/inserted/file', target_img)
> 
> Using this XPath-style syntax it's very easy to access the JSON.
> 
> QEMU users tend not to use C, except libvirt.  Even libvirt implements
> the QMP protocol dynamically and can handle optional arguments well.
> 
> I don't think a static C API makes sense when we have an extensible JSON
> protocol.  Let's use the extensibility to our advantage.

Agreed.

Re: [Qemu-devel] KVM call minutes for 2013-04-23

[Anthony Liguori]

Eric Blake <eblake@redhat.com> writes:

> On 04/23/2013 08:45 AM, Juan Quintela wrote:
>> 
>> * 1.5 pending patches (paolo)
>>   anthony thinks nothing big is outstanding
>>   rdma: not probably for this release,  too big change on migration
>>   cpu-hotplug: andreas expect to get it for 1.5
>> 
>> 
>> * What can libvirt expect in 1.5 for introspection of command-line support?
>>   command extensions?  libvirt want then
>> * What are the rules for adding optional parameters to existing QMP
>>   commands?  Would it help if we had introspection of QMP commands?
>>   what are the options that each command support.
>> 
>>   command line could work for 1.5
>>     if we got patches on the next 2 days we can get it.
>
> Goal is to provide a QMP command that provides JSON representation of
> command line options; I will help review whatever is posted to make sure
> we like the interface.  Anthony agreed the implementation should be
> relatively straightforward and okay to add after soft freeze (but must
> be before hard freeze).  Libvirt has some code that would like to make
> use of the new command-line introspection; Osier will probably be the
> first libvirt developer taking advantage of it - if we can swing it,
> we'd like libvirt 1.0.5 to use the new command (libvirt freezes this
> weekend for a May 2 release).
>
>>   rest of introspection need 1.6
>>     it is "challenging"
>>     we are interesting into feature introspection
>>     and comand extensions?
>>     one command to return the schema?
>
> Anthony was okay with the idea of a full JSON introspection of all QMP
> commands, but it is probably too big to squeeze into 1.5 timeframe.
> Furthermore, while the command will be useful, we should always be
> thinking about API - having to parse through JSON to see if a feature is
> present is not always the nicest interface; when adding a new feature,
> consider improving an existing query-* or adding a counterpart new
> query-* command that makes it much easier to tell if a feature is
> available, without having to resort to a QMP introspection.

Ack.

One of the problems with using schema introspection for feature
detection is that there isn't always a 1-1 mapping.  You can imagine
that we have an optional parameter that gets added to a structure and is
initially tied to a specific feature but later gets used by another
feature.

If a distro backports the later and not the former, but a management
tool uses this field to probe for the former feature, it will result in
a false positive.

That's why a more direct feature negotiation mechanism is better IMHO.

Regards,

Anthony Liguori


>
>>   if we change a command,  how we change the interface without
>>   changing the c-api?
>
> c-api is not yet a strong consideration (but see [1] below).  Also,
> there may be ways to design a C api that is robust to extensions (but
> that means designing it into the QMP up front when adding a new
> command); there has been some list traffic on this thought.
>
> More importantly, adding an optional parameter to an existing command is
> not okay unless something else is also available to tell whether the
> feature is usable - QMP introspection would solve this, but is not
> necessarily the most elegant way.  For now, while adding QMP
> introspection is a good idea, we still want case-by-case review of any
> command extensions.
>
>> 
>>   we can change "drive_mirror" to use a new command to see if there
>>   are the new features.
>
> drive-mirror changed in 1.4 to add optional buf-size parameter; right
> now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
> granularity) because there is no introspection and no query-* command
> that witnesses that the feature is present.  Idea was that we need to
> add a new query-drive-mirror-capabilities (name subject to bikeshedding)
> command into 1.5 that would let libvirt know that buf-size/granularity
> is usable (done right, it would also prevent the situation of buf-size
> being a write-only interface where it is set when starting the mirror
> but can not be queried later to see what size is in use).
>
> Unclear whether anyone was signing up to tackle the addition of a query
> command counterpart for drive-mirror in time for 1.5.
>
>> 
>>   if we have a stable c-api we can do test cases that work. 
>
> Having such a testsuite would make a stable C API more important.
>
>> 
>> Eric will complete this with his undrestanding from libvirt point of
>> view.
>
> Also under discussion: the existing QMP 'screendump' command is not
> ideal (not extensible, doesn't allow fd passing, hard-coded output
> format).  This was used as an example command that should not be
> extended until we have appropriate feature detection in place; probably
> easier to add a new QMP command than to add parameters to the existing
> one.  At any rate, we're late enough that 'screendump' probably won't be
> improved in 1.5, so we have the full 1.6 cycle to get it right.
>
> Not on the phone call, but a recent mail that is related to the topic -
> feature detection of whether dump-guest-memory supports paging:
> https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg04613.html
>
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org

Re: [Qemu-devel] KVM call minutes for 2013-04-23

[Luiz Capitulino]

On Tue, 23 Apr 2013 10:06:41 -0600
Eric Blake <eblake@redhat.com> wrote:

> >   we can change "drive_mirror" to use a new command to see if there
> >   are the new features.
> 
> drive-mirror changed in 1.4 to add optional buf-size parameter; right
> now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
> granularity) because there is no introspection and no query-* command
> that witnesses that the feature is present.  Idea was that we need to
> add a new query-drive-mirror-capabilities (name subject to bikeshedding)
> command into 1.5 that would let libvirt know that buf-size/granularity
> is usable (done right, it would also prevent the situation of buf-size
> being a write-only interface where it is set when starting the mirror
> but can not be queried later to see what size is in use).
> 
> Unclear whether anyone was signing up to tackle the addition of a query
> command counterpart for drive-mirror in time for 1.5.

I can do it.

Nice write-up Eric!

Re: [Qemu-devel] KVM call minutes for 2013-04-23

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2703 bytes --]

On 04/23/2013 10:06 AM, Eric Blake wrote:
>>
>>   we can change "drive_mirror" to use a new command to see if there
>>   are the new features.
> 
> drive-mirror changed in 1.4 to add optional buf-size parameter; right
> now, libvirt is forced to limit itself to 1.3 interface (no buf-size or
> granularity) because there is no introspection and no query-* command
> that witnesses that the feature is present.  Idea was that we need to
> add a new query-drive-mirror-capabilities (name subject to bikeshedding)
> command into 1.5 that would let libvirt know that buf-size/granularity
> is usable (done right, it would also prevent the situation of buf-size
> being a write-only interface where it is set when starting the mirror
> but can not be queried later to see what size is in use).
> 
> Unclear whether anyone was signing up to tackle the addition of a query
> command counterpart for drive-mirror in time for 1.5.

Further discussion on this topic:

Luiz proposed such a command:
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg04937.html

in the meantime, Paolo and I discussed on IRC, and realized that:

1. Current libvirt does not expose buf-size or granularity to the end
user.  Until a future libvirt release actually wants to use these
options, we are in no rush to get it into qemu 1.5; it's okay to leave
things in the same state they were in for 1.4, and have qemu 1.6 be the
first release that coordinates with a new libvirt release actually
wanting to use the options.

2. At least with drive-mirror, the "try-and-fail" approach generates a
reasonable-enough error message.  Having a capability query may allow
libvirt to save time and/or give a better quality error message, but
this is one case where not knowing whether the parameter exists is not a
fatal flaw to the algorithm, since libvirt can still gracefully recover
from attempting to use the parameter (only when the user requested a
non-default value).  Distros that want to prove their value-added
quality-of-implementation can easily backport whatever solution goes
into qemu 1.6 for nicer detection into the distro stable build, even if
the distro is based on 1.5; libvirt will use the nicer detection when
available but should have no problems using the try-and-fail approach
otherwise.

So at this point, I'm comfortable with not trying to add anything into
1.5 dealing with drive-mirror; it feels like too much of a new feature
past soft freeze with no known current client, where we would be better
off waiting to 1.6 to get the interface right.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]