Re: [PATCH v3 COLOPre 17/26] tools/libx{l, c}: introduce should_checkpoint callback

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote:
> Under COLO, we are doing checkpoint on demand, if this
> callback returns 1, we will take another checkpoint.
> 0 indicates unexpected error.

Is this checkpoint therefore expected to be blocking until another
checkpoint is desired (or an error occurs)?

> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
>  tools/libxc/include/xenguest.h     | 18 ++++++++++++++++++
>  tools/libxl/libxl_save_msgs_gen.pl |  7 ++++---
>  2 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
> index 3e48a97..e804a1d 100644
> --- a/tools/libxc/include/xenguest.h
> +++ b/tools/libxc/include/xenguest.h
> @@ -63,6 +63,15 @@ struct save_callbacks {
>       * 1: take another checkpoint */
>      int (*checkpoint)(void* data);
>  
> +    /*
> +     * Called after the checkpoint callback.
> +     *
> +     * returns:
> +     * 0: terminate checkpointing gracefully

The commit message describes this as an "unexpected error" which this
seems more like the desired response to such an error.

And what would gracefully involve in this context, resuming on sender or
receiver?

> +     * 1: take another checkpoint
> +     */
> +    int (*should_checkpoint)(void* data);
> +
>      /* Enable qemu-dm logging dirty pages to xen */
>      int (*switch_qemu_logdirty)(int domid, unsigned enable, void *data); /* HVM only */
>  
> @@ -108,6 +117,15 @@ struct restore_callbacks {
>      /* A checkpoint record has been found in the stream */
>      int (*checkpoint)(void* data);
>  
> +    /*
> +     * Called after the checkpoint callback.
> +     *
> +     * returns:
> +     * 0: terminate checkpointing gracefully

Again.

> +     * 1: take another checkpoint
> +     */
> +    int (*should_checkpoint)(void* data);
> +
>      /* to be provided as the last argument to each callback function */
>      void* data;
>  };
> diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl
> index dc17c6b..7284975 100755
> --- a/tools/libxl/libxl_save_msgs_gen.pl
> +++ b/tools/libxl/libxl_save_msgs_gen.pl
> @@ -26,11 +26,12 @@ our @msgs = (
>      [  3, 'scxA',   "suspend", [] ],
>      [  4, 'scxA',   "postcopy", [] ],
>      [  5, 'srcxA',   "checkpoint", [] ],
> -    [  6, 'scxA',   "switch_qemu_logdirty",  [qw(int domid
> +    [  6, 'srcxA',  "should_checkpoint", [] ],
> +    [  7, 'scxA',   "switch_qemu_logdirty",  [qw(int domid
>                                                unsigned enable)] ],
> -    [  7, 'r',      "restore_results",       ['unsigned long', 'store_mfn',
> +    [  8, 'r',      "restore_results",       ['unsigned long', 'store_mfn',
>                                                'unsigned long', 'console_mfn'] ],
> -    [  8, 'srW',    "complete",              [qw(int retval
> +    [  9, 'srW',    "complete",              [qw(int retval
>                                                   int errnoval)] ],
>  );
>