From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v6] tools/migrate: Fix regression when migrating from older version of Xen Date: Thu, 3 Oct 2013 13:41:48 +0100 Message-ID: <524D660C.9090103@citrix.com> References: <1380215727-5673-1-git-send-email-andrew.cooper3@citrix.com> <1380803223.25936.103.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1380803223.25936.103.camel@kazak.uk.xensource.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: Ian Campbell Cc: Shriram Rajagopalan , Ian Jackson , Xen-devel List-Id: xen-devel@lists.xenproject.org On 03/10/13 13:27, Ian Campbell wrote: > On Thu, 2013-09-26 at 18:15 +0100, Andrew Cooper wrote: >> Commit 00a4b65f8534c9e6521eab2e6ce796ae36037774 Sep 7 2010 >> "libxc: provide notification of final checkpoint to restore end" >> broke migration from any version of Xen using tools from prior to that commit >> >> Older tools have no idea about an XC_SAVE_ID_LAST_CHECKPOINT, causing newer >> tools xc_domain_restore() to start reading the qemu save record, as >> ctx->last_checkpoint is 0. >> >> The failure looks like: >> xc: error: Max batch size exceeded (1970103633). Giving up. >> where 1970103633 = 0x756d6551 = *(uint32_t*)"Qemu" >> >> With this fix in place, the behaviour for normal migrations is reverted to how >> it was before the regression; the migration is considered non-checkpointed >> right from the start. A XC_SAVE_ID_LAST_CHECKPOINT chunk seen in the >> migration stream is a nop. For checkpointed migrations the behaviour is >> unchanged. >> >> Signed-off-by: Andrew Cooper >> CC: Ian Campbell >> CC: Ian Jackson >> CC: Shriram Rajagopalan > Shriram -- are you OK with the remusy bits of this? > >> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h >> index 4cab294..7f8edd2 100644 >> --- a/tools/libxl/libxl.h >> +++ b/tools/libxl/libxl.h >> @@ -355,6 +355,14 @@ >> */ >> #define LIBXL_HAVE_SPICE_VDAGENT 1 >> >> +/* >> + * LIBXL_HAVE_DOMAIN_CREATE_RESTORE_FLAGS 1 >> + * >> + * If this is defined, libxl_domain_create_restore()'s API has changed to >> + * include a flags structure. >> + */ >> +#define LIBXL_HAVE_DOMAIN_CREATE_RESTORE_FLAGS 1 > flags isn't going to be right if we add e.g. rate limiting controls or > something. Lets go with "..._params". Ok. > >> + >> /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be >> * called from within libxl itself. Callers outside libxl, who >> * do not #include libxl_internal.h, are fine. */ >> @@ -578,9 +586,31 @@ int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, >> LIBXL_EXTERNAL_CALLERS_ONLY; >> int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, >> uint32_t *domid, int restore_fd, >> + const libxl_domain_restore_flags *flags, >> const libxl_asyncop_how *ao_how, >> const libxl_asyncprogress_how *aop_console_how) >> LIBXL_EXTERNAL_CALLERS_ONLY; >> + >> +#if LIBXL_API_VERSION == 0x040200 || LIBXL_API_VERSION == 0x040300 > #if LIBXL_API_VERSION <= 0x040300 please I tried that, but causes the `xl` build to fail. #if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION <= 0x040300 might work. I shall see ~Andrew > >> + >> +int static inline libxl_domain_create_restore_0x040200( >> + libxl_ctx *ctx, libxl_domain_config *d_config, >> + uint32_t *domid, int restore_fd, >> + const libxl_asyncop_how *ao_how, >> + const libxl_asyncprogress_how *aop_console_how) >> + LIBXL_EXTERNAL_CALLERS_ONLY >> +{ >> + libxl_domain_restore_flags flags; >> + flags.checkpointed_stream = 0; >> + >> + return libxl_domain_create_restore( >> + ctx, d_config, domid, restore_fd, &flags, ao_how, aop_console_how); >> +} >> + >> +#define libxl_domain_create_restore libxl_domain_create_restore_0x040200 > This all looks good. > > ...So does the rest. > > Ian. >