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