Skip to content

Fix findHostsForMigration never returning hosts from other clusters #13452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
winterhazel wants to merge 3 commits into
base: 4.20
Choose a base branch
from
+86 −44
Open

Fix findHostsForMigration never returning hosts from other clusters #13452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6c603b
Fix `findHostsForMigration` never returning hosts from other clusters
winterhazel Jun 18, 2026
8a48f3d
Address reviews
winterhazel Jun 19, 2026
a1918ee
Add srcHost back to method parameters
winterhazel Jun 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 40 additions & 29 deletions server/src/main/java/com/cloud/server/ManagementServerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,7 @@ protected boolean zoneWideVolumeRequiresStorageMotion(PrimaryDataStore volumeDat
*/
Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boolean>> getTechnicallyCompatibleHosts(
final VirtualMachine vm,
final Host srcHost,
final Long startIndex,
final Long pageSize,
final String keyword) {
Expand All @@ -1479,31 +1480,6 @@ Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boo
return new Ternary<>(new Pair<>(new ArrayList<>(), 0), new ArrayList<>(), new HashMap<>());
}

final long srcHostId = vm.getHostId();
final Host srcHost = _hostDao.findById(srcHostId);
if (srcHost == null) {
if (logger.isDebugEnabled()) {
logger.debug("Unable to find the host with ID: " + srcHostId + " of this Instance: " + vm);
}
final InvalidParameterValueException ex = new InvalidParameterValueException("Unable to find the host (with specified ID) of instance with specified ID");
ex.addProxyObject(String.valueOf(srcHostId), "hostId");
ex.addProxyObject(vm.getUuid(), "vmId");
throw ex;
}

String srcHostVersion = srcHost.getHypervisorVersion();
if (HypervisorType.KVM.equals(srcHost.getHypervisorType()) && srcHostVersion == null) {
srcHostVersion = "";
}

// Check if the vm can be migrated with storage.
boolean canMigrateWithStorage = false;

List<HypervisorType> hypervisorTypes = Arrays.asList(new HypervisorType[]{HypervisorType.VMware, HypervisorType.KVM});
if (VirtualMachine.Type.User.equals(vm.getType()) || hypervisorTypes.contains(vm.getHypervisorType())) {
canMigrateWithStorage = _hypervisorCapabilitiesDao.isStorageMotionSupported(srcHost.getHypervisorType(), srcHostVersion);
}

// Check if the vm is using any disks on local storage.
final VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(vm, null, _offeringDao.findById(vm.getId(), vm.getServiceOfferingId()), null, null);
final List<VolumeVO> volumes = _volumeDao.findCreatedByInstance(vmProfile.getId());
Expand All @@ -1517,10 +1493,12 @@ Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boo
}
}

boolean canMigrateWithStorage = isStorageMigrationSupported(vm, srcHost);
if (!canMigrateWithStorage && usesLocal) {
throw new InvalidParameterValueException("Unsupported operation, instance uses Local storage, cannot migrate");
}

final String srcHostVersion = getHypervisorVersionOfHost(srcHost);
final Type hostType = srcHost.getType();
Pair<List<HostVO>, Integer> allHostsPair = null;
List<HostVO> allHosts = null;
Expand Down Expand Up @@ -1596,6 +1574,23 @@ Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boo
return new Ternary<>(allHostsPairResult, filteredHosts, requiresStorageMotion);
}

protected boolean isStorageMigrationSupported(final VirtualMachine vm, final Host srcHost) {
final List<HypervisorType> hypervisorTypes = Arrays.asList(HypervisorType.VMware, HypervisorType.KVM);
if (VirtualMachine.Type.User.equals(vm.getType()) || hypervisorTypes.contains(vm.getHypervisorType())) {
final String srcHostVersion = getHypervisorVersionOfHost(srcHost);
return _hypervisorCapabilitiesDao.isStorageMotionSupported(srcHost.getHypervisorType(), srcHostVersion);
}
return false;
}

protected String getHypervisorVersionOfHost(final Host host) {
final String version = host.getHypervisorVersion();
if (version == null && HypervisorType.KVM.equals(host.getHypervisorType())) {
return "";
}
return version;
}

