[PATCH 3/4] qemu: Drop @cmdret argument from qemuSecurityCommandRun()

Michal Privoznik mprivozn at redhat.com
Mon Feb 20 10:00:57 UTC 2023


Every single caller of qemuSecurityCommandRun() calls the
function as:

  if (qemuSecurityCommandRun(..., &cmdret) < 0)
      goto cleanup;

  if (cmdret < 0)
      goto cleanup;

(modulo @exitstatus shenanigans)

Well, there's no need for such complication. There isn't a single
caller (and probably will never be (TM)), that would need to
distinguish the reason for the failure. Therefore,
qemuSecurityCommandRun() can be made to pass the retval of
virCommandRun() called under the hood.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/qemu/qemu_dbus.c           | 12 ++++--------
 src/qemu/qemu_passt.c          | 11 ++++-------
 src/qemu/qemu_security.c       | 12 +++++-------
 src/qemu/qemu_security.h       |  3 +--
 src/qemu/qemu_slirp.c          | 11 ++++-------
 src/qemu/qemu_tpm.c            |  9 +--------
 src/qemu/qemu_vhost_user_gpu.c | 12 +++++-------
 7 files changed, 24 insertions(+), 46 deletions(-)

diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
index a5807527a6..74cb5457ea 100644
--- a/src/qemu/qemu_dbus.c
+++ b/src/qemu/qemu_dbus.c
@@ -182,7 +182,6 @@ qemuDBusStart(virQEMUDriver *driver,
     virTimeBackOffVar timebackoff;
     const unsigned long long timeout = 500 * 1000; /* ms */
     VIR_AUTOCLOSE errfd = -1;
-    int cmdret = 0;
     int exitstatus = 0;
     pid_t cpid = -1;
     int ret = -1;
@@ -219,15 +218,12 @@ qemuDBusStart(virQEMUDriver *driver,
     virCommandDaemonize(cmd);
     virCommandAddArgFormat(cmd, "--config-file=%s", configfile);
 
-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1,
-                               &exitstatus, &cmdret) < 0)
+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus) < 0)
         goto cleanup;
 
-    if (cmdret < 0 || exitstatus != 0) {
-        if (cmdret >= 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Could not start dbus-daemon. exitstatus: %d"), exitstatus);
-        }
+    if (exitstatus != 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not start dbus-daemon. exitstatus: %d"), exitstatus);
         goto cleanup;
     }
 
diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
index 53a7cc9605..81b7917026 100644
--- a/src/qemu/qemu_passt.c
+++ b/src/qemu/qemu_passt.c
@@ -159,7 +159,6 @@ qemuPasstStart(virDomainObj *vm,
     char macaddr[VIR_MAC_STRING_BUFLEN];
     size_t i;
     int exitstatus = 0;
-    int cmdret = 0;
 
     cmd = virCommandNew(PASST);
 
@@ -271,14 +270,12 @@ qemuPasstStart(virDomainObj *vm,
     if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0)
         return -1;
 
-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) < 0)
+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus) < 0)
         goto error;
 
-    if (cmdret < 0 || exitstatus != 0) {
-        if (cmdret >= 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Could not start 'passt': %s"), NULLSTR(errbuf));
-        }
+    if (exitstatus != 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not start 'passt': %s"), NULLSTR(errbuf));
         goto error;
     }
 
diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 2548fc0ecd..07fcffb288 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -623,11 +623,9 @@ qemuSecurityDomainRestorePathLabel(virQEMUDriver *driver,
  * @uid: the uid to force
  * @gid: the gid to force
  * @existstatus: pointer to int returning exit status of process
- * @cmdret: pointer to int returning result of virCommandRun
  *
  * Run @cmd with seclabels set on it. If @uid and/or @gid are not
- * -1 then their value is enforced. If @cmdret is negative upon
- *  return, then appropriate error was already reported.
+ * -1 then their value is enforced.
  *
  * Returns: 0 on success,
  *         -1 otherwise (with error reported).
@@ -638,11 +636,11 @@ qemuSecurityCommandRun(virQEMUDriver *driver,
                        virCommand *cmd,
                        uid_t uid,
                        gid_t gid,
-                       int *exitstatus,
-                       int *cmdret)
+                       int *exitstatus)
 {
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
     qemuDomainObjPrivate *priv = vm->privateData;
+    int ret = -1;
 
     if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
                                                vm->def, cmd) < 0)
