diff mbox

[2/2] qemu-ga: Add the guest-suspend command

Message ID 1326744592-27650-3-git-send-email-lcapitulino@redhat.com
State New
Headers show

Commit Message

Luiz Capitulino Jan. 16, 2012, 8:09 p.m. UTC
The guest-suspend command supports three modes:

 o hibernate (suspend to disk)
 o sleep     (suspend to ram)
 o hybrid    (save RAM contents to disk, but suspend instead of
              powering off)

Before trying to suspend, the command queries the guest in order
to know whether the given mode is supported. The sleep and hybrid
modes are only supported in QEMU 1.1 and later though, because
QEMU's S3 support is broken in previous versions.

The guest-suspend command will use the scripts provided by the
pm-utils package if they are available. If they aren't, a manual
process which directly writes to the "/sys/power/state" file is
used.

To reap terminated children, a new signal handler is installed in
the parent to catch SIGCHLD signals and a non-blocking call to
waitpid() is done to collect their exit statuses. The statuses,
however, are discarded.

The approach used to query the guest for suspend support deserves
some explanation. It's implemented by bios_supports_mode() and shown
below:

   qemu-ga
     |
 create pipe
     |
   fork()
     -----------------
     |               |
     |               |
     |             fork()
     |               --------------------------
     |               |                        |
     |               |                        |
     |               |               exec('pm-is-supported')
     |               |
     |              wait()
     |       write exit status to pipe
     |              exit
     |
  read pipe

This might look complex, but the resulting code is quite simple.
The purpose of that approach is to allow qemu-ga to reap its
children (semi-)automatically from its SIGCHLD handler.

Implementing this the obvious way, that's, doing the exec() call
from the first child process, would force us to introduce a more
complex way to reap qemu-ga's children. Like registering PIDs to
be reaped and having a way to wait for them when returning their
exit status to qemu-ga is necessary. The approach explained
above avoids that complexity.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qapi-schema-guest.json     |   32 ++++++
 qemu-ga.c                  |   18 +++-
 qga/guest-agent-commands.c |  263 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 312 insertions(+), 1 deletions(-)

Comments

Daniel P. Berrangé Jan. 16, 2012, 9:06 p.m. UTC | #1
On Mon, Jan 16, 2012 at 06:09:52PM -0200, Luiz Capitulino wrote:

> +/* Try to find executable file 'file'. If it's found, its absolute path is
> +   returned in 'abs_path' and the function returns true. If it's not found,
> +   the function will return false and 'abs_path' will contain zeros */
> +static bool find_executable_file(const char *file, char *abs_path, size_t len)
> +{
> +    char *path, *saveptr;
> +    const char *token, *delim = ":";
> + 
> +    if (!getenv("PATH")) {
> +        return false;
> +    }
> +
> +    path = g_strdup(getenv("PATH"));
> +    if (!path) {
> +        return false;
> +    }
> +
> +    for (token = strtok_r(path, delim, &saveptr); token;
> +         token = strtok_r(NULL, delim, &saveptr)) {
> +        snprintf(abs_path, len, "%s/%s", token, file);
> +        if (access(abs_path, X_OK) == 0) {
> +            g_free(path);
> +            return true;
> +        }
> +    }
> +
> +    g_free(path);
> +    memset(abs_path, 0, len);
> +
> +    return false;
> +}

I hope you don't hate me for now pointing out...

   g_find_program_in_path()

http://developer.gnome.org/glib/stable/glib-Miscellaneous-Utility-Functions.html#g-find-program-in-path


