shwstppr commented on a change in pull request #4534:
URL: https://github.com/apache/cloudstack/pull/4534#discussion_r701628352
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1336,6 +1344,28 @@ private boolean doMaintain(final long hostId) {
return true;
}
+ private boolean isClusterWideMigrationSupported(Host host,
List<VMInstanceVO> vms, List<HostVO> hosts) {
Review comment:
Are you deliberately trying to change `hosts` arg here to have the new
list available to the caller?
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1306,7 +1307,14 @@ private boolean doMaintain(final long hostId) {
return true;
}
- final List<HostVO> hosts =
listAllUpAndEnabledHosts(Host.Type.Routing, host.getClusterId(),
host.getPodId(), host.getDataCenterId());
+ List<HostVO> hosts = listAllUpAndEnabledHosts(Host.Type.Routing,
host.getClusterId(), host.getPodId(), host.getDataCenterId());
+ if (hosts == null || hosts.isEmpty()) {
+ s_logger.warn("Unable to find a host for vm migration in
cluster: " + host.getClusterId());
+ if (! isClusterWideMigrationSupported(host, vms, hosts)) {
+ return false;
+ }
+ }
+
for (final VMInstanceVO vm : vms) {
if (hosts == null || hosts.isEmpty() || !answer.getMigrate()
Review comment:
@weizhouapache do we need to re-list `hosts` here for getting hosts from
other clusters else VM will be stopped instead of migration.
I've tested on KVM with one host each in two different clusters and the VM
just stops and doesn't migrate. Global setting was set to true.
I was able to get it working with some changes though
```
diff --git
a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
index 6a4509cefb..ef3585650a 100755
--- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
+++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
@@ -33,18 +33,6 @@ import java.util.Random;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
-import com.cloud.deploy.DataCenterDeployment;
-import com.cloud.deploy.DeployDestination;
-import com.cloud.deploy.DeploymentPlanner;
-import com.cloud.deploy.DeploymentPlanningManager;
-import com.cloud.exception.InsufficientServerCapacityException;
-import com.cloud.exception.ResourceUnavailableException;
-import com.cloud.service.ServiceOfferingVO;
-import com.cloud.service.dao.ServiceOfferingDao;
-import com.cloud.storage.dao.DiskOfferingDao;
-import com.cloud.vm.UserVmManager;
-import com.cloud.vm.VirtualMachineProfile;
-import com.cloud.vm.VirtualMachineProfileImpl;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.command.admin.cluster.AddClusterCmd;
import org.apache.cloudstack.api.command.admin.cluster.DeleteClusterCmd;
@@ -53,12 +41,11 @@ import
org.apache.cloudstack.api.command.admin.host.AddHostCmd;
import org.apache.cloudstack.api.command.admin.host.AddSecondaryStorageCmd;
import org.apache.cloudstack.api.command.admin.host.CancelHostAsDegradedCmd;
import org.apache.cloudstack.api.command.admin.host.CancelMaintenanceCmd;
-import
org.apache.cloudstack.api.command.admin.host.PrepareForMaintenanceCmd;
import
org.apache.cloudstack.api.command.admin.host.DeclareHostAsDegradedCmd;
+import
org.apache.cloudstack.api.command.admin.host.PrepareForMaintenanceCmd;
import org.apache.cloudstack.api.command.admin.host.ReconnectHostCmd;
import org.apache.cloudstack.api.command.admin.host.UpdateHostCmd;
import org.apache.cloudstack.api.command.admin.host.UpdateHostPasswordCmd;
-
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
@@ -113,6 +100,10 @@ import com.cloud.dc.dao.DataCenterDao;
import com.cloud.dc.dao.DataCenterIpAddressDao;
import com.cloud.dc.dao.DedicatedResourceDao;
import com.cloud.dc.dao.HostPodDao;
+import com.cloud.deploy.DataCenterDeployment;
+import com.cloud.deploy.DeployDestination;
+import com.cloud.deploy.DeploymentPlanner;
+import com.cloud.deploy.DeploymentPlanningManager;
import com.cloud.deploy.PlannerHostReservationVO;
import com.cloud.deploy.dao.PlannerHostReservationDao;
import com.cloud.event.ActionEvent;
@@ -121,9 +112,11 @@ import com.cloud.event.EventTypes;
import com.cloud.event.EventVO;
import com.cloud.exception.AgentUnavailableException;
import com.cloud.exception.DiscoveryException;
+import com.cloud.exception.InsufficientServerCapacityException;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.PermissionDeniedException;
import com.cloud.exception.ResourceInUseException;
+import com.cloud.exception.ResourceUnavailableException;
import com.cloud.gpu.GPU;
import com.cloud.gpu.HostGpuGroupsVO;
import com.cloud.gpu.VGPUTypesVO;
@@ -150,6 +143,8 @@ import com.cloud.org.Cluster;
import com.cloud.org.Grouping;
import com.cloud.org.Managed;
import com.cloud.serializer.GsonHelper;
+import com.cloud.service.ServiceOfferingVO;
+import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.service.dao.ServiceOfferingDetailsDao;
import com.cloud.storage.GuestOSCategoryVO;
import com.cloud.storage.StorageManager;
@@ -158,6 +153,7 @@ import com.cloud.storage.StoragePoolHostVO;
import com.cloud.storage.StoragePoolStatus;
import com.cloud.storage.StorageService;
import com.cloud.storage.VMTemplateVO;
+import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.storage.dao.GuestOSCategoryDao;
import com.cloud.storage.dao.StoragePoolHostDao;
import com.cloud.storage.dao.VMTemplateDao;
@@ -189,10 +185,13 @@ import com.cloud.utils.net.Ip;
import com.cloud.utils.net.NetUtils;
import com.cloud.utils.ssh.SSHCmdHelper;
import com.cloud.utils.ssh.SshException;
+import com.cloud.vm.UserVmManager;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.VirtualMachineManager;
+import com.cloud.vm.VirtualMachineProfile;
+import com.cloud.vm.VirtualMachineProfileImpl;
import com.cloud.vm.VmDetailConstants;
import com.cloud.vm.dao.UserVmDetailsDao;
import com.cloud.vm.dao.VMInstanceDao;
@@ -1307,12 +1306,18 @@ public class ResourceManagerImpl extends ManagerBase
implements ResourceManager,
return true;
}
- List<HostVO> hosts =
listAllUpAndEnabledHosts(Host.Type.Routing, host.getClusterId(),
host.getPodId(), host.getDataCenterId());
- if (hosts == null || hosts.isEmpty()) {
- s_logger.warn("Unable to find a host for vm migration in
cluster: " + host.getClusterId());
- if (! isClusterWideMigrationSupported(host, vms, hosts)) {
- return false;
- }
+
+ List<HostVO> hosts;
+ Long clusterId = null;
+ if (isClusterWideMigrationPossible(host.getDataCenterId(),
vms)) {
+ s_logger.info(String.format("Looking for hosts for VM
migration across different clusters in pod ID: %d", host.getPodId()));
+ } else {
+ clusterId = host.getClusterId();
+ s_logger.info(String.format("Looking for hosts for VM
migration in cluster ID: %d", clusterId));
+ }
+ hosts = listAllUpAndEnabledHosts(Host.Type.Routing, clusterId,
host.getPodId(), host.getDataCenterId());
+ if (CollectionUtils.isEmpty(hosts)) {
+ s_logger.warn(String.format("Unable to find a host for VM
migration away from host: %s", host));
}
for (final VMInstanceVO vm : vms) {
@@ -1344,23 +1349,17 @@ public class ResourceManagerImpl extends ManagerBase
implements ResourceManager,
return true;
}
- private boolean isClusterWideMigrationSupported(Host host,
List<VMInstanceVO> vms, List<HostVO> hosts) {
- if (MIGRATE_VM_ACROSS_CLUSTERS.valueIn(host.getDataCenterId())) {
- s_logger.info("Looking for hosts across different clusters in
zone: " + host.getDataCenterId());
- hosts = listAllUpAndEnabledHosts(Host.Type.Routing, null, null,
host.getDataCenterId());
- if (hosts == null || hosts.isEmpty()) {
- s_logger.warn("Unable to find a host for vm migration in
zone: " + host.getDataCenterId());
- return false;
- }
- // Dont migrate vm if it has volumes on cluster-wide pool
+ private boolean isClusterWideMigrationPossible(long zoneId,
List<VMInstanceVO> vms) {
+ if (MIGRATE_VM_ACROSS_CLUSTERS.valueIn(zoneId)) {
+ // Don't migrate vm if it has volumes on cluster-wide pool
for (final VMInstanceVO vm : vms) {
if (_vmMgr.checkIfVmHasClusterWideVolumes(vm.getId())) {
- s_logger.warn("Unable to migrate vm " +
vm.getInstanceName() + " as it has volumes on cluster-wide pool");
+ s_logger.warn(String.format("VM %s cannot be migrated
across cluster as it has volumes on cluster-wide pool", vm));
return false;
}
}
} else {
- s_logger.warn("Not migrating VM across cluster since " +
MIGRATE_VM_ACROSS_CLUSTERS.key() + " is false");
+ s_logger.warn(String.format("VMs cannot be migrated across
cluster since %s is false for zone ID: %d", MIGRATE_VM_ACROSS_CLUSTERS.key(),
zoneId));
return false;
}
return true;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
