Change in vdsm[master]: net: OVS netinfo
phoracek at redhat.com
phoracek at redhat.com
Mon May 9 13:36:27 UTC 2016
Petr Horáček has posted comments on this change.
Change subject: net: OVS netinfo
......................................................................
Patch Set 27: Verified+1
(7 comments)
Passed ovs_info_test.py
https://gerrit.ovirt.org/#/c/56972/26/lib/vdsm/network/ovs/info.py
File lib/vdsm/network/ovs/info.py:
Line 109:
Line 110: return {'slaves': slaves, 'active_slave': active_slave,
Line 111: 'fake_iface': fake_iface, 'mode': mode, 'lacp': lacp}
Line 112:
Line 113: def southbound_port(self, bridge):
> Perhaps we should cache this info at the bridge level with a 'southbond' ke
I'd prefer to do that in another patch. There is also another problem: What should we do if there are multiple southbound ports set? It should never happen, but it sounds like bug danger.
Line 114: for port, attrs in six.iteritems(self.bridges[bridge]['ports']):
Line 115: if attrs['level'] == SOUTHBOUND:
Line 116: return port
Line 117: return None
Line 184:
Line 185: def _get_bond_info(port, bond_attrs, addresses, routes):
Line 186: bond_info = {
Line 187: 'slaves': bond_attrs['slaves'],
Line 188: # TODO: what should we report when no slave is active?
> Sorry for this nit, but could you please order the funcs from top to bottom
Done
Line 189: 'active_slave': (bond_attrs['active_slave'] or
Line 190: bond_attrs['slaves'][0]),
Line 191: 'opts': _to_bond_opts(bond_attrs['mode'], bond_attrs['lacp']),
Line 192: 'switch': 'ovs'
PS26, Line 190: bond_attrs['slaves'][0]),
: 'opts': _to_bond_opt
> Why would we need a default for this?
Done
PS26, Line 201: def _to_bond_opts(mode, lacp):
: opts = {}
> For southbound we use ovs_info.southbound_port and for northbound we use th
Done
PS26, Line 203:
> Really confusing... this is the northbound port name that represents the ne
Done
https://gerrit.ovirt.org/#/c/56972/26/tests/network/ovs_info_test.py
File tests/network/ovs_info_test.py:
PS26, Line 208: else '::'))
> This requires mocking the OvsInfo object, not its __init__.
Done
PS26, Line 209: getIpInfo',
> I understand that it is easier to monkeypatch it like this, but it is a bad
Done
--
To view, visit https://gerrit.ovirt.org/56972
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list