From 8b020b5eb708091e723518907184e609350f6d41 Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Thu, 21 Jun 2018 07:21:51 -0300 Subject: qga: removing switch statements, adding run_process_child This is a cleanup of the resulting code after detaching pmutils and Linux sys state file logic: - remove the SUSPEND_MODE_* macros and use an enumeration instead. At the same time, drop the switch statements at the start of each function and use the enumeration index to get the right binary/argument; - create a new function called run_process_child(). This function uses g_spawn_sync() to execute a shell command, returning the exit code. This is a common operation in the pmutils functions and will be used in the systemd implementation as well, so this function will avoid code repetition. There are more places inside commands-posix.c where this new run_process_child function can also be used, but one step at a time. Signed-off-by: Daniel Henrique Barboza *check/propagate local_err before setting errp directly Signed-off-by: Michael Roth --- qga/commands-posix.c | 215 +++++++++++++++++++++------------------------------ 1 file changed, 88 insertions(+), 127 deletions(-) (limited to 'qga') diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 9ebd77b..1559753 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1474,152 +1474,117 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) #define LINUX_SYS_STATE_FILE "/sys/power/state" #define SUSPEND_SUPPORTED 0 #define SUSPEND_NOT_SUPPORTED 1 -#define SUSPEND_MODE_DISK 1 -#define SUSPEND_MODE_RAM 2 -#define SUSPEND_MODE_HYBRID 3 -static bool pmutils_supports_mode(int suspend_mode, Error **errp) +typedef enum { + SUSPEND_MODE_DISK = 0, + SUSPEND_MODE_RAM = 1, + SUSPEND_MODE_HYBRID = 2, +} SuspendMode; + +/* + * Executes a command in a child process using g_spawn_sync, + * returning an int >= 0 representing the exit status of the + * process. + * + * If the program wasn't found in path, returns -1. + * + * If a problem happened when creating the child process, + * returns -1 and errp is set. + */ +static int run_process_child(const char *command[], Error **errp) { - Error *local_err = NULL; - const char *pmutils_arg; - const char *pmutils_bin = "pm-is-supported"; - char *pmutils_path; - pid_t pid; - int status; - bool ret = false; + int exit_status, spawn_flag; + GError *g_err = NULL; + bool success; - switch (suspend_mode) { + spawn_flag = G_SPAWN_SEARCH_PATH | G_SPAWN_STDOUT_TO_DEV_NULL | + G_SPAWN_STDERR_TO_DEV_NULL; - case SUSPEND_MODE_DISK: - pmutils_arg = "--hibernate"; - break; - case SUSPEND_MODE_RAM: - pmutils_arg = "--suspend"; - break; - case SUSPEND_MODE_HYBRID: - pmutils_arg = "--suspend-hybrid"; - break; - default: - return ret; + success = g_spawn_sync(NULL, (char **)command, environ, spawn_flag, + NULL, NULL, NULL, NULL, + &exit_status, &g_err); + + if (success) { + return WEXITSTATUS(exit_status); } - pmutils_path = g_find_program_in_path(pmutils_bin); - if (!pmutils_path) { - return ret; + if (g_err && (g_err->code != G_SPAWN_ERROR_NOENT)) { + error_setg(errp, "failed to create child process, error '%s'", + g_err->message); } - pid = fork(); - if (!pid) { - setsid(); - execle(pmutils_path, pmutils_bin, pmutils_arg, NULL, environ); - /* - * If we get here execle() has failed. - */ - _exit(SUSPEND_NOT_SUPPORTED); - } else if (pid < 0) { - error_setg_errno(errp, errno, "failed to create child process"); - goto out; + g_error_free(g_err); + return -1; +} + +static bool pmutils_supports_mode(SuspendMode mode, Error **errp) +{ + Error *local_err = NULL; + const char *pmutils_args[3] = {"--hibernate", "--suspend", + "--suspend-hybrid"}; + const char *cmd[3] = {"pm-is-supported", pmutils_args[mode], NULL}; + int status; + + status = run_process_child(cmd, &local_err); + + if (status == SUSPEND_SUPPORTED) { + return true; } - ga_wait_child(pid, &status, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto out; + if ((status == -1) && !local_err) { + return false; } - switch (WEXITSTATUS(status)) { - case SUSPEND_SUPPORTED: - ret = true; - goto out; - case SUSPEND_NOT_SUPPORTED: - goto out; - default: + if (local_err) { + error_propagate(errp, local_err); + } else { error_setg(errp, - "the helper program '%s' returned an unexpected exit status" - " code (%d)", pmutils_path, WEXITSTATUS(status)); - goto out; + "the helper program '%s' returned an unexpected exit" + " status code (%d)", "pm-is-supported", status); } -out: - g_free(pmutils_path); - return ret; + return false; } -static void pmutils_suspend(int suspend_mode, Error **errp) +static void pmutils_suspend(SuspendMode mode, Error **errp) { Error *local_err = NULL; - const char *pmutils_bin; - char *pmutils_path; - pid_t pid; + const char *pmutils_binaries[3] = {"pm-hibernate", "pm-suspend", + "pm-suspend-hybrid"}; + const char *cmd[2] = {pmutils_binaries[mode], NULL}; 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; - } + status = run_process_child(cmd, &local_err); - pmutils_path = g_find_program_in_path(pmutils_bin); - if (!pmutils_path) { - error_setg(errp, "the helper program '%s' was not found", pmutils_bin); + if (status == 0) { 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; + if ((status == -1) && !local_err) { + error_setg(errp, "the helper program '%s' was not found", + pmutils_binaries[mode]); + return; } - ga_wait_child(pid, &status, &local_err); if (local_err) { error_propagate(errp, local_err); - goto out; - } - - if (WEXITSTATUS(status)) { + } else { error_setg(errp, - "the helper program '%s' returned an unexpected exit status" - " code (%d)", pmutils_path, WEXITSTATUS(status)); + "the helper program '%s' returned an unexpected exit" + " status code (%d)", pmutils_binaries[mode], status); } - -out: - g_free(pmutils_path); } -static bool linux_sys_state_supports_mode(int suspend_mode, Error **errp) +static bool linux_sys_state_supports_mode(SuspendMode mode, Error **errp) { - const char *sysfile_str; + const char *sysfile_strs[3] = {"disk", "mem", NULL}; + const char *sysfile_str = sysfile_strs[mode]; char buf[32]; /* hopefully big enough */ int fd; ssize_t ret; - switch (suspend_mode) { - - case SUSPEND_MODE_DISK: - sysfile_str = "disk"; - break; - case SUSPEND_MODE_RAM: - sysfile_str = "mem"; - break; - default: + if (!sysfile_str) { + error_setg(errp, "unknown guest suspend mode"); return false; } @@ -1640,22 +1605,15 @@ static bool linux_sys_state_supports_mode(int suspend_mode, Error **errp) return false; } -static void linux_sys_state_suspend(int suspend_mode, Error **errp) +static void linux_sys_state_suspend(SuspendMode mode, Error **errp) { Error *local_err = NULL; - const char *sysfile_str; + const char *sysfile_strs[3] = {"disk", "mem", NULL}; + const char *sysfile_str = sysfile_strs[mode]; pid_t pid; int status; - switch (suspend_mode) { - - case SUSPEND_MODE_DISK: - sysfile_str = "disk"; - break; - case SUSPEND_MODE_RAM: - sysfile_str = "mem"; - break; - default: + if (!sysfile_str) { error_setg(errp, "unknown guest suspend mode"); return; } @@ -1697,12 +1655,12 @@ static void linux_sys_state_suspend(int suspend_mode, Error **errp) } -static void bios_supports_mode(int suspend_mode, Error **errp) +static void bios_supports_mode(SuspendMode mode, Error **errp) { Error *local_err = NULL; bool ret; - ret = pmutils_supports_mode(suspend_mode, &local_err); + ret = pmutils_supports_mode(mode, &local_err); if (ret) { return; } @@ -1710,32 +1668,35 @@ static void bios_supports_mode(int suspend_mode, Error **errp) error_propagate(errp, local_err); return; } - ret = linux_sys_state_supports_mode(suspend_mode, errp); + ret = linux_sys_state_supports_mode(mode, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } 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 guest_suspend(SuspendMode mode, Error **errp) { Error *local_err = NULL; - bios_supports_mode(suspend_mode, &local_err); + bios_supports_mode(mode, &local_err); if (local_err) { error_propagate(errp, local_err); return; } - pmutils_suspend(suspend_mode, &local_err); + pmutils_suspend(mode, &local_err); if (!local_err) { return; } error_free(local_err); - linux_sys_state_suspend(suspend_mode, &local_err); + linux_sys_state_suspend(mode, &local_err); if (local_err) { error_propagate(errp, local_err); } -- cgit v1.1