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

[zhanghailiang]

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 <address@hidden>
> > Signed-off-by: Yang Hongyang <address@hidden>
> > Signed-off-by: Li Zhijian <address@hidden>
> > Signed-off-by: Gonglei <address@hidden>
> > ---
> >   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