[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
On 26/09/13 16:11, Ian Campbell wrote: > 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. That was considered, but it would involve playing with the IDL (with which I have no experience), and involve substantially more changes for both in and out-of-tree consumers of the function. > >> +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? > > Cannot do both this and drop the #else clause. (unless I have missed something obvious) ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |