You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/10/14 10:16:32 UTC
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4375: Fixing count for findHostsForMigration
rhtyd commented on a change in pull request #4375:
URL: https://github.com/apache/cloudstack/pull/4375#discussion_r504563714
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -1278,33 +1277,24 @@ private HypervisorType getHypervisorType(VMInstanceVO vm, StoragePool srcVolumeP
final Map<Host, Boolean> requiresStorageMotion = new HashMap<Host, Boolean>();
DataCenterDeployment plan = null;
if (canMigrateWithStorage) {
- allHostsPair = searchForServers(startIndex, pageSize, null, hostType, null, srcHost.getDataCenterId(), null, null, null, keyword, null, null, srcHost.getHypervisorType(),
- srcHost.getHypervisorVersion());
+ allHostsPair = searchForServers(startIndex, pageSize, null, hostType, null, srcHost.getDataCenterId(), null, null, null, keyword,
+ null, null, srcHost.getHypervisorType(), srcHost.getHypervisorVersion(), srcHost.getId());
allHosts = allHostsPair.first();
- allHosts.remove(srcHost);
for (final VolumeVO volume : volumes) {
StoragePool storagePool = _poolDao.findById(volume.getPoolId());
Long volClusterId = storagePool.getClusterId();
- for (Iterator<HostVO> iterator = allHosts.iterator(); iterator.hasNext();) {
- final Host host = iterator.next();
-
+ for (HostVO host : allHosts) {
if (volClusterId != null) {
if (storagePool.isLocal() || !host.getClusterId().equals(volClusterId) || usesLocal) {
- if (storagePool.isManaged()) {
- // At the time being, we do not support storage migration of a volume from managed storage unless the managed storage
- // is at the zone level and the source and target storage pool is the same.
- // If the source and target storage pool is the same and it is managed, then we still have to perform a storage migration
- // because we need to create a new target volume and copy the contents of the source volume into it before deleting the
- // source volume.
- iterator.remove();
- } else {
- if (hasSuitablePoolsForVolume(volume, host, vmProfile)) {
- requiresStorageMotion.put(host, true);
- } else {
- iterator.remove();
- }
+ // At the time being, we do not support storage migration of a volume from managed storage unless the managed storage
Review comment:
My only concern here is that now that we're not removing hosts, what is those hosts which are not removed now as shown as suitable, would that cause any regression?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org