Edward Haas has posted comments on this change. Change subject: ovs: add network with dhcp configuration ......................................................................
Patch Set 8: Code-Review-1 (6 comments) https://gerrit.ovirt.org/#/c/59039/8/lib/vdsm/network/netswitch.py File lib/vdsm/network/netswitch.py: PS8, Line 168: six.iterkeys It's not needed PS8, Line 192: _ipv4_from_attrs(attrs) The fun name has no real meaning. How about this: ipv4 = address.IPv4(_ipv4_conf_params(attrs)) def _ipv4_conf_params(attrs): return attrs.get(), ... Same for _ipv6_from_attrs https://gerrit.ovirt.org/#/c/59039/8/tests/network/func_dhclient_test.py File tests/network/func_dhclient_test.py: PS8, Line 35: CIDR BITMASK PS8, Line 58: { Is a newline necessary here? Line 65: with self.setupNetworks(netcreate, {}, NOCHK): Line 66: self.assertNetworkIp( Line 67: NETWORK_NAME, netcreate[NETWORK_NAME]) Line 68: finally: Line 69: if bridged: Why is the cleanup needed? Removing the network is not enough? Line 70: delete_dhclient_leases( Line 71: NETWORK_NAME, dhcpv4=True, dhcpv6=False) Line 72: else: Line 73: delete_dhclient_leases( https://gerrit.ovirt.org/#/c/59039/8/tests/network/netfunctestlib.py File tests/network/netfunctestlib.py: Line 244: self.assertNotIn(bond, self.running_config.bonds) Line 245: Line 246: def assertNetworkIp(self, net, attrs): Line 247: if 'ipaddr' not in attrs and 'ipv6addr' not in attrs: Line 248: return For DHCP, you will exit here and it will be a false positive. Please make sure your tests fail before they pass. Line 249: Line 250: network_netinfo = self.netinfo.networks[net] Line 251: Line 252: bridged = attrs.get('bridged', True) -- To view, visit https://gerrit.ovirt.org/59039 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I792713f27f41faf5e2ffffd4de02c51ad8d0ee02 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Edward Haas <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
