Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork ......................................................................
Patch Set 7: (6 inline comments) .................................................... File vdsm/configNetwork.py Line 47: MAX_BRIDGE_NAME_LEN = 15 Line 48: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 49: Line 50: Line 51: def objectivizeNetwork(bridge=None, vlan=None, bonding=None, This method takes a few arguments that describe a network and makes an object oriented representation out of it. It does not create a network and I am not a fan of unnecessary long names like "makeObjectOrientedNetworkRepresentation". Good point about the docstring. Line 52: bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 53: netmask=None, gateway=None, bootproto=None, Line 54: _netinfo=None, configurator=None, **opts): Line 55: if configurator is None: Line 76: if bridge: Line 77: topObj = Bridge(bridge, configurator, ports=[topObj] if topObj else [], Line 78: mtu=mtu, stp=opts.get('stp'), Line 79: forwardDelay=opts.get('forward_delay', 0)) Line 80: topObj.ip = IpConfig(inet=IPv4(ipaddr, netmask, gateway), AFAIK This code can't be reached by calls from vdsClient nor the engine without a topObj being set. I could raise a BAD_PARAMS error I guess... Line 81: bootproto=bootproto) Line 82: return topObj Line 83: Line 84: Line 81: bootproto=bootproto) Line 82: return topObj Line 83: Line 84: Line 85: def _validateInterNetworkCompatibility(ni, vlan, iface, bridged): I'd leave this for a follow-up patch. Line 86: """ Line 87: Verify network compatibility with other networks on iface (bond/nic). Line 88: Line 89: Only following combinations allowed: Line 175: if bonding not in vlanedIfaces: Line 176: for nic in nics: Line 177: if nic not in vlanedIfaces: Line 178: ifdown(nic) Line 179: netEnt.configure(network=network, **options) I'm afraid I don't understand this. Would you please rephrase? Line 180: Line 181: Line 182: def assertBridgeClean(bridge, vlan, bonding, nics): Line 183: ports = set(netinfo.ports(bridge)) .................................................... File vdsm/netmodels.py Line 27: from vdsm import netinfo Line 28: import neterrors as ne Line 29: Line 30: Line 31: class NetDevice(object): Done Line 32: def __init__(self, name, configurator, ipconfig=None, mtu=None): Line 33: self.name = name Line 34: self.ip = ipconfig Line 35: self.mtu = mtu Line 114: '''This class represents traditional kernel bridges.''' Line 115: MAX_BRIDGE_NAME_LEN = 15 Line 116: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 117: Line 118: def __init__(self, name, configurator, ipconfig=None, mtu=None, ports=(), You are right that we don't do it at the moment and there are no plans to do it. Thus, this premature extensibility point (it was here for netinfo) will be dropped. Line 119: forwardDelay=0, stp=None): Line 120: self.validateName(name) Line 121: self.ports = ports Line 122: self.forwardDelay = forwardDelay -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches