Re: [PATCH v5 COLOPre 07/18] migration/save: pass checkpointed_stream from libxl to libxc

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Wen Congyang <wency@cn.fujitsu.com>
Cc: Wei Liu <wei.liu2@citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Jiang Yunhong <yunhong.jiang@intel.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	xen devel <xen-devel@lists.xen.org>,
	Dong Eddie <eddie.dong@intel.com>,
	Gui Jianfeng <guijianfeng@cn.fujitsu.com>,
	Shriram Rajagopalan <rshriram@cs.ubc.ca>,
	Yang Hongyang <hongyang.yang@easystack.cn>
Subject: Re: [PATCH v5 COLOPre 07/18] migration/save: pass checkpointed_stream from libxl to libxc
Date: Mon, 25 Jan 2016 14:14:17 -0500	[thread overview]
Message-ID: <20160125191417.GA9768@char.us.oracle.com> (raw)
In-Reply-To: <20160125183836.GM14977@char.us.oracle.com>

On Mon, Jan 25, 2016 at 01:38:36PM -0500, Konrad Rzeszutek Wilk wrote:
> On Thu, Dec 17, 2015 at 03:48:11PM +0800, Wen Congyang wrote:
> > From: Yang Hongyang <hongyang.yang@easystack.cn>
> > 
> > Pass checkpointed_stream from libxl to libxc.
> > It won't affact legacy migration because legacy migration
> > won't use this param.
> 
> Ah, that is why you wanted to keep it as in 'int' in the struct
> domain_create. You may want to update patch #6 to mention that
> you need it to pass it on libxc.
> 
> 
> ..snip..
> > @@ -834,7 +836,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
> >      ctx.save.callbacks = callbacks;
> >      ctx.save.live  = !!(flags & XCFLAGS_LIVE);
> >      ctx.save.debug = !!(flags & XCFLAGS_DEBUG);
> > -    ctx.save.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED);
> > +    ctx.save.checkpointed = checkpointed_stream;
> 
> Should you have an check to make sure they are the right enums?
> 
> /* If altering migration_stream update this assert too. */
> assert(checkpointed_stream == LIBXL_CHECKPOINTED_STREAM_NONE || 
> 	checkpointed_stream == LIBXL_CHECKPOINTED_STREAM_REMUS);

Oops.

That should have been MIG_STREAM_REMUS and MIG_STREAM_NONE.

And sorry about responding under the wrong patchset (v5) instead
of the (v6)! They look the same so it ought to be ok?
> 
> 
> .. snip..
> 
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 3ef11aa..9aa94be 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -228,6 +228,7 @@ libxl_hdtype = Enumeration("hdtype", [
> >      (2, "AHCI"),
> >      ], init_val = "LIBXL_HDTYPE_IDE")
> >  
> > +# Consistent with the values defined for migration_stream
> 
> s/stream/stream./
> 
> >  libxl_checkpointed_stream = Enumeration("checkpointed_stream", [
> >      (0, "NONE"),
> >      (1, "REMUS"),
> > -- 
> > 2.5.0
> > 
> > 
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel