Re: [Qemu-devel] [PATCH 2/3] migration: Add qmp command for migrate_set_max_cpu_throttle

[Eric Blake]

On 07/31/2018 06:29 AM, Dr. David Alan Gilbert wrote:
> * Li Qiang (address@hidden) wrote:
> > The default max cpu throttle is 99, this is too big that may
> > influence the guest loads. Add a qmp to config it can make it
> > more flexible.
> >
> > Signed-off-by: Li Qiang <address@hidden>
>
> This should be done as a migration parameter rather than a new command.
> For example, follow the cpu-throttle-increment parameter; and this
> should work just like it.

100% agreed - extend existing commands, instead of inventing a new one. 
But in the future, if you do have a valid reason to add a new command...


> > +++ b/qapi/migration.json
> > @@ -997,6 +997,19 @@
> >   { 'command': 'migrate_set_speed', 'data': {'value': 'int'} }
> >   
> >   ##
> > +# @migrate_set_max_cpu_throttle:
> > +#
> > +# Set maximum cpu throttle for migration.
> > +#
> > +# @value: maximum cpu throttle.
> > +#
> > +# Returns: nothing on success
> > +#
> > +# Since: 2.12

You've missed 2.12 by a long shot; you've also missed freeze for 3.0. 
The earliest you can introduce this is 3.1 (which does need to be 
documented even if you properly add this as a new parameter to the 
existing migration parameter commands).

> > +##
> > +{ 'command': 'migrate_set_max_cpu_throttle', 'data': {'value': 'int'} }

...new commands should favor '-' rather than '_' for separation between 
words.  And your documentation should mention which unit the throttle is 
in (bytes per second?)

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org