Re: [Qemu-devel] [PATCH COLO-Frame v11 10/39] COLO: Implement colo checkpoint protocol

[Eric Blake]

On 11/24/2015 12:00 PM, Dr. David Alan Gilbert wrote:
> * zhanghailiang (address@hidden) wrote:
>> We need communications protocol of user-defined to control the checkpoint
>> process.
>>

>> +static int colo_ctl_get(QEMUFile *f, uint32_t require, uint64_t *value)
>> +{
>> +    int ret;
>> +    uint32_t cmd;
>> +
>> +    ret = colo_ctl_get_cmd(f, &cmd);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    if (cmd != require) {
>> +        error_report("Unexpect colo command, expect:%d, but got cmd:%d",
>> +                     require, cmd);
> 
> I think you need to use PRIu32 rather than %d  since they are uint32_t
> (I doubt it will break on anything, but it's correct).

32-bit cygwin uses 'long' for uint32_t, so there ARE platforms where it
is absolutely necessary to use PRIu32 for printing (even if qemu isn't
ported to cygwin).


>> +++ b/qapi-schema.json
>> @@ -722,6 +722,33 @@
>>  { 'command': 'migrate-start-postcopy' }
>>  
>>  ##
>> +# @COLOCommand
>> +#
>> +# The commands for COLO fault tolerance
>> +#
>> +# @invalid: unknown command
>> +#
>> +# @checkpoint-ready: SVM is ready for checkpointing
>> +#
>> +# @checkpoint-request: PVM tells SVM to prepare for new checkpointing
>> +#
>> +# @checkpoint-reply: SVM gets PVM's checkpoint request
>> +#
>> +# @vmstate-send: VM's state will be sent by PVM.
>> +#
>> +# @vmstate-size: The total size of VMstate.
>> +#
>> +# @vmstate-received: VM's state has been received by SVM
>> +#
>> +# @vmstate-loaded: VM's state has been loaded by SVM

Inconsistent use of trailing '.'

Also, do we really need 'invalid'?

>> +#
>> +# Since: 2.6
>> +##
>> +{ 'enum': 'COLOCommand',
>> +  'data': [ 'invalid', 'checkpoint-ready', 'checkpoint-request',
>> +            'checkpoint-reply', 'vmstate-send', 'vmstate-size',
>> +            'vmstate-received', 'vmstate-loaded' ] }
>> +


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature