qemu: Drop @cmdret argument from qemuSecurityCommandRun()

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>
This commit is contained in:
Michal Privoznik 2023-02-13 12:27:49 +01:00
parent 17ffdbab1f
commit caa25f75cf
7 changed files with 24 additions and 46 deletions

View File

@ -182,7 +182,6 @@ qemuDBusStart(virQEMUDriver *driver,
virTimeBackOffVar timebackoff; virTimeBackOffVar timebackoff;
const unsigned long long timeout = 500 * 1000; /* ms */ const unsigned long long timeout = 500 * 1000; /* ms */
VIR_AUTOCLOSE errfd = -1; VIR_AUTOCLOSE errfd = -1;
int cmdret = 0;
int exitstatus = 0; int exitstatus = 0;
pid_t cpid = -1; pid_t cpid = -1;
int ret = -1; int ret = -1;
@ -219,15 +218,12 @@ qemuDBusStart(virQEMUDriver *driver,
virCommandDaemonize(cmd); virCommandDaemonize(cmd);
virCommandAddArgFormat(cmd, "--config-file=%s", configfile); virCommandAddArgFormat(cmd, "--config-file=%s", configfile);
if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus) < 0)
&exitstatus, &cmdret) < 0)
goto cleanup; goto cleanup;
if (cmdret < 0 || exitstatus != 0) { if (exitstatus != 0) {
if (cmdret >= 0) { virReportError(VIR_ERR_INTERNAL_ERROR,
virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not start dbus-daemon. exitstatus: %d"), exitstatus);
_("Could not start dbus-daemon. exitstatus: %d"), exitstatus);
}
goto cleanup; goto cleanup;
} }

View File

@ -173,7 +173,6 @@ qemuPasstStart(virDomainObj *vm,
char macaddr[VIR_MAC_STRING_BUFLEN]; char macaddr[VIR_MAC_STRING_BUFLEN];
size_t i; size_t i;
int exitstatus = 0; int exitstatus = 0;
int cmdret = 0;
cmd = virCommandNew(PASST); cmd = virCommandNew(PASST);
@ -285,14 +284,12 @@ qemuPasstStart(virDomainObj *vm,
if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0) if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0)
return -1; return -1;
if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) < 0) if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus) < 0)
goto error; goto error;
if (cmdret < 0 || exitstatus != 0) { if (exitstatus != 0) {
if (cmdret >= 0) { virReportError(VIR_ERR_INTERNAL_ERROR,
virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not start 'passt': %s"), NULLSTR(errbuf));
_("Could not start 'passt': %s"), NULLSTR(errbuf));
}
goto error; goto error;
} }

View File

@ -623,11 +623,9 @@ qemuSecurityDomainRestorePathLabel(virQEMUDriver *driver,
* @uid: the uid to force * @uid: the uid to force
* @gid: the gid to force * @gid: the gid to force
* @existstatus: pointer to int returning exit status of process * @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 * 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 * -1 then their value is enforced.
* return, then appropriate error was already reported.
* *
* Returns: 0 on success, * Returns: 0 on success,
* -1 otherwise (with error reported). * -1 otherwise (with error reported).
@ -638,11 +636,11 @@ qemuSecurityCommandRun(virQEMUDriver *driver,
virCommand *cmd, virCommand *cmd,
uid_t uid, uid_t uid,
gid_t gid, gid_t gid,
int *exitstatus, int *exitstatus)
int *cmdret)
{ {
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivate *priv = vm->privateData; qemuDomainObjPrivate *priv = vm->privateData;
int ret = -1;
if (virSecurityManagerSetChildProcessLabel(driver->securityManager, if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
vm->def, cmd) < 0) vm->def, cmd) < 0)
@ -664,9 +662,9 @@ qemuSecurityCommandRun(virQEMUDriver *driver,
if (virSecurityManagerPreFork(driver->securityManager) < 0) if (virSecurityManagerPreFork(driver->securityManager) < 0)
return -1; return -1;
*cmdret = virCommandRun(cmd, exitstatus); ret = virCommandRun(cmd, exitstatus);
virSecurityManagerPostFork(driver->securityManager); virSecurityManagerPostFork(driver->securityManager);
return 0; return ret;
} }