@@ -664,9 +662,9 @@ qemuSecurityCommandRun(virQEMUDriver *driver,
     if (virSecurityManagerPreFork(driver->securityManager) < 0)
         return -1;
 
-    *cmdret = virCommandRun(cmd, exitstatus);
+    ret = virCommandRun(cmd, exitstatus);
 
     virSecurityManagerPostFork(driver->securityManager);
 
-    return 0;
+    return ret;
 }
diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h
index 8d1c6b38c3..dc8e67cc81 100644
--- a/src/qemu/qemu_security.h
+++ b/src/qemu/qemu_security.h
@@ -115,8 +115,7 @@ int qemuSecurityCommandRun(virQEMUDriver *driver,
                            virCommand *cmd,
                            uid_t uid,
                            gid_t gid,
-                           int *exitstatus,
-                           int *cmdret);
+                           int *exitstatus);
 
 /* Please note that for these APIs there is no wrapper yet. Do NOT blindly add
  * new APIs here. If an API can touch a file add a proper wrapper instead.
diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
index 1bd45cb06c..bbe919f37a 100644
--- a/src/qemu/qemu_slirp.c
+++ b/src/qemu/qemu_slirp.c
@@ -248,7 +248,6 @@ qemuSlirpStart(virDomainObj *vm,
     pid_t pid = (pid_t) -1;
     int rc;
     int exitstatus = 0;
-    int cmdret = 0;
     bool killDBusDaemon = false;
     g_autofree char *fdname = g_strdup_printf("slirpfd-%s", net->info.alias);
 
@@ -327,14 +326,12 @@ qemuSlirpStart(virDomainObj *vm,
     if (qemuExtDeviceLogCommand(driver, vm, cmd, "slirp") < 0)
         goto error;
 
-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) < 0)
+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus) < 0)
         goto error;
 
-    if (cmdret < 0 || exitstatus != 0) {
-        if (cmdret >= 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Could not start 'slirp'. exitstatus: %d"), exitstatus);
-        }
+    if (exitstatus != 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not start 'slirp'. exitstatus: %d"), exitstatus);
         goto error;
     }
 
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 5831ffc32e..982e5f13b6 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -927,7 +927,6 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
     virTimeBackOffVar timebackoff;
     const unsigned long long timeout = 1000; /* ms */
     bool setTPMStateLabel = true;
-    int cmdret = 0;
     pid_t pid = -1;
 
     cfg = virQEMUDriverGetConfig(driver);
@@ -963,15 +962,9 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
         return -1;
 
     if (qemuSecurityCommandRun(driver, vm, cmd, cfg->swtpm_user,
-                               cfg->swtpm_group, NULL, &cmdret) < 0)
+                               cfg->swtpm_group, NULL) < 0)
         goto error;
 
-    if (cmdret < 0) {
-        /* virCommandRun() hidden in qemuSecurityCommandRun()
-         * already reported error. */
-        goto error;
-    }
-
     if (virPidFileReadPath(pidfile, &pid) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("swtpm didn't show up"));
diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c
index a9a5fe3a3e..196ebc7dff 100644
--- a/src/qemu/qemu_vhost_user_gpu.c
+++ b/src/qemu/qemu_vhost_user_gpu.c
@@ -105,7 +105,7 @@ int qemuExtVhostUserGPUStart(virQEMUDriver *driver,
     g_autofree char *pidfile = NULL;
     g_autoptr(virCommand) cmd = NULL;
     int pair[2] = { -1, -1 };
-    int cmdret = 0, rc;
+    int rc;
     int exitstatus = 0;
     pid_t pid;
     int ret = -1;
@@ -153,14 +153,12 @@ int qemuExtVhostUserGPUStart(virQEMUDriver *driver,
             virCommandAddArgFormat(cmd, "--render-node=%s", video->accel->rendernode);
     }
 
-    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) < 0)
+    if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus) < 0)
         goto error;
 
-    if (cmdret < 0 || exitstatus != 0) {
-        if (cmdret >= 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Could not start 'vhost-user-gpu'. exitstatus: %d"), exitstatus);
-        }
+    if (exitstatus != 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not start 'vhost-user-gpu'. exitstatus: %d"), exitstatus);
         goto cleanup;
     }
 
-- 
2.39.2



More information about the libvir-list mailing list