[libvirt] [PATCH v1 24/31] qemu: Create NVMe disk in domain namespace

[Peter Krempa]

On Thu, Jul 11, 2019 at 17:54:11 +0200, Michal Privoznik wrote:
> If a domain has an NVMe disk configured, then we need to create
> /dev/vfio/* paths in domain's namespace so that qemu can open
> them.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_domain.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 949bbace88..cd3205a588 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -11831,7 +11831,8 @@ qemuDomainGetHostdevPath(virDomainDefPtr def,
>  
>                  perm = VIR_CGROUP_DEVICE_RW;
>                  if (teardown) {
> -                    if (!virDomainDefHasVFIOHostdev(def))
> +                    if (!virDomainDefHasVFIOHostdev(def) &&
> +                        !virDomainDefHasNVMeDisk(def))

As said previously I don't like this construct and also this hunk feels
really that it does not belong to this patch.

>                          includeVFIO = true;
>                  } else {
>                      includeVFIO = true;
> @@ -12415,6 +12416,22 @@ qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
>      int ret = -1;
>  
>      for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {
> +        /* NVMe disks must be checked before virStorageSourceIsLocalStorage()
> +         * is called. This is because while NVMe disks are local, they don't
> +         * have next->path set. */
> +        if (next->type == VIR_STORAGE_TYPE_NVME) {
> +            VIR_AUTOSTRINGLIST nvmePaths = NULL;
> +            size_t i;
> +
> +            if (!(nvmePaths = qemuDomainGetDiskNVMePaths(NULL, next, false)))
> +                goto cleanup;
> +
> +            for (i = 0; nvmePaths[i]; i++) {
> +                if (qemuDomainCreateDevice(nvmePaths[i], data, false) < 0)

/dev/vfio will be included for every NVMe-backed disk.

> +                    goto cleanup;
> +            }
> +        }
> +
>          if (!next->path || !virStorageSourceIsLocalStorage(next)) {
>              /* Not creating device. Just continue. */
>              continue;
> @@ -13462,12 +13479,28 @@ qemuDomainNamespaceSetupDisk(virDomainObjPtr vm,
>                               virStorageSourcePtr src)
>  {
>      virStorageSourcePtr next;
> +    VIR_AUTOSTRINGLIST nvmePaths = NULL;
>      const char **paths = NULL;
>      size_t npaths = 0;
>      char *dmPath = NULL;
>      int ret = -1;
>  
>      for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) {
> +        /* NVMe disks must be checked before virStorageSourceIsLocalStorage()
> +         * is called. This is because while NVMe disks are local, they don't
> +         * have next->path set. */

Well, that's why I've requested the comment for virStorageSourceIsLocalStorage
outlining why we don't consider NVMe as local. You can then say that
it's because that weirdness.

> +        if (next->type == VIR_STORAGE_TYPE_NVME) {
> +            size_t i;
> +
> +            if (!(nvmePaths = qemuDomainGetDiskNVMePaths(NULL, next, false)))
> +                goto cleanup;
> +
> +            for (i = 0; nvmePaths[i]; i++) {
> +                if (VIR_APPEND_ELEMENT_COPY(paths, npaths, nvmePaths[i]) < 0)

/dev/vfio will be present multiple times again.

> +                    goto cleanup;
> +            }
> +        }
> +
>          if (virStorageSourceIsEmpty(next) ||
>              !virStorageSourceIsLocalStorage(next)) {
>              /* Not creating device. Just continue. */
> -- 
> 2.21.0
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190716/889d0831/attachment-0001.sig>