From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47307) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RaYxv-0005zE-Eg for qemu-devel@nongnu.org; Tue, 13 Dec 2011 15:28:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RaYxt-0005v2-Q2 for qemu-devel@nongnu.org; Tue, 13 Dec 2011 15:28:43 -0500 Received: from e4.ny.us.ibm.com ([32.97.182.144]:46209) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RaYxt-0005uU-LY for qemu-devel@nongnu.org; Tue, 13 Dec 2011 15:28:41 -0500 Received: from /spool/local by e4.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 13 Dec 2011 15:28:37 -0500 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id pBDKRvv0381974 for ; Tue, 13 Dec 2011 15:28:06 -0500 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id pBDKRvH5028713 for ; Tue, 13 Dec 2011 18:27:57 -0200 Message-ID: <4EE7B54C.2050607@linux.vnet.ibm.com> Date: Tue, 13 Dec 2011 14:27:56 -0600 From: Michael Roth MIME-Version: 1.0 References: <20111213162850.4cd135a3@doriath> In-Reply-To: <20111213162850.4cd135a3@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] qemu-ga: Add the guest-suspend command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Amit Shah , aliguori@us.ibm.com, qemu-devel On 12/13/2011 12:28 PM, Luiz Capitulino wrote: > It supports two modes: "hibernate" (which corresponds to S4) and > "sleep" (which corresponds to S3). It will try to execute the > pm-hibernate or pm-suspend scripts, if the scripts don't exist > the command will try to suspend by directly writing to the > "/sys/power/state" file. > > An interesting implementation detail is how to cleanup the child's > status on termination, so that we don't create zombies. I've > choosen to ignore the SIGCHLD signal. This will cause the kernel to > automatically cleanup the child's status on its termination. > > Signed-off-by: Luiz Capitulino > --- > > I've tested this w/o any virtio driver, as they don't support S4 yet. For > S4 it seems to work ok. I couldn't fully test S3 because we lack a way to > resume from it, but by checking the logs it seems to work fine. > > changelog > --------- > > v2 > > o Rename the command to 'guest-suspend' > o Add 'mode' parameter > o Use pm-utils scripts > o Cleanup child termination status > > qapi-schema-guest.json | 17 +++++++++++ > qemu-ga.c | 11 +++++++- > qga/guest-agent-commands.c | 64 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 91 insertions(+), 1 deletions(-) > > diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json > index 29989fe..656bde9 100644 > --- a/qapi-schema-guest.json > +++ b/qapi-schema-guest.json > @@ -219,3 +219,20 @@ > ## > { 'command': 'guest-fsfreeze-thaw', > 'returns': 'int' } > + > +## > +# @guest-suspend > +# > +# Suspend guest execution by entering ACPI power state S3 or S4. > +# > +# @mode: 'hibernate' RAM content is saved in the disk and the guest is > +# powered down (this corresponds to ACPI S4) > +# 'sleep' execution is suspended but the RAM retains its contents > +# (this corresponds to ACPI S3) 'suspend' is generally associated with sleep, so maybe we should make the mode optional and default to 'sleep'? > +# > +# Notes: This is an asynchronous request. There's no guarantee it will > +# succeed. 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..b32e96c 100644 > --- a/qemu-ga.c > +++ b/qemu-ga.c > @@ -61,7 +61,7 @@ static void quit_handler(int sig) > > static void register_signal_handlers(void) > { > - struct sigaction sigact; > + struct sigaction sigact, sigact_chld; > int ret; > > memset(&sigact, 0, sizeof(struct sigaction)); > @@ -76,6 +76,15 @@ static void register_signal_handlers(void) > if (ret == -1) { > g_error("error configuring signal handler: %s", strerror(errno)); > } > + > + /* This should cause the kernel to automatically cleanup child > + termination status */ > + memset(&sigact_chld, 0, sizeof(struct sigaction)); > + sigact_chld.sa_handler = SIG_IGN; > + 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..4799638 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_PM_UTILS_PATH "/usr/sbin" > +#define LINUX_SYS_STATE_FILE "/sys/power/state" > + > +void qmp_guest_suspend(const char *mode, Error **err) > +{ > + int ret, fd = -1; > + const char *pmutils_bin; > + char pmutils_bin_path[PATH_MAX]; > + > + if (strcmp(mode, "hibernate") == 0) { > + pmutils_bin = "pm-hibernate"; > + } else if (strcmp(mode, "sleep") == 0) { > + pmutils_bin = "pm-suspend"; > + } else { > + error_set(err, QERR_INVALID_PARAMETER, "mode"); > + return; > + } > + > + snprintf(pmutils_bin_path, sizeof(pmutils_bin_path), "%s/%s", > + LINUX_PM_UTILS_PATH, pmutils_bin); I'd be surprised to find any distros where this isn't the case, but for situations where the scripts aren't in /usr/sbin, maybe we'd be better off just passing the command name to execlp() and letting it do the search via PATH? The normal use case is that qemu-ga will get launched as a service, so PATH should have the good stuff. > + > + if (access(pmutils_bin_path, X_OK) != 0) { > + pmutils_bin = NULL; > + fd = open(LINUX_SYS_STATE_FILE, O_WRONLY); > + if (fd< 0) { > + error_set(err, QERR_OPEN_FILE_FAILED, LINUX_SYS_STATE_FILE); > + return; > + } > + } > + > + ret = fork(); > + if (ret == 0) { > + /* child */ > + setsid(); > + fclose(stdin); > + fclose(stdout); > + fclose(stderr); > + > + if (pmutils_bin) { > + ret = execl(pmutils_bin_path, pmutils_bin, NULL); > + if (ret) { > + slog("%s failed: %s", pmutils_bin_path, strerror(errno)); > + } > + } else { > + const char *cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk"; > + ret = write(fd, cmd, strlen(cmd)); > + if (ret< 0) { > + slog("can't write to %s: %s\n", LINUX_SYS_STATE_FILE, > + strerror(errno)); > + } > + close(fd); > + } > + > + exit(!!ret); > + } else if (ret< 0) { > + error_set(err, QERR_UNDEFINED_ERROR); > + return; > + } > + > + if (!pmutils_bin) { > + close(fd); > + } > +} > + > /* register init/cleanup routines for stateful command groups */ > void ga_command_state_init(GAState *s, GACommandState *cs) > {