Dan Kenigsberg has posted comments on this change. Change subject: Advertise aggregator ID in bonding interfaces ......................................................................
Patch Set 13: (2 comments) Very partial review https://gerrit.ovirt.org/#/c/53100/13/lib/vdsm/netinfo/bonding.py File lib/vdsm/netinfo/bonding.py: Line 68: Line 69: def _file_value(path): Line 70: if os.path.exists(path): Line 71: with open(path, 'r') as f: Line 72: return f.read().replace("N/A", "").strip() Please, no double quotes here. They distract the reader. Line 73: Line 74: Line 75: def get_bond_slave_agg_info(nic_name): Line 76: agg_id_path = BONDING_SLAVE_OPT % (nic_name, 'ad_aggregator_id') https://gerrit.ovirt.org/#/c/53100/13/tests/functional/networkTests.py File tests/functional/networkTests.py: Line 2834: def test_bond_mode4_caps_aggregator_id(self, bond_options): Line 2835: with veth_pair() as (n1, n2), veth_pair() as (n3, n4): Line 2836: nics = [n1, n2, n3, n4] Line 2837: bonds = {BONDING_NAME: (n1, n3), BONDING_NAME + "0": (n2, n4)} Line 2838: for bond, pair in bonds.iteritems(): We try to add only Python 3 compact code. Please use six.items Line 2839: bonding = {'nics': pair} Line 2840: bonding.update({'options': bond_options}) Line 2841: status, msg = self.setupNetworks( Line 2842: {}, -- 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: 13 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
