1. Patchew
  2. Xen
  3. View series

[XEN PATCH] tools/xendomains: Don't auto save/restore/migrate on Arm*

Anthony PERARD posted 1 patch 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230519162454.50337-1-anthony.perard@citrix.com
tools/configure                                    | 11 +++++++++++
tools/configure.ac                                 | 13 +++++++++++++
tools/hotplug/Linux/init.d/sysconfig.xendomains.in |  4 ++--
3 files changed, 26 insertions(+), 2 deletions(-)
[XEN PATCH] tools/xendomains: Don't auto save/restore/migrate on Arm*
Posted by Anthony PERARD 11 months ago 
Saving, restoring and migrating domains are not currently supported on
arm and arm64 platforms, so xendomains prints the warning:

  An error occurred while saving domain:
  command not implemented

when attempting to run `xendomains stop`. It otherwise continues to shut
down the domains cleanly, with the unsupported steps skipped.

Also in sysconfig.xendomains, change "Default" to "Example" as the
real default is an empty value.

Reported-by: Peter Hoyes <Peter.Hoyes@arm.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Peter, what do you think of this approach?

For reference, there's also a way to findout if a macro exist, with
AC_CHECK_DECL(), but the libxl.h header depends on other headers that
needs to be generated.
---
 tools/configure                                    | 11 +++++++++++
 tools/configure.ac                                 | 13 +++++++++++++
 tools/hotplug/Linux/init.d/sysconfig.xendomains.in |  4 ++--
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/tools/configure b/tools/configure
index 52b4717d01..a722f72c08 100755
--- a/tools/configure
+++ b/tools/configure
@@ -624,6 +624,7 @@ ac_includes_default="\
 
 ac_subst_vars='LTLIBOBJS
 LIBOBJS
+XENDOMAINS_SAVE_DIR
 pvshim
 ninepfs
 SYSTEMD_LIBS
@@ -10155,6 +10156,16 @@ if test "$ax_found" = "0"; then :
 fi
 
 
+case "$host_cpu" in
+    arm*|aarch64)
+        XENDOMAINS_SAVE_DIR=
+        ;;
+    *)
+        XENDOMAINS_SAVE_DIR="$XEN_LIB_DIR/save"
+        ;;
+esac
+
+
 cat >confcache <<\_ACEOF
 # This file is a shell script that caches the results of configure
 # tests run on this system so they can be shared between configure
diff --git a/tools/configure.ac b/tools/configure.ac
index 3cccf41960..0f0983f6b7 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -517,4 +517,17 @@ AS_IF([test "x$pvshim" = "xy"], [
 
 AX_FIND_HEADER([INCLUDE_ENDIAN_H], [endian.h sys/endian.h])
 
+dnl Disable autosave of domain in xendomains on shutdown
+dnl due to missing support. This should be in sync with
+dnl LIBXL_HAVE_NO_SUSPEND_RESUME in libxl.h
+case "$host_cpu" in
+    arm*|aarch64)
+        XENDOMAINS_SAVE_DIR=
+        ;;
+    *)
+        XENDOMAINS_SAVE_DIR="$XEN_LIB_DIR/save"
+        ;;
+esac
+AC_SUBST(XENDOMAINS_SAVE_DIR)
+
 AC_OUTPUT()
diff --git a/tools/hotplug/Linux/init.d/sysconfig.xendomains.in b/tools/hotplug/Linux/init.d/sysconfig.xendomains.in
index f61ef9c4d1..3c49f18bb0 100644
--- a/tools/hotplug/Linux/init.d/sysconfig.xendomains.in
+++ b/tools/hotplug/Linux/init.d/sysconfig.xendomains.in
@@ -45,7 +45,7 @@ XENDOMAINS_CREATE_USLEEP=5000000
 XENDOMAINS_MIGRATE=""
 
 ## Type: string
-## Default: @XEN_LIB_DIR@/save
+## Example: @XEN_LIB_DIR@/save
 #
 # Directory to save running domains to when the system (dom0) is
 # shut down. Will also be used to restore domains from if # XENDOMAINS_RESTORE
@@ -53,7 +53,7 @@ XENDOMAINS_MIGRATE=""
 # (e.g. because you rather shut domains down).
 # If domain saving does succeed, SHUTDOWN will not be executed.
 #
-XENDOMAINS_SAVE=@XEN_LIB_DIR@/save
+XENDOMAINS_SAVE=@XENDOMAINS_SAVE_DIR@
 
 ## Type: string
 ## Default: "--wait"
-- 
Anthony PERARD
Re: [XEN PATCH] tools/xendomains: Don't auto save/restore/migrate on Arm*
Posted by Julien Grall 11 months ago 
Hi Anthony,

