qemu-ga: Add the guest-suspend command

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<[email protected]>

---

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(-)

qapi-schema-guest.json +17 -0

index 29989fe..656bde9 100644
@@ -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' } }

qemu-ga.c +10 -1

index 60d4972..b32e96c 100644
@@ -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)

qga/guest-agent-commands.c +64 -0

index a09c8ca..4799638 100644
@@ -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)
  {

Quoted 10 lines · Luiz Capitulino

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.

Quoted 152 lines

Signed-off-by Luiz Capitulino<[email protected]>

---

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(-)

qapi-schema-guest.json +17 -0

index 29989fe..656bde9 100644
@@ -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' } }

qemu-ga.c +10 -1

index 60d4972..b32e96c 100644
@@ -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)

qga/guest-agent-commands.c +64 -0

index a09c8ca..4799638 100644
@@ -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)
  {

Quoted 32 lines · On Tue, 13 Dec 2011 14:03:08 -0600 Michael Roth<[email protected]>…

Quoted 10 lines · Luiz Capitulino

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.

Quoted 2 lines

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

Quoted 158 lines

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

Quoted 152 lines

Signed-off-by Luiz Capitulino<[email protected]>

---

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(-)

qapi-schema-guest.json +17 -0

index 29989fe..656bde9 100644
@@ -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' } }

qemu-ga.c +10 -1

index 60d4972..b32e96c 100644
@@ -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)

qga/guest-agent-commands.c +64 -0

index a09c8ca..4799638 100644
@@ -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)
   {

Quoted 35 lines · Luiz Capitulino

Quoted 32 lines · On Tue, 13 Dec 2011 14:03:08 -0600 Michael Roth<[email protected]>…

Quoted 10 lines · Luiz Capitulino

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.

Quoted 5 lines

Quoted 2 lines

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

Quoted 3 lines

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.

Quoted 160 lines

Quoted 159 lines

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

Quoted 152 lines

Signed-off-by Luiz Capitulino<[email protected]>

---

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(-)

qapi-schema-guest.json +17 -0

index 29989fe..656bde9 100644
@@ -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' } }

qemu-ga.c +10 -1

index 60d4972..b32e96c 100644
@@ -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)

qga/guest-agent-commands.c +64 -0

index a09c8ca..4799638 100644
@@ -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)
   {

Quoted 8 lines

Quoted 2 lines

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

Quoted 5 lines

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).

Quoted 14 lines · On Wed, 14 Dec 2011 14:38:55 -0200 Luiz Capitulino<[email protected]>…

Quoted 8 lines

Quoted 2 lines

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

Quoted 5 lines

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.

Quoted 5 lines

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).

The version below uses g_spawn_async(). The code is a bit simpler than previous versions, but there are two important details about it:

1. I'm letting g_spawn_async() reap the child automatically. I don't know how it does it though. I'd guess it uses g_child_watch_add(), worst case it ignores SIGCHLD (although I think this would be awful)

2. The manual method of writing to the sysfs is done synchronously. This means that the command response will only be sent when the guest resumes

If you think this approach is acceptable, I'll test it more, update its doc, etc and post it again.

qapi-schema-guest.json +17 -0

index 5f8a18d..63f65a6 100644
@@ -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' } }

qga/guest-agent-commands.c +50 -0

index a09c8ca..0c6b78e 100644
@@ -574,6 +574,56 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
  }
  #endif
 
