Re: [PATCH v6 2/4] tools/xl: Allow compilation of 'xl resume' command on Arm

[Anthony PERARD <anthony@xxxxxxxxxxxxxx>]

On Thu, Jul 24, 2025 at 12:40:57PM +0300, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
> 
> The "xl resume" command was previously excluded from Arm builds because
> system suspend/resume (e.g., SYSTEM_SUSPEND via vPSCI) was not
> implemented. On x86, this command is used for ACPI S3 suspend/resume.
> 
> This change enables compilation of `xl resume` on Arm regardless of the
> underlying implementation status, making the tool available for testing
> and future feature support. The relevant libxl infrastructure and handler
> functions are already present and usable.
> 
> The macro `LIBXL_HAVE_NO_SUSPEND_RESUME` has been renamed to
> `LIBXL_HAVE_NO_SUSPEND` to better reflect the updated semantics.
> 
> Note: This does not imply full system suspend/resume support on Arm.
>       The `xl suspend` command still does not work on Arm platforms.
> 
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---
> Changes in v6:
> - Renamed macro from LIBXL_HAVE_NO_SUSPEND_RESUME to LIBXL_HAVE_NO_SUSPEND
>   to better reflect the scope of this change
> - Applied cosmetic changes based on review feedback
> ---
>  tools/include/libxl.h     |  5 ++---
>  tools/xl/xl.h             | 10 +++++-----
>  tools/xl/xl_cmdtable.c    |  8 ++++----
>  tools/xl/xl_migrate.c     |  4 ++--
>  tools/xl/xl_saverestore.c |  4 ++--
>  tools/xl/xl_vmcontrol.c   | 14 +++++++-------
>  6 files changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index d6b6e5d2dd..632264912a 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -1128,17 +1128,16 @@ typedef struct libxl__ctx libxl_ctx;
>  #define LIBXL_HAVE_SIGCHLD_SHARING 1
>  
>  /*
> - * LIBXL_HAVE_NO_SUSPEND_RESUME
> + * LIBXL_HAVE_NO_SUSPEND
>   *
>   * Is this is defined then the platform has no support for saving,
>   * restoring or migrating a domain. In this case the related functions
>   * should be expected to return failure. That is:
>   *  - libxl_domain_suspend
> - *  - libxl_domain_resume
>   *  - libxl_domain_remus_start
>   */
>  #if defined(__arm__) || defined(__aarch64__)
> -#define LIBXL_HAVE_NO_SUSPEND_RESUME 1
> +#define LIBXL_HAVE_NO_SUSPEND 1
>  #endif

I'm sorry, if you remove LIBXL_HAVE_NO_SUSPEND_RESUME, you have to
implement all the function listed. I'm pretty sure `libvirt` isn't going
to build (on arm) if you remove that macro... Actually, libvirt is going
to build, it's going to expect migration to work, and probably allow to
try to migrate Arm VMs instead of bailing out early.

I wonder what this LIBXL_HAVE_NO_SUSPEND_RESUME is for, since you don't
make any changes to libxl (tools/libs/light), but only to program that
make use of it.

Looking at 3ac3817762d1 ("xl: suppress suspend/resume functions on platforms 
which do not support it.")
    
https://xenbits.xenproject.org/gitweb/?p=xen.git;a=commitdiff;h=3ac3817762d1a8b39fa45998ec8c40cabfcfc802
it seems the real purpose was just an hint that migrate/suspend/saving
aren't going to work on that platform.

Looks like `xl resume` is a fairly new command which only makes use if
libxl_domain_resume outside of migration, but the macro
LIBXL_HAVE_NO_SUSPEND_RESUME was mostly a hint that migration doesn't
work.

So I think moving the `xl resume` command out of
LIBXL_HAVE_NO_SUSPEND_RESUME would be good enough for this patch,
without touching libxl.h.

Cheers,

-- 
Anthony PERARD