Change in vdsm[master]: vdsm: add functionality to acquire host devices

mpolednik at redhat.com mpolednik at redhat.com
Tue Sep 30 14:26:08 UTC 2014


Martin Polednik has posted comments on this change.

Change subject: vdsm: add functionality to acquire host devices
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.ovirt.org/#/c/32389/1/vdsm/hostdev.py
File vdsm/hostdev.py:

Line 23: 
Line 24: from vdsm import libvirtconnection
Line 25: 
Line 26: 
Line 27: DETACHABLE_DEVICES = ('usb', 'pci')
> these are capabilities, not devices, right? if so, please rename.
agreed
Line 28: 
Line 29: 
Line 30: def _parse_device_params(device_xml):
Line 31:     params = {}


Line 120: 
Line 121:     return devices
Line 122: 
Line 123: 
Line 124: def acquire(device_name):
> What about this name? Libvirt calls the action "detach". Why should not we?
This function is to be called inside device's constructor, the difference is that the libvirt sees that stack from below:

[host [device1 device2 device3]] -> libvirt really does dettach the device, but from VDSM's pov:

[libvirt [device1 device2 device3]] -> [vm1 vm2 vm3] here, we're acquiring the device from libvirt and giving it to one of our VMs
Line 125:     device = libvirtconnection.get().nodeDeviceLookupByName(device_name)
Line 126:     device_params = _parse_device_params(device)
Line 127: 
Line 128:     if device_params['capability'] in DETACHABLE_DEVICES:


Line 124: def acquire(device_name):
Line 125:     device = libvirtconnection.get().nodeDeviceLookupByName(device_name)
Line 126:     device_params = _parse_device_params(device)
Line 127: 
Line 128:     if device_params['capability'] in DETACHABLE_DEVICES:
> I'm a bit surprised: it is quite dangerous to silently ignore an action, ju
Acquire will be called by any device - even non-detachable (at least to get device_params) and we therefore have to limit devices for which the real detach is called
Line 129:         device.dettach()
Line 130: 


-- 
To view, visit http://gerrit.ovirt.org/32389
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I530fed56831314286942b8afa62900208812bfb5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list