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); > +}