Mark Wu has posted comments on this change.
Change subject: Change getMaxMtu to use the info provided by sysfs
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File vdsm/configNetwork.py
Line 905: configWriter = ConfigWriter()
Line 906:
Line 907: prevmtu = None
Line 908: if mtu:
Line 909: prevmtu = netinfo.getMaxMtu(nics)
Yes, in the old implementation, getMaxMtu returns the maximum of all configured
mtu and new desired mtu. And the variable name 'prevmtu' is not very accurate.
It could make more sense to call it newMaxMtu. However, we used prevmtu as it
was the maximum mtu of existing configurations.
# First we need to prepare all conf files
if bonding:
configWriter.addBonding(bonding, bridge=bridgeForNic,
bondingOptions=bondingOptions,
mtu=max(prevmtu, mtu), **options)
^^^^^^^^^^^^^^^^^
compare again.
for nic in nics:
configWriter.addNic(nic, bonding=bonding,
bridge=bridgeForNic if not bonding else None,
mtu=max(prevmtu, mtu), **options)
^^^^^^^^^^^^^^^^^^^^^
the same here.
So in the new implementation, I removed the new mtu parameter from the
function getMaxMtu(). Now the semantics of getMaxMtu is just the maximum mtu
existing configurations.
After reviewing the code again, I get a new question, why do need check all
nics to get the max mtu? My understanding multiple nics co-existing in the same
network only happens to a bonding enabled network. And if they're all slaves of
the same bonding, they should have the same MTU, right. So probably we could
just check the mtu of one nic. Is it correct?
Line 910:
Line 911: nic = nics[0] if nics else None
Line 912: iface = bonding or nic
Line 913: if bridged:
....................................................
File vdsm/netinfo.py
Line 123:
Line 124:
Line 125: def getMaxMtu(nics):
Line 126: maxMtu = int(getMtu(nics[0]))
Line 127: for nic in nics[1:]:
Cool! I like it too. Thanks for pointing out that. Just a small correction,
the 'key' argument must be provided in the keyword form. Otherwise, the
function object will be taken as an item to be compared.
Line 128: mtu = int(getMtu(nics[0]))
Line 129: if mtu > maxMtu:
Line 130: maxMtu = mtu
Line 131: return maxMtu
--
To view, visit http://gerrit.ovirt.org/11357
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I89d007eb81c2c64e642ed80af26f882f7632dc1c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
