From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36195) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RmusS-0005Ut-04 for qemu-devel@nongnu.org; Mon, 16 Jan 2012 17:18:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RmusQ-0003fj-1E for qemu-devel@nongnu.org; Mon, 16 Jan 2012 17:18:07 -0500 Received: from e37.co.us.ibm.com ([32.97.110.158]:48156) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RmusP-0003fN-Gp for qemu-devel@nongnu.org; Mon, 16 Jan 2012 17:18:05 -0500 Received: from /spool/local by e37.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 16 Jan 2012 15:18:01 -0700 Received: from d03relay04.boulder.ibm.com (d03relay04.boulder.ibm.com [9.17.195.106]) by d03dlp01.boulder.ibm.com (Postfix) with ESMTP id 001381FF0049 for ; Mon, 16 Jan 2012 15:17:34 -0700 (MST) Received: from d03av01.boulder.ibm.com (d03av01.boulder.ibm.com [9.17.195.167]) by d03relay04.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q0GMHZDw083060 for ; Mon, 16 Jan 2012 15:17:35 -0700 Received: from d03av01.boulder.ibm.com (loopback [127.0.0.1]) by d03av01.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q0GMHZ26027272 for ; Mon, 16 Jan 2012 15:17:35 -0700 Message-ID: <4F14A1FE.8020809@linux.vnet.ibm.com> Date: Mon, 16 Jan 2012 16:17:34 -0600 From: Michael Roth MIME-Version: 1.0 References: <1326744592-27650-1-git-send-email-lcapitulino@redhat.com> <1326744592-27650-3-git-send-email-lcapitulino@redhat.com> In-Reply-To: <1326744592-27650-3-git-send-email-lcapitulino@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] qemu-ga: Add the guest-suspend command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: eblake@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org 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 > --- > 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 > #include > #include > +#include > #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 > #include > +#include > #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) > {