Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator ......................................................................
Patch Set 17: (7 comments) Ayal, Thanks a lot for the insightful review! Some problems doesn't exist in the patch 16, which should be the latest change, and the other problems will be fixed in next change. .................................................... File vdsm/netconf/iproute2.py Line 84: @contextmanager Line 85: def _removeIface(self, iface): Line 86: _netinfo = netinfo.NetInfo() Line 87: toBeRemoved = not _netinfo.ifaceUsers(iface.name) Line 88: destroyed = yield not toBeRemoved this problem was already fixed in patch 16. Actually, patch 16 is the latest change of this patch. It's overrode by an old change unintentionally when Assaf updated his patch which is based on this one. Line 89: Line 90: if toBeRemoved: Line 91: if iface.master is None: Line 92: self.configApplier.removeIpConfig(iface) Line 90: if toBeRemoved: Line 91: if iface.master is None: Line 92: self.configApplier.removeIpConfig(iface) Line 93: Line 94: if not destroyed: Done Line 95: self.configApplier.setIfaceMtu(iface.name, Line 96: netinfo.DEFAULT_MTU) Line 97: self.configApplier.ifdown(iface) Line 98: else: Line 105: self.configApplier.removeBond(bonding) Line 106: for slave in bonding.slaves: Line 107: self.configApplier.removeBondSlave(bonding, slave) Line 108: slave.remove() Line 109: return True Done Line 110: Line 111: def removeNic(self, nic): Line 112: with self._removeIface(nic): Line 113: return False Line 140: ipConfig.netmask) Line 141: if ipConfig.gateway and ipConfig.defaultRoute: Line 142: ipwrapper.routeAdd(['default', 'via', ipConfig.gateway]) Line 143: except ipwrapper.IPRoute2Error as e: Line 144: if 'File exists' in e.message[0]: Done Line 145: pass Line 146: else: Line 147: raise Line 148: Line 149: def removeIpConfig(self, iface): Line 150: ipwrapper.addrFlush(iface.name) Line 151: Line 152: def setIfaceMtu(self, iface, mtu): Line 153: if mtu: in former implementation, setIfaceMtu use an instance of NetDevice as argument, and it just use iface.mtu as the mtu value if not extra mtu passed it. But we just use a plain str to represent the interface, so it doesn't make sense to call it without mtu. I am going to remove the statement 'if mtu:', the validation is already done in the netmodels layer. Line 154: ipwrapper.linkSet(iface, ['mtu', str(mtu)]) Line 155: Line 156: def setBondingMtu(self, iface, mtu): Line 157: self.setIfaceMtu(iface, mtu) Line 168: self.setIfaceMtu(iface.name, iface.mtu) Line 169: self.ifup(iface) Line 170: Line 171: def addBridge(self, bridge): Line 172: execCmd(['brctl', 'addbr', bridge.name]) Done Line 173: Line 174: def addBridgePort(self, bridge): Line 175: execCmd(['brctl', 'addif', bridge.name, bridge.port.name]) Line 176: Line 171: def addBridge(self, bridge): Line 172: execCmd(['brctl', 'addbr', bridge.name]) Line 173: Line 174: def addBridgePort(self, bridge): Line 175: execCmd(['brctl', 'addif', bridge.name, bridge.port.name]) Done Line 176: Line 177: def removeBridge(self, bridge): Line 178: execCmd([constants.EXT_BRCTL, 'delbr', bridge.name]) Line 179: -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Assaf Muller <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Mark Wu <[email protected]> Gerrit-Reviewer: Petr Šebek <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
