Just like it can't remove its own PID files, passt can't unlink its
own socket upon exit (unless the initialisation fails), because it
has no access to the filesystem at runtime.
Remove the socket file in qemuPasstKill().
Fixes: a56f0168d576 ("qemu: hook up passt config to qemu domains")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
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.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
There are two places where we kill passt:
1) qemuPasstStop() - called transitively from qemuProcessStop(),
2) qemuPasstStart() - after failed start.
Now, the code from 2) lack error preservation (so if there's
another error during cleanup we might overwrite the original
error). Therefore, move the internals of qemuPasstStop() into a
separate function and call it from both places.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
When starting passt, it may write something onto its stderr
(convincing it to print even more is addressed later). Pass this
string we read to user.
Since we're not daemonizing passt anymore (see previous commit),
we can let virCommand module do all the heavy lifting and switch
to virCommandSetErrorBuffer() instead of reading error from an
FD.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
When passt is started, it daemonizes itself by default. There's
no point in having our virCommand module daemonize it too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
If qemuPasstGetPid() fails, or the passt's PID is -1 then
qemuPasstSetupCgroup() returns early without any error message
set. Report an appropriate error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This reverts commit 0c4e716835eaf2a575bd063fde074c0fc7c4e4d4.
This patch was pushed by my mistake. Even though it got ACKed on
the list, I've raised couple of issues with it. They will be
fixed in next commits.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
I initially had the passt process being started in an identical
fashion to the slirp-helper - libvirt was daemonizing the new process
and recording its pid in a pidfile. The problem with this is that,
since it is daemonized immediately, any startup error in passt happens
after the daemonization, and thus isn't seen by libvirt - libvirt
believes that the process has started successfully and continues on
its merry way. The result was that sometimes a guest would be started,
but there would be no passt process for qemu to use for network
traffic.
Instead, we should be starting passt in the same manner we start
dnsmasq - we just exec it as normal (along with a request that passt
create the pidfile, which is just another option on the passt
commandline) and wait for the child process to exit; passt then has a
chance to parse its commandline and complete all the setup prior to
daemonizing itself; if it encounters an error and exits with a non-0
code, libvirt will see the code and know about the failure. We can
then grab the output from stderr, log that so the "user" has some idea
of what went wrong, and then fail the guest startup.
Signed-off-by: Laine Stump <laine@redhat.com>
This commented-out option was pointed out by jtomko during review, but
I missed taking it out when addressing his comments.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This attribute was added to support setting the --interface option for
passt, but in a post-push/pre-9.0-release review, danpb pointed out
that it would be better to use the existing <source dev='xxx'/>
attribute to set --interface rather than creating a new attribute (in
the wrong place). So we remove backend/upstream, and change the passt
commandline creation to grab the name for --interface from source/dev.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Jumping to the error label and reading the pidfile does not make sense
until we reached qemuSecurityCommandRun which creates the pidfile.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The pidfile is guaranteed to be non-NULL (thanks to glib allocation
functions) and it's dereferenced two lines above anyway.
Reported by coverity:
/src/qemu/qemu_passt.c: 278 in qemuPasstStart()
272 return 0;
273
274 error:
275 ignore_value(virPidFileReadPathIfLocked(pidfile, &pid));
276 if (pid != -1)
277 virProcessKillPainfully(pid, true);
>>> CID 404360: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "pidfile" suggests that it may be null, but it
>>> has already been dereferenced on all paths leading to the check.
278 if (pidfile)
279 unlink(pidfile);
280
281 return -1;
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This consists of (1) adding the necessary args to the qemu commandline
netdev option, and (2) starting a passt process prior to starting
qemu, and making sure that it is terminated when it's no longer
needed. Under normal circumstances, passt will terminate itself as
soon as qemu closes its socket, but in case of some error where qemu
is never started, or fails to startup completely, we need to terminate
passt manually.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
