On Wed, 24 Apr 2013 10:03:21 +0200
Stefan Hajnoczi <stefanha@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
