Re: [Xen-devel] [PATCH v5 RFC] tools/migrate: Fix regression when migrating from older version of Xen

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

Just commenting on the API compatibility bits here.

Can you CC Shriram (remus guy) in the next iteration please.

On Thu, 2013-09-26 at 14:45 +0100, Andrew Cooper wrote:
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 4cab294..28af9c2 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -355,6 +355,20 @@
>   */
>  #define LIBXL_HAVE_SPICE_VDAGENT 1
>  
> +/*
> + * LIBXL_HAVE_LIBXL_DOMAIN_CREATE_RESTORE_CHECKPOINTED 1

LIBXL_HAVE_DOMAIN_CREATE... (no need to repeat the LIBXL, or at least we
don't elsewhere)

> + * If this is defined, libxl_domain_create_restore()'s API has changed to
> + * include a checkpointed_stream flag which gets passed down to libxc.
> + */
> +#define LIBXL_HAVE_LIBXL_DOMAIN_CREATE_RESTORE_CHECKPOINTED 1
> +#if LIBXL_API_VERSION == 0x040200 || LIBXL_API_VERSION == 0x040300

<= 0x040300 is fine I think.

> +# define libxl_domain_create_restore libxl_domain_create_restore_V040200

I prefer "...restore_0x040200" since it looks like the #define.

> +#else
> +# define libxl_domain_create_restore libxl_domain_create_restore_V040400
> +#endif

I think you can omit the #else clause and just call the new function
libxl_domain_create_restore. libxl will never set LIBXL_API_VERSION when
building itself.

> +int libxl_domain_create_restore_V040200(
> +    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;
> +int libxl_domain_create_restore_V040400(
> +    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,
> +    int checkpointed_stream)
> +    LIBXL_EXTERNAL_CALLERS_ONLY;

We seem to habitually put the ao_how and aop_ stuff last, after the
actual "arguments".

Did we consider adding a libxl_domain_restore_params struct to contain
the checkpointed flag? That would make this interface more easily
extensible in the future.

> +int libxl_domain_create_restore_V040200(
> +    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)
> +{
> +    return do_domain_create(ctx, d_config, domid, restore_fd,
> +                            ao_how, aop_console_how, 0);
> +}

Could static inline this as a call to the non suffixed version I think?



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel