[libvirt] [PATCH] libxl: detect support for save and restore
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(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4b6b5c0..8eeaf82 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1340,7 +1340,11 @@ libxlMakeCapabilities(libxl_ctx *ctx) { virCapsPtr caps; +#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME + if ((caps = virCapabilitiesNew(virArchFromHost(), 0, 0)) == NULL) +#else if ((caps = virCapabilitiesNew(virArchFromHost(), 1, 1)) == NULL) +#endif return NULL; if (libxlCapsInitHost(ctx, caps) < 0) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 1ea99e2..ac440d2 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1370,6 +1370,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, return ret; } +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME static int libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, unsigned int flags) @@ -1488,6 +1489,7 @@ libxlDomainRestore(virConnectPtr conn, const char *from) { return libxlDomainRestoreFlags(conn, from, NULL, 0); } +#endif /* ifndef LIBXL_HAVE_NO_SUSPEND_RESUME */ static int libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) @@ -4340,6 +4342,7 @@ libxlNodeDeviceReset(virNodeDevicePtr dev) return ret; } +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME static char * libxlDomainMigrateBegin3Params(virDomainPtr domain, virTypedParameterPtr params, @@ -4559,6 +4562,7 @@ libxlDomainMigrateConfirm3Params(virDomainPtr domain, return libxlDomainMigrationConfirm(driver, vm, flags, cancelled); } +#endif /* ifndef LIBXL_HAVE_NO_SUSPEND_RESUME */ 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 .domainCoreDump = libxlDomainCoreDump, /* 0.9.2 */ .domainSetVcpus = libxlDomainSetVcpus, /* 0.9.0 */ .domainSetVcpusFlags = libxlDomainSetVcpusFlags, /* 0.9.0 */ @@ -4650,11 +4656,13 @@ static virDriver libxlDriver = { .nodeDeviceDetachFlags = libxlNodeDeviceDetachFlags, /* 1.2.3 */ .nodeDeviceReAttach = libxlNodeDeviceReAttach, /* 1.2.3 */ .nodeDeviceReset = libxlNodeDeviceReset, /* 1.2.3 */ +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME .domainMigrateBegin3Params = libxlDomainMigrateBegin3Params, /* 1.2.6 */ .domainMigratePrepare3Params = libxlDomainMigratePrepare3Params, /* 1.2.6 */ .domainMigratePerform3Params = libxlDomainMigratePerform3Params, /* 1.2.6 */ .domainMigrateFinish3Params = libxlDomainMigrateFinish3Params, /* 1.2.6 */ .domainMigrateConfirm3Params = libxlDomainMigrateConfirm3Params, /* 1.2.6 */ +#endif }; static virStateDriver libxlStateDriver = { -- 1.8.4.5
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? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@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.
I think it looks identical to that combination, in which case you can add my Tested-by: Ian Campbell <ian.campbell@citrix.com> 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.
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.
I sent a V2 which takes your preferred approach https://www.redhat.com/archives/libvir-list/2014-June/msg01305.html
Maybe get Dan's opinion on this?
Ok. I lean towards V2 since it avoids the double negative. Regards, Jim
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 :|
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Ian Campbell -
Jim Fehlig