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

[Olaf Hering <olaf@aepfle.de>]

On Tue, Feb 19, Ian Campbell wrote:

> > typedef struct {
> >     int xlflags; /* LIBXL_SUSPEND_* */
> 
> Why is this called "xlflags"? xl isn't the only user of this interface.

To make it clear that libxl is the consumer, not libxc. But I think the
comment above makes it clear that libxl flags are expected. I will
rename it to 'flags'.

> >     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?

Will rename it to 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.

Sorry, I was in a hurry and did not explain the stuff above properly
before sending the mail: 

Both versions of a backwards compatible libxl_domain_suspend interface
work, they are here just for reference.
I like the static inline variant because its C and it does not depend on
the '({ ... })' syntax, which is a gcc feature AFAIK.


Olaf