/**
* Apply affinity group constraints and other exclusion rules for VM migration.
* This builds an ExcludeList based on affinity groups, DPDK requirements, and dedicated resources.
Expand Down Expand Up @@ -1692,9 +1687,19 @@ public Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Ho

validateVmForHostMigration(vm);

final long srcHostId = vm.getHostId();
final Host srcHost = _hostDao.findById(srcHostId);
if (srcHost == null) {
logger.debug("Unable to find the host with ID: {} of this Instance: {}", srcHostId, vm);
final InvalidParameterValueException ex = new InvalidParameterValueException("Unable to find the host (with specified ID) of instance with specified ID");
ex.addProxyObject(String.valueOf(srcHostId), "hostId");
ex.addProxyObject(vm.getUuid(), "vmId");
throw ex;
}

// Get technically compatible hosts (storage, hypervisor, UEFI)
Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boolean>> compatibilityResult =
getTechnicallyCompatibleHosts(vm, startIndex, pageSize, keyword);
getTechnicallyCompatibleHosts(vm, srcHost, startIndex, pageSize, keyword);

Pair<List<? extends Host>, Integer> allHostsPair = compatibilityResult.first();
List<? extends Host> filteredHosts = compatibilityResult.second();
Expand All @@ -1707,9 +1712,7 @@ public Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Ho
}

// Create deployment plan and VM profile
final Host srcHost = _hostDao.findById(vm.getHostId());
final DataCenterDeployment plan = new DataCenterDeployment(
srcHost.getDataCenterId(), srcHost.getPodId(), srcHost.getClusterId(), null, null, null);
final DataCenterDeployment plan = createDeploymentPlanForMigrationListing(vm, srcHost);
final VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(
vm, null, _offeringDao.findById(vm.getId(), vm.getServiceOfferingId()), null, null);

Expand All @@ -1723,6 +1726,14 @@ public Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Ho
return new Ternary<>(otherHosts, suitableHosts, requiresStorageMotion);
}

protected DataCenterDeployment createDeploymentPlanForMigrationListing(final VirtualMachine vm, final Host srcHost) {
final boolean canMigrateWithStorage = isStorageMigrationSupported(vm, srcHost);
if (canMigrateWithStorage) {
return new DataCenterDeployment(srcHost.getDataCenterId(), srcHost.getPodId(), null, null, null, null);
}
return new DataCenterDeployment(srcHost.getDataCenterId(), srcHost.getPodId(), srcHost.getClusterId(), null, null, null);
}

/**
* Add non DPDK enabled hosts to the avoid list
*/
Expand Down
61 changes: 46 additions & 15 deletions server/src/test/java/com/cloud/server/ManagementServerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.cloud.dc.DataCenterVO;
import com.cloud.dc.Vlan.VlanType;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.deploy.DataCenterDeployment;
import com.cloud.deploy.DeploymentPlanningManager;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.PermissionDeniedException;
Expand Down Expand Up @@ -227,7 +228,7 @@ public void setup() throws IllegalAccessException, NoSuchFieldException {
apiDBUtilsMock = Mockito.mockStatic(ApiDBUtils.class);
// Return empty list to avoid architecture filtering in most tests
apiDBUtilsMock.when(() -> ApiDBUtils.listZoneClustersArchs(Mockito.anyLong()))
.thenReturn(new ArrayList<>());
.thenReturn(new ArrayList<>());
}

@After
Expand All @@ -246,7 +247,7 @@ private void overrideDefaultConfigValue(final ConfigKey configKey, final String
}

@Test(expected = InvalidParameterValueException.class)
public void testDuplicateRegistraitons(){
public void testDuplicateRegistrations() {
String accountName = "account";
String publicKeyString = "ssh-rsa very public";
String publicKeyMaterial = spy.getPublicKeyFromKeyKeyMaterial(publicKeyString);
Expand Down Expand Up @@ -826,9 +827,13 @@ public void testListHostsForMigrationOfVMLxcUserVM() {
@Test
public void testListHostsForMigrationOfVMGpuEnabled() {
VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM);
long hostId = vm.getHostId();
HostVO srcHost = mockHost(hostId, 4L, 5L, 6L, HypervisorType.KVM);
Account caller = mockRootAdminAccount();

Mockito.doReturn(caller).when(spy).getCaller();
Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm);
Mockito.doReturn(srcHost).when(hostDao).findById(hostId);

// Mock GPU detail
Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString()))
Expand Down Expand Up @@ -888,7 +893,7 @@ public void testListHostsForMigrationOfVMWithSystemVM() {
spy.listHostsForMigrationOfVM(1L, 0L, 20L, null);

// Verify storage motion capability was checked
Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null);
Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.VMware, null);

// Verify result structure and data
Assert.assertNotNull(result);
Expand Down Expand Up @@ -952,7 +957,7 @@ public void testListHostsForMigrationOfVMWithDomainRouter() {
spy.listHostsForMigrationOfVM(1L, 0L, 20L, null);

// Verify hypervisor capabilities were checked
Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.KVM, "");
Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.KVM, "");

// Verify result contains expected hosts
Assert.assertNotNull(result);
Expand Down Expand Up @@ -1097,7 +1102,7 @@ public void testListHostsForMigrationOfVMKVMWithNullHypervisorVersion() {
spy.listHostsForMigrationOfVM(1L, 0L, 20L, null);

// Verify KVM null version was converted to empty string
Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.KVM, "");
Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.KVM, "");

// Verify result data
Assert.assertNotNull(result);
Expand Down Expand Up @@ -1416,7 +1421,7 @@ public void testListHostsForMigrationOfVMStorageMotionCapabilityCheck() {
spy.listHostsForMigrationOfVM(1L, 0L, 20L, null);

// Verify storage motion capability was checked for User VM
Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null);
Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.VMware, null);