+#define LINUX_SYS_STATE_FILE "/sys/power/state"
+
+void qmp_guest_suspend(const char *mode, Error **err)
+{
+    GError *error = NULL;
+    gchar *argv[2];
+
+    if (strcmp(mode, "hibernate") == 0) {
+        argv[0] = (gchar *) "pm-hibernate";
+    } else if (strcmp(mode, "sleep") == 0) {
+        argv[0] = (gchar *) "pm-suspend";
+    } else if (strcmp(mode, "hybrid") == 0) {
+        argv[0] = (gchar *) "pm-hybrid";
+    } else {
+        error_set(err, QERR_INVALID_PARAMETER, "mode");
+        return;
+    }
+
+    argv[1] = NULL;
+    if (g_spawn_async(NULL, argv, NULL,G_SPAWN_SEARCH_PATH |
+                                        G_SPAWN_FILE_AND_ARGV_ZERO,
+                                        NULL, NULL, NULL,&error)<  0) {
+        int fd;
+        const char *cmd;
+
+        slog("%s\n", error->message);
+        g_error_free(error);
+
+        if (strcmp(mode, "hybrid") == 0) {
+            error_set(err, QERR_UNDEFINED_ERROR);
+            return;
+        }
+
+        slog("trying to suspend using the manual method...\n");
+
+        fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);
+        if (fd<  0) {
+            error_set(err, QERR_OPEN_FILE_FAILED, LINUX_SYS_STATE_FILE);
+            return;
+        }
+
+        cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
+        if (write(fd, cmd, strlen(cmd))<  0) {
+            error_set(err, QERR_IO_ERROR);
+        }
+
+        close(fd);
+    }
+}
+
  /* register init/cleanup routines for stateful command groups */
  void ga_command_state_init(GAState *s, GACommandState *cs)
  {

Quoted 26 lines · Luiz Capitulino

Quoted 14 lines · On Wed, 14 Dec 2011 14:38:55 -0200 Luiz Capitulino<[email protected]>…

Quoted 8 lines

Quoted 2 lines

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

Quoted 5 lines

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.

Quoted 5 lines

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).

The version below uses g_spawn_async(). The code is a bit simpler than previous versions, but there are two important details about it:

Quoted 3 lines

1. I'm letting g_spawn_async() reap the child automatically. I don't know how it does it though. I'd guess it uses g_child_watch_add(), worst case it ignores SIGCHLD (although I think this would be awful)

Weird, doesn't seem to add a watcher or do anything with SIGCHLD. You're not seeing an error that's causing it to reap it immediately after the fork?

Quoted 3 lines

2. The manual method of writing to the sysfs is done synchronously. This means that the command response will only be sent when the guest resumes

Want to avoid sync blocking calls as much as possible until timeouts are sorted out, particularly in this case. Currently, with hibernate at least, QEMU will exit (or is something being done to change that behavior?), libvirt will restart the VM later via some other interface, establish a connection to a new monitor and new agent channel, then gets a spurious response from the agent for a command it issue to a different QEMU process far in the past. So the common scenario would need nasty, special handling in libvirt and they'd probably hate us for it.

So probably best to stick with your previous approach for now, and look for a glib solution later.

Quoted 90 lines

If you think this approach is acceptable, I'll test it more, update its doc, etc and post it again.

qapi-schema-guest.json +17 -0

index 5f8a18d..63f65a6 100644
@@ -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' } }

qga/guest-agent-commands.c +50 -0

index a09c8ca..0c6b78e 100644
@@ -574,6 +574,56 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
  }
  #endif
 
+#define LINUX_SYS_STATE_FILE "/sys/power/state"
+
+void qmp_guest_suspend(const char *mode, Error **err)
+{
+    GError *error = NULL;
+    gchar *argv[2];
+
+    if (strcmp(mode, "hibernate") == 0) {
+        argv[0] = (gchar *) "pm-hibernate";
+    } else if (strcmp(mode, "sleep") == 0) {
+        argv[0] = (gchar *) "pm-suspend";
+    } else if (strcmp(mode, "hybrid") == 0) {
+        argv[0] = (gchar *) "pm-hybrid";
+    } else {
+        error_set(err, QERR_INVALID_PARAMETER, "mode");
+        return;
+    }
+
+    argv[1] = NULL;
+    if (g_spawn_async(NULL, argv, NULL,G_SPAWN_SEARCH_PATH |
+                                        G_SPAWN_FILE_AND_ARGV_ZERO,
+                                        NULL, NULL, NULL,&error)<  0) {
+        int fd;
+        const char *cmd;
+
+        slog("%s\n", error->message);
+        g_error_free(error);
+
+        if (strcmp(mode, "hybrid") == 0) {
+            error_set(err, QERR_UNDEFINED_ERROR);
+            return;
+        }
+
+        slog("trying to suspend using the manual method...\n");
+
+        fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);
+        if (fd<  0) {
+            error_set(err, QERR_OPEN_FILE_FAILED, LINUX_SYS_STATE_FILE);
+            return;
+        }
+
+        cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
+        if (write(fd, cmd, strlen(cmd))<  0) {
+            error_set(err, QERR_IO_ERROR);
+        }
+
+        close(fd);
+    }
+}
+
  /* register init/cleanup routines for stateful command groups */
  void ga_command_state_init(GAState *s, GACommandState *cs)
  {

Quoted 27 lines · On Wed, 14 Dec 2011 13:43:23 -0600 Michael Roth<[email protected]>…

Quoted 26 lines · Luiz Capitulino

Quoted 14 lines · On Wed, 14 Dec 2011 14:38:55 -0200 Luiz Capitulino<[email protected]>…

Quoted 8 lines

Quoted 2 lines

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

Quoted 5 lines

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.

Quoted 5 lines

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).