> +
>  int64_t qmp_guest_sync(int64_t id, Error **errp)
>  {
>      return id;
> @@ -574,6 +623,220 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
>  }
>  #endif
>  
> +#define LINUX_SYS_STATE_FILE "/sys/power/state"
> +#define SUS_MODE_SUPPORTED 0
> +#define SUS_MODE_NOT_SUPPORTED 1
> +
> +/**
> + * This function forks twice and the information about the mode support
> + * status is passed to the qemu-ga process via a pipe.
> + *
> + * This approach allows us to keep the way we reap terminated children
> + * in qemu-ga quite simple.
> + */
> +static bool bios_supports_mode(const char *mode, Error **err)
> +{
> +    pid_t pid;
> +    ssize_t ret;
> +    bool has_pmutils;
> +    int status, pipefds[2];
> +    char pmutils_path[PATH_MAX];
> +    const char *pmutils_bin = "pm-is-supported";
> +
> +    if (pipe(pipefds) < 0) {
> +        error_set(err, QERR_UNDEFINED_ERROR);
> +        return false;
> +    }
> +
> +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> +                                       sizeof(pmutils_path));
> +
> +    pid = fork();
> +    if (!pid) {
> +        struct sigaction act;
> +
> +        memset(&act, 0, sizeof(act));
> +        act.sa_handler = SIG_DFL;
> +        sigaction(SIGCHLD, &act, NULL);
> +
> +        setsid();
> +        close(pipefds[0]);
> +        reopen_fd_to_null(0);
> +        reopen_fd_to_null(1);
> +        reopen_fd_to_null(2);
> +
> +        pid = fork();
> +        if (!pid) {
> +            int fd;
> +            char buf[32]; /* hopefully big enough */
> +            const char *arg;
> +
> +            if (strcmp(mode, "hibernate") == 0) {
> +                arg = "--hibernate";
> +            } else if (strcmp(mode, "sleep") == 0) {
> +                arg = "--suspend";
> +            } else if (strcmp(mode, "hybrid") == 0) {
> +                arg = "--suspend-hybrid";
> +            } else {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            if (has_pmutils) {
> +                execle(pmutils_path, pmutils_bin, arg, NULL, environ);
> +            }
> +
> +            /*
> +             * If we get here either pm-utils is not installed or  execle() has
> +             * failed. Let's try the manual approach if mode is not hybrid (as
> +             * it's only supported by pm-utils)
> +             */
> +
> +            if (strcmp(mode, "hybrid") == 0) {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            fd = open(LINUX_SYS_STATE_FILE, O_RDONLY);
> +            if (fd < 0) {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            ret = read(fd, buf, sizeof(buf));

You want "sizeof(buf)-1" here, otherwise if read return 'ret'
set to exactly sizeof(buf)...

> +            if (ret <= 0) {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            buf[ret] = '\0';

...this becomes a stack overflow.

> +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> +                                       sizeof(pmutils_path));
> +
> +    pid = fork();
> +    if (pid == 0) {
> +        /* child */
> +        int fd;
> +        const char *cmd;
> +
> +        setsid();
> +        reopen_fd_to_null(0);
> +        reopen_fd_to_null(1);
> +        reopen_fd_to_null(2);
> +
> +        if (has_pmutils) {
> +            execle(pmutils_path, pmutils_bin, NULL, environ);

You could just use  execl()  and drop the trailing 'environ' here,
since that is the default anyway.

> +        }
> +
> +        /*
> +         * If we get here either pm-utils is not installed or  execle() has
> +         * failed. Let's try the manual approach if mode is not hybrid (as
> +         * it's only supported by pm-utils)
> +         */
> +
> +        slog("could not execute %s\n", pmutils_bin);
> +        if (strcmp(mode, "hybrid") == 0) {
> +            _exit(EXIT_FAILURE);
> +        }
> +
> +        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(EXIT_FAILURE);
> +        }
> +
> +        cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
> +        if (write(fd, cmd, strlen(cmd)) < 0) {
> +            slog("failued to write to %s\n", LINUX_SYS_STATE_FILE);
> +            _exit(EXIT_FAILURE);
> +        }
> +
> +        _exit(EXIT_SUCCESS);
> +    } 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)
>  {


Regards,
Daniel
Michael Roth Jan. 16, 2012, 10:17 p.m. UTC | #2
On 01/16/2012 02:09 PM, Luiz Capitulino wrote:
> The guest-suspend command supports three modes:
>
>   o hibernate (suspend to disk)
>   o sleep     (suspend to ram)
>   o hybrid    (save RAM contents to disk, but suspend instead of
>                powering off)
>
> Before trying to suspend, the command queries the guest in order
> to know whether the given mode is supported. The sleep and hybrid
> modes are only supported in QEMU 1.1 and later though, because
> QEMU's S3 support is broken in previous versions.
>
> The guest-suspend command will use the scripts provided by the
> pm-utils package if they are available. If they aren't, a manual
> process which directly writes to the "/sys/power/state" file is
> used.
>
> To reap terminated children, a new signal handler is installed in
> the parent to catch SIGCHLD signals and a non-blocking call to
> waitpid() is done to collect their exit statuses. The statuses,
> however, are discarded.
>
> The approach used to query the guest for suspend support deserves
> some explanation. It's implemented by bios_supports_mode() and shown
> below:
>
>     qemu-ga
>       |
>   create pipe
>       |
>     fork()
>       -----------------
>       |               |
>       |               |
>       |             fork()
>       |               --------------------------
>       |               |                        |
>       |               |                        |
>       |               |               exec('pm-is-supported')
>       |               |
>       |              wait()
>       |       write exit status to pipe
>       |              exit
>       |
>    read pipe
>
> This might look complex, but the resulting code is quite simple.
> The purpose of that approach is to allow qemu-ga to reap its
> children (semi-)automatically from its SIGCHLD handler.
>
> Implementing this the obvious way, that's, doing the exec() call
> from the first child process, would force us to introduce a more
> complex way to reap qemu-ga's children. Like registering PIDs to
> be reaped and having a way to wait for them when returning their
> exit status to qemu-ga is necessary. The approach explained
> above avoids that complexity.
>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> ---
>   qapi-schema-guest.json     |   32 ++++++
>   qemu-ga.c                  |   18 +++-
>   qga/guest-agent-commands.c |  263 ++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 312 insertions(+), 1 deletions(-)
>
> diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
> index 5f8a18d..7dd9267 100644
> --- a/qapi-schema-guest.json
> +++ b/qapi-schema-guest.json
> @@ -219,3 +219,35 @@
>   ##
>   { '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
> +#
> +# Returns: nothing on success
> +#          If @mode is not supported by the guest, Unsupported
> +#
> +# Notes: o This is an asynchronous request. There's no guarantee a response
> +#          will be sent.
> +#        o Errors will be logged to guest's syslog.
> +#        o It's strongly recommended to issue the guest-sync command before
> +#          sending commands when the guest resumes.
> +#
> +# Since: 1.1
> +##
> +{ 'command': 'guest-suspend', 'data': { 'mode': 'str' } }
> diff --git a/qemu-ga.c b/qemu-ga.c
> index 647df82..f531084 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,16 @@ static void quit_handler(int sig)
>       }
>   }
>
> +/* reap _all_ terminated children */
> +static void child_handler(int sig)
> +{
> +    int status;
> +    while (waitpid(-1,&status, WNOHANG)>  0) /* NOTHING */;
> +}
> +
>   static void register_signal_handlers(void)
>   {
> -    struct sigaction sigact;
> +    struct sigaction sigact, sigact_chld;
>       int ret;
>
>       memset(&sigact, 0, sizeof(struct sigaction));
> @@ -76,6 +84,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..e64b0e0 100644
> --- a/qga/guest-agent-commands.c
> +++ b/qga/guest-agent-commands.c
> @@ -23,6 +23,7 @@
>
>   #include<sys/types.h>
>   #include<sys/ioctl.h>
> +#include<sys/wait.h>
>   #include "qga/guest-agent-core.h"
>   #include "qga-qmp-commands.h"
>   #include "qerror.h"
> @@ -44,6 +45,54 @@ static void slog(const char *fmt, ...)
>       va_end(ap);
>   }
>
> +static void reopen_fd_to_null(int fd)
> +{
> +    int nullfd;
> +
> +    nullfd = open("/dev/null", O_RDWR);
> +    if (nullfd<  0) {
> +        return;
> +    }
> +
> +    dup2(nullfd, fd);
> +
> +    if (nullfd != fd) {
> +        close(nullfd);
> +    }
> +}
> +
> +/* Try to find executable file 'file'. If it's found, its absolute path is
> +   returned in 'abs_path' and the function returns true. If it's not found,
> +   the function will return false and 'abs_path' will contain zeros */
> +static bool find_executable_file(const char *file, char *abs_path, size_t len)
> +{
> +    char *path, *saveptr;
> +    const char *token, *delim = ":";
> +
> +    if (!getenv("PATH")) {
> +        return false;
> +    }
> +
> +    path = g_strdup(getenv("PATH"));
> +    if (!path) {
> +        return false;
> +    }
> +
> +    for (token = strtok_r(path, delim,&saveptr); token;
> +         token = strtok_r(NULL, delim,&saveptr)) {
> +        snprintf(abs_path, len, "%s/%s", token, file);
> +        if (access(abs_path, X_OK) == 0) {
> +            g_free(path);
> +            return true;
> +        }
> +    }
> +
> +    g_free(path);
> +    memset(abs_path, 0, len);
> +
> +    return false;
> +}
> +
>   int64_t qmp_guest_sync(int64_t id, Error **errp)
>   {
>       return id;
> @@ -574,6 +623,220 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
>   }
>   #endif
>
> +#define LINUX_SYS_STATE_FILE "/sys/power/state"
> +#define SUS_MODE_SUPPORTED 0
> +#define SUS_MODE_NOT_SUPPORTED 1
> +
> +/**
> + * This function forks twice and the information about the mode support
> + * status is passed to the qemu-ga process via a pipe.
> + *
> + * This approach allows us to keep the way we reap terminated children
> + * in qemu-ga quite simple.
> + */
> +static bool bios_supports_mode(const char *mode, Error **err)
> +{
> +    pid_t pid;
> +    ssize_t ret;
> +    bool has_pmutils;
> +    int status, pipefds[2];
> +    char pmutils_path[PATH_MAX];
> +    const char *pmutils_bin = "pm-is-supported";
> +
> +    if (pipe(pipefds)<  0) {
> +        error_set(err, QERR_UNDEFINED_ERROR);
> +        return false;
> +    }
> +
> +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> +                                       sizeof(pmutils_path));
> +
> +    pid = fork();
> +    if (!pid) {
> +        struct sigaction act;
> +
> +        memset(&act, 0, sizeof(act));
> +        act.sa_handler = SIG_DFL;
> +        sigaction(SIGCHLD,&act, NULL);
> +
> +        setsid();
> +        close(pipefds[0]);
> +        reopen_fd_to_null(0);
> +        reopen_fd_to_null(1);
> +        reopen_fd_to_null(2);
> +
> +        pid = fork();
> +        if (!pid) {
> +            int fd;
> +            char buf[32]; /* hopefully big enough */
> +            const char *arg;
> +
> +            if (strcmp(mode, "hibernate") == 0) {
> +                arg = "--hibernate";
> +            } else if (strcmp(mode, "sleep") == 0) {
> +                arg = "--suspend";
> +            } else if (strcmp(mode, "hybrid") == 0) {
> +                arg = "--suspend-hybrid";
> +            } else {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            if (has_pmutils) {
> +                execle(pmutils_path, pmutils_bin, arg, NULL, environ);
> +            }
> +
> +            /*
> +             * If we get here either pm-utils is not installed or  execle() has
> +             * failed. Let's try the manual approach if mode is not hybrid (as
> +             * it's only supported by pm-utils)
> +             */
> +
> +            if (strcmp(mode, "hybrid") == 0) {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            fd = open(LINUX_SYS_STATE_FILE, O_RDONLY);
> +            if (fd<  0) {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            ret = read(fd, buf, sizeof(buf));
> +            if (ret<= 0) {
> +                _exit(SUS_MODE_NOT_SUPPORTED);
> +            }
> +
> +            buf[ret] = '\0';
> +            if (strcmp(mode, "hibernate") == 0&&  strstr(buf, "disk")) {
> +                _exit(SUS_MODE_SUPPORTED);
> +            } else if (strcmp(mode, "sleep") == 0&&  strstr(buf, "mem")) {
> +                _exit(SUS_MODE_SUPPORTED);
> +            }
> +
> +            _exit(SUS_MODE_NOT_SUPPORTED);
> +        }
> +
> +        if (pid>  0) {
> +            wait(&status);
> +        } else {
> +            status = SUS_MODE_NOT_SUPPORTED;
> +        }
> +
> +        ret = write(pipefds[1],&status, sizeof(status));
> +        if (ret != sizeof(status)) {
> +            _exit(EXIT_FAILURE);
> +        }
> +
> +        _exit(EXIT_SUCCESS);
> +    }
> +
> +    close(pipefds[1]);
> +
> +    if (pid>  0) {
> +        ret = read(pipefds[0],&status, sizeof(status));
> +        if (ret == sizeof(status)&&  WIFEXITED(status)&&
> +            WEXITSTATUS(status) == SUS_MODE_SUPPORTED) {
> +            close(pipefds[0]);
> +            return true;
> +        }
> +    }
> +
> +    close(pipefds[0]);
> +    return false;
> +}
> +
> +static bool host_supports_mode(const char *mode)
> +{
> +    if (strcmp(mode, "hibernate")) {
> +        /* sleep&  hybrid are only supported in qemu 1.1.0 and above */
> +        return ga_has_support_level(1, 1, 0);
> +    }
> +    return true;
> +}
> +
> +void qmp_guest_suspend(const char *mode, Error **err)
> +{
> +    pid_t pid;
> +    bool has_pmutils;
> +    Error *local_err = NULL;
> +    const char *pmutils_bin;
> +    char pmutils_path[PATH_MAX];
> +
> +    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;
> +    }
> +
> +    if (!host_supports_mode(mode)) {
> +        error_set(err, QERR_UNSUPPORTED);
> +        return;
> +    }
> +
> +    if (!bios_supports_mode(mode,&local_err)) {
> +        if (error_is_set(&local_err)) {
> +            error_propagate(err, local_err);
> +        } else {
> +            error_set(err, QERR_UNSUPPORTED);
> +        }
> +        return;
> +    }
> +
> +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> +                                       sizeof(pmutils_path));
> +
> +    pid = fork();
> +    if (pid == 0) {
> +        /* child */
> +        int fd;
> +        const char *cmd;
> +
> +        setsid();
> +        reopen_fd_to_null(0);
> +        reopen_fd_to_null(1);
> +        reopen_fd_to_null(2);
> +
> +        if (has_pmutils) {
> +            execle(pmutils_path, pmutils_bin, NULL, environ);
> +        }
> +
> +        /*
> +         * If we get here either pm-utils is not installed or  execle() has
> +         * failed. Let's try the manual approach if mode is not hybrid (as
> +         * it's only supported by pm-utils)
> +         */
> +
> +        slog("could not execute %s\n", pmutils_bin);

slog() is actually a wrapper around syslog(), so I guess we need to 
scrap these in light of the previous discussion. shutdown() has the same 
problem though...

So maybe, just leave these in, and I'll follow up with a patch that logs 
to a separate file or something if pid != PARENT_PID.

> +        if (strcmp(mode, "hybrid") == 0) {
> +            _exit(EXIT_FAILURE);
> +        }
> +
> +        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(EXIT_FAILURE);
> +        }
> +
> +        cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
> +        if (write(fd, cmd, strlen(cmd))<  0) {
> +            slog("failued to write to %s\n", LINUX_SYS_STATE_FILE);
> +            _exit(EXIT_FAILURE);
> +        }
> +
> +        _exit(EXIT_SUCCESS);
> +    } 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)
>   {
Luiz Capitulino Jan. 17, 2012, 12:18 p.m. UTC | #3
On Mon, 16 Jan 2012 21:06:27 +0000
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Mon, Jan 16, 2012 at 06:09:52PM -0200, Luiz Capitulino wrote:
> 
> > +/* Try to find executable file 'file'. If it's found, its absolute path is
> > +   returned in 'abs_path' and the function returns true. If it's not found,
> > +   the function will return false and 'abs_path' will contain zeros */
> > +static bool find_executable_file(const char *file, char *abs_path, size_t len)
> > +{
> > +    char *path, *saveptr;
> > +    const char *token, *delim = ":";
> > + 
> > +    if (!getenv("PATH")) {
> > +        return false;
> > +    }
> > +
> > +    path = g_strdup(getenv("PATH"));
> > +    if (!path) {
> > +        return false;
> > +    }
> > +
> > +    for (token = strtok_r(path, delim, &saveptr); token;
> > +         token = strtok_r(NULL, delim, &saveptr)) {
> > +        snprintf(abs_path, len, "%s/%s", token, file);
> > +        if (access(abs_path, X_OK) == 0) {
> > +            g_free(path);
> > +            return true;
> > +        }
> > +    }
> > +
> > +    g_free(path);
> > +    memset(abs_path, 0, len);
> > +
> > +    return false;
> > +}
> 
> I hope you don't hate me for now pointing out...
> 
>    g_find_program_in_path()
> 
> http://developer.gnome.org/glib/stable/glib-Miscellaneous-Utility-Functions.html#g-find-program-in-path

Oh, Thanks.

Note that it returns an allocated string. I won't free it in the child,
as it will be automatically freed by exec() or by exit().

> > +
> >  int64_t qmp_guest_sync(int64_t id, Error **errp)
> >  {
> >      return id;
> > @@ -574,6 +623,220 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
> >  }
> >  #endif
> >  
> > +#define LINUX_SYS_STATE_FILE "/sys/power/state"
> > +#define SUS_MODE_SUPPORTED 0
> > +#define SUS_MODE_NOT_SUPPORTED 1
> > +
> > +/**
> > + * This function forks twice and the information about the mode support
> > + * status is passed to the qemu-ga process via a pipe.
> > + *
> > + * This approach allows us to keep the way we reap terminated children
> > + * in qemu-ga quite simple.
> > + */
> > +static bool bios_supports_mode(const char *mode, Error **err)
> > +{
> > +    pid_t pid;
> > +    ssize_t ret;
> > +    bool has_pmutils;
> > +    int status, pipefds[2];
> > +    char pmutils_path[PATH_MAX];
> > +    const char *pmutils_bin = "pm-is-supported";
> > +
> > +    if (pipe(pipefds) < 0) {
> > +        error_set(err, QERR_UNDEFINED_ERROR);
> > +        return false;
> > +    }
> > +
> > +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> > +                                       sizeof(pmutils_path));
> > +
> > +    pid = fork();
> > +    if (!pid) {
> > +        struct sigaction act;
> > +
> > +        memset(&act, 0, sizeof(act));
> > +        act.sa_handler = SIG_DFL;
> > +        sigaction(SIGCHLD, &act, NULL);
> > +
> > +        setsid();
> > +        close(pipefds[0]);
> > +        reopen_fd_to_null(0);
> > +        reopen_fd_to_null(1);
> > +        reopen_fd_to_null(2);
> > +
> > +        pid = fork();
> > +        if (!pid) {
> > +            int fd;
> > +            char buf[32]; /* hopefully big enough */
> > +            const char *arg;
> > +
> > +            if (strcmp(mode, "hibernate") == 0) {
> > +                arg = "--hibernate";
> > +            } else if (strcmp(mode, "sleep") == 0) {
> > +                arg = "--suspend";
> > +            } else if (strcmp(mode, "hybrid") == 0) {
> > +                arg = "--suspend-hybrid";
> > +            } else {
> > +                _exit(SUS_MODE_NOT_SUPPORTED);
> > +            }
> > +
> > +            if (has_pmutils) {
> > +                execle(pmutils_path, pmutils_bin, arg, NULL, environ);
> > +            }
> > +
> > +            /*
> > +             * If we get here either pm-utils is not installed or  execle() has
> > +             * failed. Let's try the manual approach if mode is not hybrid (as
> > +             * it's only supported by pm-utils)
> > +             */
> > +
> > +            if (strcmp(mode, "hybrid") == 0) {
> > +                _exit(SUS_MODE_NOT_SUPPORTED);
> > +            }
> > +
> > +            fd = open(LINUX_SYS_STATE_FILE, O_RDONLY);
> > +            if (fd < 0) {
> > +                _exit(SUS_MODE_NOT_SUPPORTED);
> > +            }
> > +
> > +            ret = read(fd, buf, sizeof(buf));
> 
> You want "sizeof(buf)-1" here, otherwise if read return 'ret'
> set to exactly sizeof(buf)...

Good catch.

> 
> > +            if (ret <= 0) {
> > +                _exit(SUS_MODE_NOT_SUPPORTED);
> > +            }
> > +
> > +            buf[ret] = '\0';
> 
> ...this becomes a stack overflow.
> 
> > +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> > +                                       sizeof(pmutils_path));
> > +
> > +    pid = fork();
> > +    if (pid == 0) {
> > +        /* child */
> > +        int fd;
> > +        const char *cmd;
> > +
> > +        setsid();
> > +        reopen_fd_to_null(0);
> > +        reopen_fd_to_null(1);
> > +        reopen_fd_to_null(2);
> > +
> > +        if (has_pmutils) {
> > +            execle(pmutils_path, pmutils_bin, NULL, environ);
> 
> You could just use  execl()  and drop the trailing 'environ' here,
> since that is the default anyway.

execl() is not in the async-signal-safe list.

> 
> > +        }
> > +
> > +        /*
> > +         * If we get here either pm-utils is not installed or  execle() has
> > +         * failed. Let's try the manual approach if mode is not hybrid (as
> > +         * it's only supported by pm-utils)
> > +         */
> > +
> > +        slog("could not execute %s\n", pmutils_bin);
> > +        if (strcmp(mode, "hybrid") == 0) {
> > +            _exit(EXIT_FAILURE);
> > +        }
> > +
> > +        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(EXIT_FAILURE);
> > +        }
> > +
> > +        cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
> > +        if (write(fd, cmd, strlen(cmd)) < 0) {
> > +            slog("failued to write to %s\n", LINUX_SYS_STATE_FILE);
> > +            _exit(EXIT_FAILURE);
> > +        }
> > +
> > +        _exit(EXIT_SUCCESS);
> > +    } 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)
> >  {
> 
> 
> Regards,
> Daniel
Luiz Capitulino Jan. 17, 2012, 12:22 p.m. UTC | #4
On Mon, 16 Jan 2012 16:17:34 -0600
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On 01/16/2012 02:09 PM, Luiz Capitulino wrote:
> > The guest-suspend command supports three modes:
> >
> >   o hibernate (suspend to disk)
> >   o sleep     (suspend to ram)
> >   o hybrid    (save RAM contents to disk, but suspend instead of
> >                powering off)
> >
> > Before trying to suspend, the command queries the guest in order
> > to know whether the given mode is supported. The sleep and hybrid
> > modes are only supported in QEMU 1.1 and later though, because
> > QEMU's S3 support is broken in previous versions.
> >
> > The guest-suspend command will use the scripts provided by the
> > pm-utils package if they are available. If they aren't, a manual
> > process which directly writes to the "/sys/power/state" file is
> > used.
> >
> > To reap terminated children, a new signal handler is installed in
> > the parent to catch SIGCHLD signals and a non-blocking call to
> > waitpid() is done to collect their exit statuses. The statuses,
> > however, are discarded.
> >
> > The approach used to query the guest for suspend support deserves
> > some explanation. It's implemented by bios_supports_mode() and shown
> > below:
> >
> >     qemu-ga
> >       |
> >   create pipe
> >       |
> >     fork()
> >       -----------------
> >       |               |
> >       |               |
> >       |             fork()
> >       |               --------------------------
> >       |               |                        |
> >       |               |                        |
> >       |               |               exec('pm-is-supported')
> >       |               |
> >       |              wait()
> >       |       write exit status to pipe
> >       |              exit
> >       |
> >    read pipe
> >
> > This might look complex, but the resulting code is quite simple.
> > The purpose of that approach is to allow qemu-ga to reap its
> > children (semi-)automatically from its SIGCHLD handler.
> >
> > Implementing this the obvious way, that's, doing the exec() call
> > from the first child process, would force us to introduce a more
> > complex way to reap qemu-ga's children. Like registering PIDs to
> > be reaped and having a way to wait for them when returning their
> > exit status to qemu-ga is necessary. The approach explained
> > above avoids that complexity.
> >
> > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> > ---
> >   qapi-schema-guest.json     |   32 ++++++
> >   qemu-ga.c                  |   18 +++-
> >   qga/guest-agent-commands.c |  263 ++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 312 insertions(+), 1 deletions(-)
> >
> > diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
> > index 5f8a18d..7dd9267 100644
> > --- a/qapi-schema-guest.json
> > +++ b/qapi-schema-guest.json
> > @@ -219,3 +219,35 @@
> >   ##
> >   { '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
> > +#
> > +# Returns: nothing on success
> > +#          If @mode is not supported by the guest, Unsupported
> > +#
> > +# Notes: o This is an asynchronous request. There's no guarantee a response
> > +#          will be sent.
> > +#        o Errors will be logged to guest's syslog.
> > +#        o It's strongly recommended to issue the guest-sync command before
> > +#          sending commands when the guest resumes.
> > +#
> > +# Since: 1.1
> > +##
> > +{ 'command': 'guest-suspend', 'data': { 'mode': 'str' } }
> > diff --git a/qemu-ga.c b/qemu-ga.c
> > index 647df82..f531084 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,16 @@ static void quit_handler(int sig)
> >       }
> >   }
> >
> > +/* reap _all_ terminated children */
> > +static void child_handler(int sig)
> > +{
> > +    int status;
> > +    while (waitpid(-1,&status, WNOHANG)>  0) /* NOTHING */;
> > +}
> > +
> >   static void register_signal_handlers(void)
> >   {
> > -    struct sigaction sigact;
> > +    struct sigaction sigact, sigact_chld;
> >       int ret;
> >
> >       memset(&sigact, 0, sizeof(struct sigaction));
> > @@ -76,6 +84,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..e64b0e0 100644
> > --- a/qga/guest-agent-commands.c
> > +++ b/qga/guest-agent-commands.c
> > @@ -23,6 +23,7 @@
> >
> >   #include<sys/types.h>
> >   #include<sys/ioctl.h>
> > +#include<sys/wait.h>
> >   #include "qga/guest-agent-core.h"
> >   #include "qga-qmp-commands.h"
> >   #include "qerror.h"
> > @@ -44,6 +45,54 @@ static void slog(const char *fmt, ...)
> >       va_end(ap);
> >   }
> >
> > +static void reopen_fd_to_null(int fd)
> > +{
> > +    int nullfd;
> > +
> > +    nullfd = open("/dev/null", O_RDWR);
> > +    if (nullfd<  0) {
> > +        return;
> > +    }
> > +
> > +    dup2(nullfd, fd);
> > +
> > +    if (nullfd != fd) {
> > +        close(nullfd);
> > +    }
> > +}
> > +
> > +/* Try to find executable file 'file'. If it's found, its absolute path is
> > +   returned in 'abs_path' and the function returns true. If it's not found,
> > +   the function will return false and 'abs_path' will contain zeros */
> > +static bool find_executable_file(const char *file, char *abs_path, size_t len)
> > +{
> > +    char *path, *saveptr;
> > +    const char *token, *delim = ":";
> > +
> > +    if (!getenv("PATH")) {
> > +        return false;
> > +    }
> > +
> > +    path = g_strdup(getenv("PATH"));
> > +    if (!path) {
> > +        return false;
> > +    }
> > +
> > +    for (token = strtok_r(path, delim,&saveptr); token;
> > +         token = strtok_r(NULL, delim,&saveptr)) {
> > +        snprintf(abs_path, len, "%s/%s", token, file);
> > +        if (access(abs_path, X_OK) == 0) {
> > +            g_free(path);
> > +            return true;
> > +        }
> > +    }
> > +
> > +    g_free(path);
> > +    memset(abs_path, 0, len);
> > +
> > +    return false;
> > +}
> > +
> >   int64_t qmp_guest_sync(int64_t id, Error **errp)
> >   {
> >       return id;
> > @@ -574,6 +623,220 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
> >   }
> >   #endif
> >
> > +#define LINUX_SYS_STATE_FILE "/sys/power/state"
> > +#define SUS_MODE_SUPPORTED 0
> > +#define SUS_MODE_NOT_SUPPORTED 1
> > +
> > +/**
> > + * This function forks twice and the information about the mode support
> > + * status is passed to the qemu-ga process via a pipe.
> > + *
> > + * This approach allows us to keep the way we reap terminated children
> > + * in qemu-ga quite simple.
> > + */
> > +static bool bios_supports_mode(const char *mode, Error **err)
> > +{
> > +    pid_t pid;
> > +    ssize_t ret;
> > +    bool has_pmutils;
> > +    int status, pipefds[2];
> > +    char pmutils_path[PATH_MAX];
> > +    const char *pmutils_bin = "pm-is-supported";
> > +
> > +    if (pipe(pipefds)<  0) {
> > +        error_set(err, QERR_UNDEFINED_ERROR);
> > +        return false;
> > +    }
> > +
> > +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> > +                                       sizeof(pmutils_path));
> > +
> > +    pid = fork();
> > +    if (!pid) {
> > +        struct sigaction act;
> > +
> > +        memset(&act, 0, sizeof(act));
> > +        act.sa_handler = SIG_DFL;
> > +        sigaction(SIGCHLD,&act, NULL);
> > +
> > +        setsid();
> > +        close(pipefds[0]);
> > +        reopen_fd_to_null(0);
> > +        reopen_fd_to_null(1);
> > +        reopen_fd_to_null(2);
> > +
> > +        pid = fork();
> > +        if (!pid) {
> > +            int fd;
> > +            char buf[32]; /* hopefully big enough */
> > +            const char *arg;
> > +
> > +            if (strcmp(mode, "hibernate") == 0) {
> > +                arg = "--hibernate";
> > +            } else if (strcmp(mode, "sleep") == 0) {
> > +                arg = "--suspend";
> > +            } else if (strcmp(mode, "hybrid") == 0) {
> > +                arg = "--suspend-hybrid";
> > +            } else {
> > +                _exit(SUS_MODE_NOT_SUPPORTED);
> > +            }
> > +
> > +            if (has_pmutils) {
> > +                execle(pmutils_path, pmutils_bin, arg, NULL, environ);
> > +            }
> > +
> > +            /*
> > +             * If we get here either pm-utils is not installed or  execle() has
> > +             * failed. Let's try the manual approach if mode is not hybrid (as
> > +             * it's only supported by pm-utils)
> > +             */
> > +
> > +            if (strcmp(mode, "hybrid") == 0) {
> > +                _exit(SUS_MODE_NOT_SUPPORTED);
> > +            }
> > +
> > +            fd = open(LINUX_SYS_STATE_FILE, O_RDONLY);
> > +            if (fd<  0) {
> > +                _exit(SUS_MODE_NOT_SUPPORTED);
> > +            }
> > +
> > +            ret = read(fd, buf, sizeof(buf));
> > +            if (ret<= 0) {
> > +                _exit(SUS_MODE_NOT_SUPPORTED);
> > +            }
> > +
> > +            buf[ret] = '\0';
> > +            if (strcmp(mode, "hibernate") == 0&&  strstr(buf, "disk")) {
> > +                _exit(SUS_MODE_SUPPORTED);
> > +            } else if (strcmp(mode, "sleep") == 0&&  strstr(buf, "mem")) {
> > +                _exit(SUS_MODE_SUPPORTED);
> > +            }
> > +
> > +            _exit(SUS_MODE_NOT_SUPPORTED);
> > +        }
> > +
> > +        if (pid>  0) {
> > +            wait(&status);
> > +        } else {
> > +            status = SUS_MODE_NOT_SUPPORTED;
> > +        }
> > +
> > +        ret = write(pipefds[1],&status, sizeof(status));
> > +        if (ret != sizeof(status)) {
> > +            _exit(EXIT_FAILURE);
> > +        }
> > +
> > +        _exit(EXIT_SUCCESS);
> > +    }
> > +
> > +    close(pipefds[1]);
> > +
> > +    if (pid>  0) {
> > +        ret = read(pipefds[0],&status, sizeof(status));
> > +        if (ret == sizeof(status)&&  WIFEXITED(status)&&
> > +            WEXITSTATUS(status) == SUS_MODE_SUPPORTED) {
> > +            close(pipefds[0]);
> > +            return true;
> > +        }
> > +    }
> > +
> > +    close(pipefds[0]);
> > +    return false;
> > +}
> > +
> > +static bool host_supports_mode(const char *mode)
> > +{
> > +    if (strcmp(mode, "hibernate")) {
> > +        /* sleep&  hybrid are only supported in qemu 1.1.0 and above */
> > +        return ga_has_support_level(1, 1, 0);
> > +    }
> > +    return true;
> > +}
> > +
> > +void qmp_guest_suspend(const char *mode, Error **err)
> > +{
> > +    pid_t pid;
> > +    bool has_pmutils;
> > +    Error *local_err = NULL;
> > +    const char *pmutils_bin;
> > +    char pmutils_path[PATH_MAX];
> > +
> > +    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;
> > +    }
> > +
> > +    if (!host_supports_mode(mode)) {
> > +        error_set(err, QERR_UNSUPPORTED);
> > +        return;
> > +    }
> > +
> > +    if (!bios_supports_mode(mode,&local_err)) {
> > +        if (error_is_set(&local_err)) {
> > +            error_propagate(err, local_err);
> > +        } else {
> > +            error_set(err, QERR_UNSUPPORTED);
> > +        }
> > +        return;
> > +    }
> > +
> > +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> > +                                       sizeof(pmutils_path));
> > +
> > +    pid = fork();
> > +    if (pid == 0) {
> > +        /* child */
> > +        int fd;
> > +        const char *cmd;
> > +
> > +        setsid();
> > +        reopen_fd_to_null(0);
> > +        reopen_fd_to_null(1);
> > +        reopen_fd_to_null(2);
> > +
> > +        if (has_pmutils) {
> > +            execle(pmutils_path, pmutils_bin, NULL, environ);
> > +        }
> > +
> > +        /*
> > +         * If we get here either pm-utils is not installed or  execle() has
> > +         * failed. Let's try the manual approach if mode is not hybrid (as
> > +         * it's only supported by pm-utils)
> > +         */
> > +
> > +        slog("could not execute %s\n", pmutils_bin);
> 
> slog() is actually a wrapper around syslog(), so I guess we need to 
> scrap these in light of the previous discussion. shutdown() has the same 
> problem though...
> 
> So maybe, just leave these in, and I'll follow up with a patch that logs 
> to a separate file or something if pid != PARENT_PID.

Yes, what I had in mind was to leave them there and fix slog() in parallel.

But as we're putting a big effort on making qmp_guest_suspend() as safe as
possible, I think I'll drop them for now and only add them back when they
get fixed.

> 
> > +        if (strcmp(mode, "hybrid") == 0) {
> > +            _exit(EXIT_FAILURE);
> > +        }
> > +
> > +        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(EXIT_FAILURE);
> > +        }
> > +
> > +        cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
> > +        if (write(fd, cmd, strlen(cmd))<  0) {
> > +            slog("failued to write to %s\n", LINUX_SYS_STATE_FILE);
> > +            _exit(EXIT_FAILURE);
> > +        }
> > +
> > +        _exit(EXIT_SUCCESS);
> > +    } 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)
> >   {
>
Daniel P. Berrangé Jan. 17, 2012, 12:27 p.m. UTC | #5
On Tue, Jan 17, 2012 at 10:18:34AM -0200, Luiz Capitulino wrote:
> On Mon, 16 Jan 2012 21:06:27 +0000
> "Daniel P. Berrange" <berrange@redhat.com> wrote:
> > > +    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
> > > +                                       sizeof(pmutils_path));
> > > +
> > > +    pid = fork();
> > > +    if (pid == 0) {
> > > +        /* child */
> > > +        int fd;
> > > +        const char *cmd;
> > > +
> > > +        setsid();
> > > +        reopen_fd_to_null(0);
> > > +        reopen_fd_to_null(1);
> > > +        reopen_fd_to_null(2);
> > > +
> > > +        if (has_pmutils) {
> > > +            execle(pmutils_path, pmutils_bin, NULL, environ);
> > 
> > You could just use  execl()  and drop the trailing 'environ' here,
> > since that is the default anyway.
> 
> execl() is not in the async-signal-safe list.

It was not in POSIX.1-2004, but POSIX.1-2008 added it. I don't thing
this is worth arguing over though, so just leave it as you have :-)

Daniel
diff mbox

Patch

diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index 5f8a18d..7dd9267 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -219,3 +219,35 @@ 
 ##
 { '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
+#
+# Returns: nothing on success
+#          If @mode is not supported by the guest, Unsupported
+#
+# Notes: o This is an asynchronous request. There's no guarantee a response
+#          will be sent.
+#        o Errors will be logged to guest's syslog.
+#        o It's strongly recommended to issue the guest-sync command before
+#          sending commands when the guest resumes.
+#
+# Since: 1.1
+##
+{ 'command': 'guest-suspend', 'data': { 'mode': 'str' } }
diff --git a/qemu-ga.c b/qemu-ga.c
index 647df82..f531084 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,16 @@  static void quit_handler(int sig)
     }
 }
 
+/* reap _all_ terminated children */
+static void child_handler(int sig)
+{
+    int status;
+    while (waitpid(-1, &status, WNOHANG) > 0) /* NOTHING */;
+}
+
 static void register_signal_handlers(void)
 {
-    struct sigaction sigact;
+    struct sigaction sigact, sigact_chld;
     int ret;
 
     memset(&sigact, 0, sizeof(struct sigaction));
@@ -76,6 +84,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..e64b0e0 100644
--- a/qga/guest-agent-commands.c
+++ b/qga/guest-agent-commands.c
@@ -23,6 +23,7 @@ 
 
 #include <sys/types.h>
 #include <sys/ioctl.h>
+#include <sys/wait.h>
 #include "qga/guest-agent-core.h"
 #include "qga-qmp-commands.h"
 #include "qerror.h"
@@ -44,6 +45,54 @@  static void slog(const char *fmt, ...)
     va_end(ap);
 }
 
+static void reopen_fd_to_null(int fd)
+{
+    int nullfd;
+
+    nullfd = open("/dev/null", O_RDWR);
+    if (nullfd < 0) {
+        return;
+    }
+
+    dup2(nullfd, fd);
+
+    if (nullfd != fd) {
+        close(nullfd);
+    }
+}
+
+/* Try to find executable file 'file'. If it's found, its absolute path is
+   returned in 'abs_path' and the function returns true. If it's not found,
+   the function will return false and 'abs_path' will contain zeros */
+static bool find_executable_file(const char *file, char *abs_path, size_t len)
+{
+    char *path, *saveptr;
+    const char *token, *delim = ":";
+ 
+    if (!getenv("PATH")) {
+        return false;
+    }
+
+    path = g_strdup(getenv("PATH"));
+    if (!path) {
+        return false;
+    }
+
+    for (token = strtok_r(path, delim, &saveptr); token;
+         token = strtok_r(NULL, delim, &saveptr)) {
+        snprintf(abs_path, len, "%s/%s", token, file);
+        if (access(abs_path, X_OK) == 0) {
+            g_free(path);
+            return true;
+        }
+    }
+
+    g_free(path);
+    memset(abs_path, 0, len);
+
+    return false;
+}
+
 int64_t qmp_guest_sync(int64_t id, Error **errp)
 {
     return id;
@@ -574,6 +623,220 @@  int64_t qmp_guest_fsfreeze_thaw(Error **err)
 }
 #endif
 
