mike-tutkowski commented on a change in pull request #2761: Add managed storage
pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205340486
##########
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -2292,73 +2292,143 @@ protected void migrate(final VMInstanceVO vm, final
long srcHostId, final Deploy
/**
* We create the mapping of volumes and storage pool to migrate the VMs
according to the information sent by the user.
+ * If the user did not enter a complete mapping, the volumes that were
left behind will be auto mapped using {@link
#createStoragePoolMappingsForVolumes(VirtualMachineProfile, Host, Map, List)}
*/
- private Map<Volume, StoragePool>
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile,
Host host, Map<Long, Long> volumeToPool) {
+ protected Map<Volume, StoragePool>
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile,
Host targetHost, Map<Long, Long> volumeToPool) {
Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<Volume,
StoragePool>();
+ buildMapUsingUserInformation(profile, targetHost, volumeToPool,
volumeToPoolObjectMap);
+
+ List<Volume> volumesNotMapped =
findVolumesThatWereNotMappedByTheUser(profile, volumeToPoolObjectMap);
+ createStoragePoolMappingsForVolumes(profile, targetHost,
volumeToPoolObjectMap, volumesNotMapped);
+ return volumeToPoolObjectMap;
+ }
+
+ /**
+ * Given the map of volume to target storage pool entered by the user, we
check for other volumes that the VM might have and were not configured.
+ * This map can be then used by CloudStack to find new target storage
pools according to the target host.
+ */
+ private List<Volume>
findVolumesThatWereNotMappedByTheUser(VirtualMachineProfile profile,
Map<Volume, StoragePool> volumeToPoolObjectMap) {
+ List<VolumeVO> allVolumes =
_volsDao.findUsableVolumesForInstance(profile.getId());
+ List<Volume> volumesNotMapped = new ArrayList<>();
+ for (Volume volume : volumeToPoolObjectMap.keySet()) {
+ if (!allVolumes.contains(volume)) {
+ volumesNotMapped.add(volume);
+ }
+ }
+ return volumesNotMapped;
+ }
+
+ /**
+ * Builds the map of storage pools and volumes with the information
entered by the user. Before creating the an entry we validate if the migration
is feasible checking if the migration is allowed and if the target host can
access the defined target storage pool.
+ */
+ private void buildMapUsingUserInformation(VirtualMachineProfile profile,
Host targetHost, Map<Long, Long> volumeToPool, Map<Volume, StoragePool>
volumeToPoolObjectMap) {
for(Long volumeId: volumeToPool.keySet()) {
VolumeVO volume = _volsDao.findById(volumeId);
Long poolId = volumeToPool.get(volumeId);
StoragePoolVO targetPool = _storagePoolDao.findById(poolId);
StoragePoolVO currentPool =
_storagePoolDao.findById(volume.getPoolId());
- if (_poolHostDao.findByPoolHost(targetPool.getId(), host.getId())
== null) {
- throw new CloudRuntimeException(String.format("Cannot migrate
the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host
[%s]. The host does not have access to the storage pool entered.",
volume.getUuid(), targetPool.getUuid(), profile.getUuid(), host.getUuid()));
+ executeManagedStorageChecks(targetHost, currentPool, volume);
+ if (_poolHostDao.findByPoolHost(targetPool.getId(),
targetHost.getId()) == null) {
+ throw new CloudRuntimeException(
+ String.format("Cannot migrate the volume [%s] to the
storage pool [%s] while migrating VM [%s] to target host [%s]. The host does
not have access to the storage pool entered.",
+ volume.getUuid(), targetPool.getUuid(),
profile.getUuid(), targetHost.getUuid()));
}
if (currentPool.getId() == targetPool.getId()) {
s_logger.info(String.format("The volume [%s] is already
allocated in storage pool [%s].", volume.getUuid(), targetPool.getUuid()));
}
volumeToPoolObjectMap.put(volume, targetPool);
}
- return volumeToPoolObjectMap;
}
/**
- * We create the default mapping of volumes and storage pools for the
migration of the VM to the target host.
- * If the current storage pool of one of the volumes is using local
storage in the host, it then needs to be migrated to a local storage in the
target host.
- * Otherwise, we do not need to migrate, and the volume can be kept in its
current storage pool.
+ * We create the default mapping of volumes and storage pools for the
migration of the VM to the target host. We execute the following checks an
operations according to some scenarios:
+ * <ul>
+ * <li>If the current storage pool of one of the volumes is using local
storage in the host, it then needs to be migrated to a local storage in the
target host. Otherwise, we do not need to migrate, and the volume can be kept
in its current storage pool.
+ * <li>If the current storage pool cluster is different from the target
host cluster, we need to migrate the volume to one storage pool at the target
cluster where the target host resides on.
+ * </ul>
*/
- private Map<Volume, StoragePool>
getDefaultMappingOfVolumesAndStoragePoolForMigration(VirtualMachineProfile
profile, Host targetHost) {
+ protected Map<Volume, StoragePool>
getDefaultMappingOfVolumesAndStoragePoolForMigration(VirtualMachineProfile
profile, Host targetHost) {
Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<Volume,
StoragePool>();
List<VolumeVO> allVolumes =
_volsDao.findUsableVolumesForInstance(profile.getId());
- for (VolumeVO volume : allVolumes) {
+ createStoragePoolMappingsForVolumes(profile, targetHost,
volumeToPoolObjectMap, new ArrayList<>(allVolumes));
+ return volumeToPoolObjectMap;
+ }
+
+ /**
+ * For each one of the volumes we will map it to a storage pool that is
available via the target host.
+ * An exception is thrown if we cannot find a storage pool that is
accessible in the target host to migrate the volume to.
+ */
+ private void createStoragePoolMappingsForVolumes(VirtualMachineProfile
profile, Host targetHost, Map<Volume, StoragePool> volumeToPoolObjectMap,
List<Volume> allVolumes) {
+ for (Volume volume : allVolumes) {
StoragePoolVO currentPool =
_storagePoolDao.findById(volume.getPoolId());
- if (ScopeType.HOST.equals(currentPool.getScope())) {
- createVolumeToStoragePoolMappingIfNeeded(profile, targetHost,
volumeToPoolObjectMap, volume, currentPool);
+
+ executeManagedStorageChecks(targetHost, currentPool, volume);
+ if (ScopeType.HOST.equals(currentPool.getScope()) ||
isStorageCrossClusterMigration(targetHost, currentPool)) {
+ createVolumeToStoragePoolMappingIfPossible(profile,
targetHost, volumeToPoolObjectMap, volume, currentPool);
} else {
volumeToPoolObjectMap.put(volume, currentPool);
Review comment:
Per a previous comment of mine, we should decide if this line should be
there. The old code didn't put the volume in the map if its current pool wasn't
going to change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