The version below uses g_spawn_async(). The code is a bit simpler than previous versions, but there are two important details about it:

Quoted 6 lines

Quoted 3 lines

1. I'm letting g_spawn_async() reap the child automatically. I don't know how it does it though. I'd guess it uses g_child_watch_add(), worst case it ignores SIGCHLD (although I think this would be awful)

Weird, doesn't seem to add a watcher or do anything with SIGCHLD. You're not seeing an error that's causing it to reap it immediately after the fork?

No. I haven't tested it much though, but the basic use case seemed to work fine.

Quoted 4 lines

Quoted 3 lines

2. The manual method of writing to the sysfs is done synchronously. This means that the command response will only be sent when the guest resumes

Quoted 8 lines

Want to avoid sync blocking calls as much as possible until timeouts are sorted out, particularly in this case. Currently, with hibernate at least, QEMU will exit (or is something being done to change that behavior?), libvirt will restart the VM later via some other interface, establish a connection to a new monitor and new agent channel, then gets a spurious response from the agent for a command it issue to a different QEMU process far in the past. So the common scenario would need nasty, special handling in libvirt and they'd probably hate us for it.

Problem is: even with the other approach there's no way to guarantee that the response will arrive before the guest suspends or shuts down, as it depends on which process runs first.

If we want to guarantee that the client will see a response before the guest suspends, then we need some form of synchronization. Like, the child should wait for the parent to send the response to the client before exec()ing (or writing to the sysfs file).

Quoted 95 lines

So probably best to stick with your previous approach for now, and look for a glib solution later.

Quoted 90 lines

If you think this approach is acceptable, I'll test it more, update its doc, etc and post it again.

qapi-schema-guest.json +17 -0

index 5f8a18d..63f65a6 100644
@@ -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' } }

qga/guest-agent-commands.c +50 -0

index a09c8ca..0c6b78e 100644
@@ -574,6 +574,56 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
   }
   #endif
 
+#define LINUX_SYS_STATE_FILE "/sys/power/state"
+
+void qmp_guest_suspend(const char *mode, Error **err)
+{
+    GError *error = NULL;
+    gchar *argv[2];
+
+    if (strcmp(mode, "hibernate") == 0) {
+        argv[0] = (gchar *) "pm-hibernate";
+    } else if (strcmp(mode, "sleep") == 0) {
+        argv[0] = (gchar *) "pm-suspend";
+    } else if (strcmp(mode, "hybrid") == 0) {
+        argv[0] = (gchar *) "pm-hybrid";
+    } else {
+        error_set(err, QERR_INVALID_PARAMETER, "mode");
+        return;
+    }
+
+    argv[1] = NULL;
+    if (g_spawn_async(NULL, argv, NULL,G_SPAWN_SEARCH_PATH |
+                                        G_SPAWN_FILE_AND_ARGV_ZERO,
+                                        NULL, NULL, NULL,&error)<   0) {
+        int fd;
+        const char *cmd;
+
+        slog("%s\n", error->message);
+        g_error_free(error);
+
+        if (strcmp(mode, "hybrid") == 0) {
+            error_set(err, QERR_UNDEFINED_ERROR);
+            return;
+        }
+
+        slog("trying to suspend using the manual method...\n");
+
+        fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);
+        if (fd<   0) {
+            error_set(err, QERR_OPEN_FILE_FAILED, LINUX_SYS_STATE_FILE);
+            return;
+        }
+
+        cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
+        if (write(fd, cmd, strlen(cmd))<   0) {
+            error_set(err, QERR_IO_ERROR);
+        }
+
+        close(fd);
+    }
+}
+
   /* register init/cleanup routines for stateful command groups */
   void ga_command_state_init(GAState *s, GACommandState *cs)
   {

Quoted 71 lines · Luiz Capitulino

Quoted 37 lines · On Wed, 14 Dec 2011 13:43:23 -0600 Michael Roth<[email protected]>…

Quoted 36 lines · Luiz Capitulino

Quoted 20 lines · On Wed, 14 Dec 2011 14:38:55 -0200 Luiz Capitulino<[email protected]>…

Quoted 13 lines

Quoted 4 lines

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

Quoted 6 lines

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.

Quoted 8 lines

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).

