Petr Horáček has posted comments on this change. Change subject: net: Link setup module - includes bond setup logic. ......................................................................
Patch Set 17: Code-Review-1 (8 comments) https://gerrit.ovirt.org/#/c/62831/17//COMMIT_MSG Commit Message: PS17, Line 16: clenup cleanup https://gerrit.ovirt.org/#/c/62831/17/lib/vdsm/network/link/setup.py File lib/vdsm/network/link/setup.py: PS17, Line 46: attrs can we create a to-be-configured bond object here and don't touch attrs anymore? it will require Bond.__init__ changes, so it won't import existing. Line 52: # allow a proper rollback to occur in case of failure. Line 53: with Bond(init_bond.master) as bond: Line 54: slaves2remove = self._slaves2remove(bond.slaves, Line 55: frozenset(attrs['nics'])) Line 56: bond.del_slaves(slaves2remove) you need to update _config here. Line 57: Line 58: for (bond, attrs) in init_bond_pool: Line 59: with bond: Line 60: bond.refresh() PS17, Line 58: (bond, attrs) parentheses are not needed PS17, Line 58: bond in the previous iteration is is called init_bond Line 61: requested_slaves = frozenset(attrs['nics']) Line 62: slaves2add = self._slaves2add(bond.slaves, requested_slaves) Line 63: bond.add_slaves(slaves2add) Line 64: Line 65: self._config.removeBonding(bond.master) we don't need to remove it if we set it. Line 66: self._config.setBonding(bond.master, attrs) Line 67: Line 68: bond.up() Line 69: _ip_flush(slaves2add) https://gerrit.ovirt.org/#/c/62831/17/tests/network/link_setup_test.py File tests/network/link_setup_test.py: Line 1: # remove this nasty extra # please Line 2: # Copyright 2016 Red Hat, Inc. Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by Line 85: setup_bonds.acquired_ifaces) Line 86: BondMock.assert_called_with(BOND1_NAME) Line 87: BondMock.return_value.add_slaves.assert_called_once_with(bond_slaves) Line 88: config_mock.removeBonding.assert_called_once_with(BOND1_NAME) Line 89: config_mock.setBonding.assert_called_once_with(BOND1_NAME, bond_attrs) can you please add a 'explode-in-the-middle' test here? setup some bonds, do some detaches during editations and then explode when trying to attach. test if _config is in consistent state (and therefore we are able to roll back). Line 90: Line 91: @staticmethod Line 92: def _assert_ip_flush_called(bond_slaves, dhclient_mock, address_mock): Line 93: for slave in bond_slaves: -- To view, visit https://gerrit.ovirt.org/62831 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib8f01a401cb1b96e357bc462e528a2a547c59c24 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas <edwa...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@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 https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
