This is an automated email from the ASF dual-hosted git repository.
weizhou pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new adec5f439df kvm: add ssvm storage nic null uri check during plug
(#11557)
adec5f439df is described below
commit adec5f439dff7fe970fb5aa33bd06260a231c8d5
Author: Abhishek Kumar <abhishek.mr...@gmail.com>
AuthorDate: Sat Sep 6 17:04:23 2025 +0530
kvm: add ssvm storage nic null uri check during plug (#11557)
* kvm: add ssvm storage nic null uri check during plug
Fixes #11552
Signed-off-by: Abhishek Kumar <abhishek.mr...@gmail.com>
* refactor
Signed-off-by: Abhishek Kumar <abhishek.mr...@gmail.com>
---------
Signed-off-by: Abhishek Kumar <abhishek.mr...@gmail.com>
---
.../hypervisor/kvm/resource/BridgeVifDriver.java | 27 ++++++----
.../kvm/resource/BridgeVifDriverTest.java | 58 +++++++++++++++++++---
2 files changed, 67 insertions(+), 18 deletions(-)
diff --git
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
index 0602fd6322f..3b66529ccaf 100644
---
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
+++
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
@@ -184,6 +184,21 @@ public class BridgeVifDriver extends VifDriverBase {
return vNetId != null && protocol != null &&
!vNetId.equalsIgnoreCase("untagged");
}
+ protected String createStorageVnetBridgeIfNeeded(NicTO nic, String
trafficLabel,
+ String storageBrName) throws InternalErrorException {
+ if
(!Networks.BroadcastDomainType.Storage.equals(nic.getBroadcastType()) ||
nic.getBroadcastUri() == null) {
+ return storageBrName;
+ }
+ String vNetId =
Networks.BroadcastDomainType.getValue(nic.getBroadcastUri());
+ String protocol = Networks.BroadcastDomainType.Vlan.scheme();
+ if (!isValidProtocolAndVnetId(vNetId, protocol)) {
+ return storageBrName;
+ }
+ logger.debug(String.format("creating a vNet dev and bridge for %s
traffic per traffic label %s",
+ Networks.TrafficType.Storage.name(), trafficLabel));
+ return createVnetBr(vNetId, storageBrName, protocol);
+ }
+
@Override
public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType,
String nicAdapter, Map<String, String> extraConfig) throws
InternalErrorException, LibvirtException {
@@ -250,15 +265,7 @@ public class BridgeVifDriver extends VifDriverBase {
intf.defBridgeNet(_bridges.get("private"), null, nic.getMac(),
getGuestNicModel(guestOsType, nicAdapter));
} else if (nic.getType() == Networks.TrafficType.Storage) {
String storageBrName = nic.getName() == null ?
_bridges.get("private") : nic.getName();
- if (nic.getBroadcastType() ==
Networks.BroadcastDomainType.Storage) {
- vNetId =
Networks.BroadcastDomainType.getValue(nic.getBroadcastUri());
- protocol = Networks.BroadcastDomainType.Vlan.scheme();
- }
- if (isValidProtocolAndVnetId(vNetId, protocol)) {
- logger.debug(String.format("creating a vNet dev and bridge for
%s traffic per traffic label %s",
- Networks.TrafficType.Storage.name(), trafficLabel));
- storageBrName = createVnetBr(vNetId, storageBrName, protocol);
- }
+ storageBrName = createStorageVnetBridgeIfNeeded(nic, trafficLabel,
storageBrName);
intf.defBridgeNet(storageBrName, null, nic.getMac(),
getGuestNicModel(guestOsType, nicAdapter));
}
if (nic.getPxeDisable()) {
@@ -291,7 +298,7 @@ public class BridgeVifDriver extends VifDriverBase {
return "brvx-" + vnetId;
}
- private String createVnetBr(String vNetId, String pifKey, String protocol)
throws InternalErrorException {
+ protected String createVnetBr(String vNetId, String pifKey, String
protocol) throws InternalErrorException {
String nic = _pifs.get(pifKey);
if (nic == null ||
protocol.equals(Networks.BroadcastDomainType.Vxlan.scheme())) {
// if not found in bridge map, maybe traffic label refers to pif
already?
diff --git
a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java
b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java
index 48bba2b78ed..00364948f82 100644
---
a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java
+++
b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java
@@ -16,24 +16,29 @@
// under the License.
package com.cloud.hypervisor.kvm.resource;
+import java.net.URI;
+import java.net.URISyntaxException;
+
import org.junit.Assert;
-import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
import com.cloud.agent.api.to.NicTO;
+import com.cloud.exception.InternalErrorException;
import com.cloud.network.Networks;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
@RunWith(MockitoJUnitRunner.class)
public class BridgeVifDriverTest {
- private BridgeVifDriver driver;
+ private static final String BRIDGE_NAME = "cloudbr1";
- @Before
- public void setUp() throws Exception {
- driver = new BridgeVifDriver();
- }
+ @Spy
+ @InjectMocks
+ private BridgeVifDriver driver = new BridgeVifDriver();
@Test
public void isBroadcastTypeVlanOrVxlan() {
@@ -58,4 +63,41 @@ public class BridgeVifDriverTest {
Assert.assertTrue(driver.isValidProtocolAndVnetId("123", "vlan"));
Assert.assertTrue(driver.isValidProtocolAndVnetId("456", "vxlan"));
}
+
+ @Test
+ public void
createStorageVnetBridgeIfNeededReturnsStorageBrNameWhenBroadcastTypeIsNotStorageButValidValues()
throws InternalErrorException {
+ NicTO nic = new NicTO();
+ nic.setBroadcastType(Networks.BroadcastDomainType.Storage);
+ int vlan = 123;
+ String newBridge = "br-" + vlan;
+ nic.setBroadcastUri(Networks.BroadcastDomainType.Storage.toUri(vlan));
+
Mockito.doReturn(newBridge).when(driver).createVnetBr(Mockito.anyString(),
Mockito.anyString(), Mockito.anyString());
+ String result = driver.createStorageVnetBridgeIfNeeded(nic,
"trafficLabel", BRIDGE_NAME);
+ Assert.assertEquals(newBridge, result);
+ }
+
+ @Test
+ public void
createStorageVnetBridgeIfNeededReturnsStorageBrNameWhenBroadcastTypeIsNotStorage()
throws InternalErrorException {
+ NicTO nic = new NicTO();
+ nic.setBroadcastType(Networks.BroadcastDomainType.Vlan);
+ String result = driver.createStorageVnetBridgeIfNeeded(nic,
"trafficLabel", BRIDGE_NAME);
+ Assert.assertEquals(BRIDGE_NAME, result);
+ }
+
+ @Test
+ public void
createStorageVnetBridgeIfNeededReturnsStorageBrNameWhenBroadcastUriIsNull()
throws InternalErrorException {
+ NicTO nic = new NicTO();
+ nic.setBroadcastType(Networks.BroadcastDomainType.Storage);
+ String result = driver.createStorageVnetBridgeIfNeeded(nic,
"trafficLabel", BRIDGE_NAME);
+ Assert.assertEquals(BRIDGE_NAME, result);
+ }
+
+ @Test
+ public void
createStorageVnetBridgeIfNeededCreatesVnetBridgeWhenUntaggedVlan() throws
InternalErrorException, URISyntaxException {
+ NicTO nic = new NicTO();
+ nic.setBroadcastType(Networks.BroadcastDomainType.Storage);
+ nic.setBroadcastUri(new
URI(Networks.BroadcastDomainType.Storage.scheme() + "://untagged"));
+ String result = driver.createStorageVnetBridgeIfNeeded(nic,
"trafficLabel", BRIDGE_NAME);
+ Assert.assertEquals(BRIDGE_NAME, result);
+ }
}