The version below uses g_spawn_async(). The code is a bit simpler than previous
versions, but there are two important details about it:

Quoted 8 lines

Quoted 4 lines

1. I'm letting g_spawn_async() reap the child automatically. I don't know
how it does it though. I'd guess it uses g_child_watch_add(), worst case it ignores SIGCHLD (although I think this would be awful)

Weird, doesn't seem to add a watcher or do anything with SIGCHLD. You're not seeing an error that's causing it to reap it immediately after the fork?

No. I haven't tested it much though, but the basic use case seemed to work
fine.

Quoted 6 lines

Quoted 5 lines

2. The manual method of writing to the sysfs is done synchronously. This
means that the command response will only be sent when the guest resumes

Quoted 8 lines

Want to avoid sync blocking calls as much as possible until timeouts are sorted out, particularly in this case. Currently, with hibernate at least, QEMU will exit (or is something being done to change that behavior?), libvirt will restart the VM later via some other interface, establish a connection to a new monitor and new agent channel, then gets a spurious response from the agent for a command it issue to a different QEMU process far in the past. So the common scenario would need nasty, special handling in libvirt and they'd probably hate us for it.

Problem is: even with the other approach there's no way to guarantee that the response will arrive before the guest suspends or shuts down, as it depends on which process runs first.

Quoted 6 lines

If we want to guarantee that the client will see a response before the guest suspends, then we need some form of synchronization. Like, the child
should wait for the parent to send the response to the client before exec()ing (or writing to the sysfs file).

There's no way to guarantee it though, even if we remove the window of

opportunity for a child to complete a shutdown/suspend/etc request before we write out a response, there's still the potential scenario where an admin kills off/restarts qemu-ga while it's processing a request.

So relying on client-side timeouts to handle these corner cases is acceptable, since being able to handle that is a requirement of a robust client, especially since we're talking to a potentially malicious guest/agent. We just need to do our best to make these situations corner cases rather than common ones.

Quoted 97 lines

Quoted 96 lines

So probably best to stick with your previous approach for now, and look for a glib solution later.

Quoted 91 lines

If you think this approach is acceptable, I'll test it more, update its doc,
etc and post it again.

qapi-schema-guest.json +17 -0

index 5f8a18d..63f65a6 100644
@@ -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' } }

qga/guest-agent-commands.c +50 -0

index a09c8ca..0c6b78e 100644
@@ -574,6 +574,56 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
 }
 #endif
 
+#define LINUX_SYS_STATE_FILE "/sys/power/state"
+
+void qmp_guest_suspend(const char *mode, Error **err)
+{
+ GError *error = NULL;
+ gchar *argv[2];
+
+ if (strcmp(mode, "hibernate") == 0) {
+ argv[0] = (gchar *) "pm-hibernate";
+ } else if (strcmp(mode, "sleep") == 0) {
+ argv[0] = (gchar *) "pm-suspend";
+ } else if (strcmp(mode, "hybrid") == 0) {
+ argv[0] = (gchar *) "pm-hybrid";
+ } else {
+ error_set(err, QERR_INVALID_PARAMETER, "mode");
+ return;
+ }
+
+ argv[1] = NULL;
+ if (g_spawn_async(NULL, argv, NULL,G_SPAWN_SEARCH_PATH |
+ G_SPAWN_FILE_AND_ARGV_ZERO,
+ NULL, NULL, NULL,&error)< 0) {
+ int fd;
+ const char *cmd;
+
+ slog("%s\n", error->message);
+ g_error_free(error);
+
+ if (strcmp(mode, "hybrid") == 0) {
+ error_set(err, QERR_UNDEFINED_ERROR);
+ return;
+ }
+
+ slog("trying to suspend using the manual method...\n");
+
+ fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);
+ if (fd< 0) {
+ error_set(err, QERR_OPEN_FILE_FAILED, LINUX_SYS_STATE_FILE);
+ return;
+ }
+
+ cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
+ if (write(fd, cmd, strlen(cmd))< 0) {
+ error_set(err, QERR_IO_ERROR);
+ }
+
+ close(fd);
+ }
+}
+
 /* register init/cleanup routines for stateful command groups */
 void ga_command_state_init(GAState *s, GACommandState *cs)
 {

Quoted 80 lines · Michael Roth

Quoted 78 lines · Luiz Capitulino

Quoted 37 lines · On Wed, 14 Dec 2011 13:43:23 -0600 Michael Roth<[email protected]>…

Quoted 36 lines · Luiz Capitulino

Quoted 20 lines · On Wed, 14 Dec 2011 14:38:55 -0200 Luiz Capitulino<[email protected]>…

Quoted 13 lines

Quoted 4 lines

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

Quoted 6 lines

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.

Quoted 8 lines

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).

