Hi
On Tue, Jun 19, 2018 at 9:38 PM, Daniel Henrique Barboza
<danielhb...@gmail.com> wrote:
> Following the same logic of the previous patch, let's also
> decouple the suspend logic from guest_suspend into specialized
> functions, one for each strategy we support at this moment.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>
> ---
> qga/commands-posix.c | 170 +++++++++++++++++++++++++++----------------
> 1 file changed, 108 insertions(+), 62 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 89ffd8dc88..a2870f9ab9 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1509,6 +1509,65 @@ out:
> return ret;
> }
>
> +static void pmutils_suspend(int suspend_mode, Error **errp)
> +{
> + Error *local_err = NULL;
> + const char *pmutils_bin;
> + char *pmutils_path;
> + pid_t pid;
> + int status;
> +
> + switch (suspend_mode) {
> +
> + case SUSPEND_MODE_DISK:
> + pmutils_bin = "pm-hibernate";
> + break;
> + case SUSPEND_MODE_RAM:
> + pmutils_bin = "pm-suspend";
> + break;
> + case SUSPEND_MODE_HYBRID:
> + pmutils_bin = "pm-suspend-hybrid";
> + break;
> + default:
> + error_setg(errp, "unknown guest suspend mode");
> + return;
> + }
> +
> + pmutils_path = g_find_program_in_path(pmutils_bin);
> + if (!pmutils_path) {
> + error_setg(errp, "the helper program '%s' was not found",
> pmutils_bin);
> + return;
> + }
> +
> + pid = fork();
> + if (!pid) {
> + setsid();
> + execle(pmutils_path, pmutils_bin, NULL, environ);
> + /*
> + * If we get here execle() has failed.
> + */
> + _exit(EXIT_FAILURE);
> + } else if (pid < 0) {
> + error_setg_errno(errp, errno, "failed to create child process");
> + goto out;
> + }
> +
> + ga_wait_child(pid, &status, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + goto out;
> + }
> +
> + if (WEXITSTATUS(status)) {
> + error_setg(errp,
> + "the helper program '%s' returned an unexpected exit
> status"
> + " code (%d)", pmutils_path, WEXITSTATUS(status));
> + }
> +
> +out:
> + g_free(pmutils_path);
> +}
> +
> static bool linux_sys_state_supports_mode(int suspend_mode, Error **errp)
> {
> const char *sysfile_str;
> @@ -1545,64 +1604,28 @@ static bool linux_sys_state_supports_mode(int
> suspend_mode, Error **errp)
> return false;
> }
>
> -static void bios_supports_mode(int suspend_mode, Error **errp)
> -{
> - Error *local_err = NULL;
> - bool ret;
> -
> - ret = pmutils_supports_mode(suspend_mode, &local_err);
> - if (ret) {
> - return;
> - }
> - if (local_err) {
> - error_propagate(errp, local_err);
> - return;
> - }
> - ret = linux_sys_state_supports_mode(suspend_mode, errp);
> - if (!ret) {
> - error_setg(errp,
> - "the requested suspend mode is not supported by the
> guest");
> - return;
> - }
> -}
> -
> -static void guest_suspend(int suspend_mode, Error **errp)
> +static void linux_sys_state_suspend(int suspend_mode, Error **errp)
> {
> Error *local_err = NULL;
> - const char *pmutils_bin, *sysfile_str;
> - char *pmutils_path;
> + const char *sysfile_str;
> pid_t pid;
> int status;
>
> - bios_supports_mode(suspend_mode, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> - return;
> - }
> -
> switch (suspend_mode) {
>
> case SUSPEND_MODE_DISK:
> - pmutils_bin = "pm-hibernate";
> sysfile_str = "disk";
> break;
> case SUSPEND_MODE_RAM:
> - pmutils_bin = "pm-suspend";
> sysfile_str = "mem";
> break;
> - case SUSPEND_MODE_HYBRID:
> - pmutils_bin = "pm-suspend-hybrid";
> - sysfile_str = NULL;
> - break;
> default:
> error_setg(errp, "unknown guest suspend mode");
> return;
> }
>
> - pmutils_path = g_find_program_in_path(pmutils_bin);
> -
> pid = fork();
> - if (pid == 0) {
> + if (!pid) {
> /* child */
> int fd;
>
> @@ -1611,19 +1634,6 @@ static void guest_suspend(int suspend_mode, Error
> **errp)
> reopen_fd_to_null(1);
> reopen_fd_to_null(2);
>
> - if (pmutils_path) {
> - execle(pmutils_path, pmutils_bin, NULL, environ);
> - }
> -
> - /*
> - * If we get here either pm-utils is not installed or execle() has
> - * failed. Let's try the manual method if the caller wants it.
> - */
> -
> - if (!sysfile_str) {
> - _exit(EXIT_FAILURE);
> - }
> -
> fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);
> if (fd < 0) {
> _exit(EXIT_FAILURE);
> @@ -1636,27 +1646,63 @@ static void guest_suspend(int suspend_mode, Error
> **errp)
> _exit(EXIT_SUCCESS);
> } else if (pid < 0) {
> error_setg_errno(errp, errno, "failed to create child process");
> - goto out;
> + return;
> }
>
> ga_wait_child(pid, &status, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> - goto out;
> - }
> -
> - if (!WIFEXITED(status)) {
> - error_setg(errp, "child process has terminated abnormally");
> - goto out;
> + return;
> }
>
> if (WEXITSTATUS(status)) {
> error_setg(errp, "child process has failed to suspend");
> - goto out;
> }
>
> -out:
> - g_free(pmutils_path);
> +}
> +
> +static void bios_supports_mode(int suspend_mode, Error **errp)
> +{
> + Error *local_err = NULL;
> + bool ret;
> +
> + ret = pmutils_supports_mode(suspend_mode, &local_err);
> + if (ret) {
> + return;
> + }
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + ret = linux_sys_state_supports_mode(suspend_mode, errp);
> + if (!ret) {
> + error_setg(errp,
> + "the requested suspend mode is not supported by the
> guest");
> + return;
> + }
> +}
> +
> +static void guest_suspend(int suspend_mode, Error **errp)
> +{
> + Error *local_err = NULL;
> +
> + bios_supports_mode(suspend_mode, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> + pmutils_suspend(suspend_mode, &local_err);
> + if (!local_err) {
> + return;
> + }
> +
> + local_err = NULL;
You should error_free(). Same in later patches.
> +
> + linux_sys_state_suspend(suspend_mode, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + }
> }
>
> void qmp_guest_suspend_disk(Error **errp)
> --
> 2.17.1
>
>
--
Marc-André Lureau
