Re: [PATCH v6] tools/migrate: Fix regression when migrating from older version of Xen

[Andrew Cooper <andrew.cooper3@citrix.com>]

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 <andrew.cooper3@citrix.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> 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.
>