From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v4 --for 4.6 COLOPre 13/25] migration/save: pass checkpointed_stream from libxl to libxc Date: Thu, 16 Jul 2015 11:47:43 +0100 Message-ID: <1437043663.32371.168.camel@citrix.com> References: <1436946351-21118-1-git-send-email-yanghy@cn.fujitsu.com> <1436946351-21118-14-git-send-email-yanghy@cn.fujitsu.com> <1436963913.32371.37.camel@citrix.com> <55A749B9.80202@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55A749B9.80202@cn.fujitsu.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Yang Hongyang Cc: wei.liu2@citrix.com, wency@cn.fujitsu.com, andrew.cooper3@citrix.com, yunhong.jiang@intel.com, eddie.dong@intel.com, xen-devel@lists.xen.org, guijianfeng@cn.fujitsu.com, rshriram@cs.ubc.ca, Ian Jackson List-Id: xen-devel@lists.xenproject.org On Thu, 2015-07-16 at 14:05 +0800, Yang Hongyang wrote: > > On 07/15/2015 08:38 PM, Ian Campbell wrote: > > On Wed, 2015-07-15 at 15:45 +0800, Yang Hongyang wrote: > >> Pass checkpointed_stream from libxl to libxc. > >> It won't affact legacy migration because legacy migration > >> won't use this param. > >> > >> Signed-off-by: Yang Hongyang > >> CC: Ian Campbell > >> CC: Ian Jackson > >> CC: Wei Liu > >> CC: Andrew Cooper > >> --- > >> tools/libxc/include/xenguest.h | 9 ++++++--- > >> tools/libxc/xc_domain_save.c | 6 ++++-- > >> tools/libxc/xc_nomigrate.c | 3 ++- > >> tools/libxc/xc_sr_common.h | 2 +- > >> tools/libxc/xc_sr_save.c | 5 +++-- > >> tools/libxl/libxl.c | 2 ++ > >> tools/libxl/libxl_dom_save.c | 11 ++++++++--- > >> tools/libxl/libxl_internal.h | 1 + > >> tools/libxl/libxl_save_callout.c | 2 +- > >> tools/libxl/libxl_save_helper.c | 3 ++- > >> 10 files changed, 30 insertions(+), 14 deletions(-) > >> > >> diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h > >> index e95af54..6e24b6c 100644 > >> --- a/tools/libxc/include/xenguest.h > >> +++ b/tools/libxc/include/xenguest.h > >> @@ -30,7 +30,6 @@ > >> #define XCFLAGS_HVM (1 << 2) > >> #define XCFLAGS_STDVGA (1 << 3) > >> #define XCFLAGS_CHECKPOINT_COMPRESS (1 << 4) > >> -#define XCFLAGS_CHECKPOINTED (1 << 5) > >> > >> #define X86_64_B_SIZE 64 > >> #define X86_32_B_SIZE 32 > >> @@ -85,16 +84,20 @@ struct save_callbacks { > >> * @parm xch a handle to an open hypervisor interface > >> * @parm fd the file descriptor to save a domain to > >> * @parm dom the id of the domain > >> + * @parm checkpointed_stream non-zero if the far end of the stream is using > >> + * checkpointing > > > > Do (or will) specific non-zero values have any meaning to the libxc > > layer? i.e. does it have any knowledge of COLO vs. Remus as the libxl > > enum added in the last patch does? > > Yes, libxc side should be aware of the type of checkpointed_stream (Remus > or COLO). In which case I'm afraid something somewhere needs to explicitly convert from the LIBXL_ namespace to the libxc one (which would be better made explicit with their own names not open coded numbers). See the handling of libxl_tsc_mode or libxl_trigger for an example. Alternatively you could add a comment to libxl_types.idl like libxl_timer_mode and libxl_shutdown_reason have indicating that these values must remain consistent with some underlying interface. I don't much like that but it is tolerable I suppose. > > I think it is better to document the non-zero values here? > for example: > * @parm checkpointed_stream non-zero if the far end of the stream is using > * checkpointing > * 0 no checkpointed stream > * 1 Remus > * 2 COLO I'd prefer named constants in the XC_ namespace. Ian.