Dan Kenigsberg has posted comments on this change. Change subject: net: introducing KernelConfig ......................................................................
Patch Set 3: Code-Review-1 (6 comments) few nits, and one supposed bug. https://gerrit.ovirt.org/#/c/42960/3/lib/vdsm/netconfpersistence.py File lib/vdsm/netconfpersistence.py: Line 202: normalized_other = self._normalize(other) Line 203: return (self.networks == normalized_other.networks Line 204: and self.bonds == normalized_other.bonds) Line 205: Line 206: def _analyze_netinfo_nets(self, netinfo): squashing commit 3de08722 into here is suboptimal. it could have been better to backport it on its own right. Line 207: for net, net_attr in netinfo.networks.iteritems(): Line 208: yield net, self._translate_netinfo_net(net, net_attr) Line 209: Line 210: def _analyze_netinfo_bonds(self, netinfo): Line 231: attributes['hostQos'] = self._remove_zero_values_in_net_qos( Line 232: net_attr['hostQos']) Line 233: Line 234: def _translate_ipaddr(self, attributes, net_attr): Line 235: attributes['bootproto'] = net_attr['bootproto4'] net_attr['dhcpv6'] is missing in the stable branch? Line 236: ifcfg = net_attr.get('cfg') Line 237: # TODO: we must not depend on 'cfg', which is configurator-dependent. Line 238: # TODO: Look up in the routing table instead. Line 239: if ifcfg and ifcfg.get('DEFROUTE') == 'yes': Line 247: if net_attr['netmask']: Line 248: attributes['netmask'] = net_attr['netmask'] Line 249: if net_attr['gateway']: Line 250: attributes['gateway'] = net_attr['gateway'] Line 251: non_local_addresses = self._translate_ipv6_addr( In master, these ipv6 translation happens regardless of bootproto=none. I think that master is correct and the bug is here. Line 252: net_attr['ipv6addrs']) Line 253: if non_local_addresses: Line 254: attributes['ipv6addr'] = non_local_addresses Line 255: if net_attr['ipv6gateway'] != '::': Line 332: self._normalize_stp(net_attr) Line 333: Line 334: def _normalize_stp(self, net_attr): Line 335: stp = net_attr.pop('stp', net_attr.pop('STP', None)) Line 336: net_attr['stp'] = self._netinfo.stpBooleanize( backporting commit 97ac9746 on its own would have been clearer. never mind now. Line 337: stp) Line 338: Line 339: def _normalize_mtu(self, config_copy): Line 340: for net_attr in config_copy.networks.itervalues(): https://gerrit.ovirt.org/#/c/42960/3/tests/functional/networkTests.py File tests/functional/networkTests.py: Line 64: IP_GATEWAY = '240.0.0.254' Line 65: DHCP_RANGE_FROM = '240.0.0.10' Line 66: DHCP_RANGE_TO = '240.0.0.100' Line 67: CUSTOM_PROPS = {'linux': 'rules', 'vdsm': 'as well'} Line 68: IP_MASK = '255.255.255.0' where do we need this? I cannot sport a usage in this module. Line 69: IPv6_ADDRESS = 'fdb3:84e5:4ff4:55e3::1/64' Line 70: IPv6_GATEWAY = 'fdb3:84e5:4ff4:55e3::ff' Line 71: Line 72: dummyPool = set() Line 725: @permutations([[True], [False]]) Line 726: @RequireDummyMod Line 727: @ValidateRunningAsRoot Line 728: @brokentest('This test is known to break until initscripts-9.03.41-1.el6 ' Line 729: 'is released to fix https://bugzilla.redhat.com/1086897') is this intentionally kept in the backport? It did not really relate to the patch in master, but why bother and create a mismatch now? Line 730: def testSetupNetworksAddVlan(self, bridged): Line 731: BRIDGE_OPTS = {'multicast_router': '0', 'multicast_snooping': '0'} Line 732: formattedOpts = ' '.join( Line 733: ['='.join(elem) for elem in BRIDGE_OPTS.items()]) -- To view, visit https://gerrit.ovirt.org/42960 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79f1eb553a42f1398ad12aa1bc33522f8af30c79 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
