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@xxxxxxxxxxxxx>
> ---
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list