From 96704454102f9c0bf8e54f38421bbcba5a54733d Mon Sep 17 00:00:00 2001 From: Sachin R <32716246+sachindoddaguni@users.noreply.github.com> Date: Tue, 11 Nov 2025 21:04:06 -0800 Subject: [PATCH 1/4] VM Migration: Storage access groups validation issue fix * VM Migration: Storage access groups validation issue fix * Refactor list empty checks --------- Co-authored-by: Sachin R Doddaguni --- .../java/com/cloud/vm/UserVmManagerImpl.java | 50 +++- .../com/cloud/vm/UserVmManagerImplTest.java | 234 +++++++++++++----- 2 files changed, 212 insertions(+), 72 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 2e30b4ecbd8c..da2689ef1ff2 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -410,6 +410,7 @@ import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; +import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; public class UserVmManagerImpl extends ManagerBase implements UserVmManager, VirtualMachineGuru, Configurable { @@ -589,6 +590,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir @Inject private StorageManager storageManager; @Inject + private StoragePoolAndAccessGroupMapDao _storagePoolAndAccessGroupMapDao; + @Inject private ServiceOfferingJoinDao serviceOfferingJoinDao; @Inject private BackupDao backupDao; @@ -7224,23 +7227,44 @@ private boolean checkEnforceStrictHostTagCheck(HostVO host, ServiceOffering serv return host.checkHostServiceOfferingAndTemplateTags(serviceOffering, template, strictHostTags); } - protected void validateStorageAccessGroupsOnHosts(Host srcHost, Host destinationHost) { - String[] storageAccessGroupsOnSrcHost = storageManager.getStorageAccessGroups(null, null, null, srcHost.getId()); - String[] storageAccessGroupsOnDestHost = storageManager.getStorageAccessGroups(null, null, null, destinationHost.getId()); - - List srcHostStorageAccessGroupsList = storageAccessGroupsOnSrcHost != null ? Arrays.asList(storageAccessGroupsOnSrcHost) : Collections.emptyList(); - List destHostStorageAccessGroupsList = storageAccessGroupsOnDestHost != null ? Arrays.asList(storageAccessGroupsOnDestHost) : Collections.emptyList(); - - if (CollectionUtils.isEmpty(srcHostStorageAccessGroupsList)) { + protected void validateStorageAccessGroupsOnHosts(VMInstanceVO vm, Host destinationHost) { + List vmVolumes = _volsDao.findByInstance(vm.getId()); + if (CollectionUtils.isEmpty(vmVolumes)) { + s_logger.debug(String.format("No volumes found for VM %s, skipping storage access group validation", vm)); return; } + String[] storageAccessGroupsOnDestHost = storageManager.getStorageAccessGroups(null, null, null, destinationHost.getId()); + List destHostStorageAccessGroupsList = Arrays.asList(storageAccessGroupsOnDestHost); if (CollectionUtils.isEmpty(destHostStorageAccessGroupsList)) { - throw new CloudRuntimeException("Source host has storage access groups, but destination host has none."); + s_logger.debug(String.format("Destination host %s has no storage access groups", destinationHost.getName())); } - if (!destHostStorageAccessGroupsList.containsAll(srcHostStorageAccessGroupsList)) { - throw new CloudRuntimeException("Storage access groups on the source and destination hosts did not match."); + for (VolumeVO volume : vmVolumes) { + if (volume.getPoolId() == null) { + s_logger.debug(String.format("Volume %s has no storage pool assigned, skipping storage access group validation", volume.getName())); + continue; + } + List poolStorageAccessGroupsList = _storagePoolAndAccessGroupMapDao.getStorageAccessGroups(volume.getPoolId()); + if (CollectionUtils.isEmpty(poolStorageAccessGroupsList)) { + s_logger.debug(String.format("Storage pool %d has no storage access groups, skipping validation for volume %s", + volume.getPoolId(), volume.getName())); + continue; + } + if (Collections.disjoint(poolStorageAccessGroupsList, destHostStorageAccessGroupsList)) { + throw new CloudRuntimeException(String.format( + "Destination host %s does not have any storage access groups that match the requirements for volume %s (Resource Class: %s) on storage pool %d. " + + "Pool requires access groups: %s, but destination host only has: %s", + destinationHost.getName(), + volume.getName(), + volume.getVolumeType(), volume.getPoolId(), + String.join(", ", poolStorageAccessGroupsList), + String.join(", ", destHostStorageAccessGroupsList) + )); + } + s_logger.debug(String.format("Storage access group validation passed for volume %s (Resource Class: %s) on storage pool %d. " + + "Destination host %s has matching access groups.", + volume.getName(), volume.getVolumeType(), volume.getPoolId(), destinationHost.getName())); } } @@ -7286,7 +7310,7 @@ private DeployDestination checkVmMigrationDestination(VMInstanceVO vm, Host srcH HostVO destinationHostVO = _hostDao.findById(destinationHost.getId()); _hostDao.loadHostTags(destinationHostVO); validateStrictHostTagCheck(vm, destinationHostVO); - validateStorageAccessGroupsOnHosts(srcHost, destinationHost); + validateStorageAccessGroupsOnHosts(vm, destinationHost); checkHostsDedication(vm, srcHost.getId(), destinationHost.getId()); @@ -7647,7 +7671,7 @@ private Pair getHostsForMigrateVmWithStorage(VMInstanceVO vm, Host d destinationHost.getName(), destinationHost.getUuid())); } - validateStorageAccessGroupsOnHosts(srcHost, destinationHost); + validateStorageAccessGroupsOnHosts(vm, destinationHost); return new Pair<>(srcHost, destinationHost); } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index fe4ea0838f16..7b9e2222df79 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -171,6 +171,7 @@ import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.template.VirtualMachineTemplate; @@ -415,6 +416,8 @@ public class UserVmManagerImplTest { @Mock StorageManager storageManager; + @Mock + private StoragePoolAndAccessGroupMapDao storagePoolAndAccessGroupMapDao; @Mock private VolumeDataFactory volumeDataFactory; @@ -3206,95 +3209,208 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab } @Test - public void validateStorageAccessGroupsOnHostsMatchingSAGsNoException() { - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + public void testValidateStorageAccessGroupsOnHostsVolumeBasedSuccessWithMatchingGroups() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(new String[]{"sag1", "sag2"}); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(new String[]{"sag1", "sag2", "sag3"}); + List vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + VolumeVO volume2 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + vmVolumes.add(volume2); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); + StoragePoolVO storagePool2 = Mockito.mock(StoragePoolVO.class); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, srcHost.getId()); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, destHost.getId()); - } + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); - @Test(expected = CloudRuntimeException.class) - public void validateSAGsOnHostsNonMatchingSAGsThrowsException() { - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); + + Mockito.when(volume2.getPoolId()).thenReturn(200L); + Mockito.when(volume2.getName()).thenReturn("volume2"); + Mockito.when(volume2.getVolumeType()).thenReturn(Volume.Type.DATADISK); + Mockito.when(storagePool2.getName()).thenReturn("pool2"); - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(new String[]{"sag1", "sag2"}); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(new String[]{"sag1", "sag3"}); + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{"group1", "group2", "group3"}); + + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(Arrays.asList("group1", "group2")); + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(200L)) + .thenReturn(Arrays.asList("group2", "group3")); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); + + Mockito.verify(volumeDaoMock).findByInstance(1L); + Mockito.verify(storageManager).getStorageAccessGroups(null, null, null, 2L); + Mockito.verify(storagePoolAndAccessGroupMapDao).getStorageAccessGroups(100L); + Mockito.verify(storagePoolAndAccessGroupMapDao).getStorageAccessGroups(200L); } @Test - public void validateEmptyStorageAccessGroupOnHosts() { - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + public void testValidateStorageAccessGroupsOnHostsVolumeBasedSuccessWithIntersection() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); + + List vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(new String[]{}); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(new String[]{}); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); + + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); + + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); + + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{"group2", "group4", "group5"}); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(Arrays.asList("group1", "group2", "group3")); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, srcHost.getId()); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, destHost.getId()); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); } @Test - public void validateSAGsOnHostsNullStorageAccessGroups() { - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + public void testValidateStorageAccessGroupsOnHostsSuccessWithEmptyStorageGroups() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); + + List vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); + + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); + + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); + + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); + + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{"group1", "group2"}); + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(List.of()); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); + } - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(null); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(null); + @Test(expected = CloudRuntimeException.class) + public void testValidateStorageAccessGroupsOnHostsDestinationHostEmptyGroups() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + List vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); + + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, srcHost.getId()); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, destHost.getId()); + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); + + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{}); + + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(List.of("group1")); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); } @Test(expected = CloudRuntimeException.class) - public void validateSAGsOnDestHostNullStorageAccessGroups() { - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + public void testValidateStorageAccessGroupsOnHostsFailureNoMatch() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); + + List vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(new String[]{"sag1", "sag2"}); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(null); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); + + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); + + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{"group4", "group5", "group6"}); + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(Arrays.asList("group1", "group2", "group3")); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); } - @Test - public void validateNullStorageAccessGroupsOnSrcHost() { + @Test(expected = CloudRuntimeException.class) + public void testValidateStorageAccessGroupsOnHostsMultipleVolumesFailure() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + List vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + VolumeVO volume2 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + vmVolumes.add(volume2); - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(null); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(new String[]{"sag1", "sag2"}); + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); + StoragePoolVO storagePool2 = Mockito.mock(StoragePoolVO.class); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); + + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); + + Mockito.when(volume2.getPoolId()).thenReturn(200L); + Mockito.when(volume2.getName()).thenReturn("volume2"); + Mockito.when(volume2.getVolumeType()).thenReturn(Volume.Type.DATADISK); + Mockito.when(storagePool2.getName()).thenReturn("pool2"); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, srcHost.getId()); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, destHost.getId()); + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); + + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{"group1", "group2"}); + + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(List.of("group1")); + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(200L)) + .thenReturn(Arrays.asList("group3", "group4")); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); } + @Test public void testAllocateVMFromBackupUsingCmdValues() throws InsufficientCapacityException, ResourceAllocationException, ResourceUnavailableException { Long backupId = 4L; From 935dcc355a0abeb0d3fd391f336f3b41c0dc79d8 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Tue, 7 Jul 2026 11:54:57 -0400 Subject: [PATCH 2/4] update logger --- .../src/main/java/com/cloud/vm/UserVmManagerImpl.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index da2689ef1ff2..974581e4920b 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7230,25 +7230,25 @@ private boolean checkEnforceStrictHostTagCheck(HostVO host, ServiceOffering serv protected void validateStorageAccessGroupsOnHosts(VMInstanceVO vm, Host destinationHost) { List vmVolumes = _volsDao.findByInstance(vm.getId()); if (CollectionUtils.isEmpty(vmVolumes)) { - s_logger.debug(String.format("No volumes found for VM %s, skipping storage access group validation", vm)); + logger.debug("No volumes found for VM {}, skipping storage access group validation", vm); return; } String[] storageAccessGroupsOnDestHost = storageManager.getStorageAccessGroups(null, null, null, destinationHost.getId()); List destHostStorageAccessGroupsList = Arrays.asList(storageAccessGroupsOnDestHost); if (CollectionUtils.isEmpty(destHostStorageAccessGroupsList)) { - s_logger.debug(String.format("Destination host %s has no storage access groups", destinationHost.getName())); + logger.debug("Destination host {} has no storage access groups", destinationHost.getName()); } for (VolumeVO volume : vmVolumes) { if (volume.getPoolId() == null) { - s_logger.debug(String.format("Volume %s has no storage pool assigned, skipping storage access group validation", volume.getName())); + logger.debug("Volume {} has no storage pool assigned, skipping storage access group validation", volume.getName()); continue; } List poolStorageAccessGroupsList = _storagePoolAndAccessGroupMapDao.getStorageAccessGroups(volume.getPoolId()); if (CollectionUtils.isEmpty(poolStorageAccessGroupsList)) { - s_logger.debug(String.format("Storage pool %d has no storage access groups, skipping validation for volume %s", - volume.getPoolId(), volume.getName())); + logger.debug("Storage pool {} has no storage access groups, skipping validation for volume {}", + volume.getPoolId(), volume.getName()); continue; } if (Collections.disjoint(poolStorageAccessGroupsList, destHostStorageAccessGroupsList)) { From f274f33deba617ffad9e1f977dd3f552e4e1b095 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Tue, 7 Jul 2026 14:14:15 -0400 Subject: [PATCH 3/4] fix logger --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 974581e4920b..e93865ef21de 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7262,7 +7262,7 @@ protected void validateStorageAccessGroupsOnHosts(VMInstanceVO vm, Host destinat String.join(", ", destHostStorageAccessGroupsList) )); } - s_logger.debug(String.format("Storage access group validation passed for volume %s (Resource Class: %s) on storage pool %d. " + + logger.debug(String.format("Storage access group validation passed for volume %s (Resource Class: %s) on storage pool %d. " + "Destination host %s has matching access groups.", volume.getName(), volume.getVolumeType(), volume.getPoolId(), destinationHost.getName())); } From 35f8ee7cd9ba5f2a929fa9c607c167589fa2de09 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 8 Jul 2026 09:50:34 -0400 Subject: [PATCH 4/4] remove unnecessary stubbings --- .../test/java/com/cloud/vm/UserVmManagerImplTest.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index b40458f6b909..8644ebf51cda 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -3219,12 +3219,10 @@ public void testValidateStorageAccessGroupsOnHostsVolumeBasedSuccessWithMatching Mockito.when(volume1.getPoolId()).thenReturn(100L); Mockito.when(volume1.getName()).thenReturn("volume1"); Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); - Mockito.when(storagePool1.getName()).thenReturn("pool1"); Mockito.when(volume2.getPoolId()).thenReturn(200L); Mockito.when(volume2.getName()).thenReturn("volume2"); Mockito.when(volume2.getVolumeType()).thenReturn(Volume.Type.DATADISK); - Mockito.when(storagePool2.getName()).thenReturn("pool2"); Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); @@ -3261,7 +3259,6 @@ public void testValidateStorageAccessGroupsOnHostsVolumeBasedSuccessWithIntersec Mockito.when(volume1.getPoolId()).thenReturn(100L); Mockito.when(volume1.getName()).thenReturn("volume1"); Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); - Mockito.when(storagePool1.getName()).thenReturn("pool1"); Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); @@ -3287,12 +3284,9 @@ public void testValidateStorageAccessGroupsOnHostsSuccessWithEmptyStorageGroups( Mockito.when(vm.getId()).thenReturn(1L); Mockito.when(destinationHost.getId()).thenReturn(2L); - Mockito.when(destinationHost.getName()).thenReturn("destHost"); Mockito.when(volume1.getPoolId()).thenReturn(100L); Mockito.when(volume1.getName()).thenReturn("volume1"); - Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); - Mockito.when(storagePool1.getName()).thenReturn("pool1"); Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); @@ -3321,7 +3315,6 @@ public void testValidateStorageAccessGroupsOnHostsDestinationHostEmptyGroups() { Mockito.when(volume1.getPoolId()).thenReturn(100L); Mockito.when(volume1.getName()).thenReturn("volume1"); Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); - Mockito.when(storagePool1.getName()).thenReturn("pool1"); Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) @@ -3350,7 +3343,6 @@ public void testValidateStorageAccessGroupsOnHostsFailureNoMatch() { Mockito.when(volume1.getPoolId()).thenReturn(100L); Mockito.when(volume1.getName()).thenReturn("volume1"); Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); - Mockito.when(storagePool1.getName()).thenReturn("pool1"); Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); @@ -3382,12 +3374,10 @@ public void testValidateStorageAccessGroupsOnHostsMultipleVolumesFailure() { Mockito.when(volume1.getPoolId()).thenReturn(100L); Mockito.when(volume1.getName()).thenReturn("volume1"); Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); - Mockito.when(storagePool1.getName()).thenReturn("pool1"); Mockito.when(volume2.getPoolId()).thenReturn(200L); Mockito.when(volume2.getName()).thenReturn("volume2"); Mockito.when(volume2.getVolumeType()).thenReturn(Volume.Type.DATADISK); - Mockito.when(storagePool2.getName()).thenReturn("pool2"); Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes);