Patchwork [v2] qemu-ga: Add the guest-suspend command

login
register
mail settings
Submitter Luiz Capitulino
Date Dec. 13, 2011, 6:28 p.m.
Message ID <20111213162850.4cd135a3@doriath>
Download mbox | patch
Permalink /patch/131161/
State New
Headers show

Comments

Luiz Capitulino - Dec. 13, 2011, 6:28 p.m.
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 <lcapitulino@redhat.com>
---

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(-)
Michael Roth - Dec. 13, 2011, 8:03 p.m.
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.

One downside to blocking SIGCHLD is it can screw with child processes 
that utilize it. `sudo` for instance will just hang indefinitely because 
it handles it's own cleanup via SIGCHLD, we might run into similar cases 
with various pm-hibernate/pm-suspend implementations as well.

This will also screw with anything we launch via guest-exec as well, 
once that goes in.

I wonder if we can tie the qemu_add_child_watch() stuff into qemu-ga's 
main loop, or maybe just implement something similar...

Basically:

  - add a qemu-ga.c:signal_channel_add() that creates a non-blocking 
pipe and associate the read-side with a GIOChannel, then ties the 
channel into the main loop via g_io_add_watch() on qemu-ga startup, with 
an associated callback that reads everything off the pipe, then iterates 
through a list of registered pids and does a non-blocking wait(pid, ...) 
on each.

  - add a SIGCHLD handler that writes a 1 to the write side of the pipe 
whenever it gets called

Then, when creating a child, you just register the child by adding the 
pid to the list that the signal channel callback checks.

>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> ---
>
> 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)
> +#
> +# 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);
> +
> +    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)
>   {
Michael Roth - Dec. 13, 2011, 8:27 p.m.
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<lcapitulino@redhat.com>
> ---
>
> 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)
>   {
Daniel P. Berrange - Dec. 13, 2011, 11:13 p.m.
On Tue, Dec 13, 2011 at 04:28:50PM -0200, 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 <lcapitulino@redhat.com>
> ---
> 
> 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)

Standard pm-utils in Linux supports three ways to suspend these
days, suspend, hibernate & suspend-hybrid. libvirt supports
all 3 modes in our recently added API for suspending the physical
host, so I think we'll want to also have all 3 for suspending
guests too.

[quote pm-suspend(8)]
       pm-suspend
           During suspend most devices are shutdown, and
           system state is saved in RAM. The system still
           requires power in this state. Most modern systems
           require 3 to 5 seconds to enter and leave suspend,
           and most laptops can stay in suspend mode for 1 to
           3 days before exhausting their battery.

       pm-hibernate
           During hibernate the system is fully powered off,
           and system state is saved to disk. The system does
           not require power, and can stay in hibernate mode
           indefinitely. Most modern systems require 15 to 45
           seconds to enter and leave hibernate, and entering
           and leaving hibernate takes longer when you have
           more memory.

       pm-suspend-hybrid
           Hybrid-suspend is the process where the system does
           everything it needs to hibernate, but suspends
           instead of shutting down. This means that your
           computer can wake up quicker than for normal
           hibernation if you do not run out of power, and you
           can resume even if you run out of power. s2both(8)
           is an hybrid-suspend implementation.
[/quote]


> 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";

Here you'd just add pm-suspend-hybrid too

Regards,
Daniel
Luiz Capitulino - Dec. 14, 2011, 1 p.m.
On Tue, 13 Dec 2011 14:03:08 -0600
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> 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.
> 
> One downside to blocking SIGCHLD is it can screw with child processes 
> that utilize it. `sudo` for instance will just hang indefinitely because 
> it handles it's own cleanup via SIGCHLD, we might run into similar cases 
> with various pm-hibernate/pm-suspend implementations as well.
> 
> This will also screw with anything we launch via guest-exec as well, 
> once that goes in.
> 
> I wonder if we can tie the qemu_add_child_watch() stuff into qemu-ga's 
> main loop, or maybe just implement something similar...
> 
> Basically:
> 
>   - add a qemu-ga.c:signal_channel_add() that creates a non-blocking 
> pipe and associate the read-side with a GIOChannel, then ties the 
> channel into the main loop via g_io_add_watch() on qemu-ga startup, with 
> an associated callback that reads everything off the pipe, then iterates 
> through a list of registered pids and does a non-blocking wait(pid, ...) 
> on each.
> 
>   - add a SIGCHLD handler that writes a 1 to the write side of the pipe 
> whenever it gets called

