| Message ID | 1451443075-27428-10-git-send-email-wency@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 30, 2015 at 10:37:39AM +0800, Wen Congyang wrote: > Under COLO, we are doing checkpoint on demand, if this > callback returns 1, we will take another checkpoint. So 1 means OK. > 0 indicates unexpected error. Why not return an error? > > Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn> > Signed-off-by: Wen Congyang <wency@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 bd133af..88d6e13 100644 > --- a/tools/libxc/include/xenguest.h > +++ b/tools/libxc/include/xenguest.h > @@ -62,6 +62,15 @@ struct save_callbacks { > * 1: take another checkpoint */ > int (*checkpoint)(void* data); > > + /* > + * Called after the checkpoint callback. > + * > + * returns: > + * 0: terminate checkpointing gracefully checkpointing terminated gracefully Why not return -EXX instead ? > + * 1: take another checkpoint
On Tue, Jan 26, 2016 at 03:50:32PM -0500, Konrad Rzeszutek Wilk wrote: > On Wed, Dec 30, 2015 at 10:37:39AM +0800, Wen Congyang wrote: > > Under COLO, we are doing checkpoint on demand, if this > > callback returns 1, we will take another checkpoint. > > So 1 means OK. > > > 0 indicates unexpected error. > > Why not return an error? > > > > Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn> > > Signed-off-by: Wen Congyang <wency@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 bd133af..88d6e13 100644 > > --- a/tools/libxc/include/xenguest.h > > +++ b/tools/libxc/include/xenguest.h > > @@ -62,6 +62,15 @@ struct save_callbacks { > > * 1: take another checkpoint */ > > int (*checkpoint)(void* data); > > > > + /* > > + * Called after the checkpoint callback. > > + * > > + * returns: > > + * 0: terminate checkpointing gracefully > > checkpointing terminated gracefully > > Why not return -EXX instead ? > > > + * 1: take another checkpoint Also perhaps the function instead of 'should_checkpoint' should just be called 'checkpoint' or 'do_checkpoint' ?
On 01/27/2016 05:09 AM, Konrad Rzeszutek Wilk wrote: > On Tue, Jan 26, 2016 at 03:50:32PM -0500, Konrad Rzeszutek Wilk wrote: >> On Wed, Dec 30, 2015 at 10:37:39AM +0800, Wen Congyang wrote: >>> Under COLO, we are doing checkpoint on demand, if this >>> callback returns 1, we will take another checkpoint. >> >> So 1 means OK. >> >>> 0 indicates unexpected error. >> >> Why not return an error? >>> >>> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn> >>> Signed-off-by: Wen Congyang <wency@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 bd133af..88d6e13 100644 >>> --- a/tools/libxc/include/xenguest.h >>> +++ b/tools/libxc/include/xenguest.h >>> @@ -62,6 +62,15 @@ struct save_callbacks { >>> * 1: take another checkpoint */ >>> int (*checkpoint)(void* data); >>> >>> + /* >>> + * Called after the checkpoint callback. >>> + * >>> + * returns: >>> + * 0: terminate checkpointing gracefully >> >> checkpointing terminated gracefully >> >> Why not return -EXX instead ? >> >>> + * 1: take another checkpoint > > Also perhaps the function instead of 'should_checkpoint' should just be > called 'checkpoint' or 'do_checkpoint' ? I will check it. IIRC, should_checkpoint() only wait for a new checkpoint. If so, I think we can call it wait_checkpoint(). Thanks Wen Congyang > > > . >
On 01/27/2016 04:50 AM, Konrad Rzeszutek Wilk wrote: > On Wed, Dec 30, 2015 at 10:37:39AM +0800, Wen Congyang wrote: >> Under COLO, we are doing checkpoint on demand, if this >> callback returns 1, we will take another checkpoint. > > So 1 means OK. > >> 0 indicates unexpected error. > > Why not return an error? >> >> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn> >> Signed-off-by: Wen Congyang <wency@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 bd133af..88d6e13 100644 >> --- a/tools/libxc/include/xenguest.h >> +++ b/tools/libxc/include/xenguest.h >> @@ -62,6 +62,15 @@ struct save_callbacks { >> * 1: take another checkpoint */ >> int (*checkpoint)(void* data); >> >> + /* >> + * Called after the checkpoint callback. >> + * >> + * returns: >> + * 0: terminate checkpointing gracefully > > checkpointing terminated gracefully > > Why not return -EXX instead ? Other callbacks also use 0 to indicate an error. Thanks Wen Congyang > >> + * 1: take another checkpoint > > > . >
diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h index bd133af..88d6e13 100644 --- a/tools/libxc/include/xenguest.h +++ b/tools/libxc/include/xenguest.h @@ -62,6 +62,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 (*should_checkpoint)(void* data); + /* Enable qemu-dm logging dirty pages to xen */ int (*switch_qemu_logdirty)(int domid, unsigned enable, void *data); /* HVM only */ @@ -93,6 +102,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 (*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 d6d2967..9107a86 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)] ], );