[libvirt PATCH v2 08/16] qemu: Add qemuNbdkitProcess

Peter Krempa pkrempa at redhat.com
Tue Sep 27 12:02:30 UTC 2022


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 at redhat.com>
> ---
>  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);
> +}


More information about the libvir-list mailing list