From nobody Tue Nov 4 18:51:46 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1530651753279704.8183801157605; Tue, 3 Jul 2018 14:02:33 -0700 (PDT) Received: from localhost ([::1]:42635 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1faSRU-0006fC-ID for importer@patchew.org; Tue, 03 Jul 2018 17:02:32 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1faSIz-0008QN-R4 for qemu-devel@nongnu.org; Tue, 03 Jul 2018 16:53:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1faSIw-0000z7-DD for qemu-devel@nongnu.org; Tue, 03 Jul 2018 16:53:45 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:53676) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1faSIw-0000yR-3o for qemu-devel@nongnu.org; Tue, 03 Jul 2018 16:53:42 -0400 Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w63KnB7p056052 for ; Tue, 3 Jul 2018 16:53:40 -0400 Received: from e35.co.us.ibm.com (e35.co.us.ibm.com [32.97.110.153]) by mx0a-001b2d01.pphosted.com with ESMTP id 2k0dpjxhrc-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 03 Jul 2018 16:53:40 -0400 Received: from localhost by e35.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 3 Jul 2018 14:53:39 -0600 Received: from b03cxnp08027.gho.boulder.ibm.com (9.17.130.19) by e35.co.us.ibm.com (192.168.1.135) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Tue, 3 Jul 2018 14:53:36 -0600 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp08027.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id w63KrZWL12190178 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 3 Jul 2018 13:53:35 -0700 Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CEF697805E; Tue, 3 Jul 2018 14:53:35 -0600 (MDT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A8F197805C; Tue, 3 Jul 2018 14:53:35 -0600 (MDT) Received: from localhost (unknown [9.53.92.156]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Tue, 3 Jul 2018 14:53:35 -0600 (MDT) From: Michael Roth To: qemu-devel@nongnu.org Date: Tue, 3 Jul 2018 15:52:19 -0500 X-Mailer: git-send-email 2.11.0 In-Reply-To: <20180703205221.24788-1-mdroth@linux.vnet.ibm.com> References: <20180703205221.24788-1-mdroth@linux.vnet.ibm.com> X-TM-AS-GCONF: 00 x-cbid: 18070320-0012-0000-0000-000016869C49 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00009303; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000266; SDB=6.01056103; UDB=6.00541729; IPR=6.00834026; MB=3.00021980; MTD=3.00000008; XFM=3.00000015; UTC=2018-07-03 20:53:38 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18070320-0013-0000-0000-0000538B80D7 Message-Id: <20180703205221.24788-13-mdroth@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2018-07-03_09:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=3 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1806210000 definitions=main-1807030235 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy] X-Received-From: 148.163.156.1 Subject: [Qemu-devel] [PULL 12/14] qga: removing switch statements, adding run_process_child X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: peter.maydell@linaro.org, Daniel Henrique Barboza Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Daniel Henrique Barboza 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(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 9ebd77b894..1559753d85 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1474,152 +1474,117 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimu= m, 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 =20 -static bool pmutils_supports_mode(int suspend_mode, Error **errp) +typedef enum { + SUSPEND_MODE_DISK =3D 0, + SUSPEND_MODE_RAM =3D 1, + SUSPEND_MODE_HYBRID =3D 2, +} SuspendMode; + +/* + * Executes a command in a child process using g_spawn_sync, + * returning an int >=3D 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 =3D NULL; - const char *pmutils_arg; - const char *pmutils_bin =3D "pm-is-supported"; - char *pmutils_path; - pid_t pid; - int status; - bool ret =3D false; + int exit_status, spawn_flag; + GError *g_err =3D NULL; + bool success; =20 - switch (suspend_mode) { + spawn_flag =3D G_SPAWN_SEARCH_PATH | G_SPAWN_STDOUT_TO_DEV_NULL | + G_SPAWN_STDERR_TO_DEV_NULL; =20 - case SUSPEND_MODE_DISK: - pmutils_arg =3D "--hibernate"; - break; - case SUSPEND_MODE_RAM: - pmutils_arg =3D "--suspend"; - break; - case SUSPEND_MODE_HYBRID: - pmutils_arg =3D "--suspend-hybrid"; - break; - default: - return ret; + success =3D g_spawn_sync(NULL, (char **)command, environ, spawn_flag, + NULL, NULL, NULL, NULL, + &exit_status, &g_err); + + if (success) { + return WEXITSTATUS(exit_status); } =20 - pmutils_path =3D g_find_program_in_path(pmutils_bin); - if (!pmutils_path) { - return ret; + if (g_err && (g_err->code !=3D G_SPAWN_ERROR_NOENT)) { + error_setg(errp, "failed to create child process, error '%s'", + g_err->message); } =20 - pid =3D 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 =3D NULL; + const char *pmutils_args[3] =3D {"--hibernate", "--suspend", + "--suspend-hybrid"}; + const char *cmd[3] =3D {"pm-is-supported", pmutils_args[mode], NULL}; + int status; + + status =3D run_process_child(cmd, &local_err); + + if (status =3D=3D SUSPEND_SUPPORTED) { + return true; } =20 - ga_wait_child(pid, &status, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto out; + if ((status =3D=3D -1) && !local_err) { + return false; } =20 - switch (WEXITSTATUS(status)) { - case SUSPEND_SUPPORTED: - ret =3D 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 st= atus" - " code (%d)", pmutils_path, WEXITSTATUS(status)); - goto out; + "the helper program '%s' returned an unexpected exit" + " status code (%d)", "pm-is-supported", status); } =20 -out: - g_free(pmutils_path); - return ret; + return false; } =20 -static void pmutils_suspend(int suspend_mode, Error **errp) +static void pmutils_suspend(SuspendMode mode, Error **errp) { Error *local_err =3D NULL; - const char *pmutils_bin; - char *pmutils_path; - pid_t pid; + const char *pmutils_binaries[3] =3D {"pm-hibernate", "pm-suspend", + "pm-suspend-hybrid"}; + const char *cmd[2] =3D {pmutils_binaries[mode], NULL}; int status; =20 - switch (suspend_mode) { - - case SUSPEND_MODE_DISK: - pmutils_bin =3D "pm-hibernate"; - break; - case SUSPEND_MODE_RAM: - pmutils_bin =3D "pm-suspend"; - break; - case SUSPEND_MODE_HYBRID: - pmutils_bin =3D "pm-suspend-hybrid"; - break; - default: - error_setg(errp, "unknown guest suspend mode"); - return; - } + status =3D run_process_child(cmd, &local_err); =20 - pmutils_path =3D g_find_program_in_path(pmutils_bin); - if (!pmutils_path) { - error_setg(errp, "the helper program '%s' was not found", pmutils_= bin); + if (status =3D=3D 0) { return; } =20 - pid =3D 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 =3D=3D -1) && !local_err) { + error_setg(errp, "the helper program '%s' was not found", + pmutils_binaries[mode]); + return; } =20 - 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 st= atus" - " 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); } =20 -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] =3D {"disk", "mem", NULL}; + const char *sysfile_str =3D sysfile_strs[mode]; char buf[32]; /* hopefully big enough */ int fd; ssize_t ret; =20 - switch (suspend_mode) { - - case SUSPEND_MODE_DISK: - sysfile_str =3D "disk"; - break; - case SUSPEND_MODE_RAM: - sysfile_str =3D "mem"; - break; - default: + if (!sysfile_str) { + error_setg(errp, "unknown guest suspend mode"); return false; } =20 @@ -1640,22 +1605,15 @@ static bool linux_sys_state_supports_mode(int suspe= nd_mode, Error **errp) return false; } =20 -static void linux_sys_state_suspend(int suspend_mode, Error **errp) +static void linux_sys_state_suspend(SuspendMode mode, Error **errp) { Error *local_err =3D NULL; - const char *sysfile_str; + const char *sysfile_strs[3] =3D {"disk", "mem", NULL}; + const char *sysfile_str =3D sysfile_strs[mode]; pid_t pid; int status; =20 - switch (suspend_mode) { - - case SUSPEND_MODE_DISK: - sysfile_str =3D "disk"; - break; - case SUSPEND_MODE_RAM: - sysfile_str =3D "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_mod= e, Error **errp) =20 } =20 -static void bios_supports_mode(int suspend_mode, Error **errp) +static void bios_supports_mode(SuspendMode mode, Error **errp) { Error *local_err =3D NULL; bool ret; =20 - ret =3D pmutils_supports_mode(suspend_mode, &local_err); + ret =3D pmutils_supports_mode(mode, &local_err); if (ret) { return; } @@ -1710,32 +1668,35 @@ static void bios_supports_mode(int suspend_mode, Er= ror **errp) error_propagate(errp, local_err); return; } - ret =3D linux_sys_state_supports_mode(suspend_mode, errp); + ret =3D 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 gue= st"); - return; } } =20 -static void guest_suspend(int suspend_mode, Error **errp) +static void guest_suspend(SuspendMode mode, Error **errp) { Error *local_err =3D NULL; =20 - bios_supports_mode(suspend_mode, &local_err); + bios_supports_mode(mode, &local_err); if (local_err) { error_propagate(errp, local_err); return; } =20 - pmutils_suspend(suspend_mode, &local_err); + pmutils_suspend(mode, &local_err); if (!local_err) { return; } =20 error_free(local_err); =20 - linux_sys_state_suspend(suspend_mode, &local_err); + linux_sys_state_suspend(mode, &local_err); if (local_err) { error_propagate(errp, local_err); } --=20 2.11.0