Is the pipe really needed? Is there any side effect if we call waitpid()
from the signal handler considering it won't block?

I'm also wondering if we could use g_child_watch_add(), but it's not clear
to me if it works with processes not created with g_spawn_*() functions.

> 
> Then, when creating a child, you just register the child by adding the 
> pid to the list that the signal channel callback checks.
> 
> >
> > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> > ---
> >
> > 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)
> > +#
> > +# 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);
> > +
> > +    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)
> >   {
>
Luiz Capitulino - Dec. 14, 2011, 1:07 p.m.
On Tue, 13 Dec 2011 14:27:56 -0600
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> 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<lcapitulino@redhat.com>
> > ---
> >
> > 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'?

I'm not sure I like having defaults because we're choosing for the client.
If this were a human interface then it would make sense, but for a machine
one I don't think it does. Besides we don't seem to have a way to resume
from S3 so we're probably going to get reports about this command not working
(and that reminds me to add a note about that in the doc).

> 
> > +#
> > +# 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.

Ok, I'll make that change.

> 
> > +
> > +    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)
> >   {
>
Luiz Capitulino - Dec. 14, 2011, 1:08 p.m.
On Tue, 13 Dec 2011 23:13:23 +0000
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Tue, Dec 13, 2011 at 04:28:50PM -0200, 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 <lcapitulino@redhat.com>
> > ---
> > 
> > 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)
> 
> Standard pm-utils in Linux supports three ways to suspend these
> days, suspend, hibernate & suspend-hybrid. libvirt supports
> all 3 modes in our recently added API for suspending the physical
> host, so I think we'll want to also have all 3 for suspending
> guests too.

Ok.

> 
> [quote pm-suspend(8)]
>        pm-suspend
>            During suspend most devices are shutdown, and
>            system state is saved in RAM. The system still
>            requires power in this state. Most modern systems
>            require 3 to 5 seconds to enter and leave suspend,
>            and most laptops can stay in suspend mode for 1 to
>            3 days before exhausting their battery.
> 
>        pm-hibernate
>            During hibernate the system is fully powered off,
>            and system state is saved to disk. The system does
>            not require power, and can stay in hibernate mode
>            indefinitely. Most modern systems require 15 to 45
>            seconds to enter and leave hibernate, and entering
>            and leaving hibernate takes longer when you have
>            more memory.
> 
>        pm-suspend-hybrid
>            Hybrid-suspend is the process where the system does
>            everything it needs to hibernate, but suspends
>            instead of shutting down. This means that your
>            computer can wake up quicker than for normal
>            hibernation if you do not run out of power, and you
>            can resume even if you run out of power. s2both(8)
>            is an hybrid-suspend implementation.
> [/quote]
> 
> 
> > 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";
> 
> Here you'd just add pm-suspend-hybrid too
> 
> Regards,
> Daniel
Michael Roth - Dec. 14, 2011, 3:50 p.m.
On 12/14/2011 07:07 AM, Luiz Capitulino wrote:
> On Tue, 13 Dec 2011 14:27:56 -0600
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>> 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<lcapitulino@redhat.com>
>>> ---
>>>
>>> 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'?
>
> I'm not sure I like having defaults because we're choosing for the client.
> If this were a human interface then it would make sense, but for a machine
> one I don't think it does. Besides we don't seem to have a way to resume
> from S3 so we're probably going to get reports about this command not working
> (and that reminds me to add a note about that in the doc).

