Re: [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters

[Luiz Capitulino <lcapitulino@redhat.com>]

On Mon, 30 Jul 2012 12:11:52 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 07/29/2012 03:42 AM, Orit Wasserman wrote:
> > The management can enable/disable a capability for the next migration by using
> > migrate_set_parameter command.
> > The management can query the current migration capabilities using
> > query-migrate-parameters
> 
> In addition to Luiz' nitpicks:
> 
> Here in the commit message, you are mixing HMP (migrate_set_parameter)
> with QMP (query-migrate-parameters).  It might be better to stick to HMP
> only (migrate_set_parameter, info migrate_parameters) or QMP only
> (migrate-set-parameter, query-migrate-parameters), or even list the
> spellings for both protocols.
> 
> > +++ b/hmp-commands.hx
> 
> > @@ -1419,6 +1433,8 @@ show user network stack connection states
> >  show migration status
> >  @item info migration_capabilities
> >  show migration capabilities
> > +@item info migrate_parameters
> > +show current migration parameters
> 
> > +++ b/monitor.c
> > @@ -2669,6 +2669,13 @@ static mon_cmd_t info_cmds[] = {
> >          .mhandler.info = hmp_info_migration_capabilities,
> >      },
> >      {
> > +        .name       = "migrate-parameters",
> 
> Is this the correct spelling?  Or is this feeding the HMP interface,
> where you documented it above as 'info migrate_parameters'?

It's the HMP interface, yes.

> 
> > +++ b/qapi-schema.json
> > @@ -345,6 +345,38 @@
> >  { 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] }
> >  
> >  ##
> > +# @migrate-set-parameters
> > +#
> > +# Enable/Disable the following migration capabilities (like xbzrle)
> > +#
> > +# Since: 1.2
> 
> Same comment as in 1/11 about 1.2 vs. 1.2.0.
>