Change in vdsm[master]: hostdev: change ownership for passthrough USB devices

mpolednik at redhat.com mpolednik at redhat.com
Mon Sep 21 07:47:37 UTC 2015


Martin Polednik has posted comments on this change.

Change subject: hostdev: change ownership for passthrough USB devices
......................................................................


Patch Set 16:

(2 comments)

https://gerrit.ovirt.org/#/c/44679/16/vdsm/supervdsmServer
File vdsm/supervdsmServer:

Line 390: 
Line 391:         if not error:
Line 392:             self._udevTrigger(subsystem_matches=('vfio',))
Line 393: 
Line 394:     @logDecorator
> Both method names are not descriptive. I am not sure what the code is doing
Not very fortunate but "consistent in a module" :/
Line 395:     def appropriateUSBDevice(self, bus, device):
Line 396:         rule_file = _UDEV_RULE_FILE_NAME_USB % (bus, device)
Line 397:         rule = ('SUBSYSTEM=="usb", ATTRS{{busnum}}=="{}", '
Line 398:                 'ATTRS{{devnum}}=="{}", OWNER:="{}", GROUP:="{}"\n').format(


Line 421:         self.log.debug('Changing ownership (to root:root) of device '
Line 422:                        'bus: %s, device:: %s',
Line 423:                        bus, device, QEMU_PROCESS_USER, QEMU_PROCESS_GROUP)
Line 424:         device_path = _USB_DEVICE_PATH % (int(bus), int(device))
Line 425:         cmd = [EXT_CHOWN, 'root:root', device_path]
> Do we need to revert the ownership of the device or default it to root even
If someone changes the permission via udev rule then we retrigger udev few lines below. If someone chmoded the file, I'm not sure how could we take care of that without introducing dangerous persistence.
Line 426:         rc, out, err = utils.execCmd(cmd)
Line 427:         if err:
Line 428:             raise OSError(errno.EINVAL, 'Could not change ownership'
Line 429:                           'out %s\nerr %s' % (out, err))


-- 
To view, visit https://gerrit.ovirt.org/44679
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1f72f63186187254cd4aded0a2e6c396001ca28b
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list