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

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

On Wed, Jun 25, 2014 at 01:10:20PM -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.
> > 
> >  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.
> 
> 
> >  
> >  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?

I think it'd end up pretty much the same in both cases since we'd
end up using VIR_ERR_NO_SUPPORT in both cases. The argument in
favour of providing the driver registration and #ifdef in the
impl is that you could give a slightly more precise error report.

eg instead of "This function isn't supported" you could say
"This function isn't supported on this architecture/version",
but that's pretty much the only difference you'd get.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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