[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.