[libvirt PATCH v5 21/32] qemu: use nbdkit to serve network disks if available

[Peter Krempa]

On Tue, Feb 14, 2023 at 11:08:08 -0600, Jonathon Jongsma wrote:
> For virStorageSource objects that contain an nbdkitProcess, start that
> nbdkit process to serve that network drive and then pass the nbdkit
> socket to qemu rather than sending the network url to qemu directly.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---

[...]

> @@ -308,10 +327,36 @@ qemuExtDevicesHasDevice(virDomainDef *def)
>              return true;
>      }
>  
> +    for (i = 0; i < def->ndisks; i++) {
> +        qemuDomainStorageSourcePrivate *priv =
> +            QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(def->disks[i]->src);
> +        if (priv->nbdkitProcess)
> +            return true;

This is insufficient. Also the backing images of 'src' can have 'nbdkit'

> +    }
> +
> +
>      return false;
>  }
>  
>  
> +/* recursively setup nbdkit cgroups for backing chain of src */
> +static int qemuExtDevicesSetupCgroupNbdkit(virStorageSource *src,
> +                                           virCgroup *cgroup)

Header style doesn't conform to our coding style and the rest of the
file.

> +{
> +        qemuDomainStorageSourcePrivate *priv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
> +
> +        if (src->backingStore)
> +            if (qemuExtDevicesSetupCgroupNbdkit(src->backingStore, cgroup) < 0)
> +                return -1;
> +
> +        if (priv && priv->nbdkitProcess &&
> +            qemuNbdkitProcessSetupCgroup(priv->nbdkitProcess, cgroup) < 0)
> +            return -1;

Any particular reason why you need to setup the cgroups starting from
the bottom-most image?

Specifically for nbdkit it should not matter too much as we need to
start them before starting qemu anyways.

In QEMU we indeed must construct the backing chain from the bottom image
first.

> +
> +        return 0;
> +}
> +
> +
>  int
>  qemuExtDevicesSetupCgroup(virQEMUDriver *driver,
>                            virDomainObj *vm,

[...]

> +/* recursively start nbdkit for backing chain of src */
> +int
> +qemuNbdkitStartStorageSource(virQEMUDriver *driver,
> +                             virDomainObj *vm,
> +                             virStorageSource *src)
> +{
> +    virStorageSource *backing;
> +
> +    for (backing = src->backingStore; backing != NULL; backing = backing->backingStore)
> +        if (qemuNbdkitStartStorageSourceOne(driver, vm, backing) < 0)
> +            return -1;
> +
> +    return qemuNbdkitStartStorageSourceOne(driver, vm, src);
> +}

This is again the weird iteration mechanism where backing images are
iterated from top to bottom and then the topmost image is iterated.

Make it into a linear one please. Either from the end or from the start.

> +/* recursively stop nbdkit processes for backing chain of src */
> +void
> +qemuNbdkitStopStorageSource(virStorageSource *src)
> +{
> +    virStorageSource *backing;
> +
> +    qemuNbdkitStopStorageSourceOne(src);
> +
> +    for (backing = src->backingStore; backing != NULL; backing = backing->backingStore)
> +        qemuNbdkitStopStorageSourceOne(backing);
> +}

Same here.


Reviewed-by: Peter Krempa <pkrempa at redhat.com>