[SolidFire] Fix inter-cluster VM migrations on zone-wide SolidFire pools #11633

Copilot · 2026-01-26T11:35:38Z

Throwing a RuntimeException wrapping AgentUnavailableException or OperationTimedoutException may cause the entire migration to fail unexpectedly. Consider logging the error and handling it gracefully instead of propagating it as a RuntimeException, since this is a cleanup operation that happens after the VM has successfully migrated. The migration has already succeeded at this point (line 2887), so failing here could leave the system in an inconsistent state.

Suggested change

throw new RuntimeException(e);

s_logger.warn("Failed to unmount datastores on origin host " + srcHostId +

" after migration of VM id=" + vmId + ". Datastores to unmount: " + poolsToReleaseOnOrigin, e);

Copilot · 2026-01-26T11:35:37Z

The boolean logic in this condition is incorrect. The expression !details.containsKey(DiskTO.MANAGED) && !details.containsKey(DiskTO.INTER_CLUSTER_MIGRATION) should be (!details.containsKey(DiskTO.MANAGED) || !details.containsKey(DiskTO.INTER_CLUSTER_MIGRATION)) with OR instead of AND.

The current logic means the method returns only when BOTH MANAGED and INTER_CLUSTER_MIGRATION are missing, but the subsequent checks on lines 4666-4668 explicitly check for the presence of these keys. This means if either key is present (but not both), the method won't return early but will later evaluate false in the checks below, making the early return ineffective.

The intended logic should be: return early if SCOPE is missing OR if either MANAGED or INTER_CLUSTER_MIGRATION is missing.

Suggested change

!details.containsKey(DiskTO.MANAGED) && !details.containsKey(DiskTO.INTER_CLUSTER_MIGRATION)) {

(!details.containsKey(DiskTO.MANAGED) || !details.containsKey(DiskTO.INTER_CLUSTER_MIGRATION))) {

Copilot · 2026-01-26T11:35:38Z

There's a potential NumberFormatException if details.get(DiskTO.STORAGE_PORT) returns null or a non-numeric value. While the comment on line 4672 suggests details should not be null for managed storage, there's no explicit validation that STORAGE_PORT is present or valid before parsing. Consider adding a null check and validation before calling Integer.parseInt().

Suggested change

_storageProcessor.prepareManagedDatastore(context, hyperHost, datastoreName,

details.get(DiskTO.IQN), details.get(DiskTO.STORAGE_HOST),

Integer.parseInt(details.get(DiskTO.STORAGE_PORT)));

String storagePortString = details.get(DiskTO.STORAGE_PORT);

if (StringUtils.isBlank(storagePortString) || !NumberUtils.isDigits(storagePortString)) {

s_logger.warn(String.format("Skipping preparation of managed datastore %s due to invalid storage port value: %s",

datastoreName, storagePortString));

return;

}

int storagePort = Integer.parseInt(storagePortString);

_storageProcessor.prepareManagedDatastore(context, hyperHost, datastoreName,

details.get(DiskTO.IQN), details.get(DiskTO.STORAGE_HOST), storagePort);

-Original file line number
+Diff line change
@@ Expand Up / @@ -29,6 +29,8 @@ public class DiskTO { @@
         public static final String CHAP_TARGET_SECRET = "chapTargetSecret";
         public static final String SCSI_NAA_DEVICE_ID = "scsiNaaDeviceId";
         public static final String MANAGED = "managed";
+        public static final String SCOPE = "scope";
+        public static final String INTER_CLUSTER_MIGRATION = "interClusterMigration";
         public static final String IQN = "iqn";
         public static final String STORAGE_HOST = "storageHost";
         public static final String STORAGE_PORT = "storagePort";
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -0,0 +1,42 @@
+    //
+    // Licensed to the Apache Software Foundation (ASF) under one
+    // or more contributor license agreements.  See the NOTICE file
+    // distributed with this work for additional information
+    // regarding copyright ownership.  The ASF licenses this file
+    // to you under the Apache License, Version 2.0 (the
+    // "License"); you may not use this file except in compliance
+    // with the License.  You may obtain a copy of the License at
+    //
+    //   http://www.apache.org/licenses/LICENSE-2.0
+    //
+    // Unless required by applicable law or agreed to in writing,
+    // software distributed under the License is distributed on an
+    // "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    // KIND, either express or implied.  See the License for the
+    // specific language governing permissions and limitations
+    // under the License.
+    //
+    package com.cloud.agent.api;
+    import java.util.List;
+    public class UnmountDatastoresCommand extends Command {
+        private List<String> datastorePools;
+        public UnmountDatastoresCommand() {
+        }
+        public UnmountDatastoresCommand(List<String> datastorePools) {
+            this.datastorePools = datastorePools;
+        }
+        public List<String> getDatastorePools() {
+            return datastorePools;
+        }
+        @Override
+        public boolean executeInSequence() {
+            return false;
+        }
+    }

-Original file line number
+Diff line change
@@ Expand Up @@
         boolean storageMigration(VirtualMachineProfile vm, Map<Volume, StoragePool> volumeToPool) throws StorageUnavailableException;
-        void prepareForMigration(VirtualMachineProfile vm, DeployDestination dest);
+        void prepareForMigration(VirtualMachineProfile vm, DeployDestination dest, Long srcHostId);
         void prepare(VirtualMachineProfile vm, DeployDestination dest) throws StorageUnavailableException, InsufficientStorageCapacityException, ConcurrentOperationException, StorageAccessException;
@@ Expand Down Expand Up @@
          * Unmanage VM volumes
          */
         void unmanageVolumes(long vmId);
+        List<String> postMigrationReleaseDatastoresOnOriginHost(VirtualMachineProfile profile, long vmId);
     }

-Original file line number
+Diff line change
@@ Expand Up / @@ -183,4 +183,15 @@ default boolean volumesRequireGrantAccessWhenUsed() { @@
         default boolean zoneWideVolumesAvailableWithoutClusterMotion() {
             return false;
         }
+        /**
+         * Disabled by default. Set to true if the data store driver needs to unmount/revoke volumes datastore on the
+         * origin host in case of:
+         * - zone wide storage
+         * - inter-cluster VM migration (without storage motion)
+         * - the hypervisor has restrictions on the number of mounted datastores per host/cluster
+         */
+        default boolean zoneWideVolumesDatastoreCleanupOnOriginHostAfterInterClusterMigration() {
+            return false;
+        }
     }

-Original file line number
+Diff line change
@@ Expand Up / @@ -84,6 +84,7 @@ @@
     import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
     import org.apache.commons.collections.CollectionUtils;
     import org.apache.commons.collections.MapUtils;
+    import org.apache.commons.lang.BooleanUtils;
     import org.apache.commons.lang3.ObjectUtils;
     import org.apache.commons.lang3.StringUtils;
     import org.apache.log4j.Logger;
@@ Expand Down Expand Up @@
         }
         @Override
