Re: [libvirt] [PATCH] libxl: detect support for save and restore
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(a)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(a)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.