The version below uses g_spawn_async(). The code is a bit simpler than previous
versions, but there are two important details about it:

Quoted 8 lines

Quoted 4 lines

1. I'm letting g_spawn_async() reap the child automatically. I don't know
how it does it though. I'd guess it uses g_child_watch_add(), worst case it ignores SIGCHLD (although I think this would be awful)

Weird, doesn't seem to add a watcher or do anything with SIGCHLD. You're not seeing an error that's causing it to reap it immediately after the fork?

No. I haven't tested it much though, but the basic use case seemed to work
fine.

Quoted 6 lines

Quoted 5 lines

2. The manual method of writing to the sysfs is done synchronously. This
means that the command response will only be sent when the guest resumes

Quoted 8 lines

Want to avoid sync blocking calls as much as possible until timeouts are sorted out, particularly in this case. Currently, with hibernate at least, QEMU will exit (or is something being done to change that behavior?), libvirt will restart the VM later via some other interface, establish a connection to a new monitor and new agent channel, then gets a spurious response from the agent for a command it issue to a different QEMU process far in the past. So the common scenario would need nasty, special handling in libvirt and they'd probably hate us for it.

Problem is: even with the other approach there's no way to guarantee that the response will arrive before the guest suspends or shuts down, as it depends on which process runs first.

If we want to guarantee that the client will see a response before the guest suspends, then we need some form of synchronization. Like, the child
should wait for the parent to send the response to the client before exec()ing (or writing to the sysfs file).

Quoted 1 line

There's no way to guarantee it though, even if we remove the window of

Rather, there's no way to guarantee the client will get a response (or that, given a response, the async command will succeed).

But you're that we could make it so that time-out/lack of response at least implies that the command *won't* be carried out...

That might be a nice guarantee, but your call on whether you want to do the signalling approach for hibernate. Otherwise, I think async will work well enough in the common case, and response timeout would imply neither success nor failure of the request, which I think is the norm for most things.

Quoted 109 lines

opportunity for a child to complete a shutdown/suspend/etc request before we write out a response, there's still the potential scenario where an admin kills off/restarts qemu-ga while it's processing a request.

So relying on client-side timeouts to handle these corner cases is acceptable, since being able to handle that is a requirement of a robust client, especially since we're talking to a potentially malicious guest/agent. We just need to do our best to make these situations corner cases rather than common ones.

Quoted 97 lines

Quoted 96 lines

So probably best to stick with your previous approach for now, and look for a glib solution later.

Quoted 91 lines

If you think this approach is acceptable, I'll test it more, update its doc,
etc and post it again.

qapi-schema-guest.json +17 -0

index 5f8a18d..63f65a6 100644
@@ -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' } }

qga/guest-agent-commands.c +50 -0

index a09c8ca..0c6b78e 100644
@@ -574,6 +574,56 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
 }
 #endif
 
