Re: [Qemu-devel] [PATCH v3] qemu-ga: Add the guest-suspend command

[Luiz Capitulino]

On Thu, 15 Dec 2011 10:51:15 -0600
Michael Roth <address@hidden> wrote:

> On 12/15/2011 09:09 AM, Luiz Capitulino wrote:
> > It supports three modes: "hibernate" (suspend to disk), "sleep"
> > (suspend to ram) and "hybrid" (save RAM contents to disk, but
> > suspend instead of powering off).
> >
> > The command will try to execute the scripts provided by the pm-utils
> > package. If that fails, it will try to suspend manually by writing
> > to the "/sys/power/state" file.
> >
> > To reap terminated children, a new signal handler is installed to
> > catch SIGCHLD signals and a non-blocking call to waitpid() is done to
> > collect their exit statuses.
> 
> Spotted a trailing whitespace in one of the comments, but patch looks good.
> 
> Not sure what to expect right now with regarding to testing though...
> 
> I'm aware of the issue with hibernate and virtio S4 support, but what's 
> the expected behavior for sleep? I'd seen Dor mention that QEMU 
> currently wakes the guest back up immediately after, and that some 
> pause/cont logic would need to be added to address that, but for me 
> (Fedora 15 x86_64), the guest appears to stay asleep.

Do you think so because the SDL screen is blank? I could run more tests
now that I'm back from vacation and I think Dor is right because I'm
able to talk to qemu-ga when I issue the sleep command. The fact that
the display is blank is likely to be bug in the VGA card emulation.

Now, it doesn't seem to make sense to add an API to qemu-ga that's not
well supported by qemu. I think Gerd is going to work on the pause/cont
functionality and maybe he can work on the display bug too (I could try
taking a look myself too).

Meanwhile, what do you think about adding the guest-suspend command but
supporting only the 'hibernate' argument? The others can be added once
sleeping is fully supported by qemu.