-        public void prepareForMigration(VirtualMachineProfile vm, DeployDestination dest) {
+        public void prepareForMigration(VirtualMachineProfile vm, DeployDestination dest, Long srcHostId) {
             List<VolumeVO> vols = _volsDao.findUsableVolumesForInstance(vm.getId());
             if (s_logger.isDebugEnabled()) {
                 s_logger.debug(String.format("Preparing to migrate [%s] volumes for VM [%s].", vols.size(), vm.getVirtualMachine()));
@@ Expand All @@
                 // make sure this is done AFTER grantAccess, as grantAccess may change the volume's state
                 DataTO volTO = volumeInfo.getTO();
                 DiskTO disk = storageMgr.getDiskWithThrottling(volTO, vol.getVolumeType(), vol.getDeviceId(), vol.getPath(), vm.getServiceOfferingId(), vol.getDiskOfferingId());
-                disk.setDetails(getDetails(volumeInfo, dataStore));
+                disk.setDetails(getDetails(volumeInfo, dataStore, vm, srcHostId));
                 vm.addDisk(disk);
             }
@@ Expand All @@
             }
         }
-        private Map<String, String> getDetails(VolumeInfo volumeInfo, DataStore dataStore) {
+        private Map<String, String> getDetails(VolumeInfo volumeInfo, DataStore dataStore, VirtualMachineProfile vmProfile, Long srcHostId) {
             Map<String, String> details = new HashMap<String, String>();
             StoragePoolVO storagePool = _storagePoolDao.findById(dataStore.getId());
@@ Expand Down Expand Up @@
                 details.put(DiskTO.CHAP_TARGET_SECRET, chapInfo.getTargetSecret());
             }
+            // Zone wide storage Solidfire inter-cluster VM migrations needs the destination host mount the LUN before migrating
+            if (storagePool.isManaged() && ScopeType.ZONE.equals(storagePool.getScope()) &&
+                    isCleanupNeededOnOriginHostAfterInterClusterMigration(dataStore)) {
+                details.put(DiskTO.SCOPE, storagePool.getScope().name());
+                if (vmProfile.getHostId() != null && srcHostId != null) {
+                    HostVO host = _hostDao.findById(vmProfile.getHostId());
+                    HostVO lastHost = _hostDao.findById(srcHostId);
+                    boolean interClusterMigration = isVmwareInterClusterMigration(lastHost, host);
+                    details.put(DiskTO.INTER_CLUSTER_MIGRATION, BooleanUtils.toStringTrueFalse(interClusterMigration));
+                }
+            }
             return details;
         }
