Ido Barkan has uploaded a new change for review. Change subject: relax the validation multiple networks on the same interface ......................................................................
relax the validation multiple networks on the same interface the only validation that is now left on the vdsm side is that there are no 2 networks using the same vlan tag that are connected to the same interface concurrently. Also, no 2 untagged networks can be connected to the same interface. Change-Id: I0030433ef519ae6699ee8a921b95c0a67f7b2eae Signed-off-by: ibarkan <[email protected]> --- M lib/vdsm/netinfo.py M tests/functional/networkTests.py M vdsm/network/api.py 3 files changed, 55 insertions(+), 261 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/38/34538/1 diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index d818249..f2409a8 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -774,6 +774,11 @@ self.bondings = _netinfo['bondings'] self.bridges = _netinfo['bridges'] + def getNetsByVlans(self, iface): + return dict((vlan, net) + for (net, vlan) + in self.getNetworksAndVlansForIface(iface)) + def getNetworksAndVlansForIface(self, iface): """ Returns tuples of (bridge/network, vlan) connected to nic/bond """ return chain(self.getBridgedNetworksAndVlansForIface(iface), @@ -787,7 +792,7 @@ if iface == interface: yield (network, None) elif interface.startswith(iface + '.'): - yield (network, interface.split('.', 1)[1]) + yield (network, int(interface.split('.', 1)[1])) def getBridgelessNetworksAndVlansForIface(self, iface): """ Returns tuples of (network, vlan) connected to nic/bond """ diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 34741ab..313a317 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -913,247 +913,59 @@ self.assertVlanDoesntExist(nic + '.' + tag) @cleanupNet + @permutations([[True], [False]]) @RequireDummyMod @ValidateRunningAsRoot - def testSetupNetworksNetCompatibilityBondSingleBridge(self): - with dummyIf(1) as nics: - # Only single non-VLANed bridged network allowed - d = dict(bonding=BONDING_NAME, bridged=True) - status, msg = self.vdsm_net.setupNetworks({NETWORK_NAME: d}, - {BONDING_NAME: - dict(nics=nics)}, - NOCHK) - self.assertEqual(status, SUCCESS, msg) - self.assertNetworkExists(NETWORK_NAME, bridged=True) + def testSetupNetworksNetCompatibilityMultipleNetsSameNic(self, bridged): + with dummyIf(2) as (nic, another_nic): - # Try to add additional bridgeless network, should fail - netNameBridgeless = NETWORK_NAME + '-2' - d['bridged'] = False - status, msg = self.vdsm_net.setupNetworks({netNameBridgeless: - d}, {}, NOCHK) - self.assertTrue(status != SUCCESS, msg) + net_untagged = NETWORK_NAME + networks = {net_untagged: dict(nic=nic, bridged=bridged)} + status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) + self.assertEquals(status, SUCCESS, msg) + self.assertNetworkExists(net_untagged, bridged=bridged) - # Try to add additional bridged network, should fail - netNameBridged = NETWORK_NAME + '-3' - d['bridged'] = True - status, msg = self.vdsm_net.setupNetworks({netNameBridged: d}, - {}, NOCHK) - self.assertTrue(status != SUCCESS, msg) - - # Try to add additional VLANed bridgeless network, should fail - netNameVlanBridgeless = NETWORK_NAME + '-4' - networks = dict(netNameVlanBridgeless={'bonding': BONDING_NAME, - 'vlan': '100', - 'bridged': False}) + other_net_untagged = NETWORK_NAME + '2' + networks = {other_net_untagged: dict(nic=nic, bridged=bridged)} status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) self.assertTrue(status != SUCCESS, msg) + self.assertNetworkDoesntExist(other_net_untagged) - # Try to add additional VLANed bridged network, should fail - netNameVlanBridged = NETWORK_NAME + '-5' - networks['vlan'] = '200' - networks['bridged'] = True - status, msg = self.vdsm_net.setupNetworks({netNameVlanBridged: - networks}, {}, NOCHK) + net_tagged = NETWORK_NAME + '3' + networks = {net_tagged: dict(nic=nic, bridged=bridged, vlan='100')} + status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) + self.assertEquals(status, SUCCESS, msg) + self.assertNetworkExists(net_tagged, bridged=bridged) + + other_net_same_tag = NETWORK_NAME + '4' + networks = {other_net_same_tag: dict(nic=nic, bridged=bridged, + vlan='100')} + status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) self.assertTrue(status != SUCCESS, msg) + self.assertNetworkDoesntExist(other_net_same_tag) - self.assertNetworkDoesntExist(netNameBridgeless) - self.assertNetworkDoesntExist(netNameBridged) - self.assertNetworkDoesntExist(netNameVlanBridgeless) - self.assertNetworkDoesntExist(netNameVlanBridged) + networks = {other_net_same_tag: dict(nic=another_nic, + bridged=bridged, vlan='100')} + status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) + self.assertEquals(status, SUCCESS, msg) + self.assertNetworkExists(other_net_same_tag) + + other_net_different_tag = NETWORK_NAME + '5' + networks = {other_net_different_tag: dict(nic=nic, bridged=bridged, + vlan='200')} + status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) + self.assertEquals(status, SUCCESS, msg) + self.assertNetworkExists(other_net_different_tag, bridged=bridged) # Clean all - status, msg = self.vdsm_net.setupNetworks({NETWORK_NAME: - dict(remove=True)}, - {BONDING_NAME: - dict(remove=True)}, - NOCHK) - self.assertEquals(status, SUCCESS, msg) - - self.assertNetworkDoesntExist(NETWORK_NAME) - self.assertBondDoesntExist(BONDING_NAME, nics) - - @cleanupNet - @RequireDummyMod - @ValidateRunningAsRoot - def testSetupNetworksNetCompatibilityBondSingleBridgeless(self): - with dummyIf(1) as nics: - # Multiple VLANed networks (bridged/bridgeless) with only one - # non-VLANed bridgeless network permited - d = dict(bonding=BONDING_NAME, bridged=False) - status, msg = self.vdsm_net.setupNetworks({NETWORK_NAME: d}, - {BONDING_NAME: - dict(nics=nics)}, - NOCHK) - self.assertEqual(status, SUCCESS, msg) - self.assertNetworkExists(NETWORK_NAME, bridged=False) - - # Try to add additional bridgeless network, should fail - netNameBridgeless = NETWORK_NAME + '-2' - status, msg = self.vdsm_net.setupNetworks({netNameBridgeless: - d}, {}, NOCHK) - self.assertTrue(status != SUCCESS, msg) - - # Try to add additional bridged network, should fail - netNameBridged = NETWORK_NAME + '-3' - d['bridged'] = True - status, msg = self.vdsm_net.setupNetworks({netNameBridged: d}, - {}, NOCHK) - self.assertTrue(status != SUCCESS, msg) - - # Try to add additional VLANed bridgeless network, - # should succeed - netNameVlanBridgeless = NETWORK_NAME + '-4' - d['vlan'], d['bridged'] = '100', False - networks = {netNameVlanBridgeless: d} + nets_to_clean = [net_untagged, net_tagged, other_net_same_tag, + other_net_different_tag] + networks = dict((net, dict(remove=True)) for net in nets_to_clean) status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - self.assertEqual(status, SUCCESS, msg) - # Try to add additional VLANed bridged network, should succeed - netNameVlanBridged = NETWORK_NAME + '-5' - d['vlan'], d['bridged'] = '200', True - status, msg = self.vdsm_net.setupNetworks({netNameVlanBridged: - d}, {}, NOCHK) - self.assertEqual(status, SUCCESS, msg) - - self.assertNetworkDoesntExist(netNameBridgeless) - self.assertNetworkDoesntExist(netNameBridged) - - self.assertNetworkExists(netNameVlanBridgeless) - self.assertNetworkExists(netNameVlanBridged) - - # Clean all - r = dict(remove=True) - networks = {NETWORK_NAME: r, - netNameVlanBridgeless: r, - netNameVlanBridged: r} - status, msg = self.vdsm_net.setupNetworks(networks, - {BONDING_NAME: r}, - NOCHK) - - self.assertEqual(status, SUCCESS, msg) - - self.assertNetworkDoesntExist(NETWORK_NAME) - self.assertNetworkDoesntExist(netNameVlanBridgeless) - self.assertNetworkDoesntExist(netNameVlanBridged) - self.assertBondDoesntExist(BONDING_NAME, nics) - - @cleanupNet - @RequireDummyMod - @ValidateRunningAsRoot - def testSetupNetworksNetCompatibilityNicSingleBridge(self): - with dummyIf(1) as nics: - nic, = nics - # Only single non-VLANed bridged network allowed - networks = {NETWORK_NAME: dict(nic=nic, bridged=True)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertEquals(status, SUCCESS, msg) - self.assertNetworkExists(NETWORK_NAME, bridged=True) - - # Try to add additional bridgeless network, should fail - netNameBridgeless = NETWORK_NAME + '-2' - networks = {netNameBridgeless: dict(nic=nic, bridged=False)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertTrue(status != SUCCESS, msg) - - # Try to add additional bridged network, should fail - netNameBridged = NETWORK_NAME + '-3' - networks = {netNameBridged: dict(nic=nic, bridged=True)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertTrue(status != SUCCESS, msg) - - # Try to add additional VLANed bridgeless network, should fail - netNameVlanBridgeless = NETWORK_NAME + '-4' - networks = {netNameVlanBridgeless: dict(nic=nic, vlan='100', - bridged=False)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertTrue(status != SUCCESS, msg) - - # Try to add additional VLANed bridged network, should fail - netNameVlanBridged = NETWORK_NAME + '-5' - networks = {netNameVlanBridged: dict(nic=nic, vlan='200', - bridged=True)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertTrue(status != SUCCESS, msg) - - self.assertNetworkDoesntExist(netNameBridgeless) - self.assertNetworkDoesntExist(netNameBridged) - self.assertNetworkDoesntExist(netNameVlanBridgeless) - self.assertNetworkDoesntExist(netNameBridged) - - # Clean all - status, msg = self.vdsm_net.setupNetworks({NETWORK_NAME: - dict(remove=True)}, - {}, NOCHK) - self.assertEquals(status, SUCCESS, msg) - self.assertNetworkDoesntExist(NETWORK_NAME) - - @cleanupNet - @RequireDummyMod - @ValidateRunningAsRoot - def testSetupNetworksNetCompatibilityNicSingleBridgeless(self): - with dummyIf(1) as nics: - nic, = nics - # Multiple VLANed networks (bridged/bridgeless) with only one - # non-VLANed bridgeless network permited - networks = {NETWORK_NAME: dict(nic=nic, bridged=False)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertEquals(status, SUCCESS, msg) - self.assertNetworkExists(NETWORK_NAME, bridged=False) - - # Try to add additional bridgeless network, should fail - netNameBridgeless = NETWORK_NAME + '-2' - networks = {netNameBridgeless: dict(nic=nic, bridged=False)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertTrue(status != SUCCESS, msg) - - # Try to add additional bridged network, should fail - netNameBridged = NETWORK_NAME + '-3' - networks = {netNameBridged: dict(nic=nic, bridged=True)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertTrue(status != SUCCESS, msg) - - # Try to add additional VLANed bridgeless network, - # should succeed - netNameVlanBridgeless = NETWORK_NAME + '-4' - networks = {netNameVlanBridgeless: dict(nic=nic, vlan='100', - bridged=False)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertEquals(status, SUCCESS, msg) - - # Try to add additional VLANed bridged network, should succeed - netNameVlanBridged = NETWORK_NAME + '-5' - networks = {netNameVlanBridged: dict(nic=nic, vlan='200', - bridged=True)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertEquals(status, SUCCESS, msg) - - self.assertNetworkDoesntExist(netNameBridgeless) - self.assertNetworkDoesntExist(netNameBridged) - self.assertNetworkExists(netNameVlanBridgeless) - self.assertNetworkExists(netNameVlanBridged, bridged=True) - - # Clean all - networks = {NETWORK_NAME: dict(remove=True), - netNameVlanBridgeless: dict(remove=True), - netNameVlanBridged: dict(remove=True)} - status, msg = self.vdsm_net.setupNetworks(networks, {}, NOCHK) - - self.assertEqual(status, SUCCESS, msg) - - self.assertNetworkDoesntExist(NETWORK_NAME) - self.assertNetworkDoesntExist(netNameVlanBridgeless) - self.assertNetworkDoesntExist(netNameVlanBridged) + for net in nets_to_clean: + self.assertNetworkDoesntExist(net) @cleanupNet @permutations([[True], [False]]) diff --git a/vdsm/network/api.py b/vdsm/network/api.py index 302deb9..72fe038 100755 --- a/vdsm/network/api.py +++ b/vdsm/network/api.py @@ -180,7 +180,7 @@ raise ValueError('Invalid value for bridge stp') -def _validateInterNetworkCompatibility(ni, vlan, iface, bridged, qosOutbound): +def _validateInterNetworkCompatibility(ni, vlan, iface): """ Verify network compatibility with other networks on iface (bond/nic). @@ -190,35 +190,14 @@ non-VLANed bridgeless network - QoSed networks can't coexist with non-QoSed nets """ - def _validateNoDirectNet(ifaces): - # validate that none of the ifaces - # is a non-VLANed network over our iface - for (iface_net, iface_vlan) in ifaces: - if iface_vlan is None: - raise ConfigNetworkError(ne.ERR_BAD_PARAMS, 'interface %r ' - 'already member of network %r' % - (iface, iface_net)) + iface_nets_by_vlans = ni.getNetsByVlans(iface) - ifaces_bridgeless = tuple(ni.getBridgelessNetworksAndVlansForIface(iface)) - ifaces_bridged = tuple(ni.getBridgedNetworksAndVlansForIface(iface)) - - # If non-VLANed bridged network exists - # we can't add nothing else - _validateNoDirectNet(ifaces_bridged) - - # Multiple VLANed networks (bridged/bridgeless) with only one - # non-VLANed bridgeless network permited - if vlan is None: - # Want to add non-VLANed bridgeless network, - # check whether interface already has such network. - # Only one non-VLANed bridgeless network permited - if not bridged: - _validateNoDirectNet(ifaces_bridgeless) - # Want to add non-VLANed bridged network, - # check whether interface is empty - elif ifaces_bridged or ifaces_bridgeless: - raise ConfigNetworkError(ne.ERR_BAD_PARAMS, 'interface %r already ' - 'has networks' % (iface)) + if vlan in iface_nets_by_vlans: + raise ConfigNetworkError(ne.ERR_BAD_PARAMS, + 'interface %r cannot be define with this ' + 'network since it is already defined with ' + 'network %s' % (iface, + iface_nets_by_vlans[vlan])) def _alterRunningConfig(func): @@ -305,12 +284,10 @@ raise ConfigNetworkError(ne.ERR_USED_BRIDGE, 'Network already exists') if bonding: - _validateInterNetworkCompatibility(_netinfo, vlan, bonding, - bridged, qosOutbound) + _validateInterNetworkCompatibility(_netinfo, vlan, bonding) else: for nic in nics: - _validateInterNetworkCompatibility(_netinfo, vlan, nic, - bridged, qosOutbound) + _validateInterNetworkCompatibility(_netinfo, vlan, nic) # defaultRoute is set either explicitly by the client, OR if we're adding # the management network. -- To view, visit http://gerrit.ovirt.org/34538 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0030433ef519ae6699ee8a921b95c0a67f7b2eae Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
