Edward Haas has posted comments on this change.

Change subject: net: Remove OVS bond implementation.
......................................................................


Patch Set 8:

(2 comments)

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': []}.
It was changed to pass a failure of no passing 'nics' key at all.
If it is passed empty, we have no problem creating the bond without slaves. We 
will not explode on it.
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.
I think we should explicit document (through tests) that we are ok if two same 
slaves are added.
Some other implementation may raise an error on this, but we do not.
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

Reply via email to