On Sat, Oct 30, 2010 at 12:25:30AM +0200, Diego Elio Pettenò wrote:
Add a new interface to hostusb.h to add an USB device abstraction to a cgroup whitelist; then use it both when attaching a new USB device and when adding it to the commandline so that the device can be accessed by the QEmu-specific cgroup. --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 36 ++++++++++++++++++++++++++++++++++++ src/util/hostusb.c | 6 ++++++ src/util/hostusb.h | 4 +++- 4 files changed, 46 insertions(+), 1 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cf06256..0c6f308 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -707,6 +707,7 @@ virMutexUnlock;
# usb.h +usbAllowDeviceCgroup; usbDeviceFileIterate; usbDeviceGetBus; usbDeviceGetDevno; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1eea3a9..acf319e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3505,6 +3505,23 @@ static int qemuSetupCgroup(struct qemud_driver *driver, qemuSetupChardevCgroup, cgroup) < 0) goto cleanup; + + for (i = 0; i < vm->def->nhostdevs; i++) { + virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i]; + usbDevice *usb; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + continue; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) + continue; + + if ((usb = usbFindDevice(hostdev->source.subsys.u.usb.vendor, + hostdev->source.subsys.u.usb.product)) == NULL) + goto cleanup;
This should actually use usbGetDevice with bus+device, since the (optional) vendor+product has already been resolved into a bus+device ID pair.
+ if (usbAllowDeviceCgroup(usb, cgroup) < 0) + goto cleanup; + } }
This should call out to usbDeviceFileIterate, passing a callback todo the cgroup relabelling.
if ((rc = qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY))) { @@ -8454,6 +8471,25 @@ static int qemudDomainAttachHostUsbDevice(struct qemud_driver *driver, goto error; }
+ if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + virCgroupPtr cgroup = NULL; + usbDevice *usb; + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find cgroup for %s\n"), + vm->def->name); + goto error; + } + + if ((usb = usbFindDevice(hostdev->source.subsys.u.usb.vendor, + hostdev->source.subsys.u.usb.product)) == NULL) + goto error;
Likewise here.
+ + if (usbAllowDeviceCgroup(usb, cgroup) < 0) + goto error; + }
Also usbDeviceFileIterate.
+ qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) ret = qemuMonitorAddDevice(priv->mon, devstr); diff --git a/src/util/hostusb.c b/src/util/hostusb.c index 2d6e414..2cbe1fb 100644 --- a/src/util/hostusb.c +++ b/src/util/hostusb.c @@ -225,3 +225,9 @@ int usbDeviceFileIterate(usbDevice *dev, { return (actor)(dev, dev->path, opaque); } + +int usbAllowDeviceCgroup(usbDevice *dev, + virCgroupPtr group) +{ + return virCgroupAllowDevicePath(group, dev->path); +}
This introduces a strict coupling between the usb + cgroups code which isn't something we want. Using the usbDeviceFileIterate API avoids this coupling. One general point is that the indentation in your patch is not correct - needs to be 4 space indented rather than 2. See the HACKING file for .emacs rules. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|