From 9e97e35031572e0f6ace32e2fb094f0f358f0391 Mon Sep 17 00:00:00 2001

Message-Id: <9e97e35031572e0f6ace32e2fb094f0f358f0391@dist-git>

From: Thomas Huth <thuth@redhat.com>

Date: Tue, 11 May 2021 14:10:28 +0200

Subject: [PATCH] hostdev: mdev: Lookup mdevs by sysfs path rather than mdev

 struct

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

The lookup didn't do anything apart from comparing the sysfs paths

anyway since that's what makes each mdev unique.

The most ridiculous usage of the old logic was in

virHostdevReAttachMediatedDevices where in order to drop an mdev

hostdev from the list of active devices we first had to create a new

mdev and use it in the lookup call. Why couldn't we have used the

hostdev directly? Because the hostdev and mdev structures are

incompatible.

The way mdevs are currently removed is via a write to a specific sysfs

attribute. If you do it while the machine which has the mdev assigned

is running, the write call may block (with a new enough kernel, with

older kernels it would return a write error!) until the device

is no longer in use which is when the QEMU process exits.

The interesting part here comes afterwards when we're cleaning up and

call virHostdevReAttachMediatedDevices. The domain doesn't exist

anymore, so the list of active hostdevs needs to be updated and the

respective hostdevs removed from the list, but remember we had to

create an mdev object in the memory in order to find it in the list

first which will fail because the write to sysfs had already removed

the mdev instance from the host system.

And so the next time you try to start the same domain you'll get:

"Requested operation is not valid: mediated device <path> is in use by

driver QEMU, domain <name>"

Fixes: https://gitlab.com/libvirt/libvirt/-/issues/119

Signed-off-by: Erik Skultety <eskultet@redhat.com>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

(cherry picked from commit 49cb59778a4e6c2d04bb9383a9d97fbbc83f9fce)

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1940449

Signed-off-by: Thomas Huth <thuth@redhat.com>

Message-Id: <20210511121028.304070-3-thuth@redhat.com>

Reviewed-by: Erik Skultety <eskultet@redhat.com>

---

 src/util/virhostdev.c | 10 ++++------

 src/util/virmdev.c    | 16 ++++++++--------

 src/util/virmdev.h    |  4 ++--

 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c

index b7050e99e4..392e94307c 100644

--- a/src/util/virhostdev.c

+++ b/src/util/virhostdev.c

@@ -2025,7 +2025,7 @@ virHostdevReAttachMediatedDevices(virHostdevManagerPtr mgr,

     virObjectLock(mgr->activeMediatedHostdevs);

     for (i = 0; i < nhostdevs; i++) {

-        g_autoptr(virMediatedDevice) mdev = NULL;

+        g_autofree char *sysfspath = NULL;

         virMediatedDevicePtr tmp;

         virDomainHostdevSubsysMediatedDevPtr mdevsrc;

         virDomainHostdevDefPtr hostdev = hostdevs[i];

@@ -2034,14 +2034,12 @@ virHostdevReAttachMediatedDevices(virHostdevManagerPtr mgr,

             continue;

         mdevsrc = &hostdev->source.subsys.u.mdev;

-

-        if (!(mdev = virMediatedDeviceNew(mdevsrc->uuidstr,

-                                          mdevsrc->model)))

-            continue;

+        sysfspath = virMediatedDeviceGetSysfsPath(mdevsrc->uuidstr);

         /* Remove from the list only mdevs assigned to @drv_name/@dom_name */

-        tmp = virMediatedDeviceListFind(mgr->activeMediatedHostdevs, mdev);

+        tmp = virMediatedDeviceListFind(mgr->activeMediatedHostdevs,

+                                        sysfspath);

         /* skip inactive devices */

         if (!tmp)

diff --git a/src/util/virmdev.c b/src/util/virmdev.c

index c2499c0a20..bae4a7d2c1 100644

--- a/src/util/virmdev.c

+++ b/src/util/virmdev.c

@@ -312,7 +312,7 @@ int

 virMediatedDeviceListAdd(virMediatedDeviceListPtr list,

                          virMediatedDevicePtr *dev)

 {

-    if (virMediatedDeviceListFind(list, *dev)) {

+    if (virMediatedDeviceListFind(list, (*dev)->path)) {

         virReportError(VIR_ERR_INTERNAL_ERROR,

                        _("device %s is already in use"), (*dev)->path);

         return -1;

@@ -358,7 +358,7 @@ virMediatedDevicePtr

 virMediatedDeviceListSteal(virMediatedDeviceListPtr list,

                            virMediatedDevicePtr dev)

 {

-    int idx = virMediatedDeviceListFindIndex(list, dev);

+    int idx = virMediatedDeviceListFindIndex(list, dev->path);

     return virMediatedDeviceListStealIndex(list, idx);

 }

@@ -374,13 +374,13 @@ virMediatedDeviceListDel(virMediatedDeviceListPtr list,

 int

 virMediatedDeviceListFindIndex(virMediatedDeviceListPtr list,

-                               virMediatedDevicePtr dev)

+                               const char *sysfspath)

 {

     size_t i;

     for (i = 0; i < list->count; i++) {

-        virMediatedDevicePtr other = list->devs[i];

-        if (STREQ(other->path, dev->path))

+        virMediatedDevicePtr dev = list->devs[i];

+        if (STREQ(sysfspath, dev->path))

             return i;

     }

     return -1;

@@ -389,11 +389,11 @@ virMediatedDeviceListFindIndex(virMediatedDeviceListPtr list,

 virMediatedDevicePtr

 virMediatedDeviceListFind(virMediatedDeviceListPtr list,

-                          virMediatedDevicePtr dev)

+                          const char *sysfspath)

 {

     int idx;

-    if ((idx = virMediatedDeviceListFindIndex(list, dev)) >= 0)

+    if ((idx = virMediatedDeviceListFindIndex(list, sysfspath)) >= 0)

         return list->devs[idx];

     else

         return NULL;

@@ -407,7 +407,7 @@ virMediatedDeviceIsUsed(virMediatedDevicePtr dev,

     const char *drvname, *domname;

     virMediatedDevicePtr tmp = NULL;

-    if ((tmp = virMediatedDeviceListFind(list, dev))) {

+    if ((tmp = virMediatedDeviceListFind(list, dev->path))) {

         virMediatedDeviceGetUsedBy(tmp, &drvname, &domname);

         virReportError(VIR_ERR_OPERATION_INVALID,

                        _("mediated device %s is in use by "

diff --git a/src/util/virmdev.h b/src/util/virmdev.h

index e0905a3f6e..3022ab9948 100644

--- a/src/util/virmdev.h

+++ b/src/util/virmdev.h

@@ -120,11 +120,11 @@ virMediatedDeviceListDel(virMediatedDeviceListPtr list,

 virMediatedDevicePtr

 virMediatedDeviceListFind(virMediatedDeviceListPtr list,

-                          virMediatedDevicePtr dev);

+                          const char *sysfspath);

 int

 virMediatedDeviceListFindIndex(virMediatedDeviceListPtr list,

-                               virMediatedDevicePtr dev);

+                               const char *sysfspath);

 int

 virMediatedDeviceListMarkDevices(virMediatedDeviceListPtr dst,

-- 

2.31.1