Ido Barkan has posted comments on this change.

Change subject: refactor StaticSourceRoute for better testability
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.ovirt.org/#/c/34067/4/tests/functional/networkTests.py
File tests/functional/networkTests.py:

Line 1620
Line 1621
Line 1622
Line 1623
Line 1624
> The advantage of keeping these is that if someone introduces a regression t
incorrect. This tests (unlike their names) test source routing explicitly, 
which is (now) tested better elsewhere.


Line 58: BONDING_NAME = 'bond11'
Line 59: IP_ADDRESS = '240.0.0.1'
Line 60: IP_NETWORK = '240.0.0.0'
Line 61: IP_ADDRESS_IN_NETWORK = '240.0.0.50'
Line 62: IP_CIDR = '24'
> You can calculate this from IP_CIDR via netaddr.
Done
Line 63: IP_MASK = '255.255.255.0'
Line 64: IP_NETWORK_AND_CIDR = IP_NETWORK + '/' + IP_CIDR
Line 65: IP_GATEWAY = '240.0.0.254'
Line 66: DHCP_RANGE_FROM = '240.0.0.10'


http://gerrit.ovirt.org/#/c/34067/4/vdsm/network/configurators/__init__.py
File vdsm/network/configurators/__init__.py:

Line 139: 
Line 140:     def _removeSourceRoute(self, netEnt, sourceRouteClass):
Line 141:         if netEnt.ipConfig.bootproto != 'dhcp' and netEnt.master is 
None:
Line 142:             ip = netEnt.ipConfig
Line 143:             logging.debug("Removing source route for device %s", 
netEnt.name)
> Just pass None, None, None, no?
Done
Line 144:             sourceRouteClass(netEnt.name, self, ip.ipaddr, ip.netmask,
Line 145:                              ip.gateway).remove()
Line 146: 
Line 147:     def _setNewMtu(self, iface, ifaceVlans):


-- 
To view, visit http://gerrit.ovirt.org/34067
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If81c71b83670ee0c721dcce449e48ccc21e3bbec
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibar...@redhat.com>
Gerrit-Reviewer: Assaf Muller <amul...@redhat.com>
Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to