View File

@ -115,8 +115,7 @@ int qemuSecurityCommandRun(virQEMUDriver *driver,
virCommand *cmd, virCommand *cmd,
uid_t uid, uid_t uid,
gid_t gid, gid_t gid,
int *exitstatus, int *exitstatus);
int *cmdret);
/* Please note that for these APIs there is no wrapper yet. Do NOT blindly add /* 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. * new APIs here. If an API can touch a file add a proper wrapper instead.

View File

@ -248,7 +248,6 @@ qemuSlirpStart(virDomainObj *vm,
pid_t pid = (pid_t) -1; pid_t pid = (pid_t) -1;
int rc; int rc;
int exitstatus = 0; int exitstatus = 0;
int cmdret = 0;
bool killDBusDaemon = false; bool killDBusDaemon = false;
g_autofree char *fdname = g_strdup_printf("slirpfd-%s", net->info.alias); 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) if (qemuExtDeviceLogCommand(driver, vm, cmd, "slirp") < 0)
goto error; goto error;
if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) < 0) if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus) < 0)
goto error; goto error;
if (cmdret < 0 || exitstatus != 0) { if (exitstatus != 0) {
if (cmdret >= 0) { virReportError(VIR_ERR_INTERNAL_ERROR,
virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not start 'slirp'. exitstatus: %d"), exitstatus);
_("Could not start 'slirp'. exitstatus: %d"), exitstatus);
}
goto error; goto error;
} }

View File

@ -927,7 +927,6 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
virTimeBackOffVar timebackoff; virTimeBackOffVar timebackoff;
const unsigned long long timeout = 1000; /* ms */ const unsigned long long timeout = 1000; /* ms */
bool setTPMStateLabel = true; bool setTPMStateLabel = true;
int cmdret = 0;
pid_t pid = -1; pid_t pid = -1;
cfg = virQEMUDriverGetConfig(driver); cfg = virQEMUDriverGetConfig(driver);
@ -963,15 +962,9 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
return -1; return -1;
if (qemuSecurityCommandRun(driver, vm, cmd, cfg->swtpm_user, if (qemuSecurityCommandRun(driver, vm, cmd, cfg->swtpm_user,
cfg->swtpm_group, NULL, &cmdret) < 0) cfg->swtpm_group, NULL) < 0)
goto error; goto error;
if (cmdret < 0) {
/* virCommandRun() hidden in qemuSecurityCommandRun()
* already reported error. */
goto error;
}
if (virPidFileReadPath(pidfile, &pid) < 0) { if (virPidFileReadPath(pidfile, &pid) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("swtpm didn't show up")); _("swtpm didn't show up"));

View File

@ -105,7 +105,7 @@ int qemuExtVhostUserGPUStart(virQEMUDriver *driver,
g_autofree char *pidfile = NULL; g_autofree char *pidfile = NULL;
g_autoptr(virCommand) cmd = NULL; g_autoptr(virCommand) cmd = NULL;
int pair[2] = { -1, -1 }; int pair[2] = { -1, -1 };
int cmdret = 0, rc; int rc;
int exitstatus = 0; int exitstatus = 0;
pid_t pid; pid_t pid;
int ret = -1; int ret = -1;
@ -153,14 +153,12 @@ int qemuExtVhostUserGPUStart(virQEMUDriver *driver,
virCommandAddArgFormat(cmd, "--render-node=%s", video->accel->rendernode); 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; goto error;
if (cmdret < 0 || exitstatus != 0) { if (exitstatus != 0) {
if (cmdret >= 0) { virReportError(VIR_ERR_INTERNAL_ERROR,
virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not start 'vhost-user-gpu'. exitstatus: %d"), exitstatus);
_("Could not start 'vhost-user-gpu'. exitstatus: %d"), exitstatus);
}
goto cleanup; goto cleanup;
} }