Dan Kenigsberg has posted comments on this change. Change subject: Advertise aggregator ID in bonding interfaces ......................................................................
Patch Set 9: Code-Review-1 (4 comments) https://gerrit.ovirt.org/#/c/53100/9/lib/vdsm/netinfo/nics.py File lib/vdsm/netinfo/nics.py: Line 18: # Refer to the README and COPYING files for full details of the license Line 19: from __future__ import absolute_import Line 20: import errno Line 21: import io Line 22: import os actually, "os" comes after "logging" Line 23: from functools import partial Line 24: import logging Line 25: Line 26: from . import NET_PATH Line 80: with open(path, "r") as f: Line 81: return f.read().strip() Line 82: Line 83: Line 84: def get_bond_agg_info(nic_name): I am not sure about the other function, but this one should clearly sit in bonds.py Line 85: agg_id_path = os.path.join(NET_PATH, nic_name, 'bonding', 'ad_aggregator') Line 86: ad_mac_path = os.path.join(NET_PATH, nic_name, 'bonding', 'ad_partner_mac') Line 87: return { Line 88: 'ad_aggregator_id': _file_value(agg_id_path), Line 81: return f.read().strip() Line 82: Line 83: Line 84: def get_bond_agg_info(nic_name): Line 85: agg_id_path = os.path.join(NET_PATH, nic_name, 'bonding', 'ad_aggregator') you are basically rebuilding bonds.BONDING_OPT Line 86: ad_mac_path = os.path.join(NET_PATH, nic_name, 'bonding', 'ad_partner_mac') Line 87: return { Line 88: 'ad_aggregator_id': _file_value(agg_id_path), Line 89: 'ad_partner_mac': _file_value(ad_mac_path) https://gerrit.ovirt.org/#/c/53100/9/tests/functional/networkTests.py File tests/functional/networkTests.py: Line 2828: @permutations([[{}], [{'options': 'mode=4 lacp_rate=1 miimon=100'}]]) Line 2829: @cleanupNet Line 2830: @ValidateRunningAsRoot Line 2831: def test_bond_mode4_caps_aggregator_id(self, mode4): Line 2832: with dummyIf(3) as nics: Edy had a nice idea: can you create with veth_pair() as (a1, a2), veth_pair() as (b1, b2): create bond1(a1,b1) and bond2(a2,b2), both with mode=4. It would be great if we can actually see assert the true value of ad_parner_mac on both ends. Line 2833: bonding = {'nics': nics[:2]} Line 2834: bonding.update(mode4) Line 2835: status, msg = self.setupNetworks( Line 2836: {}, -- To view, visit https://gerrit.ovirt.org/53100 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I85267967c9cb1b0a626d91cb1953361ed4de727a Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sagi Shnaidman <[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: Sagi Shnaidman <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
