Change in vdsm[master]: Simplify setNewMtu()
gvallare at redhat.com
gvallare at redhat.com
Wed Jun 5 10:18:29 UTC 2013
Giuseppe Vallarelli has posted comments on this change.
Change subject: Simplify setNewMtu()
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/netconf/ifcfg.py
Line 209:
Line 210: """
Line 211: ifaceMtu = netinfo.getMtu(iface.name)
Line 212: if isinstance(iface.master, Vlan):
Line 213: if iface.master.mtu < ifaceMtu:
Is ifaceMtu an int ? I'm not sure, because you've used it in a different way later on, casting it's value to an int. If having a string as mtu has no value/purpose I propose to refactor getMtu to return an int value instead of a string so we get rid of this castings which are error prone, what do you think?
Line 214: return
Line 215:
Line 216: vlans = _netinfo.getVlansForIface(iface.name)
Line 217: try:
Line 214: return
Line 215:
Line 216: vlans = _netinfo.getVlansForIface(iface.name)
Line 217: try:
Line 218: maxMtu = max(int(netinfo.getMtu(iface.name + '.' + vlan))
netinfo has already a function getMaxMtu.
Line 219: for vlan in vlans)
Line 220: except ValueError:
Line 221: maxMtu = None
Line 222:
Line 220: except ValueError:
Line 221: maxMtu = None
Line 222:
Line 223: if maxMtu and maxMtu < ifaceMtu:
Line 224: if isinstance(iface, Bond):
I don't like this approach of manual message dispatch, we should try to exploit python's natural duck typing, this adds unnecessary complexity to the code, a refactoring of configWriter may be needed, but I understand it stands outside the scope of this patch.
Line 225: self.configWriter.setBondingMtu(iface.name, maxMtu)
Line 226: else:
Line 227: self.configWriter.setIfaceMtu(iface.name, maxMtu)
Line 228:
--
To view, visit http://gerrit.ovirt.org/15355
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e8e0d45b7887af8fd1b003fd4c358826b77289
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
More information about the vdsm-patches
mailing list