| Message ID | 1457080891-26054-13-git-send-email-xiecl.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Changlong Xie writes ("[PATCH v11 12/27] tools/libx{l,c}: introduce wait_checkpoint callback"):
> From: Wen Congyang <wency@cn.fujitsu.com>
>
> Under COLO, we are doing checkpoint on demand, if this
> callback returns 1, we will take another checkpoint.
> 0 indicates unexpected error.
This doesn't seem to have a corresponding implementation. I think the
implementation ought to be in the same patch.
If 0 is always an `unexpected error', perhaps the return value should
be an error code or something ? I'm not sure.
Ian.
On Fri, Mar 04, 2016 at 05:03:16PM +0000, Ian Jackson wrote: > Changlong Xie writes ("[PATCH v11 12/27] tools/libx{l,c}: introduce wait_checkpoint callback"): > > From: Wen Congyang <wency@cn.fujitsu.com> > > > > Under COLO, we are doing checkpoint on demand, if this > > callback returns 1, we will take another checkpoint. > > 0 indicates unexpected error. > > This doesn't seem to have a corresponding implementation. I think the > implementation ought to be in the same patch. > > If 0 is always an `unexpected error', perhaps the return value should > be an error code or something ? I'm not sure. I struggled with this API. I like the idea of that negative value would imply 'unexpected error'. And 1 for 'OK, take another checkpoint'. Not sure if zero would be a valid return value.. > > Ian.
On 03/05/2016 04:23 AM, Konrad Rzeszutek Wilk wrote: > On Fri, Mar 04, 2016 at 05:03:16PM +0000, Ian Jackson wrote: >> Changlong Xie writes ("[PATCH v11 12/27] tools/libx{l,c}: introduce wait_checkpoint callback"): >>> From: Wen Congyang <wency@cn.fujitsu.com> >>> >>> Under COLO, we are doing checkpoint on demand, if this >>> callback returns 1, we will take another checkpoint. >>> 0 indicates unexpected error. >> >> This doesn't seem to have a corresponding implementation. I think the >> implementation ought to be in the same patch. >> >> If 0 is always an `unexpected error', perhaps the return value should >> be an error code or something ? I'm not sure. > > I struggled with this API. > > I like the idea of that negative value would imply 'unexpected error'. > And 1 for 'OK, take another checkpoint'. Not sure if zero would be a valid > return value.. IIRC, save/restore callback always use 0 for unexpected error, 1 for OK. negative value for pipe is broken. Thanks Wen Congyang > > >> >> Ian. > > > . >
On 03/05/2016 01:03 AM, Ian Jackson wrote: > Changlong Xie writes ("[PATCH v11 12/27] tools/libx{l,c}: introduce wait_checkpoint callback"): >> From: Wen Congyang <wency@cn.fujitsu.com> >> >> Under COLO, we are doing checkpoint on demand, if this >> callback returns 1, we will take another checkpoint. >> 0 indicates unexpected error. > > This doesn't seem to have a corresponding implementation. I think the > implementation ought to be in the same patch. > Surely, will merge it in the implement patch in next version Thanks -Xie > If 0 is always an `unexpected error', perhaps the return value should > be an error code or something ? I'm not sure. > > Ian. > > > . >
diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h index 8040ac7..f598bec 100644 --- a/tools/libxc/include/xenguest.h +++ b/tools/libxc/include/xenguest.h @@ -68,6 +68,15 @@ struct save_callbacks { * 1: take another checkpoint */ int (*checkpoint)(void* data); + /* + * Called after the checkpoint callback. + * + * returns: + * 0: terminate checkpointing gracefully + * 1: take another checkpoint + */ + int (*wait_checkpoint)(void* data); + /* Enable qemu-dm logging dirty pages to xen */ int (*switch_qemu_logdirty)(int domid, unsigned enable, void *data); /* HVM only */ @@ -99,6 +108,15 @@ struct restore_callbacks { #define XGR_CHECKPOINT_FAILOVER 2 /* Failover and resume VM */ int (*checkpoint)(void* data); + /* + * Called after the checkpoint callback. + * + * returns: + * 0: terminate checkpointing gracefully + * 1: take another checkpoint + */ + int (*wait_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 d6d2967..51d004d 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', "wait_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)] ], );