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@suse.com> ---
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 :|