On Wed, Aug 31, 2022 at 13:40:53 -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@xxxxxxxxxx>
> ---
> src/qemu/qemu_conf.c | 22 ++++++++++++
> src/qemu/qemu_conf.h | 3 ++
> src/qemu/qemu_domain.c | 29 +++++++++++++++
> src/qemu/qemu_domain.h | 4 +++
> src/qemu/qemu_nbdkit.c | 82 ++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_nbdkit.h | 24 +++++++++++++
> 6 files changed, 164 insertions(+)
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 3b75cdeb95..b5f451a8f9 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1571,3 +1571,25 @@ qemuGetMemoryBackingPath(virQEMUDriver *driver,
> *memPath = g_strdup_printf("%s/%s", domainPath, alias);
> 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 414bcfb8a2..91212d6608 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -309,6 +309,7 @@ struct _virQEMUDriver {
> virHashAtomic *migrationErrors;
>
> virFileCache *nbdkitCapsCache;
> + char *nbdkitBinary;
> };
>
> virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
> @@ -362,3 +363,5 @@ int qemuGetMemoryBackingPath(virQEMUDriver *driver,
> const virDomainDef *def,
> const char *alias,
> char **memPath);
> +
> +qemuNbdkitCaps* qemuGetNbdkitCaps(virQEMUDriver *driver);
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index fe3ce023a4..5b3b2d3e9c 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -819,6 +819,7 @@ qemuDomainStorageSourcePrivateDispose(void *obj)
> g_clear_pointer(&priv->encinfo, qemuDomainSecretInfoFree);
> g_clear_pointer(&priv->httpcookie, qemuDomainSecretInfoFree);
> g_clear_pointer(&priv->tlsKeySecret, qemuDomainSecretInfoFree);
> + g_clear_pointer(&priv->nbdkitProcess, qemuNbdkitProcessFree);
Note that private data for a storage source are disposed also in cases
when the VM is not being terminated, such as at libvirtd/virtqemud
restart, qemuNbdkitProcessFree is not simply a freeing function but also
kills the nbdkit process, so in this instance it will break on restart
of libvirtd.
> }
>
>
> @@ -10013,6 +10014,32 @@ qemuDomainPrepareStorageSourceNFS(virStorageSource *src)
> }
>
>
> +/* qemuPrepareStorageSourceNbdkit:
> + * @src: source for a disk
> + *
> + * If src is an network source that is managed by nbdkit, prepare data so that
> + * nbdkit can be launched before the domain is started
> + *
> + * Returns true if nbdkit will be used for this source,
> + */
> +static bool
> +qemuDomainPrepareStorageSourceNbdkit(virStorageSource *src,
So for now the only caller [2] doesn't use the return value. I'm not
sure if that will change but if not it's pointless to return it.
> + virQEMUDriverConfig *cfg,
> + const char *alias,
> + qemuDomainObjPrivate *priv)
> +{
> + g_autoptr(qemuNbdkitCaps) nbdkit = qemuGetNbdkitCaps(priv->driver);
> + if (!nbdkit)
> + return false;
> +
> + if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK)
> + return false;
IMO you should first check whether 'nbdkit' will even be used before
fetching the capabilities.
> +
> + return qemuNbdkitInitStorageSource(nbdkit, src, priv->libDir,
> + alias, cfg->user, cfg->group);
> +}
> +
> +
> /* qemuProcessPrepareStorageSourceTLS:
> * @source: source for a disk
> * @cfg: driver configuration
> @@ -10787,6 +10814,8 @@ qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk,
> if (qemuDomainPrepareStorageSourceNFS(src) < 0)
> return -1;
>
> + qemuDomainPrepareStorageSourceNbdkit(src, cfg, src->nodestorage, priv);
[2]
> +
> return 0;
> }
>
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 592ee9805b..563b0aa925 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -33,6 +33,7 @@
> #include "qemu_conf.h"
> #include "qemu_capabilities.h"
> #include "qemu_migration_params.h"
> +#include "qemu_nbdkit.h"
> #include "qemu_slirp.h"
> #include "qemu_fd.h"
> #include "virchrdev.h"
> @@ -293,6 +294,9 @@ struct _qemuDomainStorageSourcePrivate {
>
> /* key for decrypting TLS certificate */
> qemuDomainSecretInfo *tlsKeySecret;
> +
> + /* an nbdkit process for serving network storage sources */
> + qemuNbdkitProcess *nbdkitProcess;
> };
>
> virObject *qemuDomainStorageSourcePrivateNew(void);
> diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
> index fc83f80f1f..4df8957196 100644
> --- a/src/qemu/qemu_nbdkit.c
> +++ b/src/qemu/qemu_nbdkit.c
> @@ -579,3 +579,85 @@ qemuNbdkitCapsCacheNew(const char *cachedir)
> g_autofree char *dir = g_build_filename(cachedir, "nbdkitcapabilities", NULL);
> return virFileCacheNew(dir, "xml", &nbdkitCapsCacheHandlers);
> }
> +
> +
> +static qemuNbdkitProcess *
> +qemuNbdkitProcessNew(qemuNbdkitCaps *caps,
> + virStorageSource *source,
> + char *statedir,
> + const char *alias,
> + uid_t user,
> + gid_t group)
> +{
> + qemuNbdkitProcess *proc = g_new0(qemuNbdkitProcess, 1);
> + g_autofree char *pidfile = g_strdup_printf("nbdkit-%s.pid", alias);
> + g_autofree char *socketfile = g_strdup_printf("nbdkit-%s.socket", alias);
> +
> + proc->caps = g_object_ref(caps);
> + /* weak reference -- source owns this object, so it will always outlive us */
> + proc->source = source;
> + proc->user = user;
> + proc->group = group;
> + proc->pidfile = g_build_filename(statedir, pidfile, NULL);
> + proc->socketfile = g_build_filename(statedir, socketfile, NULL);
> +
> + return proc;
> +}
> +
> +
> +bool
> +qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps,
> + virStorageSource *source,
> + char *statedir,
> + const char *alias,
> + uid_t user,
> + gid_t group)
> +{
> + qemuDomainStorageSourcePrivate *srcPriv = qemuDomainStorageSourcePrivateFetch(source);
> +
> + if (srcPriv->nbdkitProcess)
> + return false;
> +
> + switch (source->protocol) {
> + case VIR_STORAGE_NET_PROTOCOL_HTTP:
> + case VIR_STORAGE_NET_PROTOCOL_HTTPS:
> + case VIR_STORAGE_NET_PROTOCOL_FTP:
> + case VIR_STORAGE_NET_PROTOCOL_FTPS:
> + case VIR_STORAGE_NET_PROTOCOL_TFTP:
> + if (!virBitmapIsBitSet(caps->flags, QEMU_NBDKIT_CAPS_PLUGIN_CURL))
> + return false;
> + break;
> + case VIR_STORAGE_NET_PROTOCOL_SSH:
> + if (!virBitmapIsBitSet(caps->flags, QEMU_NBDKIT_CAPS_PLUGIN_SSH))
> + return false;
> + break;
> + case VIR_STORAGE_NET_PROTOCOL_NONE:
> + case VIR_STORAGE_NET_PROTOCOL_NBD:
> + case VIR_STORAGE_NET_PROTOCOL_RBD:
> + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
> + case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
> + case VIR_STORAGE_NET_PROTOCOL_ISCSI:
> + case VIR_STORAGE_NET_PROTOCOL_VXHS:
> + case VIR_STORAGE_NET_PROTOCOL_NFS:
> + case VIR_STORAGE_NET_PROTOCOL_LAST:
> + return false;
> + }
> + srcPriv->nbdkitProcess = qemuNbdkitProcessNew(caps, source, statedir, alias, user, group);
> +
> + return true;
> +}
> +
> +
> +void
> +qemuNbdkitProcessFree(qemuNbdkitProcess *proc)
> +{
> + if (virProcessKillPainfully(proc->pid, true) < 0)
> + VIR_WARN("Unable to kill nbdkit process");
[2].
> +
> + unlink(proc->pidfile);
> + g_clear_pointer(&proc->pidfile, g_free);
> + unlink(proc->socketfile);
> + g_clear_pointer(&proc->socketfile, g_free);
> + g_clear_object(&proc->caps);
> + g_free(proc);
> +}
