[libvirt PATCH 2/2] hostdev: mdev: Lookup mdevs by sysfs path rather than mdev struct

[Ján Tomko]

On a Thursday in 2021, Erik Skultety wrote:
>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 (this should return a write error
>instead and it is likely a bug in the vendor driver) until the device
>is no longer bound to vfio 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>"
>

Is there a public bug/issue you could link here?

>Signed-off-by: Erik Skultety <eskultet at redhat.com>
>---
> src/hypervisor/virhostdev.c | 10 ++++------
> src/util/virmdev.c          | 16 ++++++++--------
> src/util/virmdev.h          |  4 ++--
> 3 files changed, 14 insertions(+), 16 deletions(-)
>
>diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
>index aa3fc8738f..ea04382d0d 100644
>--- a/src/hypervisor/virhostdev.c
>+++ b/src/hypervisor/virhostdev.c
>@@ -1975,7 +1975,7 @@ virHostdevReAttachMediatedDevices(virHostdevManagerPtr mgr,
>
>     virObjectLock(mgr->activeMediatedHostdevs);
>     for (i = 0; i < nhostdevs; i++) {
>-        g_autoptr(virMediatedDevice) mdev = NULL;
>+        g_autofree char * sysfspath = NULL;

extra space

>         virMediatedDevicePtr tmp;
>         virDomainHostdevSubsysMediatedDevPtr mdevsrc;
>         virDomainHostdevDefPtr hostdev = hostdevs[i];

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

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210107/e9a8bf78/attachment-0001.sig>