Petr Horáček has posted comments on this change. Change subject: net: Remove OVS bond implementation. ......................................................................
Patch Set 8: Code-Review-1 (3 comments) just a nit https://gerrit.ovirt.org/#/c/63850/5/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: PS5, Line 221: '', > '' Done https://gerrit.ovirt.org/#/c/63850/8/lib/vdsm/network/ovs/validator.py File lib/vdsm/network/ovs/validator.py: Line 58: # TODO: Pass all nets and bonds to validator at once, not one by one. Line 59: def validate_bond_configuration(bond, attrs, nets, running_nets, kernel_nics): Line 60: if 'remove' in attrs: Line 61: _validate_bond_removal(bond, nets, running_nets) Line 62: elif 'nics' in attrs: i can still pass {'nics': []}. should not this change be in following patch? Line 63: _validate_bond_addition(attrs['nics'], kernel_nics) Line 64: else: Line 65: raise ne.ConfigNetworkError(ne.ERR_BAD_NIC, 'No slaves defined') Line 66: https://gerrit.ovirt.org/#/c/63850/8/tests/network/ovs_test.py File tests/network/ovs_test.py: Line 119: def test_add_bond_with_one_slave_twice(self): Line 120: fake_kernel_nics = ['eth0'] Line 121: nets = {} Line 122: running_nets = {} Line 123: with self.assertNotRaises(): i would remove this test. it used to make sense when we needed it to explode. now it does no make sense anymore as an unit test (we should test it on functional level) Line 124: ovs_validator.validate_bond_configuration( Line 125: 'bond1', {'nics': ['eth0', 'eth0'], 'switch': 'ovs'}, nets, Line 126: running_nets, fake_kernel_nics) Line 127: -- To view, visit https://gerrit.ovirt.org/63850 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e3d6ba6ecd64bbf22d4be88af6a69ed2f476cea Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas <edwa...@redhat.com> Gerrit-Reviewer: Edward Haas <edwa...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček <phora...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
