Re: [Qemu-devel] [PATCH] Move max-bandwidth and downtime-limit into migrate_set_parameter for both hmp and qmp

[Ashijeet Acharya]

On Mon, Sep 5, 2016 at 11:37 PM, Ashijeet Acharya
<address@hidden> wrote:
> On Mon, Sep 5, 2016 at 11:26 PM, Daniel P. Berrange <address@hidden> wrote:
>> On Mon, Sep 05, 2016 at 11:20:11PM +0530, Ashijeet Acharya wrote:
>>> Mark old-commands for speed and downtime as deprecated.
>>> Move max-bandwidth and downtime-limit into migrate-set-parameters for
>>> setting maximum migration speed and expected downtime limit parameters
>>> respectively.
>>> Change downtime units to milliseconds and update the query part in both
>>> hmp and qmp qemu control interfaces.
>>>
>>> Signed-off-by: Ashijeet Acharya <address@hidden>
>>> ---
>>>  hmp-commands.hx               |   8 +--
>>>  hmp.c                         |  29 +++++++++-
>>>  include/migration/migration.h |   1 -
>>>  migration/migration.c         | 122 
>>> +++++++++++++++++++++++++++++++-----------
>>>  qapi-schema.json              |  37 ++++++++++---
>>>  qmp-commands.hx               |  21 +++++---
>>>  6 files changed, 169 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>> index 848efee..c7f1ef9 100644
>>> --- a/hmp-commands.hx
>>> +++ b/hmp-commands.hx
>>> @@ -980,16 +980,16 @@ ETEXI
>>>
>>>      {
>>>          .name       = "migrate_set_downtime",
>>> -        .args_type  = "value:T",
>>> +        .args_type  = "value:i",
>>>          .params     = "value",
>>> -        .help       = "set maximum tolerated downtime (in seconds) for 
>>> migrations",
>>> +        .help       = "set maximum tolerated downtime (in milliseconds) 
>>> for migrations",
>>>          .mhandler.cmd = hmp_migrate_set_downtime,
>>>      },
>>>
>>>  STEXI
>>> address@hidden migrate_set_downtime @var{second}
>>> address@hidden migrate_set_downtime @var{milliseconds}
>>>  @findex migrate_set_downtime
>>> -Set maximum tolerated downtime (in seconds) for migration.
>>> +Set maximum tolerated downtime (in milliseconds) for migration.
>>>  ETEXI
>>
>> Sorry, you misunderstood - while we should use 'milliseconds' for
>> downtime in the 'migrate_set_parameters' command, we cannot
>> change the existing 'migrate_set_downtime' command. Changing that
>> would break compatibility with existing apps.
>
> Oh, so that means the calculations of downtime for both the parts will
> be separate at the moment and i cannot glue them together.
> Unfortunately I will only be able send the updated patch by tomorrow
> now. It's really late here.
>
> Thanks
> Ashijeet

I had one more query since you mentioned the backwards compatibility
issue, regarding the bandwidth part.
Should I drop the rounding of -ve speed values to zero for both or
just the new-commands.

Ashijeet
>>
>>
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index 5658723..66ed582 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -637,12 +637,19 @@
>>>  #                hostname must be provided so that the server's x509
>>>  #                certificate identity can be validated. (Since 2.7)
>>>  #
>>> +# @max-bandwidth: to set maximum speed for migration. maximum speed in
>>> +#                 bytes. (Since 2.8)
>>
>> s/bytes/bytes per second/  (and elsewhere in this patch)
>>
>>> +#
>>> +# @downtime-limit: set maximum tolerated downtime for migration. maximum 
>>> downtime
>>> +#                  in milliseconds (Since 2.8)
>>> +#
>>
>> Regards,
>> Daniel
>> --
>> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
>> |: http://libvirt.org              -o-             http://virt-manager.org :|
>> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
>> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|