mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2025-02-22 03:12:22 +00:00
hostdev: mdev: Lookup mdevs by sysfs path rather than mdev struct
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>
This commit is contained in:
parent
964738cff3
commit
49cb59778a
@ -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;
|
||||
virMediatedDevicePtr tmp;
|
||||
virDomainHostdevSubsysMediatedDevPtr mdevsrc;
|
||||
virDomainHostdevDefPtr hostdev = hostdevs[i];
|
||||
@ -1984,14 +1984,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)
|
||||
|
@ -308,7 +308,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;
|
||||
@ -354,7 +354,7 @@ virMediatedDevicePtr
|
||||
virMediatedDeviceListSteal(virMediatedDeviceListPtr list,
|
||||
virMediatedDevicePtr dev)
|
||||
{
|
||||
int idx = virMediatedDeviceListFindIndex(list, dev);
|
||||
int idx = virMediatedDeviceListFindIndex(list, dev->path);
|
||||
|
||||
return virMediatedDeviceListStealIndex(list, idx);
|
||||
}
|
||||
@ -370,13 +370,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;
|
||||
@ -385,11 +385,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;
|
||||
@ -403,7 +403,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 "
|
||||
|
@ -130,11 +130,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,
|
||||
|
Loading…
x
Reference in New Issue
Block a user