Re: [libvirt PATCH v3 09/18] qemu: add functions to start and stop nbdkit

On Thu, Oct 20, 2022 at 16:59:00 -0500, Jonathon Jongsma wrote: > Add some helper functions to build a virCommand object and run the > nbdkit process for a given virStorageSource. > > Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; > --- > src/qemu/qemu_nbdkit.c | 251 +++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_nbdkit.h | 10 ++ > 2 files changed, 261 insertions(+) [...] > +static virCommand * > +qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc) > +{ > + g_autoptr(virCommand) cmd = virCommandNewArgList(proc->caps->path, > + "--exit-with-parent", The documentation for this option states: If the parent process exits, we exit. This can be used to avoid complicated cleanup or orphaned nbdkit processes. There are some important caveats with this, see "EXIT WITH PARENT" in nbdkit-captive(1). But this doesn't really make much sense in our case. We very specifically daemonize the process so it becomes parent of init, thus there's no point waiting for the parent. > + "--unix", > + proc->socketfile, > + "--foreground", > + //"--selinux-label", > + //selinux_label, > + NULL); [...] > +qemuNbdkitProcessStart(qemuNbdkitProcess *proc, > + virDomainObj *vm, > + virQEMUDriver *driver) > +{ > + g_autoptr(virCommand) cmd = NULL; > + int rc; > + int exitstatus = 0; > + int cmdret = 0; > + VIR_AUTOCLOSE errfd = -1; > + virTimeBackOffVar timebackoff; > + bool socketCreated = false; > + > + if (!(cmd = qemuNbdkitProcessBuildCommand(proc))) > + return -1; > + > + VIR_DEBUG("starting nbdkit process for %s", proc->source->nodestorage); > + virCommandSetErrorFD(cmd, &errfd); > + virCommandSetPidFile(cmd, proc->pidfile); > + > + if (qemuExtDeviceLogCommand(driver, vm, cmd, "nbdkit") < 0) > + goto error; > + > + if (qemuSecurityCommandRun(driver, vm, cmd, proc->user, proc->group, &exitstatus, &cmdret) < 0) > + goto error; > + > + if (cmdret < 0 || exitstatus != 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Could not start 'nbdkit'. exitstatus: %d"), exitstatus); > + goto error; > + } > + > + if ((rc = virPidFileReadPath(proc->pidfile, &proc->pid)) < 0) { > + virReportSystemError(-rc, > + _("Failed to read pidfile %s"), > + proc->pidfile); > + goto error; > + } > + > + if (virTimeBackOffStart(&timebackoff, 1, 1000) < 0) > + goto error; > + > + while (virTimeBackOffWait(&timebackoff)) { > + if ((socketCreated = virFileExists(proc->socketfile))) > + break; > + > + if (virProcessKill(proc->pid, 0) == 0) > + continue; > + > + goto error; > + } > + > + if (!socketCreated) { > + virReportError(VIR_ERR_OPERATION_TIMEOUT, "%s", > + _("nbdkit socket did not show up")); > + goto error; > + } > + > + return 0; > + > + error: > + if (errfd > 0) { > + g_autofree char *errbuf = g_new0(char, 1024); > + if (read(errfd, errbuf, 1024) > 0) > + virReportError(VIR_ERR_OPERATION_FAILED, > + _("nbdkit failed to start and reported: %s"), errbuf); This error reporting doesn't seem to work. I tried starting a VM and got an error from qemu that the NBD socket is not available: error: internal error: process exited while connecting to monitor: 2022-12-09T15:12:25.558050Z qemu-system-x86_64: -blockdev {"driver":"nbd","server":{"type":"unix","path":"/var/lib/libvirt/qemu/domain-11-cd/nbdkit-libvirt-1-storage.socket"},"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}: Requested export not available So I enabled logging from nbdkit into logging into the system journal and got: Dec 09 16:07:55 speedmetal nbdkit[2115190]: curl[1]: problem doing HEAD request to fetch size of URL [http://HOST:80/name]: Unsupported protocol: Protocol "https" not supported or disabled in libcurl Which is true, the tested host forces https, but libvirt didn't report it at all. I think you'll need to convert this to properly capture the stdout/err via virtlogd as we do with other external process helpers.