Re: [PATCH V1 03/32] savevm: QMP command for cprsave

[Steven Sistare <steven.sistare@oracle.com>]

On 7/30/2020 12:12 PM, Eric Blake wrote:
> On 7/30/20 10:14 AM, Steve Sistare wrote:
>> To enable live reboot, provide the cprsave QMP command and the VMS_REBOOT
>> vmstate-saving operation, which saves the state of the virtual machine in a
>> simple file.
>>
>> Syntax:
>>    {'command':'cprsave', 'data':{'file':'str', 'mode':'str'}}
>>
>>    The mode argument must be 'reboot'.  Additional modes will be defined in
>>    the future.
>>
> 
> Focusing on just the UI:
> 
>> +++ b/qapi/migration.json
>> @@ -1621,3 +1621,17 @@
>>   ##
>>   { 'event': 'UNPLUG_PRIMARY',
>>     'data': { 'device-id': 'str' } }
>> +
>> +##
>> +# @cprsave:
>> +#
>> +# Create a checkpoint of the virtual machine device state in @file.
>> +# Guest RAM and guest block device blocks are not saved.
>> +#
>> +# @file: name of checkpoint file
> 
> Since you used qemu_open() in the code, this can include a '/dev/fdset/NNN' magic name for saving into a previously-passed-in file descriptor instead of directly opening a local file name.  That's a good thing, but I don't know if it needs explicit mention in the docs.

OK, I'll look for other uses of file and fdset in the docs and see if it fits naturally here.

>> +# @mode: 'reboot' : checkpoint can be cprload'ed after a host kexec reboot.
>> +#
>> +# Since 5.0
> 
> 5.2 (you've missed 5.0 by a long shot, and even 5.1 is too late now).

Yup!  Will fix here and in the other patches, thanks.

>> +##
>> +{ 'command': 'cprsave', 'data': { 'file': 'str', 'mode': 'str' } }
> 
> 'mode' should be an enum type, rather than an open-coded string:
> 
> { 'enum': 'CprMode', 'data': ['reboot'] }
> { 'command': 'cprsave', 'data': {'file': 'str', 'mode': 'CprMode' } }

Will do, thanks for the syntax.

- Steve