Re: [Qemu-devel] [PATCH COLO-Frame v9 09/32] COLO: Implement colo checkpoint protocol

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

On 2015/10/21 20:17, Eric Blake wrote:
> On 09/02/2015 02:22 AM, zhanghailiang wrote:
>> We need communications protocol of user-defined to control the checkpoint
>> process.
>>
>> The new checkpoint request is started by Primary VM, and the interactive process
>> like below:
>> Checkpoint synchronizing points,
>>
>>                         Primary                         Secondary
>> 'checkpoint-request'   @ ----------------------------->
>>                                                         Suspend (In hybrid mode)
>> 'checkpoint-reply'     <------------------------------ @
>>                         Suspend&Save state
>> 'vmstate-send'         @ ----------------------------->
>>                         Send state                      Receive state
>> 'vmstate-received'     <------------------------------ @
>>                         Release packets                 Load state
>> 'vmstate-load'         <------------------------------ @
>>                         Resume                          Resume (In hybrid mode)
>>
>>                         Start Comparing (In hybrid mode)
>> NOTE:
>>   1) '@' who sends the message
>>   2) Every sync-point is synchronized by two sides with only
>>      one handshake(single direction) for low-latency.
>>      If more strict synchronization is required, a opposite direction
>>      sync-point should be added.
>>   3) Since sync-points are single direction, the remote side may
>>      go forward a lot when this side just receives the sync-point.
>>   4) For now, we only support 'periodic' checkpoint, for which
>>     the Secondary VM is not running, later we will support 'hybrid' mode.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>>   migration/colo.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   qapi-schema.json |  26 ++++++++
>>   trace-events     |   3 +-
>>   3 files changed, 218 insertions(+), 3 deletions(-)
>
> Just a qapi review:
>
>
>> +++ b/qapi-schema.json
>> @@ -664,6 +664,32 @@
>>               '*tls-port': 'int', '*cert-subject': 'str' } }
>>
>>   ##
>> +# @COLOCmd
>
> Any reason this can't be COLOCommand?  We tend to avoid abbreviations in
> the public interface, although arguably type names are not ABI.
>

No special reason, will rename it in next version. :)

>> +#
>> +# The colo command
>> +#
>> +# @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-received: VM's state has been received by SVM
>> +#
>> +# @vmstate-loaded: VM's state has been loaded by SVM
>
> 7 documentation strings...
>
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'enum': 'COLOCmd',
>> +  'data': [ 'invalid', 'checkpoint-ready', 'checkpoint-request',
>> +            'checkpoint-reply', 'vmstate-send', 'vmstate-size',
>> +            'vmstate-received', 'vmstate-loaded', 'guest-shutdown',
>> +            'ram-steal'] }
>
> ...10 enum values.  Missing vmstate-size, guest-shutdown, ram-steal.
>

Yes, this is a mistake, these three values shouldn't be added in this patch, we
didn't refer to them in this patch, they should appear in the later corresponding
patch. I will fix it in next version.

Thanks,
zhanghailiang