Change in vdsm[master]: hostdev: fix hostdev addressing

mpolednik at redhat.com mpolednik at redhat.com
Wed Jul 8 12:35:26 UTC 2015


Martin Polednik has posted comments on this change.

Change subject: hostdev: fix hostdev addressing
......................................................................


Patch Set 7:

(2 comments)

https://gerrit.ovirt.org/#/c/43042/7/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

Line 63:         Detach the device from the host. This method *must* be
Line 64:         called before getXML in order to populate _deviceParams.
Line 65:         """
Line 66:         self._deviceParams = detach_detachable(self.device)
Line 67:         self._update_address(self.hostAddress)
> Shouldn't this be 
The function is designed to mutate the address AND return it. In the first step, the address needs to be retrieved anyway and therefore, return value can be used and we don't need separate line to call the function. Future occurrences rely on the state change only.
Line 68: 
Line 69:     def getXML(self):
Line 70:         if any((self.macAddr, self.vlanId)):
Line 71:             xml = self._create_network_interface_xml()


Line 105:             managed='no', mode='subsystem',
Line 106:             type=CAPABILITY_TO_XML_ATTR[self._deviceParams['capability']])
Line 107:         source = hostdev.appendChildWithArgs('source')
Line 108: 
Line 109:         self._add_source_address(source, self._deviceParams['address'])
> question: shouldn't we use self_hostAddress here?
We should. :) But the functionality doesn't rely on it due to state mutation.
Line 110: 
Line 111:         return hostdev
Line 112: 
Line 113:     def _create_network_interface_xml(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d21555ec18d0e8f9766064755443785be3ef697
Gerrit-PatchSet: 7
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: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list