+#define LINUX_SYS_STATE_FILE "/sys/power/state"
+
+void qmp_guest_suspend(const char *mode, Error **err)
+{
+ GError *error = NULL;
+ gchar *argv[2];
+
+ if (strcmp(mode, "hibernate") == 0) {
+ argv[0] = (gchar *) "pm-hibernate";
+ } else if (strcmp(mode, "sleep") == 0) {
+ argv[0] = (gchar *) "pm-suspend";
+ } else if (strcmp(mode, "hybrid") == 0) {
+ argv[0] = (gchar *) "pm-hybrid";
+ } else {
+ error_set(err, QERR_INVALID_PARAMETER, "mode");
+ return;
+ }
+
+ argv[1] = NULL;
+ if (g_spawn_async(NULL, argv, NULL,G_SPAWN_SEARCH_PATH |
+ G_SPAWN_FILE_AND_ARGV_ZERO,
+ NULL, NULL, NULL,&error)< 0) {
+ int fd;
+ const char *cmd;
+
+ slog("%s\n", error->message);
+ g_error_free(error);
+
+ if (strcmp(mode, "hybrid") == 0) {
+ error_set(err, QERR_UNDEFINED_ERROR);
+ return;
+ }
+
+ slog("trying to suspend using the manual method...\n");
+
+ fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);
+ if (fd< 0) {
+ error_set(err, QERR_OPEN_FILE_FAILED, LINUX_SYS_STATE_FILE);
+ return;
+ }
+
+ cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
+ if (write(fd, cmd, strlen(cmd))< 0) {
+ error_set(err, QERR_IO_ERROR);
+ }
+
+ close(fd);
+ }
+}
+
 /* register init/cleanup routines for stateful command groups */
 void ga_command_state_init(GAState *s, GACommandState *cs)
 {

Quoted 81 lines · On Wed, 14 Dec 2011 15:14:40 -0600 Michael Roth<[email protected]>…

Quoted 80 lines · Michael Roth

Quoted 78 lines · Luiz Capitulino

Quoted 37 lines · On Wed, 14 Dec 2011 13:43:23 -0600 Michael Roth<[email protected]>…

Quoted 36 lines · Luiz Capitulino

Quoted 20 lines · On Wed, 14 Dec 2011 14:38:55 -0200 Luiz Capitulino<[email protected]>…

Quoted 13 lines

Quoted 4 lines

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

Quoted 6 lines

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.

Quoted 8 lines

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).

The version below uses g_spawn_async(). The code is a bit simpler than previous
versions, but there are two important details about it:

Quoted 8 lines

Quoted 4 lines

1. I'm letting g_spawn_async() reap the child automatically. I don't know
how it does it though. I'd guess it uses g_child_watch_add(), worst case it ignores SIGCHLD (although I think this would be awful)

Weird, doesn't seem to add a watcher or do anything with SIGCHLD. You're not seeing an error that's causing it to reap it immediately after the fork?

No. I haven't tested it much though, but the basic use case seemed to work
fine.

Quoted 6 lines

Quoted 5 lines

2. The manual method of writing to the sysfs is done synchronously. This
means that the command response will only be sent when the guest resumes

Quoted 8 lines

Want to avoid sync blocking calls as much as possible until timeouts are sorted out, particularly in this case. Currently, with hibernate at least, QEMU will exit (or is something being done to change that behavior?), libvirt will restart the VM later via some other interface, establish a connection to a new monitor and new agent channel, then gets a spurious response from the agent for a command it issue to a different QEMU process far in the past. So the common scenario would need nasty, special handling in libvirt and they'd probably hate us for it.

Problem is: even with the other approach there's no way to guarantee that the response will arrive before the guest suspends or shuts down, as it depends on which process runs first.

If we want to guarantee that the client will see a response before the guest suspends, then we need some form of synchronization. Like, the child
should wait for the parent to send the response to the client before exec()ing (or writing to the sysfs file).

Quoted 4 lines

Quoted 1 line

There's no way to guarantee it though, even if we remove the window of

Rather, there's no way to guarantee the client will get a response (or that, given a response, the async command will succeed).

Well, it's the best we can do, ie. only allow the child to run when the parent is done sending the response. For the error case, we don't report errors in the child back to the client anyway, we do it via syslog.

Quoted 8 lines

But you're that we could make it so that time-out/lack of response at least implies that the command *won't* be carried out...

That might be a nice guarantee, but your call on whether you want to do the signalling approach for hibernate. Otherwise, I think async will work well enough in the common case, and response timeout would imply neither success nor failure of the request, which I think is the norm for most things.

Yes, I think the signaling approach is complex and I'm not sure we really we have a problem here. Maybe I should do it async for now, if this turns out to be a problem for libvirt we can try more complex approaches.

Quoted 10 lines

Quoted 10 lines

opportunity for a child to complete a shutdown/suspend/etc request before we write out a response, there's still the potential scenario where an admin kills off/restarts qemu-ga while it's processing a request.

