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

fromani at redhat.com fromani at redhat.com
Thu Jul 9 12:03:07 UTC 2015


Francesco Romani has posted comments on this change.

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


Patch Set 8: Code-Review-1

(2 comments)

I think we're reaching a good enough point. A couple more suggestions inside. -1 only for visibility.

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

Line 65:         Detach the device from the host. This method *must* be
Line 66:         called before getXML in order to populate _deviceParams.
Line 67:         """
Line 68:         self._deviceParams = detach_detachable(self.device)
Line 69:         self._update_address(self.hostAddress)
perhaps

  self.hostAddress = self._update_address(self.hostAddress)

give us a better reminder that this method is mutating the argument
Line 70: 
Line 71:     def getXML(self):
Line 72:         if any((self.macAddr, self.vlanId)):
Line 73:             xml = self._create_network_interface_xml()


Line 144:             vlan.appendChildWithArgs('tag', id=str(self.vlanId))
Line 145: 
Line 146:         return interface
Line 147: 
Line 148:     def _add_source_address(self, parent_element, address):
I think now we can get rid of this method and inline it. It doesn't seem to add much value anymore.


-- 
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: 8
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