> 
> I guess it depends on what devices are currently whitelisted in 
> /proc/acpi/wakeup? On Fedora I don't have anything:
> 
> address@hidden qemu-build2]$ cat /proc/acpi/wakeup
> Device        S-state   Status   Sysfs node
> address@hidden qemu-build2]$
> 
> Which I guess explains why I don't wake up immediately after, but I 
> guess it also means there's no way to wake up, ever? On my Ubuntu host I 
> have:
> 
> address@hidden:~$ cat /proc/acpi/wakeup
> Device        S-state   Status   Sysfs node
> LID     S3    *enabled
> SLPB    S3    *enabled
> IGBE    S4    *enabled   pci:0000:00:19.0
> EXP0    S4    *disabled  pci:0000:00:1c.0
> EXP1    S4    *disabled  pci:0000:00:1c.1
> EXP2    S4    *disabled  pci:0000:00:1c.2
> EXP3    S4    *disabled  pci:0000:00:1c.3
> EXP4    S4    *disabled  pci:0000:00:1c.4
> PCI1    S4    *disabled  pci:0000:00:1e.0
> USB0    S3    *disabled  pci:0000:00:1d.0
> USB1    S3    *disabled  pci:0000:00:1d.1
> USB2    S3    *disabled  pci:0000:00:1d.2
> USB3    S3    *disabled  pci:0000:00:1a.0
> USB4    S3    *disabled  pci:0000:00:1a.1
> EHC0    S3    *disabled  pci:0000:00:1d.7
> EHC1    S3    *disabled  pci:0000:00:1a.7
> HDEF    S4    *disabled  pci:0000:00:1b.0
> DURT    S3    *disabled  pnp:00:0a
> address@hidden:~$
> 
> Are we lacking an equivalent to SLPB right now? Or is that list 
> incomplete and there's some other mechanism available?
> 
> >
> > Signed-off-by: Luiz Capitulino<address@hidden>
> > ---
> >
> > v3
> >
> > o Add 'hybrid' mode
> > o Use execlp() instead of execl()
> > o Don't ignore SIGCHLD, catch it instead and call waitpid() from the
> >    signal handler to avoid zombies
> > o Improve the schema doc
> >
> >   qapi-schema-guest.json     |   26 ++++++++++++++++++
> >   qemu-ga.c                  |   17 +++++++++++-
> >   qga/guest-agent-commands.c |   64 
> > ++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 106 insertions(+), 1 deletions(-)
> >
> > diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
> > index 5f8a18d..6b9657a 100644
> > --- a/qapi-schema-guest.json
> > +++ b/qapi-schema-guest.json
> > @@ -219,3 +219,29 @@
> >   ##
> >   { 'command': 'guest-fsfreeze-thaw',
> >     'returns': 'int' }
> > +
> > +##
> > +# @guest-suspend
> > +#
> > +# Suspend guest execution by entering ACPI power state S3 or S4.
> > +#
> > +# This command tries to execute the scripts provided by the pm-utils
> > +# package. If they are not available, it will perform the suspend
> > +# operation by manually writing to a sysfs file.
> > +#
> > +# For the best results it's strongly recommended to have the pm-utils
> > +# package installed in the guest.
> > +#
> > +# @mode: 'hibernate' RAM content is saved to the disk and the guest is
> > +#                    powered off (this corresponds to ACPI S4)
> > +#        'sleep'     execution is suspended but the RAM retains its 
> > contents
> > +#                    (this corresponds to ACPI S3)
> > +#        'hybrid'    RAM content is saved to the disk but the guest is
> > +#                    suspended instead of powering off
> > +#
> > +# Notes: This is an asynchronous request. There's no guarantee a response
> > +# will be sent. Errors will be logged to guest's syslog.
> > +#
> > +# Since: 1.1
> > +##
> > +{ 'command': 'guest-suspend', 'data': { 'mode': 'str' } }
> > diff --git a/qemu-ga.c b/qemu-ga.c
> > index 60d4972..a7c5973 100644
> > --- a/qemu-ga.c
> > +++ b/qemu-ga.c
> > @@ -17,6 +17,7 @@
> >   #include<getopt.h>
> >   #include<termios.h>
> >   #include<syslog.h>
> > +#include<sys/wait.h>
> >   #include "qemu_socket.h"
> >   #include "json-streamer.h"
> >   #include "json-parser.h"
> > @@ -59,9 +60,15 @@ static void quit_handler(int sig)
> >       }
> >   }
> >
> > +static void child_handler(int sig)
> > +{
> > +    int status;
> > +    waitpid(-1,&status, WNOHANG);
> > +}
> > +
> >   static void register_signal_handlers(void)
> >   {
> > -    struct sigaction sigact;
> > +    struct sigaction sigact, sigact_chld;
> >       int ret;
> >
> >       memset(&sigact, 0, sizeof(struct sigaction));
> > @@ -76,6 +83,14 @@ static void register_signal_handlers(void)
> >       if (ret == -1) {
> >           g_error("error configuring signal handler: %s", strerror(errno));
> >       }
> > +
> > +    memset(&sigact_chld, 0, sizeof(struct sigaction));
> > +    sigact_chld.sa_handler = child_handler;
> > +    sigact_chld.sa_flags = SA_NOCLDSTOP;
> > +    ret = sigaction(SIGCHLD,&sigact_chld, NULL);
> > +    if (ret == -1) {
> > +        g_error("error configuring signal handler: %s", strerror(errno));
> > +    }
> >   }
> >
> >   static void usage(const char *cmd)
> > diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c
> > index a09c8ca..cc466ba 100644
> > --- a/qga/guest-agent-commands.c
> > +++ b/qga/guest-agent-commands.c
> > @@ -574,6 +574,70 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
> >   }
> >   #endif
> >
> > +#define LINUX_SYS_STATE_FILE "/sys/power/state"
> > +
> > +void qmp_guest_suspend(const char *mode, Error **err)
> > +{
> > +    pid_t pid;
> > +    const char *pmutils_bin;
> > +
> > +    if (strcmp(mode, "hibernate") == 0) {
> > +        pmutils_bin = "pm-hibernate";
> > +    } else if (strcmp(mode, "sleep") == 0) {
> > +        pmutils_bin = "pm-suspend";
> > +    } else if (strcmp(mode, "hybrid") == 0) {
> > +        pmutils_bin = "pm-suspend-hybrid";
> > +    } else {
> > +        error_set(err, QERR_INVALID_PARAMETER, "mode");
> > +        return;
> > +    }
> > +
> > +    pid = fork();
> > +    if (pid == 0) {
> > +        /* child */
> > +        int fd;
> > +        const char *cmd;
> > +
> > +        setsid();
> > +        fclose(stdin);
> > +        fclose(stdout);
> > +        fclose(stderr);
> > +
> > +        execlp(pmutils_bin, pmutils_bin, NULL);
> > +
> > +        /*
> > +         * The exec call should not return, if it does something went 
> > wrong.
> > +         * In this case we try to suspend manually if 'mode' is 'hibernate'
> > +         * or 'sleep'
> > +         */
> > +        slog("could not execute %s: %s\n", pmutils_bin, strerror(errno));
> > +        if (strcmp(mode, "hybrid") == 0) {
> > +            exit(1);
> > +        }
> > +
> > +        slog("trying to suspend using the manual method...\n");
> > +
> > +        fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);
> > +        if (fd<  0) {
> > +            slog("can't open file %s: %s\n", LINUX_SYS_STATE_FILE,
> > +                    strerror(errno));
> > +            exit(1);
> > +        }
> > +
> > +        cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
> > +        if (write(fd, cmd, strlen(cmd))<  0) {
> > +            slog("can't write to %s: %s\n", LINUX_SYS_STATE_FILE,
> > +                    strerror(errno));
> > +            exit(1);
> > +        }
> > +
> > +        exit(0);
> > +    } else if (pid<  0) {
> > +        error_set(err, QERR_UNDEFINED_ERROR);
> > +        return;
> > +    }
> > +}
> > +
> >   /* register init/cleanup routines for stateful command groups */
> >   void ga_command_state_init(GAState *s, GACommandState *cs)
> >   {
>