Re: [Xen-devel] [PATCH v4] tools: set migration constraints from cmdline

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-02-19 at 11:42 +0000, Olaf Hering wrote:
> On Mon, Feb 04, Ian Campbell wrote:
> 
> > On Fri, 2013-02-01 at 19:34 +0000, Olaf Hering wrote:
> > > +++ b/tools/libxl/libxl.h
> > > @@ -500,12 +500,25 @@ int libxl_domain_create_restore(libxl_ct
> > >  void libxl_domain_config_init(libxl_domain_config *d_config);
> > >  void libxl_domain_config_dispose(libxl_domain_config *d_config);
> > > 
> > > -int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> > > +int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
> > >                           int flags, /* LIBXL_SUSPEND_* */
> > >                           const libxl_asyncop_how *ao_how)
> > >                           LIBXL_EXTERNAL_CALLERS_ONLY;
> > > +#ifdef LIBXL_API_VERSION
> > > +#if LIBXL_API_VERSION == 0x040200
> > > +#define libxl_domain_suspend libxl_domain_suspend_0x040200
> > 
> > int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> >                          int flags, /* LIBXL_SUSPEND_* */
> >                          int max_iters, int max_factor,
> >                          const libxl_asyncop_how *ao_how)
> >                          LIBXL_EXTERNAL_CALLERS_ONLY;
> > #ifdef LIBXL_API_VERSION
> > #if LIBXL_API_VERSION == 0x040200
> > #define libxl_domain_suspend(ctx, domid, fd, flags, ao_how) \
> >             libxl_domain_suspend(ctx, domid, fd, flags, 0, 0, ao_how) 
> > #endif /* LIBXL_API_VERSION == 0x040200 */
> > #endif /* defined(LIBXL_API_VERSION) */
> > 
> > Should work?
> > 
> > Not sure if
> > #if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION == 0x040200
> > works in CPP.
> > 
> > Maybe we should
> > #ifndef LIBXL_API_VERSION
> > #define LIBXL_API_VERSION LIBXL_API_VERSION_LATEST
> > #endif
> > ?
> 
> Here is another attempt to make use of LIBXL_API_VERSION. Two versions
> just for testing. I would use the static inline variant because we code
> in C, not cpp.
> 
> 
> ...
> /* API compatibility. */
> #ifdef LIBXL_API_VERSION
> #if LIBXL_API_VERSION != 0x040200 && LIBXL_API_VERSION != 0x040300
> #error Unknown LIBXL_API_VERSION
> #endif
> #endif
> 
> typedef struct {
>     int xlflags; /* LIBXL_SUSPEND_* */

Why is this called "xlflags"? xl isn't the only user of this interface.

>     int max_iters;
>     int max_factor;
> } libxl_save_properties;

s/save/suspend/ to match the function it is passed to? Perhaps
libxl_domain_suspend_properties?

> int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
>                          const libxl_save_properties *props,
>                          const libxl_asyncop_how *ao_how)
>                          LIBXL_EXTERNAL_CALLERS_ONLY;
> #ifdef LIBXL_API_VERSION
> #if LIBXL_API_VERSION == 0x040200
> #define libxl_domain_suspend(__ctx, __domid, __fd, __flags, __ao_how) \
> ({ \
>     libxl_save_properties __props = { .xlflags = (__flags) }; \
>     int __ret = libxl_domain_suspend((__ctx), (__domid), (__fd), &__props, 
> (__ao_how)); \
>     __ret; \
> })
> #elif LIBXL_API_VERSION == 0x040300
> static inline int libxl_domain_suspend_0x040300(libxl_ctx *ctx, uint32_t 
> domid, int fd,
>                           int flags, const libxl_asyncop_how *ao_how)
> {
>     libxl_save_properties props = { .xlflags = flags };
>     return libxl_domain_suspend(ctx, domid, fd, &props, ao_how);
> }
> #define libxl_domain_suspend libxl_domain_suspend_0x040300

I don't understand the need for this alternative, you are defining
version 0x040300 in this patch and in the absence of LIBXL_API_VERSION
this should therefore be the expected interface I think. We haven't
released Xen 0x040300 yet.

> #endif
> #endif
> 
> 
> /* cat > t.c
>  * gcc -Wall -o t t.c --save-temps -g
>  * -I tools/libxc -I tools/libxl
>  * -Ltools/libxc -L tools/libxl -lxenlight
>  * -Wl,-rpath,$PWD/tools/libxc,-rpath,$PWD/tools/libxl
>  */
> #define LIBXL_API_VERSION  0x040300
> 
> #include "tools/libxl/libxl.h"
> 
> int main(void)
> {
>       int ret;
> #if LIBXL_API_VERSION == 0x040200
>       ret = libxl_domain_suspend(NULL, 0, 0, 0, NULL);
> #elif LIBXL_API_VERSION == 0x040300
>       ret = libxl_domain_suspend(NULL, 0, 0, 0, NULL);
> #else
>       ret = libxl_domain_suspend(NULL, 0, 0, NULL, NULL);


> #endif
>       return ret;
> }
> 
> 
> Olaf



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