+        private boolean isCleanupNeededOnOriginHostAfterInterClusterMigration(DataStore dataStore) {
+            if (dataStore == null || dataStore.getDriver() == null) {
+                return false;
+            }
+            DataStoreDriver driver = dataStore.getDriver();
+            if (driver instanceof PrimaryDataStoreDriver) {
+                return ((PrimaryDataStoreDriver)driver).zoneWideVolumesDatastoreCleanupOnOriginHostAfterInterClusterMigration();
+            }
+            return false;
+        }
         private void setIoDriverPolicy(Map<String, String> details, StoragePoolVO storagePool, VolumeVO volume) {
             if (volume.getInstanceId() != null) {
                 UserVmDetailVO ioDriverPolicy = userVmDetailsDao.findDetail(volume.getInstanceId(),
@@ Expand Down Expand Up @@
                 DiskTO disk = storageMgr.getDiskWithThrottling(volTO, vol.getVolumeType(), vol.getDeviceId(), vol.getPath(), vm.getServiceOfferingId(), vol.getDiskOfferingId());
                 DataStore dataStore = dataStoreMgr.getDataStore(vol.getPoolId(), DataStoreRole.Primary);
-                disk.setDetails(getDetails(volumeInfo, dataStore));
+                disk.setDetails(getDetails(volumeInfo, dataStore, vm, null));
                 vm.addDisk(disk);
@@ Expand Down Expand Up @@
                 }
             });
         }
+        protected boolean isVmwareInterClusterMigration(Host lastHost, Host host) {
+            if (ObjectUtils.anyNull(lastHost, host)) {
+                return false;
+            }
+            if (host.getHypervisorType() != HypervisorType.VMware) {
+                return false;
+            }
+            Long lastHostClusterId = lastHost.getClusterId();
+            Long clusterId = host.getClusterId();
+            if (ObjectUtils.anyNull(lastHostClusterId, clusterId)) {
+                return false;
+            }
+            return lastHostClusterId.compareTo(clusterId) != 0;
+        }
+        @Override
+        public List<String> postMigrationReleaseDatastoresOnOriginHost(VirtualMachineProfile profile, long vmId) {
+            List<String> pools = new ArrayList<>();
+            if (profile.getVirtualMachine() == null) {
+                s_logger.debug("Cannot release from source as the virtual machine profile is not set");
+                return pools;
+            }
+            HostVO lastHost = _hostDao.findById(profile.getVirtualMachine().getLastHostId());
+            HostVO host = _hostDao.findById(profile.getHostId());
+            // Consider only Vmware inter-cluster migration
+            if (!isVmwareInterClusterMigration(lastHost, host)) {
+                return pools;
+            }
+            List<VolumeVO> volumesForVm = _volsDao.findUsableVolumesForInstance(vmId);
+            for (VolumeVO volumeForVm : volumesForVm) {
+                if (volumeForVm.getPoolId() != null) {
+                    DataStore dataStore = dataStoreMgr.getDataStore(volumeForVm.getPoolId(), DataStoreRole.Primary);
+                    PrimaryDataStore primaryDataStore = (PrimaryDataStore) dataStore;
+                    if (primaryDataStore.getScope() != null && primaryDataStore.getScope().getScopeType() == ScopeType.ZONE) {
+                        // Consider zone-wide storage
+                        PrimaryDataStoreDriver driver = (PrimaryDataStoreDriver) primaryDataStore.getDriver();
+                        if (driver.zoneWideVolumesDatastoreCleanupOnOriginHostAfterInterClusterMigration()) {
+                            // Currently enabled for SolidFire only on inter-cluster migrations on zone-wide pools.
+                            pools.add(volumeForVm.get_iScsiName());
+                        }
+                    }
+                }
+            }
+            return pools;
+        }
     }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SolidFire] Fix inter-cluster VM migrations on zone-wide SolidFire pools #11633

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Copilot AI Jan 26, 2026

Uh oh!

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up @@
             }
         }
-        private void unmountVmfsDatastore(VmwareContext context, VmwareHypervisorHost hyperHost, String datastoreName,
+        public void unmountVmfsDatastore(VmwareContext context, VmwareHypervisorHost hyperHost, String datastoreName,
                                           List<Pair<ManagedObjectReference, String>> hosts) throws Exception {
             for (Pair<ManagedObjectReference, String> host : hosts) {
                 HostMO hostMO = new HostMO(context, host.first());
@@ Expand Down @@

[SolidFire] Fix inter-cluster VM migrations on zone-wide SolidFire pools #11633

Uh oh!

[SolidFire] Fix inter-cluster VM migrations on zone-wide SolidFire pools #11633

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!