| Message ID | 4e9f3590be9f19edb87e05c7b7c2efeae8863109.1709190010.git.slack@rabbit.lu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/xl: add suspend-to-ram and resume subcommands | expand |
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
Hi Zithro, On 2024-04-24 10:03, Jason Andryuk wrote: > 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'd like to re-submit this while retaining your authorship. Are you able to provide your S-o-B? I'll do all the follow ups, but I need your S-o-B for me to make the submission. > 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. I've found the issue. xl exits when the domain suspends. When the domain finally shuts down, xl isn't there to perform the remaining cleanup. >> --- >> - 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. I see you were following the existing style, so this may remain as you wrote it. >> 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. I'm thinking we may want to just use "suspend" for the command name and all the functions. Thanks, Jason
Added my S-o-B (no other change). On 29 Feb 2024 08: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> Signed-off-by: Cyril Rébert (zithro) <slack@rabbit.lu> > > --- > - 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 ... > --- > --- > docs/man/xl.1.pod.in | 10 ++++++++++ > tools/xl/xl.h | 2 ++ > tools/xl/xl_cmdtable.c | 10 ++++++++++ > tools/xl/xl_vmcontrol.c | 36 ++++++++++++++++++++++++++++++++++++ > 4 files changed, 58 insertions(+) > > diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in > index bed8393473..63d80f1449 100644 > --- a/docs/man/xl.1.pod.in > +++ b/docs/man/xl.1.pod.in > @@ -682,6 +682,10 @@ Pass the VNC password to vncviewer via stdin. > > =back > > +=item B<resume> I<domain-id> > + > +Resume a domain, after having been suspended to RAM. > + > =item B<save> [I<OPTIONS>] I<domain-id> I<checkpointfile> [I<configfile>] > > Saves a running domain to a state file so that it can be restored > @@ -760,6 +764,12 @@ in response to this event. > > =back > > +=item B<suspend-to-ram> I<domain-id> > + > +Suspend a domain to RAM. When in a suspended state the domain will still > +consume allocated resources (such as memory), but will not be eligible for > +scheduling by the Xen hypervisor. It is in a shutdown state, but not dying. > + > =item B<sysrq> I<domain-id> I<letter> > > Send a <Magic System Request> to the domain, each type of request is > diff --git a/tools/xl/xl.h b/tools/xl/xl.h > index 9c86bb1d98..716ad32423 100644 > --- a/tools/xl/xl.h > +++ b/tools/xl/xl.h > @@ -133,6 +133,8 @@ int main_migrate(int argc, char **argv); > int main_dump_core(int argc, char **argv); > int main_pause(int argc, char **argv); > int main_unpause(int argc, char **argv); > +int main_suspendtoram(int argc, char **argv); > +int main_resume(int argc, char **argv); > int main_destroy(int argc, char **argv); > int main_shutdown(int argc, char **argv); > int main_reboot(int argc, char **argv); > diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c > index 62bdb2aeaa..1382282252 100644 > --- a/tools/xl/xl_cmdtable.c > +++ b/tools/xl/xl_cmdtable.c > @@ -136,6 +136,16 @@ const struct cmd_spec cmd_table[] = { > "Unpause a paused domain", > "<Domain>", > }, > + { "suspend-to-ram", > + &main_suspendtoram, 0, 1, > + "Suspend a domain to RAM", > + "<Domain>", > + }, > + { "resume", > + &main_resume, 0, 1, > + "Resume a domain from RAM", > + "<Domain>", > + }, > { "console", > &main_console, 0, 0, > "Attach to domain's console", > 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); > +} > + > 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) > +{ > + int opt; > + > + SWITCH_FOREACH_OPT(opt, "", NULL, "suspend-to-ram", 1) { > + /* No options */ > + } > + > + suspend_domain_toram(find_domain(argv[optind])); > + > + return EXIT_SUCCESS; > +} > + > +int main_resume(int argc, char **argv) > +{ > + int opt; > + > + SWITCH_FOREACH_OPT(opt, "", NULL, "resume", 1) { > + /* No options */ > + } > + > + resume_domain(find_domain(argv[optind])); > + > + return EXIT_SUCCESS; > +} > + > int main_destroy(int argc, char **argv) > { > int opt;
Added my S-o-B (no other change). PS: re-sent to account for Anthony mail address change On 29 Feb 2024 08: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> Signed-off-by: Cyril Rébert (zithro) <slack@rabbit.lu> > > --- > - 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 ... > --- > --- > docs/man/xl.1.pod.in | 10 ++++++++++ > tools/xl/xl.h | 2 ++ > tools/xl/xl_cmdtable.c | 10 ++++++++++ > tools/xl/xl_vmcontrol.c | 36 ++++++++++++++++++++++++++++++++++++ > 4 files changed, 58 insertions(+) > > diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in > index bed8393473..63d80f1449 100644 > --- a/docs/man/xl.1.pod.in > +++ b/docs/man/xl.1.pod.in > @@ -682,6 +682,10 @@ Pass the VNC password to vncviewer via stdin. > > =back > > +=item B<resume> I<domain-id> > + > +Resume a domain, after having been suspended to RAM. > + > =item B<save> [I<OPTIONS>] I<domain-id> I<checkpointfile> [I<configfile>] > > Saves a running domain to a state file so that it can be restored > @@ -760,6 +764,12 @@ in response to this event. > > =back > > +=item B<suspend-to-ram> I<domain-id> > + > +Suspend a domain to RAM. When in a suspended state the domain will still > +consume allocated resources (such as memory), but will not be eligible for > +scheduling by the Xen hypervisor. It is in a shutdown state, but not dying. > + > =item B<sysrq> I<domain-id> I<letter> > > Send a <Magic System Request> to the domain, each type of request is > diff --git a/tools/xl/xl.h b/tools/xl/xl.h > index 9c86bb1d98..716ad32423 100644 > --- a/tools/xl/xl.h > +++ b/tools/xl/xl.h > @@ -133,6 +133,8 @@ int main_migrate(int argc, char **argv); > int main_dump_core(int argc, char **argv); > int main_pause(int argc, char **argv); > int main_unpause(int argc, char **argv); > +int main_suspendtoram(int argc, char **argv); > +int main_resume(int argc, char **argv); > int main_destroy(int argc, char **argv); > int main_shutdown(int argc, char **argv); > int main_reboot(int argc, char **argv); > diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c > index 62bdb2aeaa..1382282252 100644 > --- a/tools/xl/xl_cmdtable.c > +++ b/tools/xl/xl_cmdtable.c > @@ -136,6 +136,16 @@ const struct cmd_spec cmd_table[] = { > "Unpause a paused domain", > "<Domain>", > }, > + { "suspend-to-ram", > + &main_suspendtoram, 0, 1, > + "Suspend a domain to RAM", > + "<Domain>", > + }, > + { "resume", > + &main_resume, 0, 1, > + "Resume a domain from RAM", > + "<Domain>", > + }, > { "console", > &main_console, 0, 0, > "Attach to domain's console", > 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); > +} > + > 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) > +{ > + int opt; > + > + SWITCH_FOREACH_OPT(opt, "", NULL, "suspend-to-ram", 1) { > + /* No options */ > + } > + > + suspend_domain_toram(find_domain(argv[optind])); > + > + return EXIT_SUCCESS; > +} > + > +int main_resume(int argc, char **argv) > +{ > + int opt; > + > + SWITCH_FOREACH_OPT(opt, "", NULL, "resume", 1) { > + /* No options */ > + } > + > + resume_domain(find_domain(argv[optind])); > + > + return EXIT_SUCCESS; > +} > + > int main_destroy(int argc, char **argv) > { > int opt;
On 30.07.2024 00:31, zithro wrote: > Added my S-o-B (no other change). > > PS: re-sent to account for Anthony mail address change > > On 29 Feb 2024 08: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> > Signed-off-by: Cyril Rébert (zithro) <slack@rabbit.lu> Imo you want to properly re-submit, ... >> --- >> - Tested on v4.17, x86 ... with this testing also advanced to 4.19. Jan
On 2024-07-30 02:42, Jan Beulich wrote: > On 30.07.2024 00:31, zithro wrote: >> Added my S-o-B (no other change). >> >> PS: re-sent to account for Anthony mail address change >> >> On 29 Feb 2024 08: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> >> Signed-off-by: Cyril Rébert (zithro) <slack@rabbit.lu> > > Imo you want to properly re-submit, ... > >>> --- >>> - Tested on v4.17, x86 > > ... with this testing also advanced to 4.19. I'll test and re-submit this. Thanks, Jason
diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in index bed8393473..63d80f1449 100644 --- a/docs/man/xl.1.pod.in +++ b/docs/man/xl.1.pod.in @@ -682,6 +682,10 @@ Pass the VNC password to vncviewer via stdin. =back +=item B<resume> I<domain-id> + +Resume a domain, after having been suspended to RAM. + =item B<save> [I<OPTIONS>] I<domain-id> I<checkpointfile> [I<configfile>] Saves a running domain to a state file so that it can be restored @@ -760,6 +764,12 @@ in response to this event. =back +=item B<suspend-to-ram> I<domain-id> + +Suspend a domain to RAM. When in a suspended state the domain will still +consume allocated resources (such as memory), but will not be eligible for +scheduling by the Xen hypervisor. It is in a shutdown state, but not dying. + =item B<sysrq> I<domain-id> I<letter> Send a <Magic System Request> to the domain, each type of request is diff --git a/tools/xl/xl.h b/tools/xl/xl.h index 9c86bb1d98..716ad32423 100644 --- a/tools/xl/xl.h +++ b/tools/xl/xl.h @@ -133,6 +133,8 @@ int main_migrate(int argc, char **argv); int main_dump_core(int argc, char **argv); int main_pause(int argc, char **argv); int main_unpause(int argc, char **argv); +int main_suspendtoram(int argc, char **argv); +int main_resume(int argc, char **argv); int main_destroy(int argc, char **argv); int main_shutdown(int argc, char **argv); int main_reboot(int argc, char **argv); diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c index 62bdb2aeaa..1382282252 100644 --- a/tools/xl/xl_cmdtable.c +++ b/tools/xl/xl_cmdtable.c @@ -136,6 +136,16 @@ const struct cmd_spec cmd_table[] = { "Unpause a paused domain", "<Domain>", }, + { "suspend-to-ram", + &main_suspendtoram, 0, 1, + "Suspend a domain to RAM", + "<Domain>", + }, + { "resume", + &main_resume, 0, 1, + "Resume a domain from RAM", + "<Domain>", + }, { "console", &main_console, 0, 0, "Attach to domain's console", 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); +} + 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) +{ + int opt; + + SWITCH_FOREACH_OPT(opt, "", NULL, "suspend-to-ram", 1) { + /* No options */ + } + + suspend_domain_toram(find_domain(argv[optind])); + + return EXIT_SUCCESS; +} + +int main_resume(int argc, char **argv) +{ + int opt; + + SWITCH_FOREACH_OPT(opt, "", NULL, "resume", 1) { + /* No options */ + } + + resume_domain(find_domain(argv[optind])); + + return EXIT_SUCCESS; +} + int main_destroy(int argc, char **argv) { int opt;