3:28 p.m.
The USB-specific cgroup setup had been inserted inline in
qemuDomainAttachHostUsbDevice and qemuSetupCgroup, but now there is a
common cgroup setup function called for all hostdevs, so it makes sens
to put the usb-specific setup there and just rely on that function
being called.
The one thing I'm uncertain of here (and a reason for not pushing
until after release) is that previously hostdev->missing was checked
only when starting a domain (and cgroup setup for the device skipped
if missing was true), but with this consolidation, it is now checked
in the case of hotplug as well. I don't know if this will have any
practical effect (does it make sense to hotplug a "missing" usb
device?)
---
src/qemu/qemu_cgroup.c | 61 ++++++++++++++++++++++++++-----------------------
src/qemu/qemu_cgroup.h | 3 ---
src/qemu/qemu_hotplug.c | 16 -------------
3 files changed, 32 insertions(+), 48 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 92c53d9..7a7824d 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -191,9 +191,10 @@ qemuSetupTPMCgroup(virDomainDefPtr def,
}
-int qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev ATTRIBUTE_UNUSED,
- const char *path,
- void *opaque)
+static int
+qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev ATTRIBUTE_UNUSED,
+ const char *path,
+ void *opaque)
{
virDomainObjPtr vm = opaque;
qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -221,6 +222,7 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm,
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
virPCIDevicePtr pci = NULL;
+ virUSBDevicePtr usb = NULL;
char *path = NULL;
/* currently this only does something for PCI devices using vfio
@@ -263,6 +265,28 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm,
}
}
break;
+
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
+ /* NB: hostdev->missing wasn't previously checked in the
+ * case of hotplug, only when starting a domain. Now it is
+ * always checked, and the cgroup setup skipped if true.
+ */
+ if (dev->missing)
+ break;
+ if ((usb = virUSBDeviceNew(dev->source.subsys.u.usb.bus,
+ dev->source.subsys.u.usb.device,
+ NULL)) == NULL) {
+ goto cleanup;
+ }
+
+ /* oddly, qemuSetupHostUsbDeviceCgroup doesn't ever
+ * reference the usb object we just created
+ */
+ if (virUSBDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,
+ vm) < 0) {
+ goto cleanup;
+ }
+ break;
default:
break;
}
@@ -271,6 +295,7 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm,
ret = 0;
cleanup:
virPCIDeviceFree(pci);
+ virUSBDeviceFree(usb);
VIR_FREE(path);
return ret;
}
@@ -326,6 +351,9 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
}
}
break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
+ /* nothing to tear down for USB */
+ break;
default:
break;
}
@@ -545,33 +573,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver,
goto cleanup;
for (i = 0; i < vm->def->nhostdevs; i++) {
- virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i];
- virUSBDevicePtr usb;
-
- if (qemuSetupHostdevCGroup(vm, hostdev) < 0)
+ if (qemuSetupHostdevCGroup(vm, vm->def->hostdevs[i]) < 0)
goto cleanup;
-
- /* NB: the code below here should be moved into
- * qemuSetupHostdevCGroup()
- */
- if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
- continue;
- if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
- continue;
- if (hostdev->missing)
- continue;
-
- if ((usb = virUSBDeviceNew(hostdev->source.subsys.u.usb.bus,
- hostdev->source.subsys.u.usb.device,
- NULL)) == NULL)
- goto cleanup;
-
- if (virUSBDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,
- vm) < 0) {
- virUSBDeviceFree(usb);
- goto cleanup;
- }
- virUSBDeviceFree(usb);
}
}
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 64d71a5..048c2fa 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -33,9 +33,6 @@ int qemuSetupDiskCgroup(virDomainObjPtr vm,
virDomainDiskDefPtr disk);
int qemuTeardownDiskCgroup(virDomainObjPtr vm,
virDomainDiskDefPtr disk);
-int qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev,
- const char *path,
- void *opaque);
int qemuSetupHostdevCGroup(virDomainObjPtr vm,
virDomainHostdevDefPtr dev) ATTRIBUTE_RETURN_CHECK;
int qemuTeardownHostdevCgroup(virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index eeee507..ad46a3d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1151,22 +1151,6 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
goto error;
}
- if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) {
- virUSBDevicePtr usb;
-
- if ((usb = virUSBDeviceNew(hostdev->source.subsys.u.usb.bus,
- hostdev->source.subsys.u.usb.device,
- NULL)) == NULL)
- goto error;
-
- if (virUSBDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,
- vm) < 0) {
- virUSBDeviceFree(usb);
- goto error;
- }
- virUSBDeviceFree(usb);
- }
-
qemuDomainObjEnterMonitor(driver, vm);
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE))
ret = qemuMonitorAddDevice(priv->mon, devstr);
--
1.7.11.7