From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hongyang Yang Subject: Re: [RFC PATCH 1/3] remus: add a bool var to indicate checkpointed stream Date: Wed, 9 Jul 2014 17:53:30 +0800 Message-ID: <53BD111A.3080403@cn.fujitsu.com> References: <1404892050-24650-1-git-send-email-yanghy@cn.fujitsu.com> <1404892050-24650-2-git-send-email-yanghy@cn.fujitsu.com> <53BD0F3D.8090808@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53BD0F3D.8090808@citrix.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: Andrew Cooper , xen-devel@lists.xen.org Cc: rshriram@cs.ubc.ca, Ian.Jackson@eu.citrix.com, Ian.Campbell@citrix.com List-Id: xen-devel@lists.xenproject.org Hi Andrew, Thanks for the comments! On 07/09/2014 05:45 PM, Andrew Cooper wrote: > On 09/07/14 08:47, Yang Hongyang wrote: >> add a bool variable to migration context to indicate checkpointed stream >> >> Signed-off-by: Yang Hongyang >> --- >> tools/libxc/saverestore/common.h | 1 + >> tools/libxc/saverestore/save.c | 7 +++++++ >> tools/libxc/xenguest.h | 1 + >> tools/libxl/libxl_dom.c | 1 + >> 4 files changed, 10 insertions(+) >> >> diff --git a/tools/libxc/saverestore/common.h b/tools/libxc/saverestore/common.h >> index 4840d3f..24ba95b 100644 >> --- a/tools/libxc/saverestore/common.h >> +++ b/tools/libxc/saverestore/common.h >> @@ -152,6 +152,7 @@ struct xc_sr_context >> int fd; >> >> xc_dominfo_t dominfo; >> + bool checkpointed; > > This appears to only be used on the save side, so should live in the > .save union. Also, a comment indicating that this is a remus option. I also consider to put this in the .save union, but I assume it will also be used in restore code... So does ctx.firsttime There's a checkpointed_stream parameter in xc_domain_restore()... > >> >> union >> { >> diff --git a/tools/libxc/saverestore/save.c b/tools/libxc/saverestore/save.c >> index 653dc8e..d2fa8a6 100644 >> --- a/tools/libxc/saverestore/save.c >> +++ b/tools/libxc/saverestore/save.c >> @@ -652,6 +652,13 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_ite >> ctx.save.callbacks = callbacks; >> ctx.save.live = !!(flags & XCFLAGS_LIVE); >> ctx.save.debug = !!(flags & XCFLAGS_DEBUG); >> + ctx.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED); >> + >> + if ( ctx.checkpointed ) { > > For better or for worse, I have been using the Xen style, as libxc is a > mix of the two. For consistency with the rest of the migration v2 code, > this brace should be on the next line. Get it! > > ~Andrew > >> + /* This is a checkpointed save, we need these callbacks */ >> + assert(ctx.save.callbacks->checkpoint); >> + assert(ctx.save.callbacks->postcopy); >> + } >> >> IPRINTF("In experimental %s", __func__); >> DPRINTF("fd %d, dom %"PRIu32", max_iters %"PRIu32", max_factor %"PRIu32 >> diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h >> index 55755cf..72be3bf 100644 >> --- a/tools/libxc/xenguest.h >> +++ b/tools/libxc/xenguest.h >> @@ -28,6 +28,7 @@ >> #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 >> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c >> index 83eb29a..4a3fd73 100644 >> --- a/tools/libxl/libxl_dom.c >> +++ b/tools/libxl/libxl_dom.c >> @@ -1573,6 +1573,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss) >> >> if (r_info != NULL) { >> dss->interval = r_info->interval; >> + dss->xcflags |= XCFLAGS_CHECKPOINTED; >> if (r_info->compression) >> dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS; >> } > > . > -- Thanks, Yang.