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_r205239038
 
 

 ##########
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##########
 @@ -2332,15 +2339,44 @@ protected void migrate(final VMInstanceVO vm, final 
long srcHostId, final Deploy
         return volumeToPoolObjectMap;
     }
 
+    /**
+     *  If we the volume is placed in a managed storage we execute the 
following checks:
+     *  <ul>
+     *      <li> If the volume is not placed in a managed storage, then we do 
not need to proceed with these checks
+     *      <li> Cross cluster migration with cluster-wide storage pool. 
Volumes in managed storage cannot be migrated out of their current pool. 
Therefore, an exception is thrown.
+     *  </ul>
+     */
+    protected void executeManagedStorageChecks(Host targetHost, StoragePoolVO 
currentPool, VolumeVO volume) {
+        if (!currentPool.isManaged()) {
+            return;
+        }
+        if (currentPool.getClusterId() == targetHost.getClusterId()) {
+            return;
+        }
+        if (ScopeType.ZONE.equals(currentPool.getScope())) {
+            return;
+        }
+        throw new CloudRuntimeException(String.format("Currently, a volume on 
managed storage can only be 'migrated' to itself [volumeId=%s, storageId=%s, 
targetHostId=%s].", volume.getUuid(),
+                currentPool.getUuid(), targetHost.getUuid()));
+    }
+
+    /**
+     *  Return true if the VM migration is a cross cluster migration. To 
execute that, we check if the volume current storage pool cluster is different 
from the target host cluster.
+     */
+    protected boolean isStorageCrossClusterMigration(Host targetHost, 
StoragePoolVO currentPool) {
+        return ScopeType.CLUSTER.equals(currentPool.getScope()) && 
currentPool.getClusterId() != targetHost.getClusterId();
+    }
+
     /**
      * We will add a mapping of volume to storage pool if needed. The 
conditions to add a mapping are the following:
      * <ul>
-     *  <li> The current storage pool where the volume is allocated can be 
accessed by the target host
-     *  <li> If not storage pool is found to allocate the volume we throw an 
exception.
+     *  <li> The candidate storage pool where the volume is to be allocated 
can be accessed by the target host
+     *  <li> If no storage pool is found to allocate the volume we throw an 
exception.
      * </ul>
      *
      */
-    private void 
createVolumeToStoragePoolMappingIfNeeded(VirtualMachineProfile profile, Host 
targetHost, Map<Volume, StoragePool> volumeToPoolObjectMap, VolumeVO volume, 
StoragePoolVO currentPool) {
+    protected void 
createVolumeToStoragePoolMappingIfNeeded(VirtualMachineProfile profile, Host 
targetHost, Map<Volume, StoragePool> volumeToPoolObjectMap, VolumeVO volume,
 
 Review comment:
   Perhaps canTargetHostAccessVolumeStoragePool should be called 
canTargetHostAccessVolumeCurrentStoragePool. Not a big deal, but when I think 
about it this way, it makes it more clear to me that we are going though this 
list of candidate storage pools in an attempt to see if one of them is the 
current storage pool.

----------------------------------------------------------------
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

Reply via email to