Re: [PATCH] tools/xl: add suspend-to-ram and resume subcommands

From: Jason Andryuk <jason.andryuk@amd.com>
To: "zithro / Cyril Rébert" <slack@rabbit.lu>,
	xen-devel@lists.xenproject.org
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>,
	"Wei Liu" <wl@xen.org>,
	"Anthony PERARD" <anthony.perard@citrix.com>
Subject: Re: [PATCH] tools/xl: add suspend-to-ram and resume subcommands
Date: Wed, 24 Apr 2024 10:03:05 -0400	[thread overview]
Message-ID: <d3c9ea82-ff33-4211-9e76-f2e599f21a8b@amd.com> (raw)
In-Reply-To: <4e9f3590be9f19edb87e05c7b7c2efeae8863109.1709190010.git.slack@rabbit.lu>

On 2024-02-29 02:00, zithro / Cyril Rébert wrote:
> The xl command doesn't provide suspend/resume, so add them :
>    xl suspend-to-ram <Domain>
>    xl resume <Domain>
> 
> This patch follows a discussion on XenDevel: when you want the
> virtualized equivalent of "sleep"-ing a host, it's better to
> suspend/resume than to pause/unpause a domain.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Suggested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

You should include you S-o-B here to certify your patch under the 
Developer's Certificate of Origin.  You can read what that means in 
CONTRIBUTING.  tl;dr: You are stating you can make the open source 
contribution.

I tested this with a PVH and HVM guest.  suspend-to-ram and resume seem 
to function properly.  The VCPUs stop, but the domain & qemu remain. 
Resume works - the VCPUs start running again.

However, the domain destruction seems to hang on poweroff.  The VM 
transitions to powered off - state shutdown - but the domain and QEMU 
instance are not cleaned up.

If I power off without a suspend-to-ram, everything is cleaned up properly.

Have you seen this?  It's not your code, but I guess something with 
libxl or qemu.

> ---
> - Tested on v4.17, x86
> - the function "libxl_domain_resume" is called like libvirt does, so
>    using a "co-operative resume", but I don't know what it means (:
> - there may be a problem with the words resume <-> restore, like
>    for "LIBXL_HAVE_NO_SUSPEND_RESUME"
> - for the docs, I only slightly adapted a copy/paste from xl pause ...
> ---

> diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> index 98f6bd2e76..ba45f89c5a 100644
> --- a/tools/xl/xl_vmcontrol.c
> +++ b/tools/xl/xl_vmcontrol.c
> @@ -42,6 +42,16 @@ static void unpause_domain(uint32_t domid)
>       libxl_domain_unpause(ctx, domid, NULL);
>   }
>   
> +static void suspend_domain_toram(uint32_t domid)
> +{
> +    libxl_domain_suspend_only(ctx, domid, NULL);
> +}
> +
> +static void resume_domain(uint32_t domid)
> +{
> +    libxl_domain_resume(ctx, domid, 1, NULL);
> +}
> +

I would just inline these functions below.

>   static void destroy_domain(uint32_t domid, int force)
>   {
>       int rc;
> @@ -82,6 +92,32 @@ int main_unpause(int argc, char **argv)
>       return EXIT_SUCCESS;
>   }
>   
> +int main_suspendtoram(int argc, char **argv)

Maybe main_suspend_to_ram to be closer to the command line suspend-to-ram.

Thanks,
Jason