On Wed, Mar 22, 2023 at 01:58:00PM +0000, Peter Hoyes wrote:
> From: Peter Hoyes <peter.ho...@arm.com>
>
> 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.
The patch looks kind of ok, but shouldn't $XENDOMAINS_SAVE be set to an
empty string in the config by the admin instead?
Or is the issue that $XENDOMAINS_SAVE is set by default, even on arm* ?
Maybe it's easier to check that the command is implemented at run time
rather than trying to have a good default value for XENDOMAINS_SAVE at
install/package time.
> Use `xl help` to detect whether save/restore/migrate is supported by the
> platform. If not, do not attempt to run the corresponding command.
>
> Signed-off-by: Peter Hoyes <peter.ho...@arm.com>
> ---
> tools/hotplug/Linux/xendomains.in | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/tools/hotplug/Linux/xendomains.in
> b/tools/hotplug/Linux/xendomains.in
> index 70f4129ef4..bafcb874e1 100644
> --- a/tools/hotplug/Linux/xendomains.in
> +++ b/tools/hotplug/Linux/xendomains.in
> @@ -229,6 +229,15 @@ parseln()
> [ -n "$name" -a -n "$id" ] && return 0 || return 1
> }
>
> +subcmd_supported()
> +{
> + local output
> + output=$("$CMD help | grep "^ $1"")
> + if [ ! "$output" ]; then
> + return 1
> + fi
It looks like some quote are in the wrong place. You probably wanted to
write:
output="$($CMD help | grep "^ $1")"
But I'd like to have this slightly more robust, to match the whole
command, rather than the beginning.
(For example `subcmd_supported "pci"` would reture true even if no "pci"
command exist.)
Something like:
$CMD help | grep "^ $1\( \|$\)"
To check that the command name is followed by a space or end-of-line
(even if it seems that there's always a space printed.)
A similar pattern is used in "tools/xl/bash-completion", so it should
probably be fine in the future.
Then, we don't really need the "$output" from grep, we could just ask it
if there's a match or not, with --quiet:
$CMD help | grep -q "^ $1\( \|$\)"
And that would be the whole function. Would that works for you?
The rest of the patch looks fine.
Thanks,
--
Anthony PERARD
