Re: [Qemu-devel] [PATCH COLO-Frame (Base) v21 06/17] COLO: Introduce checkpointing protocol

[Amit Shah <amit.shah@redhat.com>]

On (Tue) 18 Oct 2016 [20:10:02], zhanghailiang wrote:
> We need communications protocol of user-defined to control
> the checkpointing process.
> 
> The new checkpointing request is started by Primary VM,
> and the interactive process like below:
> 
> Checkpoint synchronizing points:
> 
>                    Primary               Secondary
>                                             initial work
> 'checkpoint-ready'    <-------------------- @
> 
> 'checkpoint-request'  @ -------------------->
>                                             Suspend (Only in hybrid mode)
> 'checkpoint-reply'    <-------------------- @
>                       Suspend&Save state
> 'vmstate-send'        @ -------------------->
>                       Send state            Receive state
> 'vmstate-received'    <-------------------- @
>                       Release packets       Load state
> 'vmstate-load'        <-------------------- @
>                       Resume                Resume (Only in hybrid mode)
> 
>                       Start Comparing (Only 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: Li Zhijian <lizhijian@cn.fujitsu.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

> +static int colo_do_checkpoint_transaction(MigrationState *s)
> +{
> +    Error *local_err = NULL;
> +
> +    colo_send_message(s->to_dst_file, COLO_MESSAGE_CHECKPOINT_REQUEST,
> +                      &local_err);
> +    if (local_err) {
> +        goto out;
> +    }
> +
> +    colo_receive_check_message(s->rp_state.from_dst_file,
> +                    COLO_MESSAGE_CHECKPOINT_REPLY, &local_err);
> +    if (local_err) {
> +        goto out;
> +    }
> +
> +    /* TODO: suspend and save vm state to colo buffer */

I like how you've split up the patches - makes it easier to review.
Thanks for doing this!

> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -785,6 +785,31 @@
>  { 'command': 'migrate-start-postcopy' }
>  
>  ##
> +# @COLOMessage
> +#
> +# The message transmission between PVM and SVM

Can you expand PVM and SVM for the first use?  It's obvious to someone
who's familiar with COLO, but someone looking at the api may not know
what it all means.  Also, please expand COLO if not already done in
the qapi-schema file.


		Amit