So relying on client-side timeouts to handle these corner cases is acceptable, since being able to handle that is a requirement of a robust client, especially since we're talking to a potentially malicious guest/agent. We just need to do our best to make these situations corner cases rather than common ones.

Quoted 100 lines

Quoted 99 lines

Quoted 98 lines

Quoted 97 lines

So probably best to stick with your previous approach for now, and look for a glib solution later.

Quoted 91 lines

If you think this approach is acceptable, I'll test it more, update its doc,
etc and post it again.

qapi-schema-guest.json +17 -0

index 5f8a18d..63f65a6 100644
@@ -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' } }

qga/guest-agent-commands.c +50 -0

index a09c8ca..0c6b78e 100644
@@ -574,6 +574,56 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
 }
 #endif
 
+#define LINUX_SYS_STATE_FILE "/sys/power/state"
+
+void qmp_guest_suspend(const char *mode, Error **err)
+{
+ GError *error = NULL;
+ gchar *argv[2];
+
+ if (strcmp(mode, "hibernate") == 0) {
+ argv[0] = (gchar *) "pm-hibernate";
+ } else if (strcmp(mode, "sleep") == 0) {
+ argv[0] = (gchar *) "pm-suspend";
+ } else if (strcmp(mode, "hybrid") == 0) {
+ argv[0] = (gchar *) "pm-hybrid";
+ } else {
+ error_set(err, QERR_INVALID_PARAMETER, "mode");
+ return;
+ }
+
+ argv[1] = NULL;
+ if (g_spawn_async(NULL, argv, NULL,G_SPAWN_SEARCH_PATH |
+ G_SPAWN_FILE_AND_ARGV_ZERO,
+ NULL, NULL, NULL,&error)<  0) {
+ int fd;
+ const char *cmd;
+
+ slog("%s\n", error->message);
+ g_error_free(error);
+
+ if (strcmp(mode, "hybrid") == 0) {
+ error_set(err, QERR_UNDEFINED_ERROR);
+ return;
+ }
+
+ slog("trying to suspend using the manual method...\n");
+
+ fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);
+ if (fd<  0) {
+ error_set(err, QERR_OPEN_FILE_FAILED, LINUX_SYS_STATE_FILE);
+ return;
+ }
+
+ cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk";
+ if (write(fd, cmd, strlen(cmd))<  0) {
+ error_set(err, QERR_IO_ERROR);
+ }
+
+ close(fd);
+ }
+}
+
 /* register init/cleanup routines for stateful command groups */
 void ga_command_state_init(GAState *s, GACommandState *cs)
 {

Quoted 42 lines · On Wed, 14 Dec 2011 09:54:29 -0600 Michael Roth<[email protected]>…

Quoted 35 lines · Luiz Capitulino

Quoted 32 lines · On Tue, 13 Dec 2011 14:03:08 -0600 Michael Roth<[email protected]>…

Quoted 10 lines · Luiz Capitulino

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.

Quoted 5 lines

Quoted 2 lines

- 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?

Quoted 6 lines

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.

Quoted 12 lines

Quoted 4 lines

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.

Quoted 3 lines

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).

Quoted 162 lines

Quoted 161 lines

Quoted 160 lines

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

Quoted 152 lines

Signed-off-by Luiz Capitulino<[email protected]>

---

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(-)

qapi-schema-guest.json +17 -0

index 29989fe..656bde9 100644
@@ -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' } }

qemu-ga.c +10 -1

index 60d4972..b32e96c 100644
@@ -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)

qga/guest-agent-commands.c +64 -0

index a09c8ca..4799638 100644
@@ -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)
    {

Quoted 52 lines · Luiz Capitulino

Quoted 42 lines · On Wed, 14 Dec 2011 09:54:29 -0600 Michael Roth<[email protected]>…

Quoted 35 lines · Luiz Capitulino

Quoted 32 lines · On Tue, 13 Dec 2011 14:03:08 -0600 Michael Roth<[email protected]>…

Quoted 10 lines · Luiz Capitulino

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.

Quoted 5 lines

Quoted 2 lines

- 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?

Quoted 6 lines

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.

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 13 lines

Quoted 13 lines

Quoted 5 lines

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.

Quoted 7 lines

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 164 lines

Quoted 163 lines

Quoted 162 lines

Quoted 161 lines

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

Quoted 152 lines

Signed-off-by Luiz Capitulino<[email protected]>

---

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(-)

qapi-schema-guest.json +17 -0

index 29989fe..656bde9 100644
@@ -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' } }

qemu-ga.c +10 -1

index 60d4972..b32e96c 100644
@@ -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)

