Re: [Qemu-devel] KVM call minutes for 2013-04-23
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 signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] KVM call minutes for 2013-04-23
On Wed, 24 Apr 2013 10:03:21 +0200
Stefan Hajnoczi 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.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call minutes for 2013-04-23
On Tue, 23 Apr 2013 10:06:41 -0600 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. I can do it. Nice write-up Eric! -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call minutes for 2013-04-23
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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