True, defaulting to broken behavior probably isn't a good idea :)

>
>>
>>> +#
>>> +# 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.
>
> Ok, I'll make that change.
>
>>
>>> +
>>> +    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)
>>>    {
>>
>
Michael Roth - Dec. 14, 2011, 3:54 p.m.
On 12/14/2011 07:00 AM, Luiz Capitulino wrote:
> On Tue, 13 Dec 2011 14:03:08 -0600
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>> 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.
>>
>> One downside to blocking SIGCHLD is it can screw with child processes
>> that utilize it. `sudo` for instance will just hang indefinitely because
>> it handles it's own cleanup via SIGCHLD, we might run into similar cases
>> with various pm-hibernate/pm-suspend implementations as well.
>>
>> This will also screw with anything we launch via guest-exec as well,
>> once that goes in.
>>
>> I wonder if we can tie the qemu_add_child_watch() stuff into qemu-ga's
>> main loop, or maybe just implement something similar...
>>
>> Basically:
>>
>>    - add a qemu-ga.c:signal_channel_add() that creates a non-blocking
>> pipe and associate the read-side with a GIOChannel, then ties the
>> channel into the main loop via g_io_add_watch() on qemu-ga startup, with
>> an associated callback that reads everything off the pipe, then iterates
>> through a list of registered pids and does a non-blocking wait(pid, ...)
>> on each.
>>
>>    - add a SIGCHLD handler that writes a 1 to the write side of the pipe
>> whenever it gets called
>
> Is the pipe really needed? Is there any side effect if we call waitpid()
> from the signal handler considering it won't block?

In the current state of things, I believe that might actually be 
sufficient. I was thinking of the eventual guest-exec case though: we 
need to be able to poll for a guest-exec'd process' return status 
asynchronously by calling a guest-exec-status command that does the 
wait(), so if we use a waitpid(-1, ...) SIGCHLD handler, or block 
SIGCHLD, the return status would be intercepted/lost.

>
> I'm also wondering if we could use g_child_watch_add(), but it's not clear
> to me if it works with processes not created with g_spawn_*() functions.

GPid's map to something other than PIDs on Windows, so I think we'd have 
issues there. But our fork() approach probably wouldn't work at all on 
Windows except maybe under cygwin, so at some point we'd probably want 
to switch over to g_spawn for this kind of stuff anyway...

So this might be a good point to switch over to using the glib functions.

Would you mind trying to do the hibernate/zombie reaping stuff using 
g_spawn+g_child_watch_add()? It might end up being the easiest route. 
Otherwise I can take a look at it later today.

>
>>
>> Then, when creating a child, you just register the child by adding the
>> pid to the list that the signal channel callback checks.
>>
>>>
>>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>>> ---
>>>
>>> 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)
>>> +#
>>> +# 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);
>>> +
>>> +    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)
>>>    {
>>
>
Luiz Capitulino - Dec. 14, 2011, 4:38 p.m.
On Wed, 14 Dec 2011 09:54:29 -0600
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On 12/14/2011 07:00 AM, Luiz Capitulino wrote:
> > On Tue, 13 Dec 2011 14:03:08 -0600
> > Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
> >
> >> 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.
> >>
> >> One downside to blocking SIGCHLD is it can screw with child processes
> >> that utilize it. `sudo` for instance will just hang indefinitely because
> >> it handles it's own cleanup via SIGCHLD, we might run into similar cases
> >> with various pm-hibernate/pm-suspend implementations as well.
> >>
> >> This will also screw with anything we launch via guest-exec as well,
> >> once that goes in.
> >>
> >> I wonder if we can tie the qemu_add_child_watch() stuff into qemu-ga's
> >> main loop, or maybe just implement something similar...
> >>
> >> Basically:
> >>
> >>    - add a qemu-ga.c:signal_channel_add() that creates a non-blocking
> >> pipe and associate the read-side with a GIOChannel, then ties the
> >> channel into the main loop via g_io_add_watch() on qemu-ga startup, with
> >> an associated callback that reads everything off the pipe, then iterates
> >> through a list of registered pids and does a non-blocking wait(pid, ...)
> >> on each.
> >>
> >>    - add a SIGCHLD handler that writes a 1 to the write side of the pipe
> >> whenever it gets called
> >
> > Is the pipe really needed? Is there any side effect if we call waitpid()
> > from the signal handler considering it won't block?
> 
> In the current state of things, I believe that might actually be 
> sufficient. I was thinking of the eventual guest-exec case though: we 
> need to be able to poll for a guest-exec'd process' return status 
> asynchronously by calling a guest-exec-status command that does the 
> wait(), so if we use a waitpid(-1, ...) SIGCHLD handler, or block 
> SIGCHLD, the return status would be intercepted/lost.

What I had in mind was to do what qemu_add_child_watch() does, ie. it
iterates through a list of child pids calling waitpid() on them instead
of doing waitpid(-1,...). The only difference is that we would do it from
a signal handler.

Maybe that could work for guest-exec too: we could let the signal handler
collect the exit status and store them. Then guest-status could retrieve
the status from there.

We have several options here. Maybe I should do the simplest solution for
the guest-suspend command and you work on improving it for guest-exec.

> >
> > I'm also wondering if we could use g_child_watch_add(), but it's not clear
> > to me if it works with processes not created with g_spawn_*() functions.
> 
> GPid's map to something other than PIDs on Windows, so I think we'd have 
> issues there. But our fork() approach probably wouldn't work at all on 
> Windows except maybe under cygwin, so at some point we'd probably want 
> to switch over to g_spawn for this kind of stuff anyway...
> 
> So this might be a good point to switch over to using the glib functions.
> 
> Would you mind trying to do the hibernate/zombie reaping stuff using 
> g_spawn+g_child_watch_add()? It might end up being the easiest route. 
> Otherwise I can take a look at it later today.

Well, there are two problems with g_spawn wrt to the manual method of
writing to the sysfs file. The first one is that I'm not sure if g_spawn()
reports the file not found error synchronously. The other problem is that,
I'd have to fork() anyway to write to the sysfs file (unless we decide that
it's ok to do this synchronously, which seems ok to me).

> 
> >
> >>
> >> Then, when creating a child, you just register the child by adding the
> >> pid to the list that the signal channel callback checks.
> >>
> >>>
> >>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> >>> ---
> >>>
> >>> 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)
> >>> +#
> >>> +# 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);
> >>> +
> >>> +    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)
> >>>    {
> >>
> >
>
Michael Roth - Dec. 14, 2011, 6:06 p.m.
On 12/14/2011 10:38 AM, Luiz Capitulino wrote:
> On Wed, 14 Dec 2011 09:54:29 -0600
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>> On 12/14/2011 07:00 AM, Luiz Capitulino wrote:
>>> On Tue, 13 Dec 2011 14:03:08 -0600
>>> Michael Roth<mdroth@linux.vnet.ibm.com>   wrote:
>>>
>>>> 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.
>>>>
>>>> One downside to blocking SIGCHLD is it can screw with child processes
>>>> that utilize it. `sudo` for instance will just hang indefinitely because
>>>> it handles it's own cleanup via SIGCHLD, we might run into similar cases
>>>> with various pm-hibernate/pm-suspend implementations as well.
>>>>
>>>> This will also screw with anything we launch via guest-exec as well,
>>>> once that goes in.
>>>>
>>>> I wonder if we can tie the qemu_add_child_watch() stuff into qemu-ga's
>>>> main loop, or maybe just implement something similar...
>>>>
>>>> Basically:
>>>>
>>>>     - add a qemu-ga.c:signal_channel_add() that creates a non-blocking
>>>> pipe and associate the read-side with a GIOChannel, then ties the
>>>> channel into the main loop via g_io_add_watch() on qemu-ga startup, with
>>>> an associated callback that reads everything off the pipe, then iterates
>>>> through a list of registered pids and does a non-blocking wait(pid, ...)
>>>> on each.
>>>>
>>>>     - add a SIGCHLD handler that writes a 1 to the write side of the pipe
>>>> whenever it gets called
>>>
>>> Is the pipe really needed? Is there any side effect if we call waitpid()
>>> from the signal handler considering it won't block?
>>
>> In the current state of things, I believe that might actually be
>> sufficient. I was thinking of the eventual guest-exec case though: we
>> need to be able to poll for a guest-exec'd process' return status
>> asynchronously by calling a guest-exec-status command that does the
>> wait(), so if we use a waitpid(-1, ...) SIGCHLD handler, or block
>> SIGCHLD, the return status would be intercepted/lost.
>
> What I had in mind was to do what qemu_add_child_watch() does, ie. it
> iterates through a list of child pids calling waitpid() on them instead
> of doing waitpid(-1,...). The only difference is that we would do it from
> a signal handler.

Ah, yah that might work. I'm not sure how well our list functions will 
behave when accessed via an interrupt handler. We'll have race 
conditions at least:

#define QTAILQ_INSERT_TAIL(head, elm, field) do {                      \
         (elm)->field.tqe_next = NULL;                                  \
         (elm)->field.tqe_prev = (head)->tqh_last;                      \
         /* interrupt */
         *(head)->tqh_last = (elm);                                     \
         (head)->tqh_last = &(elm)->field.tqe_next;

So if we fork() and the process completes and triggers the interrupt 
before we add the pid to the list, it will remain a zombie until 
something else triggers the handler. It's not a huge deal...we at least 
avoid accumulating zombies, but I'd be weary of other side-effects as 
well, especially if we also remove entries from the list if they've been 
waited on successfully.

>
> Maybe that could work for guest-exec too: we could let the signal handler
> collect the exit status and store them. Then guest-status could retrieve
> the status from there.

Yah, I think we can tie all this together rather nicely with a little work.

>
> We have several options here. Maybe I should do the simplest solution for
> the guest-suspend command and you work on improving it for guest-exec.

That's fine, I need to look at this more. But if we're gonna take the 
simplest approach for now, let's not even bother with the pid 
registration (and potential races), and just do a waitpid(-1, ...) in a 
SIGCHLD handler like QEMU did before the child watch stuff was added, 
and I'll make the changes necessary for the guest-exec stuff before that 
gets added.

>
>>>
>>> I'm also wondering if we could use g_child_watch_add(), but it's not clear
>>> to me if it works with processes not created with g_spawn_*() functions.
>>
>> GPid's map to something other than PIDs on Windows, so I think we'd have
>> issues there. But our fork() approach probably wouldn't work at all on
>> Windows except maybe under cygwin, so at some point we'd probably want
>> to switch over to g_spawn for this kind of stuff anyway...
>>
>> So this might be a good point to switch over to using the glib functions.
>>
>> Would you mind trying to do the hibernate/zombie reaping stuff using
>> g_spawn+g_child_watch_add()? It might end up being the easiest route.
>> Otherwise I can take a look at it later today.
>
> Well, there are two problems with g_spawn wrt to the manual method of
> writing to the sysfs file. The first one is that I'm not sure if g_spawn()
> reports the file not found error synchronously. The other problem is that,

"error can be NULL to ignore errors, or non-NULL to report errors. If an 
error is set, the function returns FALSE. Errors are reported even if 
they occur in the child (for example if the executable in argv[0] is not 
found). Typically the message field of returned errors should be 
displayed to users. Possible errors are those from the G_SPAWN_ERROR 
domain."

So I'd think we'd be able to report the FNF synchronously even if we 
launch into the background.

> I'd have to fork() anyway to write to the sysfs file (unless we decide that
> it's ok to do this synchronously, which seems ok to me).

I think what we'd have to do is open the file, then pass it in as stdout 
for a g_spawn'd `echo "disk"` command. I'm not sure how we'd manage 
cleaning up the FD though, if we're doing it asynchronously. And 
synchronous means every guest-suspend call will eventually time out, so 
I don't think that's doable right now.

So I'd say keep it the way it is for now. Probably not worth spending 
too much time on until we see what the Windows stuff looks like anyway.

>
>>
>>>
>>>>
>>>> Then, when creating a child, you just register the child by adding the
>>>> pid to the list that the signal channel callback checks.
>>>>
>>>>>
>>>>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>>>>> ---
>>>>>
>>>>> 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)
>>>>> +#
>>>>> +# 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);
>>>>> +
>>>>> +    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)
>>>>>     {
>>>>
>>>
>>
>
Luiz Capitulino - Dec. 14, 2011, 11:44 p.m.
On Wed, 14 Dec 2011 12:06:13 -0600
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On 12/14/2011 10:38 AM, Luiz Capitulino wrote:
> > On Wed, 14 Dec 2011 09:54:29 -0600
> > Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
> >
> >> On 12/14/2011 07:00 AM, Luiz Capitulino wrote:
> >>> On Tue, 13 Dec 2011 14:03:08 -0600
> >>> Michael Roth<mdroth@linux.vnet.ibm.com>   wrote:
> >>>
> >>>> 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.
> >>>>
> >>>> One downside to blocking SIGCHLD is it can screw with child processes
> >>>> that utilize it. `sudo` for instance will just hang indefinitely because
> >>>> it handles it's own cleanup via SIGCHLD, we might run into similar cases
> >>>> with various pm-hibernate/pm-suspend implementations as well.
> >>>>
> >>>> This will also screw with anything we launch via guest-exec as well,
> >>>> once that goes in.
> >>>>
> >>>> I wonder if we can tie the qemu_add_child_watch() stuff into qemu-ga's
> >>>> main loop, or maybe just implement something similar...
> >>>>
> >>>> Basically:
> >>>>
> >>>>     - add a qemu-ga.c:signal_channel_add() that creates a non-blocking
> >>>> pipe and associate the read-side with a GIOChannel, then ties the
> >>>> channel into the main loop via g_io_add_watch() on qemu-ga startup, with
> >>>> an associated callback that reads everything off the pipe, then iterates
> >>>> through a list of registered pids and does a non-blocking wait(pid, ...)
> >>>> on each.
> >>>>
> >>>>     - add a SIGCHLD handler that writes a 1 to the write side of the pipe
> >>>> whenever it gets called
> >>>
> >>> Is the pipe really needed? Is there any side effect if we call waitpid()
> >>> from the signal handler considering it won't block?
> >>
> >> In the current state of things, I believe that might actually be
> >> sufficient. I was thinking of the eventual guest-exec case though: we
> >> need to be able to poll for a guest-exec'd process' return status
> >> asynchronously by calling a guest-exec-status command that does the
> >> wait(), so if we use a waitpid(-1, ...) SIGCHLD handler, or block
> >> SIGCHLD, the return status would be intercepted/lost.
> >
> > What I had in mind was to do what qemu_add_child_watch() does, ie. it
> > iterates through a list of child pids calling waitpid() on them instead
> > of doing waitpid(-1,...). The only difference is that we would do it from
> > a signal handler.
> 
> Ah, yah that might work. I'm not sure how well our list functions will 
> behave when accessed via an interrupt handler. We'll have race 
> conditions at least:
> 
> #define QTAILQ_INSERT_TAIL(head, elm, field) do {                      \
>          (elm)->field.tqe_next = NULL;                                  \
>          (elm)->field.tqe_prev = (head)->tqh_last;                      \
>          /* interrupt */
>          *(head)->tqh_last = (elm);                                     \
>          (head)->tqh_last = &(elm)->field.tqe_next;
> 
> So if we fork() and the process completes and triggers the interrupt 
> before we add the pid to the list, it will remain a zombie until 
> something else triggers the handler. It's not a huge deal...we at least 
> avoid accumulating zombies, but I'd be weary of other side-effects as 
> well, especially if we also remove entries from the list if they've been 
> waited on successfully.

We can block SIGCHLD temporarily while inserting. If a new signal arrives
while it's blocked, it will be queued and will be delivered when unblocked.

> > Maybe that could work for guest-exec too: we could let the signal handler
> > collect the exit status and store them. Then guest-status could retrieve
> > the status from there.
> 
> Yah, I think we can tie all this together rather nicely with a little work.
> 
> >
> > We have several options here. Maybe I should do the simplest solution for
> > the guest-suspend command and you work on improving it for guest-exec.
> 
> That's fine, I need to look at this more. But if we're gonna take the 
> simplest approach for now, let's not even bother with the pid 
> registration (and potential races), and just do a waitpid(-1, ...) in a 
> SIGCHLD handler like QEMU did before the child watch stuff was added, 
> and I'll make the changes necessary for the guest-exec stuff before that 
> gets added.

Okay, that works for me.

> 
> >
> >>>
> >>> I'm also wondering if we could use g_child_watch_add(), but it's not clear
> >>> to me if it works with processes not created with g_spawn_*() functions.
> >>
> >> GPid's map to something other than PIDs on Windows, so I think we'd have
> >> issues there. But our fork() approach probably wouldn't work at all on
> >> Windows except maybe under cygwin, so at some point we'd probably want
> >> to switch over to g_spawn for this kind of stuff anyway...
> >>
> >> So this might be a good point to switch over to using the glib functions.
> >>
> >> Would you mind trying to do the hibernate/zombie reaping stuff using
> >> g_spawn+g_child_watch_add()? It might end up being the easiest route.
> >> Otherwise I can take a look at it later today.
> >
> > Well, there are two problems with g_spawn wrt to the manual method of
> > writing to the sysfs file. The first one is that I'm not sure if g_spawn()
> > reports the file not found error synchronously. The other problem is that,
> 
> "error can be NULL to ignore errors, or non-NULL to report errors. If an 
> error is set, the function returns FALSE. Errors are reported even if 
> they occur in the child (for example if the executable in argv[0] is not 
> found). Typically the message field of returned errors should be 
> displayed to users. Possible errors are those from the G_SPAWN_ERROR 
> domain."
> 
> So I'd think we'd be able to report the FNF synchronously even if we 
> launch into the background.

Yeah, from my testing looks like it does.

> 
> > I'd have to fork() anyway to write to the sysfs file (unless we decide that
> > it's ok to do this synchronously, which seems ok to me).
> 
> I think what we'd have to do is open the file, then pass it in as stdout 
> for a g_spawn'd `echo "disk"` command. I'm not sure how we'd manage 
> cleaning up the FD though, if we're doing it asynchronously. And 
> synchronous means every guest-suspend call will eventually time out, so 
> I don't think that's doable right now.
> 
> So I'd say keep it the way it is for now. Probably not worth spending 
> too much time on until we see what the Windows stuff looks like anyway.

Ok.

> 
> >
> >>
> >>>
> >>>>
> >>>> Then, when creating a child, you just register the child by adding the
> >>>> pid to the list that the signal channel callback checks.
> >>>>
> >>>>>
> >>>>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> >>>>> ---
> >>>>>
> >>>>> 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)
> >>>>> +#
> >>>>> +# 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);
> >>>>> +
> >>>>> +    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)
> >>>>>     {
> >>>>
> >>>
> >>
> >
>

Patch

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)
+#
+# 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);
+
+    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)
 {