From 00c2a702090598dfcf0aa93a928c1390f8df8666 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 3 Aug 2026 07:27:47 +0200 Subject: [PATCH 1/3] make updating the vmdk descriptor optional --- .../vmware/mo/VirtualMachineMO.java | 6 +++- .../vmware/mo/VirtualMachineMOTest.java | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index 950ec9010cbd..2245bcadc036 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -1466,7 +1466,11 @@ public void attachDisk(String[] vmdkDatastorePathChain, ManagedObjectReference m VirtualDevice newDisk = VmwareHelper.prepareDiskDevice(this, null, controllerKey, vmdkDatastorePathChain, morDs, unitNumber, 1, maxIops); if (StringUtils.isNotBlank(diskController)) { String vmdkFileName = vmdkDatastorePathChain[0]; - updateVmdkAdapter(vmdkFileName, diskController); + try { + updateVmdkAdapter(vmdkFileName, diskController); + } catch (Exception e) { + logger.warn("Unable to verify/update adapter type for VMDK file " + vmdkFileName + ", proceeding with disk attach: " + e.getMessage(), e); + } } VirtualMachineConfigSpec reConfigSpec = new VirtualMachineConfigSpec(); VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); diff --git a/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java index 570f2a7b4a16..fd68ac6fb945 100644 --- a/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java +++ b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java @@ -21,6 +21,7 @@ import com.cloud.hypervisor.vmware.util.VmwareContext; import com.cloud.utils.exception.CloudRuntimeException; import com.vmware.vim25.ManagedObjectReference; +import com.vmware.vim25.VimPortType; import com.vmware.vim25.VirtualDevice; import com.vmware.vim25.VirtualLsiLogicController; import com.vmware.vim25.VirtualLsiLogicSASController; @@ -42,6 +43,14 @@ import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) @@ -135,4 +144,26 @@ public void testGetVmxFormattedVirtualHardwareVersionTwoDigits() { public void testGetVmxFormattedVirtualHardwareVersionInvalid() { VirtualMachineMO.getVmxFormattedVirtualHardwareVersion(-1); } + + @Test + public void testAttachDiskSucceedsWhenAdapterTypeUpdateFails() throws Exception { + VirtualMachineMO spyVmMo = spy(vmMo); + ManagedObjectReference morDs = mock(ManagedObjectReference.class); + ManagedObjectReference morTask = mock(ManagedObjectReference.class); + VimPortType service = mock(VimPortType.class); + + doReturn(1).when(spyVmMo).getScsiDiskControllerKey(anyString()); + doReturn(200).when(spyVmMo).getIDEDeviceControllerKey(); + doReturn(0).when(spyVmMo).getNextDeviceNumber(anyInt()); + doThrow(new Exception("HTTP 500 from vCenter datastore browser")).when(spyVmMo).updateVmdkAdapter(anyString(), anyString()); + + when(mor.getValue()).thenReturn("vm-1"); + when(context.getService()).thenReturn(service); + when(service.reconfigVMTask(eq(mor), any())).thenReturn(morTask); + when(client.waitForTask(morTask)).thenReturn(true); + + spyVmMo.attachDisk(new String[]{"[ds] i-2-3-VM/data.vmdk"}, morDs, "pvscsi", null, null); + + verify(spyVmMo).updateVmdkAdapter(eq("[ds] i-2-3-VM/data.vmdk"), eq("pvscsi")); + } } From b79169a2d36455877f6b4628fe32d9e958fa6268 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 3 Aug 2026 11:09:52 +0200 Subject: [PATCH 2/3] tighten exception scope --- .../com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java | 5 +++-- .../com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index 2245bcadc036..fd6ad8e6aa57 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -24,6 +24,7 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileOutputStream; +import java.io.IOException; import java.io.InputStreamReader; import java.io.OutputStreamWriter; import java.net.URLEncoder; @@ -1468,8 +1469,8 @@ public void attachDisk(String[] vmdkDatastorePathChain, ManagedObjectReference m String vmdkFileName = vmdkDatastorePathChain[0]; try { updateVmdkAdapter(vmdkFileName, diskController); - } catch (Exception e) { - logger.warn("Unable to verify/update adapter type for VMDK file " + vmdkFileName + ", proceeding with disk attach: " + e.getMessage(), e); + } catch (IOException e) { + logger.warn("Unable to verify/update adapter type for VMDK file " + vmdkFileName + " due to a datastore browser I/O failure, proceeding with disk attach: " + e.getMessage(), e); } } VirtualMachineConfigSpec reConfigSpec = new VirtualMachineConfigSpec(); diff --git a/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java index fd68ac6fb945..4257b8f17582 100644 --- a/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java +++ b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java @@ -38,6 +38,7 @@ import org.mockito.MockitoAnnotations; import org.mockito.junit.MockitoJUnitRunner; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -155,7 +156,7 @@ public void testAttachDiskSucceedsWhenAdapterTypeUpdateFails() throws Exception doReturn(1).when(spyVmMo).getScsiDiskControllerKey(anyString()); doReturn(200).when(spyVmMo).getIDEDeviceControllerKey(); doReturn(0).when(spyVmMo).getNextDeviceNumber(anyInt()); - doThrow(new Exception("HTTP 500 from vCenter datastore browser")).when(spyVmMo).updateVmdkAdapter(anyString(), anyString()); + doThrow(new IOException("HTTP 500 from vCenter datastore browser")).when(spyVmMo).updateVmdkAdapter(anyString(), anyString()); when(mor.getValue()).thenReturn("vm-1"); when(context.getService()).thenReturn(service); @@ -165,5 +166,7 @@ public void testAttachDiskSucceedsWhenAdapterTypeUpdateFails() throws Exception spyVmMo.attachDisk(new String[]{"[ds] i-2-3-VM/data.vmdk"}, morDs, "pvscsi", null, null); verify(spyVmMo).updateVmdkAdapter(eq("[ds] i-2-3-VM/data.vmdk"), eq("pvscsi")); + verify(service).reconfigVMTask(eq(mor), any()); + verify(client).waitForTask(morTask); } } From 55c38887058c2272a9adb915694da7302f5cfef2 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 3 Aug 2026 17:21:32 +0200 Subject: [PATCH 3/3] test disk attach when failing adaptor update --- .../vmware/mo/VirtualMachineMOTest.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java index 4257b8f17582..b1ec1ef90b8f 100644 --- a/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java +++ b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMOTest.java @@ -49,6 +49,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -169,4 +170,26 @@ public void testAttachDiskSucceedsWhenAdapterTypeUpdateFails() throws Exception verify(service).reconfigVMTask(eq(mor), any()); verify(client).waitForTask(morTask); } + + @Test(expected = Exception.class) + public void testAttachDiskFailsWhenAdapterTypeIsInvalid() throws Exception { + VirtualMachineMO spyVmMo = spy(vmMo); + ManagedObjectReference morDs = mock(ManagedObjectReference.class); + ManagedObjectReference morTask = mock(ManagedObjectReference.class); + VimPortType service = mock(VimPortType.class); + + doReturn(1).when(spyVmMo).getScsiDiskControllerKey(anyString()); + doReturn(200).when(spyVmMo).getIDEDeviceControllerKey(); + doReturn(0).when(spyVmMo).getNextDeviceNumber(anyInt()); + doThrow(new Exception("Failed to attach disk due to invalid vmdk adapter type")).when(spyVmMo).updateVmdkAdapter(anyString(), anyString()); + + when(mor.getValue()).thenReturn("vm-1"); + // Lenient: these mirror the success path and are only reached if the + // invalid-adapter-type exception is (incorrectly) swallowed by attachDisk. + lenient().when(context.getService()).thenReturn(service); + lenient().when(service.reconfigVMTask(eq(mor), any())).thenReturn(morTask); + lenient().when(client.waitForTask(morTask)).thenReturn(true); + + spyVmMo.attachDisk(new String[]{"[ds] i-2-3-VM/data.vmdk"}, morDs, "pvscsi", null, null); + } }