| Message ID | 20230519162454.50337-1-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN] tools/xendomains: Don't auto save/restore/migrate on Arm* | expand |
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"
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,
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,
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"
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(-)