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 <edwardh(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes