On Mon, 16 Jan 2012 16:17:34 -0600
Michael Roth <mdr...@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<lcapitul...@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)
> > {
>
