[libvirt PATCH v3 08/18] qemu: Add qemuNbdkitProcess

[Peter Krempa]

On Thu, Oct 20, 2022 at 16:58:59 -0500, Jonathon Jongsma wrote:
> An object for storing information about a nbdkit process that is serving
> a specific virStorageSource. At the moment, this information is just
> stored in the private data of virStorageSource and not used at all.
> Future commits will use this data to actually start a nbdkit process.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
>  src/qemu/qemu_conf.c   | 23 ++++++++++++
>  src/qemu/qemu_conf.h   |  3 ++
>  src/qemu/qemu_domain.c | 31 ++++++++++++++++
>  src/qemu/qemu_domain.h |  4 +++
>  src/qemu/qemu_nbdkit.c | 82 ++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_nbdkit.h | 26 ++++++++++++++
>  6 files changed, 169 insertions(+)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index ae5bbcd138..0370429da0 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1641,3 +1641,26 @@ qemuHugepageMakeBasedir(virQEMUDriver *driver,
>  
>      return 0;
>  }
> +
> +
> +/*
> + * qemuGetNbdkitCaps:
> + * @driver: the qemu driver
> + *
> + * Gets the capabilities for Nbdkit for the specified driver. These can be used
> + * to determine whether a particular disk source can be served by nbdkit or
> + * not.
> + *
> + * Returns: a reference to qemuNbdkitCaps or NULL
> + */
> +qemuNbdkitCaps*
> +qemuGetNbdkitCaps(virQEMUDriver *driver)
> +{
> +    if (!driver->nbdkitBinary)
> +        driver->nbdkitBinary = virFindFileInPath("nbdkit");
> +
> +    if (driver->nbdkitBinary)
> +        return virFileCacheLookup(driver->nbdkitCapsCache, driver->nbdkitBinary);
> +
> +    return NULL;
> +}
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 95c05e6888..8b4f6ce669 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -323,6 +323,7 @@ struct _virQEMUDriver {
>  
>      /* Immutable pointer, self-locking APIs */
>      virFileCache *nbdkitCapsCache;
> +    char *nbdkitBinary;

The above statement doesn't apply to 'nbdkitBinary'. It's not immutable
because you update it in qemuGetNbdkitCaps and it also certainly doesn't
have self-locking APIS.

You can claim immutability if and only if it's assigned in
qemuStateInitialize and never changed afterwards.

I don't think we should cache the path though as it creates weird
situations where a different version of nbdkit placed in $PATH
preferentially will be picked up, but only after a restart.

This brings another issue though I think that the path to
modules/plugins should be detected from the actual NBDkit instance
rather than hardcoded:

 $ nbdkit --dump-config
binary=/usr/sbin/nbdkit
bindir=/usr/bin
exit_with_parent=yes
filterdir=/usr/lib64/nbdkit/filters
host_cpu=x86_64
host_os=linux-gnu
libdir=/usr/lib64
mandir=/usr/share/man
name=nbdkit
plugindir=/usr/lib64/nbdkit/plugins
root_tls_certificates_dir=/etc/pki/nbdkit
sbindir=/usr/sbin
selinux=yes
sysconfdir=/etc
tls=yes
version=1.32.4
version_extra=nbdkit-1.32.4-1.fc37
version_major=1
version_minor=32
zstd=yes


This way each cached version will properly see the corresponding plugin
version. You also won't have a config-time option for it which can't be
changed after the package is installed.