10:54 a.m.
On Thu, Jan 18, 2018 at 17:04:41 +0100, Michal Privoznik wrote:
Before we exec() qemu we have to spawn pr-helper processes for
all managed reservations (well, technically there can only one).
The only caveat there is that we should place the process into
the same namespace and cgroup as qemu (so that it shares the same
view of the system). But we can do that only after we've forked.
That means calling the setup function between fork() and exec().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_process.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 151 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 25ec464d3..02608c1f3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2507,6 +2507,151 @@ qemuProcessSetupEmulator(virDomainObjPtr vm)
}
+static int
+qemuProcessSetupOnePRDaemonHook(void *opaque)
+{
+ virDomainObjPtr vm = opaque;
+ size_t i, nfds = 0;
+ int *fds = NULL;
+ int ret = -1;
+
+ if (virProcessGetNamespaces(vm->pid, &nfds, &fds) < 0)
If this detects '0' namespaces ...
+ return ret;
+
+ if (virProcessSetNamespaces(nfds, fds) < 0)
... this call will be unhappy.
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ for (i = 0; i < nfds; i++)
+ VIR_FORCE_CLOSE(fds[i]);
+ VIR_FREE(fds);
+ return ret;
+}
+
+
+static int
+qemuProcessSetupOnePRDaemon(void *payload,
+ const void *name,
+ void *opaque)
+{
+ qemuDomainDiskPRObjectPtr prObj = payload;
+ virDomainObjPtr vm = opaque;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virQEMUDriverPtr driver = priv->driver;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ char *pidfile = NULL;
+ pid_t cpid = -1;
+ virCommandPtr cmd = NULL;
+ virTimeBackOffVar timebackoff;
+ const unsigned long long timeout = 500000; /* ms */
+ int ret = -1;
+
+ if (!prObj->managed)
+ return 0;
Again. It does not make much sense to me to have the hash table and
everything if this is ever going to be called for the managed daemon.
+
+ if (!virFileIsExecutable(cfg->prHelperName)) {
+ virReportSystemError(errno, _("'%s' is not a suitable pr
helper"),
This error message does not really report what it's checking.
+ cfg->prHelperName);
+ goto cleanup;
+ }
+
+ if (!(pidfile = virPidFileBuildPath(cfg->stateDir, name)))
+ goto cleanup;
+
+ if (unlink(pidfile) < 0 &&
+ errno != ENOENT) {
+ virReportSystemError(errno,
+ _("Cannot remove stale PID file %s"),
+ pidfile);
+ goto cleanup;
+ }
+
+ if (!(cmd = virCommandNewArgList(cfg->prHelperName,
+ "-k", prObj->path,
+ NULL)))
+ goto cleanup;
+
+ virCommandDaemonize(cmd);
+ virCommandSetPidFile(cmd, pidfile);
+ virCommandSetPreExecHook(cmd, qemuProcessSetupOnePRDaemonHook, vm);
+
+#ifdef CAP_SYS_RAWIO
+ virCommandAllowCap(cmd, CAP_SYS_RAWIO);
+#else
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Raw I/O is not supported on this platform"));
+ goto cleanup;
This also looks like worth putting in the validation code for this so
that we don't really get to here to find out.
+#endif
+
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ if (virPidFileReadPath(pidfile, &cpid) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("pr helper %s didn't show up"), (const char *)
name);
+ goto cleanup;
+ }
+
+ if (virTimeBackOffStart(&timebackoff, 1, timeout) < 0)
+ goto cleanup;
+ while (virTimeBackOffWait(&timebackoff)) {
+ if (virFileExists(prObj->path))
+ break;
+
+ if (virProcessKill(cpid, 0) == 0)
+ continue;
+
+ virReportSystemError(errno, "%s",
+ _("pr helper died unexpectedly"));
+ goto cleanup;
+ }
Sooo, after the timeout you assume success? That does not seem like a
reasonable idea.
+
+ if (priv->cgroup &&
+ virCgroupAddMachineTask(priv->cgroup, cpid) < 0)
+ goto cleanup;
+
+ if (qemuSecurityDomainSetPathLabel(driver->securityManager,
+ vm->def, prObj->path, true) < 0)
+ goto cleanup;
+
+ prObj->pid = cpid;
+ ret = 0;
+ cleanup:
+ if (ret < 0) {
+ virCommandAbort(cmd);
+ virProcessKillPainfully(cpid, true);
+ }
+ virCommandFree(cmd);
+ if (unlink(pidfile) < 0 &&
+ errno != ENOENT &&
+ !virGetLastError())
+ virReportSystemError(errno,
+ _("Cannot remove stale PID file %s"),
+ pidfile);
+ VIR_FREE(pidfile);
+ virObjectUnref(cfg);
+ return ret;
+}