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

Mykola Kvach posted 4 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v6 2/4] tools/xl: Allow compilation of 'xl resume' command on Arm
Posted by Mykola Kvach 3 months, 1 week ago
From: Mykola Kvach <mykola_kvach@epam.com>

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@epam.com>
---
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
 
 /*
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index 45745f0dbb..4d4a5bb1c8 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -56,7 +56,7 @@ int create_domain(struct domain_create *dom_info);
 static const char savefileheader_magic[32]=
     "Xen saved domain, xl format\n \0 \r";
 
-#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
+#ifndef LIBXL_HAVE_NO_SUSPEND
 static const char migrate_receiver_banner[]=
     "xl migration receiver ready, send binary domain data.\n";
 static const char migrate_receiver_ready[]=
@@ -65,7 +65,7 @@ static const char migrate_permission_to_go[]=
     "domain is yours, you are cleared to unpause";
 static const char migrate_report[]=
     "my copy unpause results are as follows";
-#endif
+#endif /* !LIBXL_HAVE_NO_SUSPEND */
 
   /* followed by one byte:
    *     0: everything went well, domain is running
@@ -124,14 +124,14 @@ int main_pciattach(int argc, char **argv);
 int main_pciassignable_add(int argc, char **argv);
 int main_pciassignable_remove(int argc, char **argv);
 int main_pciassignable_list(int argc, char **argv);
-#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
+#ifndef LIBXL_HAVE_NO_SUSPEND
 int main_restore(int argc, char **argv);
 int main_migrate_receive(int argc, char **argv);
 int main_save(int argc, char **argv);
 int main_migrate(int argc, char **argv);
 int main_suspend(int argc, char **argv);
+#endif /* !LIBXL_HAVE_NO_SUSPEND */
 int main_resume(int argc, char **argv);
-#endif
 int main_dump_core(int argc, char **argv);
 int main_pause(int argc, char **argv);
 int main_unpause(int argc, char **argv);
@@ -202,7 +202,7 @@ int main_cpupoolnumasplit(int argc, char **argv);
 int main_getenforce(int argc, char **argv);
 int main_setenforce(int argc, char **argv);
 int main_loadpolicy(int argc, char **argv);
-#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
+#ifndef LIBXL_HAVE_NO_SUSPEND
 int main_remus(int argc, char **argv);
 #endif
 int main_devd(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 06a0039718..03f0970bb7 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -152,7 +152,7 @@ const struct cmd_spec cmd_table[] = {
       "                         -autopass\n"
       "--vncviewer-autopass     (consistency alias for --autopass)"
     },
-#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
+#ifndef LIBXL_HAVE_NO_SUSPEND
     { "save",
       &main_save, 0, 1,
       "Save a domain state to restore later",
@@ -198,12 +198,12 @@ const struct cmd_spec cmd_table[] = {
       "Suspend a domain to RAM",
       "<Domain>",
     },
+#endif /* !LIBXL_HAVE_NO_SUSPEND */
     { "resume",
       &main_resume, 0, 1,
       "Resume a domain from RAM",
       "<Domain>",
     },
-#endif
     { "dump-core",
       &main_dump_core, 0, 1,
       "Core dump a domain",
@@ -524,7 +524,7 @@ const struct cmd_spec cmd_table[] = {
       "Loads a new policy into the Flask Xen security module",
       "<policy file>",
     },
-#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
+#ifndef LIBXL_HAVE_NO_SUSPEND
     { "remus",
       &main_remus, 0, 1,
       "Enable Remus HA for domain",
@@ -548,7 +548,7 @@ const struct cmd_spec cmd_table[] = {
       "                        checkpoint must be disabled.\n"
       "-p                      Use COLO userspace proxy."
     },
-#endif
+#endif /* !LIBXL_HAVE_NO_SUSPEND */
     { "devd",
       &main_devd, 0, 1,
       "Daemon that listens for devices and launches backends",
diff --git a/tools/xl/xl_migrate.c b/tools/xl/xl_migrate.c
index b8594f44a5..a8e2c39944 100644
--- a/tools/xl/xl_migrate.c
+++ b/tools/xl/xl_migrate.c
@@ -30,7 +30,7 @@
 #include "xl_utils.h"
 #include "xl_parse.h"
 
-#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
+#ifndef LIBXL_HAVE_NO_SUSPEND
 
 static pid_t create_migration_child(const char *rune, int *send_fd,
                                         int *recv_fd)
@@ -767,7 +767,7 @@ int main_remus(int argc, char **argv)
     close(send_fd);
     return EXIT_FAILURE;
 }
-#endif
+#endif /* !LIBXL_HAVE_NO_SUSPEND */
 
 
 /*
diff --git a/tools/xl/xl_saverestore.c b/tools/xl/xl_saverestore.c
index 953d791d1a..cb10b869b9 100644
--- a/tools/xl/xl_saverestore.c
+++ b/tools/xl/xl_saverestore.c
@@ -29,7 +29,7 @@
 #include "xl_utils.h"
 #include "xl_parse.h"
 
-#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
+#ifndef LIBXL_HAVE_NO_SUSPEND
 
 void save_domain_core_begin(uint32_t domid,
                             int preserve_domid,
@@ -270,7 +270,7 @@ int main_save(int argc, char **argv)
     return EXIT_SUCCESS;
 }
 
-#endif /* LIBXL_HAVE_NO_SUSPEND_RESUME */
+#endif /* !LIBXL_HAVE_NO_SUSPEND */
 
 
 
diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index c813732838..49484ca3e2 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -32,17 +32,12 @@
 
 static int fd_lock = -1;
 
-#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
+#ifndef LIBXL_HAVE_NO_SUSPEND
 static void suspend_domain(uint32_t domid)
 {
     libxl_domain_suspend_only(ctx, domid, NULL);
 }
 
-static void resume_domain(uint32_t domid)
-{
-    libxl_domain_resume(ctx, domid, 1, NULL);
-}
-
 int main_suspend(int argc, char **argv)
 {
     int opt;
@@ -55,6 +50,12 @@ int main_suspend(int argc, char **argv)
 
     return EXIT_SUCCESS;
 }
+#endif /* !LIBXL_HAVE_NO_SUSPEND */
+
+static void resume_domain(uint32_t domid)
+{
+    libxl_domain_resume(ctx, domid, 1, NULL);
+}
 
 int main_resume(int argc, char **argv)
 {
@@ -68,7 +69,6 @@ int main_resume(int argc, char **argv)
 
     return EXIT_SUCCESS;
 }
-#endif
 
 static void pause_domain(uint32_t domid)
 {
-- 
2.48.1
Re: [PATCH v6 2/4] tools/xl: Allow compilation of 'xl resume' command on Arm
Posted by Anthony PERARD 3 months, 1 week ago
On Thu, Jul 24, 2025 at 12:40:57PM +0300, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@epam.com>
> 
> 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@epam.com>
> ---
> 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
Re: [PATCH v6 2/4] tools/xl: Allow compilation of 'xl resume' command on Arm
Posted by Mykola Kvach 3 months, 1 week ago
Hi Anthony,

On Thu, Jul 24, 2025 at 5:01 PM Anthony PERARD <anthony@xenproject.org> wrote:
>
> On Thu, Jul 24, 2025 at 12:40:57PM +0300, Mykola Kvach wrote:
> > From: Mykola Kvach <mykola_kvach@epam.com>
> >
> > 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@epam.com>
> > ---
> > 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.

Got it. I'll revert the patch to the previous version.
Thanks for the review and clarification -- much appreciated!

>
> Cheers,
>
> --
> Anthony PERARD

Best regards,
Mykola