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

[Hailiang Zhang <zhang.zhanghailiang@huawei.com>]

On 2015/11/25 22:01, Eric Blake wrote:
> On 11/24/2015 12:00 PM, Dr. David Alan Gilbert wrote:
>> * zhanghailiang (zhang.zhanghailiang@huawei.com) 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).
>

Got it, i will fix it.

>
>>> +++ 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'?
>

No, we can remove it. Thanks.

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