qga/guest-agent-commands.c +64 -0

index a09c8ca..4799638 100644
@@ -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)
    {

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<[email protected]>

---

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(-)

qapi-schema-guest.json +0 -0

index 29989fe..656bde9 100644
@@ -219,3 +219,20 @@
  ##
  { 'command': 'guest-fsfreeze-thaw',
    'returns': 'int' }

qapi-schema-guest.json +7 -0

+#
+# 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' } }

qemu-ga.c +10 -1

index 60d4972..b32e96c 100644
@@ -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)

qga/guest-agent-commands.c +10 -0

index a09c8ca..4799638 100644
@@ -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) {

qga/guest-agent-commands.c +44 -0

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

Quoted 46 lines · Luiz Capitulino

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<[email protected]>

---

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(-)

qapi-schema-guest.json +5 -0

index 29989fe..656bde9 100644
@@ -219,3 +219,20 @@
  ##
  { 'command': 'guest-fsfreeze-thaw',
    'returns': 'int' }
+
+##
+# @guest-suspend
+#
+# Suspend guest execution by entering ACPI power state S3 or S4.

qapi-schema-guest.json line 231

Quoted 5 lines

qapi-schema-guest.json +5 -0

+#
+# @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'?

Quoted 63 lines

qapi-schema-guest.json +7 -0

+#
+# 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' } }

qemu-ga.c +10 -1

index 60d4972..b32e96c 100644
@@ -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)

qga/guest-agent-commands.c +18 -0

index a09c8ca..4799638 100644
@@ -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;
+    }
+

qga/guest-agent-commands.c int64_t qmp_guest_fsfreeze_thaw(Error **err) · line 596

Quoted 2 lines

qga/guest-agent-commands.c +2 -0

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

Quoted 48 lines

qga/guest-agent-commands.c +44 -0

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

Quoted 47 lines · On Tue, 13 Dec 2011 14:27:56 -0600 Michael Roth<[email protected]>…

Quoted 46 lines · Luiz Capitulino

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<[email protected]>

---

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(-)

qapi-schema-guest.json +5 -0

index 29989fe..656bde9 100644
@@ -219,3 +219,20 @@
   ##
   { 'command': 'guest-fsfreeze-thaw',
     'returns': 'int' }
+
+##
+# @guest-suspend
+#
+# Suspend guest execution by entering ACPI power state S3 or S4.

Quoted 8 lines

qapi-schema-guest.json line 231

Quoted 5 lines

qapi-schema-guest.json +5 -0

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

Quoted 64 lines

Quoted 64 lines

qapi-schema-guest.json +7 -0

+#
+# 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' } }

qemu-ga.c +10 -1

index 60d4972..b32e96c 100644
@@ -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)

qga/guest-agent-commands.c +18 -0

index a09c8ca..4799638 100644
@@ -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;
+    }
+

Quoted 8 lines

qga/guest-agent-commands.c int64_t qmp_guest_fsfreeze_thaw(Error **err) · line 596

Quoted 2 lines

qga/guest-agent-commands.c +2 -0

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

Quoted 49 lines

Quoted 48 lines

qga/guest-agent-commands.c +44 -0

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

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 <[email protected]>

---

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(-)

qapi-schema-guest.json +0 -0

index 29989fe..656bde9 100644
@@ -219,3 +219,20 @@
 ##
 { 'command': 'guest-fsfreeze-thaw',
   'returns': 'int' }

qga/guest-agent-commands.c +3 -0

index a09c8ca..4799638 100644
@@ -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"
+

Quoted 49 lines · Luiz Capitulino

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 <[email protected]>

---

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(-)

qapi-schema-guest.json +8 -0

index 29989fe..656bde9 100644
@@ -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)

qapi-schema-guest.json line 231

Quoted 2 lines

qapi-schema-guest.json +2 -0

+#        '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]

Quoted 11 lines

qga/guest-agent-commands.c +3 -0

index a09c8ca..4799638 100644
@@ -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"
+

qga/guest-agent-commands.c int64_t qmp_guest_fsfreeze_thaw(Error **err) · line 589

Quoted 10 lines

qga/guest-agent-commands.c +10 -0

+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