]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
qemu: Drop @cmdret argument from qemuSecurityCommandRun()
authorMichal Privoznik <mprivozn@redhat.com>
Mon, 13 Feb 2023 11:27:49 +0000 (12:27 +0100)
committerMichal Privoznik <mprivozn@redhat.com>
Fri, 3 Mar 2023 11:02:59 +0000 (12:02 +0100)
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@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
src/qemu/qemu_dbus.c
src/qemu/qemu_passt.c
src/qemu/qemu_security.c
src/qemu/qemu_security.h
src/qemu/qemu_slirp.c
src/qemu/qemu_tpm.c
src/qemu/qemu_vhost_user_gpu.c

index a5807527a671ed8bfddba23fe0729b69ea426cba..74cb5457ea6f8768379fc3755d51cd7e5de00ed4 100644 (file)
@@ -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;
     }
 
index 158bd5b5b24b1ca6576fdfb8de1b02bfbe2c3ba4..7f67c3dcc304077b8256069c88a53c56352d46d1 100644 (file)
@@ -173,7 +173,6 @@ qemuPasstStart(virDomainObj *vm,
     char macaddr[VIR_MAC_STRING_BUFLEN];
     size_t i;
     int exitstatus = 0;
-    int cmdret = 0;
 
     cmd = virCommandNew(PASST);
 
@@ -285,14 +284,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;
     }
 
index 2548fc0ecd464ca141f5ddf625f9966895ca798a..07fcffb28864128197de06c1baf1dfef469d1de4 100644 (file)
@@ -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;
 }
index 8d1c6b38c3a74101c9205acb31a2bc47f72be43e..dc8e67cc81c805a11d9d2d3e2e0894503e825a09 100644 (file)
@@ -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.
index 1bd45cb06ceae16b5bfe7964534bf56d401a0077..bbe919f37a2f755ca4480a6f976fcf005f78b584 100644 (file)
@@ -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;
     }
 
index 5831ffc32e992655ec48cff0193879bca4389cb4..982e5f13b6235519c31956d69906a5e5e9e09116 100644 (file)
@@ -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"));
index a9a5fe3a3e3d7327f73ea6ba774fb72c5ddac578..196ebc7dff12037f110a16d49416ed111c936ea8 100644 (file)
@@ -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;
     }