+#define LINUX_SYS_STATE_FILE "/sys/power/state"
+#define SUS_MODE_SUPPORTED 0
+#define SUS_MODE_NOT_SUPPORTED 1
+
+/**
+ * This function forks twice and the information about the mode support
+ * status is passed to the qemu-ga process via a pipe.
+ *
+ * This approach allows us to keep the way we reap terminated children
+ * in qemu-ga quite simple.
+ */
+static bool bios_supports_mode(const char *mode, Error **err)
+{
+    pid_t pid;
+    ssize_t ret;
+    bool has_pmutils;
+    int status, pipefds[2];
+    char pmutils_path[PATH_MAX];
+    const char *pmutils_bin = "pm-is-supported";
+
+    if (pipe(pipefds) < 0) {
+        error_set(err, QERR_UNDEFINED_ERROR);
+        return false;
+    }
+
+    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
+                                       sizeof(pmutils_path));
+
+    pid = fork();
+    if (!pid) {
+        struct sigaction act;
+
+        memset(&act, 0, sizeof(act));
+        act.sa_handler = SIG_DFL;
+        sigaction(SIGCHLD, &act, NULL);
+
+        setsid();
+        close(pipefds[0]);
+        reopen_fd_to_null(0);
+        reopen_fd_to_null(1);
+        reopen_fd_to_null(2);
+
+        pid = fork();
+        if (!pid) {
+            int fd;
+            char buf[32]; /* hopefully big enough */
+            const char *arg;
+
+            if (strcmp(mode, "hibernate") == 0) {
+                arg = "--hibernate";
+            } else if (strcmp(mode, "sleep") == 0) {
+                arg = "--suspend";
+            } else if (strcmp(mode, "hybrid") == 0) {
+                arg = "--suspend-hybrid";
+            } else {
+                _exit(SUS_MODE_NOT_SUPPORTED);
+            }
+
+            if (has_pmutils) {
+                execle(pmutils_path, pmutils_bin, arg, NULL, environ);
+            }
+
+            /*
+             * If we get here either pm-utils is not installed or  execle() has
+             * failed. Let's try the manual approach if mode is not hybrid (as
+             * it's only supported by pm-utils)
+             */
+
+            if (strcmp(mode, "hybrid") == 0) {
+                _exit(SUS_MODE_NOT_SUPPORTED);
+            }
+
+            fd = open(LINUX_SYS_STATE_FILE, O_RDONLY);
+            if (fd < 0) {
+                _exit(SUS_MODE_NOT_SUPPORTED);
+            }
+
+            ret = read(fd, buf, sizeof(buf));
+            if (ret <= 0) {
+                _exit(SUS_MODE_NOT_SUPPORTED);
+            }
+
+            buf[ret] = '\0';
+            if (strcmp(mode, "hibernate") == 0 && strstr(buf, "disk")) {
+                _exit(SUS_MODE_SUPPORTED);
+            } else if (strcmp(mode, "sleep") == 0 && strstr(buf, "mem")) {
+                _exit(SUS_MODE_SUPPORTED);
+            }
+
+            _exit(SUS_MODE_NOT_SUPPORTED);
+        }
+
+        if (pid > 0) {
+            wait(&status);
+        } else {
+            status = SUS_MODE_NOT_SUPPORTED;
+        }
+
+        ret = write(pipefds[1], &status, sizeof(status));
+        if (ret != sizeof(status)) {
+            _exit(EXIT_FAILURE);
+        }
+
+        _exit(EXIT_SUCCESS);
+    }
+
+    close(pipefds[1]);
+
+    if (pid > 0) {
+        ret = read(pipefds[0], &status, sizeof(status));
+        if (ret == sizeof(status) && WIFEXITED(status) &&
+            WEXITSTATUS(status) == SUS_MODE_SUPPORTED) {
+            close(pipefds[0]);
+            return true;
+        }
+    }
+
+    close(pipefds[0]);
+    return false;
+}
+
+static bool host_supports_mode(const char *mode)
+{
+    if (strcmp(mode, "hibernate")) {
+        /* sleep & hybrid are only supported in qemu 1.1.0 and above */
+        return ga_has_support_level(1, 1, 0);
+    }
+    return true;
+}
+
+void qmp_guest_suspend(const char *mode, Error **err)
+{
+    pid_t pid;
+    bool has_pmutils;
+    Error *local_err = NULL;
+    const char *pmutils_bin;
+    char pmutils_path[PATH_MAX];
+
+    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;
+    }
+
+    if (!host_supports_mode(mode)) {
+        error_set(err, QERR_UNSUPPORTED);
+        return;
+    }
+
+    if (!bios_supports_mode(mode, &local_err)) {
+        if (error_is_set(&local_err)) {
+            error_propagate(err, local_err);
+        } else {
+            error_set(err, QERR_UNSUPPORTED);
+        }
+        return;
+    }
+
+    has_pmutils = find_executable_file(pmutils_bin, pmutils_path,
+                                       sizeof(pmutils_path));
+
+    pid = fork();
+    if (pid == 0) {
+        /* child */
+        int fd;
+        const char *cmd;
+
+        setsid();
+        reopen_fd_to_null(0);
+        reopen_fd_to_null(1);
+        reopen_fd_to_null(2);
+
+        if (has_pmutils) {
+            execle(pmutils_path, pmutils_bin, NULL, environ);
+        }
+
+        /*
+         * If we get here either pm-utils is not installed or  execle() has
+         * failed. Let's try the manual approach if mode is not hybrid (as
+         * it's only supported by pm-utils)
+         */
+
+        slog("could not execute %s\n", pmutils_bin);
+        if (strcmp(mode, "hybrid") == 0) {
+            _exit(EXIT_FAILURE);
+        }
+
+        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(EXIT_FAILURE);
+        }
+
+        cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
+        if (write(fd, cmd, strlen(cmd)) < 0) {
+            slog("failued to write to %s\n", LINUX_SYS_STATE_FILE);
+            _exit(EXIT_FAILURE);
+        }
+
+        _exit(EXIT_SUCCESS);
+    } 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)
 {