Dan Kenigsberg has posted comments on this change.
Change subject: added mtu support for network interface
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(8 inline comments)
incomplete review, ETOOTIRED.
....................................................
File vdsm/configNetwork.py
Line 238: def addVlan(self, vlanId, iface, bridge, mtu=None):
we should read/test whether MTU here (and in the bridge conttected to this)
includes the 4 byte vlan tag
Line 245: f.write('MTU=%s\n' % mtu)
quote
Line 282: f.write('MTU=%s\n' % mtu)
better safe than sorry - let's quote mtu, too.
Line 312: ''' method returns entry value or None
Search entry in conffile and return its value or None if not found.
Line 313: if we have MTU=9000 and we get 'MTU' as ethry
ethry->entry
Line 316: if line.upper().startswith(entry.upper() + '=') ]
why upper()? case is significant in ifcfg-*
Line 324: def _updateConfigValue(self, conffile, entry, value,
delete=False):
all these functions just cry "use augeas", please take a look.
Line 332: f.writelines(entries)
this write is not atomic, and does not record a back backup file.
--
To view, visit http://gerrit.ovirt.org/754
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d8f38627dd1a59830e426484228c1796081ad1b
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Shahar Havivi <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/vdsm-patches