// Verify response data
Assert.assertNotNull(result);
Expand Down Expand Up @@ -1481,7 +1486,7 @@ public void testListHostsForMigrationOfVMWithAllSupportedHypervisors() {
spy.listHostsForMigrationOfVM(1L, 0L, 20L, null);

// Verify hypervisor is in supported hypervisors list
Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(hypervisorType, version);
Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(hypervisorType, version);

// Verify validation passed for this hypervisor
Assert.assertNotNull("Result should not be null for " + hypervisorType, result);
Expand All @@ -1508,8 +1513,6 @@ public void testListHostsForMigrationOfVMSourceHostNotFound() {
Account caller = mockRootAdminAccount();
Mockito.doReturn(caller).when(spy).getCaller();
Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm);
Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString()))
.thenReturn(null);
Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(null);

spy.listHostsForMigrationOfVM(1L, 0L, 20L, null);
Expand Down Expand Up @@ -1589,7 +1592,7 @@ public void testListHostsForMigrationOfVMStorageMotionCheckForSystemVM() {
spy.listHostsForMigrationOfVM(1L, 0L, 20L, null);

// Verify that storage motion capability was checked for system VM (VMware is in hypervisorTypes list)
Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null);
Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.VMware, null);

// Verify response structure
Assert.assertNotNull(result);
Expand Down Expand Up @@ -1642,7 +1645,7 @@ public void testListHostsForMigrationOfVMStorageMotionCheckForUserVM() {
spy.listHostsForMigrationOfVM(1L, 0L, 20L, null);

// Verify User VM can migrate with storage (User VM type always checks)
Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.KVM, "");
Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.KVM, "");

// Verify response data
Assert.assertNotNull(result);
Expand Down Expand Up @@ -1695,7 +1698,7 @@ public void testListHostsForMigrationOfVMWithoutStorageMotionClusterScope() {
spy.listHostsForMigrationOfVM(1L, 0L, 20L, null);

// Verify XenServer without storage motion was checked
Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.XenServer, null);
Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.XenServer, null);
// Verify cluster-scoped search was used (not zone-wide)
Mockito.verify(spy).searchForServers(
Mockito.eq(0L), Mockito.eq(20L), Mockito.isNull(), Mockito.any(Type.class),
Expand Down Expand Up @@ -1845,14 +1848,14 @@ public void testListHostsForMigrationOfVMVmwareStorageMotionCheck() {
Mockito.doReturn(caller).when(spy).getCaller();
Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm);
Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString()))
.thenReturn(null);
.thenReturn(null);

HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.VMware);
Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost);

// VMware with DomainRouter should still check storage motion
Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.VMware, null))
.thenReturn(true);
.thenReturn(true);

ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class);
Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering);
Expand Down Expand Up @@ -1880,7 +1883,7 @@ public void testListHostsForMigrationOfVMVmwareStorageMotionCheck() {
spy.listHostsForMigrationOfVM(1L, 0L, 20L, null);

// Verify VMware always checks storage motion (hypervisorTypes list includes VMware)
Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null);
Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.VMware, null);

// Verify response
Assert.assertNotNull(result);
Expand Down Expand Up @@ -2074,4 +2077,32 @@ private DiskOfferingVO mockSharedDiskOffering(Long id) {
Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(false);
return diskOffering;
}

@Test
public void createDeploymentPlanForMigrationListingTestAllocatesInAnyClusterWhenStorageMigrationIsSupported() {
VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM);
HostVO srcHost = mockHost(vm.getHostId(), 1L, 2L, 3L, HypervisorType.KVM);

Mockito.doReturn(true).when(spy).isStorageMigrationSupported(vm, srcHost);

DataCenterDeployment deploymentPlan = spy.createDeploymentPlanForMigrationListing(vm, srcHost);

Assert.assertEquals(3L, deploymentPlan.getDataCenterId());
Assert.assertEquals(2L, (long) deploymentPlan.getPodId());
Assert.assertNull(deploymentPlan.getClusterId());
}

@Test
public void createDeploymentPlanForMigrationListingTestAllocatesInSourceClusterWhenStorageMigrationIsNotSupported() {
VMInstanceVO vm = mockRunningVM(1L, HypervisorType.XenServer);
HostVO srcHost = mockHost(vm.getHostId(), 4L, 5L, 6L, HypervisorType.XenServer);

Mockito.doReturn(false).when(spy).isStorageMigrationSupported(vm, srcHost);

DataCenterDeployment deploymentPlan = spy.createDeploymentPlanForMigrationListing(vm, srcHost);

Assert.assertEquals(6L, deploymentPlan.getDataCenterId());
Assert.assertEquals(5L, (long) deploymentPlan.getPodId());
Assert.assertEquals(4L, (long) deploymentPlan.getClusterId());
}
}
Loading