Re: [PATCH v2 5/5] qemu_passt: Let passt write the PID file
On 2/16/23 8:32 AM, Michal Privoznik wrote:
The way we start passt currently is: we use
virCommandSetPidFile() to use our virCommand machinery to acquire
the PID file and leak opened FD into passt. Then, we use
virPidFile*() APIs to read the PID file (which is needed when
placing it into CGroups or killing it). But this does not fly
really because passt daemonizes itself. Thus the process we
started dies soon and thus the PID file is closed and unlocked.
We could work around this by passing '--foreground' argument, but
that weakens passt as it can't create new PID namespace (because
it doesn't fork()).
The solution is to let passt write the PID file, but since it
does not lock the file and closes it as soon as it is written, we
have to switch to those virPidFile APIs which don't expect PID
file to be locked.
So *this* is the part that was functionally wrong after my earlier patch
was applied - I had switched to using an externally-generated pidfile,
but was still using the APIs that should have only been used for
pidfiles created by libvirt.
(You had mentioned some sort of cgroup issue last week. Did that solve
itself? I never saw the problem, and passt has been starting/stopping
fine for me all along (both before and after I changed the daemonizing)
as long as selinux is disabled - still need to fix that).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Reviewed-by: Laine Stump <laine(a)redhat.com>
---
src/qemu/qemu_passt.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
index a4cc9e7166..47f4b5fcae 100644
--- a/src/qemu/qemu_passt.c
+++ b/src/qemu/qemu_passt.c
@@ -72,7 +72,7 @@ qemuPasstGetPid(virDomainObj *vm,
{
g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
- return virPidFileReadPathIfLocked(pidfile, pid);
+ return virPidFileReadPath(pidfile, pid);
}
@@ -106,11 +106,14 @@ static void
qemuPasstKill(const char *pidfile)
{
virErrorPtr orig_err;
+ pid_t pid = 0;
virErrorPreserveLast(&orig_err);
- if (virPidFileForceCleanupPath(pidfile) < 0)
- VIR_WARN("Unable to kill passt process");
+ ignore_value(virPidFileReadPath(pidfile, &pid));
+ if (pid != 0)
+ virProcessKillPainfully(pid, true);
+ unlink(pidfile);
virErrorRestore(&orig_err);
}
@@ -161,13 +164,13 @@ qemuPasstStart(virDomainObj *vm,
cmd = virCommandNew(PASST);
virCommandClearCaps(cmd);
- virCommandSetPidFile(cmd, pidfile);
virCommandSetErrorBuffer(cmd, &errbuf);
virCommandAddArgList(cmd,
"--one-off",
"--socket", passtSocketName,
"--mac-addr", virMacAddrFormat(&net->mac,
macaddr),
+ "--pid", pidfile,
NULL);
if (net->mtu) {