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

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On 01/04/2012 01:45 PM, Luiz Capitulino wrote:
> For now it only supports the "hibernate" mode, which suspends the
> guest to disk.
>
> This 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.
>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>

Looks good.

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>   qapi-schema-guest.json     |   23 ++++++++++++++++++
>   qemu-ga.c                  |   17 ++++++++++++-
>   qga/guest-agent-commands.c |   55 ++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 94 insertions(+), 1 deletions(-)
>
> diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
> index 5f8a18d..b151670 100644
> --- a/qapi-schema-guest.json
> +++ b/qapi-schema-guest.json
> @@ -219,3 +219,26 @@
>   ##
>   { 'command': 'guest-fsfreeze-thaw',
>     'returns': 'int' }
> +
> +##
> +# @guest-suspend
> +#
> +# Suspend guest execution by changing the guest's ACPI power state.
> +#
> +# 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)
> +#
> +# Notes: This is an asynchronous request. There's no guarantee a response
> +# will be sent. Errors will be logged to guest's syslog. More modes are
> +# expected in the future.
> +#
> +# Since: 1.1
> +##
> +{ 'command': 'guest-suspend', 'data': { 'mode': 'str' } }
> diff --git a/qemu-ga.c b/qemu-ga.c
> index 98e4dfe..5b7a7a5 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..19f29c6 100644
> --- a/qga/guest-agent-commands.c
> +++ b/qga/guest-agent-commands.c
> @@ -574,6 +574,61 @@ 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;
> +
> +    /* TODO implement 'sleep' and 'hybrid' modes once qemu is fixed to
> +       support them */
> +    if (strcmp(mode, "hibernate") == 0) {
> +        pmutils_bin = "pm-hibernate";
> +    } else {
> +        error_set(err, QERR_INVALID_PARAMETER, "mode");
> +        return;
> +    }
> +
> +    pid = fork();
> +    if (pid == 0) {
> +        /* child */
> +        int fd;
> +
> +        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'
> +         */
> +        slog("could not execute %s: %s\n", pmutils_bin, strerror(errno));
> +        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);
> +        }
> +
> +        if (write(fd, "disk", 4)<  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)
>   {