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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
