Re: [Xen-devel] [libvirt] [PATCH] libxl: detect support for save and restore

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-06-25 at 13:10 -0600, Eric Blake wrote:
> On 06/25/2014 12:13 PM, Jim Fehlig wrote:
> > libxl does not support save, restore, or migrate on all architectures,
> > notably ARM.  Detect whether libxl supports these operations using
> > LIBXL_HAVE_NO_SUSPEND_RESUME.  If not supported, drop advertisement of
> > <migration_features>.
> > 
> > Found by Ian Campbell while improving Xen's OSSTEST infrastructure
> > 
> > http://lists.xen.org/archives/html/xen-devel/2014-06/msg02171.html
> > Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> > ---
> > 
> > Derived from a test patch I sent to Ian Campbell
> > 
> > http://lists.xen.org/archives/html/xen-devel/2014-06/msg03150.html
> > 
> > Includes fixups Ian provided later in the thread.

I think it looks identical to that combination, in which case you can
add my Tested-by: Ian Campbell <ian.campbell@xxxxxxxxxx> if you want.

> > 
> >  src/libxl/libxl_conf.c   | 4 ++++
> >  src/libxl/libxl_driver.c | 8 ++++++++
> >  2 files changed, 12 insertions(+)
> 
> >  
> > +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
> 
> Double negative logic is hard to read. Oh well.

libxl didn't initially supply a #define (because it only supported x86
which always did migration) and when ARM came along we could only add
something to new versions since obviously we can't change already
released stuff, so it had to be this way, sadly.

> >  static virDriver libxlDriver = {
> > @@ -4594,10 +4598,12 @@ static virDriver libxlDriver = {
> >      .domainSetMemoryFlags = libxlDomainSetMemoryFlags, /* 0.9.0 */
> >      .domainGetInfo = libxlDomainGetInfo, /* 0.9.0 */
> >      .domainGetState = libxlDomainGetState, /* 0.9.2 */
> > +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
> >      .domainSave = libxlDomainSave, /* 0.9.2 */
> >      .domainSaveFlags = libxlDomainSaveFlags, /* 0.9.4 */
> >      .domainRestore = libxlDomainRestore, /* 0.9.2 */
> >      .domainRestoreFlags = libxlDomainRestoreFlags, /* 0.9.4 */
> > +#endif
> 
> Hmm - do we do conditional registration in any other driver based on
> configure-time results?  I'd almost rather always provide the driver
> registration, and then use #ifdefs in the body of that function to
> either provide a sane result or else report that the compilation
> environment was too old, rather than omit the support altogether.  Maybe
> get Dan's opinion on this?

From the Xen test harness' point of view we'd like virsh capabilities to
be accurate, FWIW.

Ian.


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