Damans227 commented on code in PR #14267:
URL: https://github.com/apache/cloudstack/pull/14267#discussion_r4283740298
##########
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##########
@@ -3278,14 +3278,46 @@ public StoragePoolVO findLocalStorageOnHost(long
hostId) {
@Override
public Host findUpAndEnabledHostWithAccessToStoragePools(List<Long>
poolIds) {
List<Long> hostIds =
_storagePoolHostDao.findHostsConnectedToPools(poolIds);
- if (hostIds.isEmpty()) {
+ if (CollectionUtils.isNotEmpty(hostIds)) {
+ Collections.shuffle(hostIds);
+
+ for (Long hostId : hostIds) {
+ Host host = _hostDao.findById(hostId);
+ if (canHostAccessStoragePools(host, poolIds)) {
+ return host;
+ }
+ }
+ }
+
+ return findUpAndEnabledHostAbleToPrepareAccessToStoragePools(poolIds);
+ }
+
+ protected Host
findUpAndEnabledHostAbleToPrepareAccessToStoragePools(List<Long> poolIds) {
+ if (CollectionUtils.isEmpty(poolIds)) {
return null;
}
- Collections.shuffle(hostIds);
- for (Long hostId : hostIds) {
- Host host = _hostDao.findById(hostId);
- if (canHostAccessStoragePools(host, poolIds)) {
+ Long zoneId = null;
+ for (Long poolId : poolIds) {
+ StoragePoolVO pool = _storagePoolDao.findById(poolId);
+ if (pool == null) {
+ return null;
+ }
+ if (zoneId == null) {
+ zoneId = pool.getDataCenterId();
+ } else if (!zoneId.equals(pool.getDataCenterId())) {
+ return null;
+ }
+ }
+
+ List<HostVO> hosts =
_resourceMgr.listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, zoneId);
Review Comment:
can this pick a host from a different cluster when the pool is added at
cluster level? looks like only the access groups are checked here
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -3550,7 +3550,8 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) {
throw new CloudRuntimeException(checkResult.second());
}
- if (!liveMigrateVolume && vm != null) {
+ if (!liveMigrateVolume && vm != null
+ &&
storageMgr.findUpAndEnabledHostWithAccessToStoragePools(Arrays.asList(vol.getPoolId(),
destPool.getId())) == null) {
Review Comment:
should this only run for storage that can connect on demand? for other
storage every offline migration now goes through all hosts in the zone first
--
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]
