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

Jonathon Jongsma jjongsma at redhat.com
Tue Dec 13 15:23:18 UTC 2022


On 12/9/22 9:21 AM, Peter Krempa wrote:
> 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 at redhat.com>
>> ---
>>   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.
> 


Adding Rich to cc on this subthread as well.

Peter and I were discussing this on IRC and were wondering if nbdkit 
provides (or could provide) something that could help with error 
reporting here. The issue is that if you configure the the network 
source incorrectly, nbdkit will start up fine, and won't report an error 
until qemu connects to the unix socket and tries to access the nbd 
export. At that point, we report the unhelpful error Peter mentions 
above ("Requested export not available") rather than something helpful 
(e.g. "Couldn't resolve host name: site-that-doesnt-exist.com").

One possibility that we discussed was that libvirt could link against 
libnbd and try to open the socket ourselves before attempting to pass it 
to qemu. This would theoretically trigger an error from nbdkit and allow 
us to report that error before we even get to the point of starting 
qemu. That would at least help catch misconfigurations. But it seems 
unfortunate to add a new dependency just for that. If we could simply 
tell nbdkit to try to access the remote host and fail early rather than 
waiting for a nbd client, that would be nicer from my point of view.

Any thoughts? Based on my past experience, I wouldn't be surprised if 
you've already though of this and there's a plugin that already provides 
this functionality ;)

Jonathon



More information about the libvir-list mailing list