diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java index 11b22c494f46..bd410df419e3 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java @@ -206,6 +206,15 @@ private static boolean isInterface(final String fname) { return fname.matches(commonPattern.toString()); } + protected boolean isBroadcastTypeVlanOrVxlan(final NicTO nic) { + return nic != null && (nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan + || nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan); + } + + protected boolean isValidProtocolAndVnetId(final String vNetId, final String protocol) { + return vNetId != null && protocol != null && !vNetId.equalsIgnoreCase("untagged"); + } + @Override public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicAdapter) throws InternalErrorException, LibvirtException { @@ -220,7 +229,7 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA String vNetId = null; String protocol = null; - if (nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan || nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan) { + if (isBroadcastTypeVlanOrVxlan(nic)) { vNetId = Networks.BroadcastDomainType.getValue(nic.getBroadcastUri()); protocol = Networks.BroadcastDomainType.getSchemeValue(nic.getBroadcastUri()).scheme(); } else if (nic.getBroadcastType() == Networks.BroadcastDomainType.Lswitch) { @@ -233,8 +242,7 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA } if (nic.getType() == Networks.TrafficType.Guest) { - if ((nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan) && (vNetId != null) && (protocol != null) && (!vNetId.equalsIgnoreCase("untagged")) || - (nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan)) { + if (isBroadcastTypeVlanOrVxlan(nic) && isValidProtocolAndVnetId(vNetId, protocol)) { if (trafficLabel != null && !trafficLabel.isEmpty()) { s_logger.debug("creating a vNet dev and bridge for guest traffic per traffic label " + trafficLabel); String brName = createVnetBr(vNetId, trafficLabel, protocol); @@ -257,8 +265,7 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA createControlNetwork(); intf.defBridgeNet(_bridges.get("linklocal"), null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter)); } else if (nic.getType() == Networks.TrafficType.Public) { - if ((nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan) && (vNetId != null) && (protocol != null) && (!vNetId.equalsIgnoreCase("untagged")) || - (nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan)) { + if (isBroadcastTypeVlanOrVxlan(nic) && isValidProtocolAndVnetId(vNetId, protocol)) { if (trafficLabel != null && !trafficLabel.isEmpty()) { s_logger.debug("creating a vNet dev and bridge for public traffic per traffic label " + trafficLabel); String brName = createVnetBr(vNetId, trafficLabel, protocol); @@ -276,7 +283,7 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA String storageBrName = nic.getName() == null ? _bridges.get("private") : nic.getName(); intf.defBridgeNet(storageBrName, null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter)); } - if (nic.getPxeDisable() == true) { + if (nic.getPxeDisable()) { intf.setPxeDisable(true); } diff --git a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java new file mode 100644 index 000000000000..ad0f92bd3f88 --- /dev/null +++ b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java @@ -0,0 +1,58 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.kvm.resource; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.cloud.agent.api.to.NicTO; +import com.cloud.network.Networks; + +public class BridgeVifDriverTest { + + private BridgeVifDriver driver; + + @Before + public void setUp() throws Exception { + driver = new BridgeVifDriver(); + } + + @Test + public void isBroadcastTypeVlanOrVxlan() { + final NicTO nic = new NicTO(); + nic.setBroadcastType(Networks.BroadcastDomainType.Native); + Assert.assertFalse(driver.isBroadcastTypeVlanOrVxlan(null)); + Assert.assertFalse(driver.isBroadcastTypeVlanOrVxlan(nic)); + // Test VLAN + nic.setBroadcastType(Networks.BroadcastDomainType.Vlan); + Assert.assertTrue(driver.isBroadcastTypeVlanOrVxlan(nic)); + // Test VXLAN + nic.setBroadcastType(Networks.BroadcastDomainType.Vxlan); + Assert.assertTrue(driver.isBroadcastTypeVlanOrVxlan(nic)); + } + + @Test + public void isValidProtocolAndVnetId() { + Assert.assertFalse(driver.isValidProtocolAndVnetId(null, null)); + Assert.assertFalse(driver.isValidProtocolAndVnetId("123", null)); + Assert.assertFalse(driver.isValidProtocolAndVnetId(null, "vlan")); + Assert.assertFalse(driver.isValidProtocolAndVnetId("untagged", "vxlan")); + Assert.assertTrue(driver.isValidProtocolAndVnetId("123", "vlan")); + Assert.assertTrue(driver.isValidProtocolAndVnetId("456", "vxlan")); + } +} \ No newline at end of file