On 19/05/2023 17:24, Anthony PERARD wrote:
> Saving, restoring and migrating domains are not currently supported on
> arm and arm64 platforms, so xendomains prints the warning:
> 
>    An error occurred while saving domain:
>    command not implemented
> 
> when attempting to run `xendomains stop`. It otherwise continues to shut
> down the domains cleanly, with the unsupported steps skipped.
> 
> Also in sysconfig.xendomains, change "Default" to "Example" as the
> real default is an empty value.
> 
> Reported-by: Peter Hoyes <Peter.Hoyes@arm.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> Peter, what do you think of this approach?
> 
> For reference, there's also a way to findout if a macro exist, with
> AC_CHECK_DECL(), but the libxl.h header depends on other headers that
> needs to be generated.
> ---
>   tools/configure                                    | 11 +++++++++++
>   tools/configure.ac                                 | 13 +++++++++++++
>   tools/hotplug/Linux/init.d/sysconfig.xendomains.in |  4 ++--
>   3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/configure b/tools/configure
> index 52b4717d01..a722f72c08 100755
> --- a/tools/configure
> +++ b/tools/configure
> @@ -624,6 +624,7 @@ ac_includes_default="\
>   
>   ac_subst_vars='LTLIBOBJS
>   LIBOBJS
> +XENDOMAINS_SAVE_DIR
>   pvshim
>   ninepfs
>   SYSTEMD_LIBS
> @@ -10155,6 +10156,16 @@ if test "$ax_found" = "0"; then :
>   fi
>   
>   
> +case "$host_cpu" in
> +    arm*|aarch64)
> +        XENDOMAINS_SAVE_DIR=
> +        ;;
> +    *)
> +        XENDOMAINS_SAVE_DIR="$XEN_LIB_DIR/save"
> +        ;;
> +esac
> +
> +
>   cat >confcache <<\_ACEOF
>   # This file is a shell script that caches the results of configure
>   # tests run on this system so they can be shared between configure
> diff --git a/tools/configure.ac b/tools/configure.ac
> index 3cccf41960..0f0983f6b7 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -517,4 +517,17 @@ AS_IF([test "x$pvshim" = "xy"], [
>   
>   AX_FIND_HEADER([INCLUDE_ENDIAN_H], [endian.h sys/endian.h])
>   
> +dnl Disable autosave of domain in xendomains on shutdown
> +dnl due to missing support. This should be in sync with
> +dnl LIBXL_HAVE_NO_SUSPEND_RESUME in libxl.h

Quite likely, a developer adding a new arch will first look at the 
definition of LIXBL_HAVE_NO_SUSPEND_RESUME. So it would be good if we 
have a similar message there to remind them to update this case. That 
said...

> +case "$host_cpu" in
> +    arm*|aarch64)
> +        XENDOMAINS_SAVE_DIR=
> +        ;;
> +    *)
> +        XENDOMAINS_SAVE_DIR="$XEN_LIB_DIR/save"
> +        ;;
> +esac

... I am wondering if the switch should be the other way around. IOW, 
the default should be no support for suspend/resume. This will make 
easier to add support for RISC-V (I suspect support for suspend/resume 
will not be in the first version) or any new other arch.

Cheers,

-- 
Julien Grall
Re: [XEN PATCH] tools/xendomains: Don't auto save/restore/migrate on Arm*
Posted by Anthony PERARD 11 months ago 
On Mon, May 22, 2023 at 08:34:52PM +0100, Julien Grall wrote:
> On 19/05/2023 17:24, Anthony PERARD wrote:
> > diff --git a/tools/configure.ac b/tools/configure.ac
> > index 3cccf41960..0f0983f6b7 100644
> > --- a/tools/configure.ac
> > +++ b/tools/configure.ac
> > @@ -517,4 +517,17 @@ AS_IF([test "x$pvshim" = "xy"], [
> >   AX_FIND_HEADER([INCLUDE_ENDIAN_H], [endian.h sys/endian.h])
> > +dnl Disable autosave of domain in xendomains on shutdown
> > +dnl due to missing support. This should be in sync with
> > +dnl LIBXL_HAVE_NO_SUSPEND_RESUME in libxl.h
> 
> Quite likely, a developer adding a new arch will first look at the
> definition of LIXBL_HAVE_NO_SUSPEND_RESUME. So it would be good if we have a
> similar message there to remind them to update this case. That said...

Probably true, I'll look at adding a comment there.

> > +case "$host_cpu" in
> > +    arm*|aarch64)
> > +        XENDOMAINS_SAVE_DIR=
> > +        ;;
> > +    *)
> > +        XENDOMAINS_SAVE_DIR="$XEN_LIB_DIR/save"
> > +        ;;
> > +esac
> 
> ... I am wondering if the switch should be the other way around. IOW, the
> default should be no support for suspend/resume. This will make easier to
> add support for RISC-V (I suspect support for suspend/resume will not be in
> the first version) or any new other arch.

Sounds good, I'll look at that.

Thanks,

-- 
Anthony PERARD
Re: [XEN PATCH] tools/xendomains: Don't auto save/restore/migrate on Arm*
Posted by Peter Hoyes 11 months ago 
On 19/05/2023 17:24, Anthony PERARD wrote:
> Saving, restoring and migrating domains are not currently supported on
> arm and arm64 platforms, so xendomains prints the warning:
>
>    An error occurred while saving domain:
>    command not implemented
>
> when attempting to run `xendomains stop`. It otherwise continues to shut
> down the domains cleanly, with the unsupported steps skipped.
>
> Also in sysconfig.xendomains, change "Default" to "Example" as the
> real default is an empty value.
>
> Reported-by: Peter Hoyes <Peter.Hoyes@arm.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>
> Peter, what do you think of this approach?

Thanks for preparing this. Just validated that the warning above is not 
present with both qemu-aarch64 and an internal stack.

Tested-by: Peter Hoyes <peter.hoyes@arm.com>

>
> For reference, there's also a way to findout if a macro exist, with
> AC_CHECK_DECL(), but the libxl.h header depends on other headers that
> needs to be generated.
> ---
>   tools/configure                                    | 11 +++++++++++
>   tools/configure.ac                                 | 13 +++++++++++++
>   tools/hotplug/Linux/init.d/sysconfig.xendomains.in |  4 ++--
>   3 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/tools/configure b/tools/configure
> index 52b4717d01..a722f72c08 100755
> --- a/tools/configure
> +++ b/tools/configure
> @@ -624,6 +624,7 @@ ac_includes_default="\
>   
>   ac_subst_vars='LTLIBOBJS
>   LIBOBJS
> +XENDOMAINS_SAVE_DIR
>   pvshim
>   ninepfs
>   SYSTEMD_LIBS
> @@ -10155,6 +10156,16 @@ if test "$ax_found" = "0"; then :
>   fi
>   
>   
> +case "$host_cpu" in
> +    arm*|aarch64)
> +        XENDOMAINS_SAVE_DIR=
> +        ;;
> +    *)
> +        XENDOMAINS_SAVE_DIR="$XEN_LIB_DIR/save"
> +        ;;
> +esac
> +
> +
>   cat >confcache <<\_ACEOF
>   # This file is a shell script that caches the results of configure
>   # tests run on this system so they can be shared between configure
> diff --git a/tools/configure.ac b/tools/configure.ac
> index 3cccf41960..0f0983f6b7 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -517,4 +517,17 @@ AS_IF([test "x$pvshim" = "xy"], [
>   
>   AX_FIND_HEADER([INCLUDE_ENDIAN_H], [endian.h sys/endian.h])
>   
> +dnl Disable autosave of domain in xendomains on shutdown
> +dnl due to missing support. This should be in sync with
> +dnl LIBXL_HAVE_NO_SUSPEND_RESUME in libxl.h
> +case "$host_cpu" in
> +    arm*|aarch64)
> +        XENDOMAINS_SAVE_DIR=
> +        ;;
> +    *)
> +        XENDOMAINS_SAVE_DIR="$XEN_LIB_DIR/save"
> +        ;;
> +esac
> +AC_SUBST(XENDOMAINS_SAVE_DIR)
> +
>   AC_OUTPUT()
> diff --git a/tools/hotplug/Linux/init.d/sysconfig.xendomains.in b/tools/hotplug/Linux/init.d/sysconfig.xendomains.in
> index f61ef9c4d1..3c49f18bb0 100644
> --- a/tools/hotplug/Linux/init.d/sysconfig.xendomains.in
> +++ b/tools/hotplug/Linux/init.d/sysconfig.xendomains.in
> @@ -45,7 +45,7 @@ XENDOMAINS_CREATE_USLEEP=5000000
>   XENDOMAINS_MIGRATE=""
>   
>   ## Type: string
> -## Default: @XEN_LIB_DIR@/save
> +## Example: @XEN_LIB_DIR@/save
>   #
>   # Directory to save running domains to when the system (dom0) is
>   # shut down. Will also be used to restore domains from if # XENDOMAINS_RESTORE
> @@ -53,7 +53,7 @@ XENDOMAINS_MIGRATE=""
>   # (e.g. because you rather shut domains down).
>   # If domain saving does succeed, SHUTDOWN will not be executed.
>   #
> -XENDOMAINS_SAVE=@XEN_LIB_DIR@/save
> +XENDOMAINS_SAVE=@XENDOMAINS_SAVE_DIR@
>   
>   ## Type: string
>   ## Default: "--wait"

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.