[libvirt] [PATCH] qemu: fix hot unplug of PCI devices with VFIO
Currently, on hot unplug of PCI devices with VFIO driver for QEMU, libvirt is trying to restore the host devices to it's previous value (basically a chown on the previous user/group). However for devices with VFIO driver, when the device is unbinded it is removed from the /dev/vfio file system causing the restore label to fail. The fix is to not restore the label for those PCI devices since they are going to be teared down anyway. Signed-off-by: Ludovic Beliveau <ludovic.beliveau@windriver.com> --- diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f8db960..f5beabd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2996,6 +2996,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; char *drivestr = NULL; + virDomainHostdevSubsysPCIPtr pcisrc = NULL; + bool is_vfio = false; VIR_DEBUG("Removing host device %s from domain %p %s", hostdev->info->alias, vm, vm->def->name); @@ -3039,6 +3041,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + pcisrc = &hostdev->source.subsys.u.pci; + is_vfio = pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; qemuDomainRemovePCIHostDevice(driver, vm, hostdev); /* QEMU might no longer need to lock as much memory, eg. we just * detached the last VFIO device, so adjust the limit here */ @@ -3058,7 +3062,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, if (qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Failed to remove host device cgroup ACL"); - if (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + if (!is_vfio && + virSecurityManagerRestoreHostdevLabel(driver->securityManager, vm->def, hostdev, NULL) < 0) { VIR_WARN("Failed to restore host device labelling"); }
Hi all, I'm new to this community and this is the first patch/fix that I'm trying to get merged. I would appreciate if people could review it and give me some feedback. Do I need to raise a bug as well ? Regards, /ludovic On 02/05/2016 02:11 PM, Ludovic Beliveau wrote:
Currently, on hot unplug of PCI devices with VFIO driver for QEMU, libvirt is trying to restore the host devices to it's previous value (basically a chown on the previous user/group).
However for devices with VFIO driver, when the device is unbinded it is removed from the /dev/vfio file system causing the restore label to fail.
The fix is to not restore the label for those PCI devices since they are going to be teared down anyway.
Signed-off-by: Ludovic Beliveau <ludovic.beliveau@windriver.com> --- diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f8db960..f5beabd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2996,6 +2996,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; char *drivestr = NULL; + virDomainHostdevSubsysPCIPtr pcisrc = NULL; + bool is_vfio = false;
VIR_DEBUG("Removing host device %s from domain %p %s", hostdev->info->alias, vm, vm->def->name); @@ -3039,6 +3041,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + pcisrc = &hostdev->source.subsys.u.pci; + is_vfio = pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; qemuDomainRemovePCIHostDevice(driver, vm, hostdev); /* QEMU might no longer need to lock as much memory, eg. we just * detached the last VFIO device, so adjust the limit here */ @@ -3058,7 +3062,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, if (qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Failed to remove host device cgroup ACL");
- if (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + if (!is_vfio && + virSecurityManagerRestoreHostdevLabel(driver->securityManager, vm->def, hostdev, NULL) < 0) { VIR_WARN("Failed to restore host device labelling"); }
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 05.02.2016 20:07, Ludovic Beliveau wrote:
Currently, on hot unplug of PCI devices with VFIO driver for QEMU, libvirt is trying to restore the host devices to it's previous value (basically a chown on the previous user/group).
However for devices with VFIO driver, when the device is unbinded it is removed from the /dev/vfio file system causing the restore label to fail.
The fix is to not restore the label for those PCI devices since they are going to be teared down anyway.
Signed-off-by: Ludovic Beliveau <ludovic.beliveau@windriver.com> ---
Sorry for the delay. I had this patch marked for review but haven't been doing much review lately.
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f8db960..f5beabd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2996,6 +2996,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; char *drivestr = NULL; + virDomainHostdevSubsysPCIPtr pcisrc = NULL; + bool is_vfio = false;
VIR_DEBUG("Removing host device %s from domain %p %s", hostdev->info->alias, vm, vm->def->name); @@ -3039,6 +3041,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + pcisrc = &hostdev->source.subsys.u.pci; + is_vfio = pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO;
I'd prefer: int backend = hostdev->source.subsys.u.pci.backend; is_vfio = backend == ...;
qemuDomainRemovePCIHostDevice(driver, vm, hostdev); /* QEMU might no longer need to lock as much memory, eg. we just * detached the last VFIO device, so adjust the limit here */ @@ -3058,7 +3062,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, if (qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Failed to remove host device cgroup ACL");
- if (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + if (!is_vfio && + virSecurityManagerRestoreHostdevLabel(driver->securityManager, vm->def, hostdev, NULL) < 0) { VIR_WARN("Failed to restore host device labelling"); }
This approach is correct. But the same problem exists in qemuDomainAttachHostPCIDevice() under error label. So ACK with this squashed in: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3e02c86..afa99f1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1278,7 +1278,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (virSecurityManagerSetHostdevLabel(driver->securityManager, vm->def, hostdev, NULL) < 0) goto error; - teardownlabel = true; + if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) + teardownlabel = true; if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0) @@ -2985,7 +2986,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; char *drivestr = NULL; - virDomainHostdevSubsysPCIPtr pcisrc = NULL; + int backend; bool is_vfio = false; VIR_DEBUG("Removing host device %s from domain %p %s", @@ -3030,8 +3031,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - pcisrc = &hostdev->source.subsys.u.pci; - is_vfio = pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + backend = hostdev->source.subsys.u.pci.backend; + is_vfio = backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; qemuDomainRemovePCIHostDevice(driver, vm, hostdev); /* QEMU might no longer need to lock as much memory, eg. we just * detached the last VFIO device, so adjust the limit here */ Michal
participants (3)
-
Beliveau, Ludovic -
Ludovic Beliveau
-
Michal Privoznik