Re: [libvirt PATCH] qemu: allow passt to self-daemonize

["Michal Prívozník" <mprivozn@redhat.com>]

On 2/9/23 00:13, Laine Stump wrote:
> 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>
> ---
>  src/qemu/qemu_passt.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)


OOOPS, somehow I've accidentally merged this. Let me post follow up patches.

> 
> diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
> index 0f09bf3db8..f640a69c00 100644
> --- a/src/qemu/qemu_passt.c
> +++ b/src/qemu/qemu_passt.c
> @@ -141,24 +141,23 @@ qemuPasstStart(virDomainObj *vm,
>      g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net);
>      g_autoptr(virCommand) cmd = NULL;
>      g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
> +    g_autofree char *errbuf = NULL;
>      char macaddr[VIR_MAC_STRING_BUFLEN];
>      size_t i;
>      pid_t pid = (pid_t) -1;
>      int exitstatus = 0;
>      int cmdret = 0;
> -    VIR_AUTOCLOSE errfd = -1;
>  
>      cmd = virCommandNew(PASST);
>  
>      virCommandClearCaps(cmd);
> -    virCommandSetPidFile(cmd, pidfile);
> -    virCommandSetErrorFD(cmd, &errfd);
> -    virCommandDaemonize(cmd);
> +    virCommandSetErrorBuffer(cmd, &errbuf);
>  
>      virCommandAddArgList(cmd,
>                           "--one-off",

BTW: we definitely need something better than this. IF, something goes
wrong after we've executed passt but before we execute QEMU, then passt
just hangs there. This is because passt clone()-s itself (i.e. creates a
child process), but QEMU that would connect to the socket never comes
around. Thus, the child process never sees the EOF on the socket and
just hangs in there thinking there will be somebody connecting, soon.

I thought this could be solved by just killing the whole process group,
but the child process calls setsid(), which creates its own process
group. I've managed to work around this by passing --foreground, but I'm
unclear about the consequences. Though, it looks like it's still
dropping caps, creating its own namespaces, etc. So this may actually be
the way to go.

But in general, we may need to make our pid handling better. I remember
Jano mentioned some problems with virtiofsd which also fork()-s.

Michal