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

On 2/9/23 09:36, Peter Krempa wrote:
...
  On Wed, Feb 08, 2023 at 18:13:10 -0500, 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(a)redhat.com>
 ---
  src/qemu/qemu_passt.c | 9 ++++-----
  1 file changed, 4 insertions(+), 5 deletions(-) 
 
 [..]
 
       if (cmdret < 0 || exitstatus != 0) {
          virReportError(VIR_ERR_INTERNAL_ERROR,
 -                       _("Could not start 'passt'. exitstatus: %d"),
exitstatus);
 +                       _("Could not start 'passt': %s"), errbuf);
          goto error;
      } 
 
 So the 'passt' binary doesn't do any logging later on during runtime
 which we'd have to capture into a specific log file? 
It does:

  -e, --stderr          Log to stderr too
    default: log to system logger only if started from a TTY
  -l, --log-file PATH   Log (only) to given file
  --log-size BYTES      Maximum size of log file
    default: 1 MiB

Maybe, we can keep the errfd and let our event loop read from it? But
that looks like a stretch, unnecessary - what would we do with the error
if it's reported after guest is started, there's no client connected and
no API running? The best we could do is to relay the error into our
logs. Which is probably as good as '-l' option then.

BTW: I don't see us passing --stderr. Is that intentional? Maybe I don't
understand the default.

Michal