[CLOUDSTACK-10323] Allow changing disk offering during volume migration

This is a continuation of work developed on PR #2425 (CLOUDSTACK-10240), which provided root admins an override mechanism to move volumes between storage systems types (local/shared) even when the disk offering would not allow such operation. To complete the work, we will now provide a way for administrators to enter a new disk offering that can reflect the new placement of the volume. We will add an extra parameter to allow the root admin inform a new disk offering for the volume. Therefore, when the volume is being migrated, it will be possible to replace the disk offering to reflect the new placement of the volume.

The API method will have the following parameters:

* storageid (required)
* volumeid (required)
* livemigrate(optional)
* newdiskofferingid (optional) – this is the new parameter

The expected behavior is the following:

* If “newdiskofferingid” is not provided the current behavior is maintained. Override mechanism will also keep working as we have seen so far.
* If the “newdiskofferingid” is provided by the admin, we will execute the following checks
** new disk offering mode (local/shared) must match the target storage mode. If it does not match, an exception will be thrown and the operator will receive a message indicating the problem.
** we will check if the new disk offering tags match the target storage tags. If it does not match, an exception will be thrown and the operator will receive a message indicating the problem.
** check if the target storage has the capacity for the new volume. If it does not have enough space, then an exception is thrown and the operator will receive a message indicating the problem.
** check if the size of the volume is the same as the size of the new disk offering. If it is not the same, we will ALLOW the change of the service offering, and a warning message will be logged.

We execute the change of the Disk offering as soon as the migration of the volume finishes. Therefore, if an error happens during the migration and the volume remains in the original storage system, the disk offering will keep reflecting this situation.

Author: rafaelweingartner

api/src/main/java/org/apache/cloudstack/api/ApiConstants.java

@@ -86,6 +86,7 @@ public class ApiConstants {
     public static final String DEVICE_ID = "deviceid";
     public static final String DIRECT_DOWNLOAD = "directdownload";
     public static final String DISK_OFFERING_ID = "diskofferingid";
+    public static final String NEW_DISK_OFFERING_ID = "newdiskofferingid";
     public static final String DISK_SIZE = "disksize";
     public static final String UTILIZATION = "utilization";
     public static final String DRIVER = "driver";

api/src/main/java/org/apache/cloudstack/api/command/admin/volume/MigrateVolumeCmdByAdmin.java

@@ -33,9 +33,7 @@ public class MigrateVolumeCmdByAdmin extends MigrateVolumeCmd {
 
     @Override
     public void execute(){
-        Volume result;
-
-        result = _volumeService.migrateVolume(this);
+        Volume result = _volumeService.migrateVolume(this);
         if (result != null) {
             VolumeResponse response = _responseGenerator.createVolumeResponse(ResponseView.Full, result);
             response.setResponseName(getCommandName());

api/src/main/java/org/apache/cloudstack/api/command/user/volume/MigrateVolumeCmd.java

@@ -55,6 +55,9 @@ public class MigrateVolumeCmd extends BaseAsyncCmd {
                description = "if the volume should be live migrated when it is attached to a running vm")
     private Boolean liveMigrate;
 
+    @Parameter(name = ApiConstants.NEW_DISK_OFFERING_ID, type = CommandType.STRING, description = "The new disk offering ID that replaces the current one used by the volume. This new disk offering is used to better reflect the new storage where the volume is going to be migrated to.")
+    private String newDiskOfferingUuid;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -105,6 +108,10 @@ public String getEventDescription() {
         return "Attempting to migrate volume Id: " + getVolumeId() + " to storage pool Id: " + getStoragePoolId();
     }
 
+    public String getNewDiskOfferingUuid() {
+        return newDiskOfferingUuid;
+    }
+
     @Override
     public void execute() {
         Volume result;

engine/components-api/src/main/java/com/cloud/vm/VmWorkMigrateVolume.java

@@ -22,12 +22,14 @@ public class VmWorkMigrateVolume extends VmWork {
     private long volumeId;
     private long destPoolId;
     private boolean liveMigrate;
+    private Long newDiskOfferingId;
 
-    public VmWorkMigrateVolume(long userId, long accountId, long vmId, String handlerName, long volumeId, long destPoolId, boolean liveMigrate) {
+    public VmWorkMigrateVolume(long userId, long accountId, long vmId, String handlerName, long volumeId, long destPoolId, boolean liveMigrate, Long newDiskOfferingId) {
         super(userId, accountId, vmId, handlerName);
         this.volumeId = volumeId;
         this.destPoolId = destPoolId;
         this.liveMigrate = liveMigrate;
+        this.newDiskOfferingId = newDiskOfferingId;
     }
 
     public long getVolumeId() {
@@ -41,4 +43,8 @@ public long getDestPoolId() {
     public boolean isLiveMigrate() {
         return liveMigrate;
     }
+
+    public Long getNewDiskOfferingId() {
+        return newDiskOfferingId;
+    }
 }

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

@@ -986,7 +986,9 @@ public void revokeAccess(long vmId, long hostId) {
     @DB
     public Volume migrateVolume(Volume volume, StoragePool destPool) throws StorageUnavailableException {
         VolumeInfo vol = volFactory.getVolume(volume.getId());
-        AsyncCallFuture<VolumeApiResult> future = volService.copyVolume(vol, (DataStore)destPool);
+
+        DataStore dataStoreTarget = dataStoreMgr.getDataStore(destPool.getId(), DataStoreRole.Primary);
+        AsyncCallFuture<VolumeApiResult> future = volService.copyVolume(vol, dataStoreTarget);
         try {
             VolumeApiResult result = future.get();
             if (result.isFailed()) {

engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java

@@ -514,4 +514,8 @@ public void setHypervisorSnapshotReserve(Integer hypervisorSnapshotReserve) {
     public Integer getHypervisorSnapshotReserve() {
         return hypervisorSnapshotReserve;
     }
+
+    public boolean isShared() {
+        return !useLocalStorage;
+    }
 }

engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java

@@ -122,4 +122,9 @@ public interface VolumeDao extends GenericDao<VolumeVO, Long>, StateDao<Volume.S
      * @return returns true if transaction is successful.
      */
     boolean updateUuid(long srcVolId, long destVolId);
+
+    /**
+     *  Updates the disk offering for the given volume. 
+     */
+    void updateDiskOffering(long volumeId, long diskOfferingId);
 }

engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java

@@ -685,4 +685,17 @@ public ScopeType getVolumeStoragePoolScope(long volumeId) {
         }
         return null;
     }
+
+    private String sqlUpdateDiskOffering = "UPDATE volumes SET disk_offering_id = ? where id =?";
+    public void updateDiskOffering(long volumeId, long diskOfferingId) {
+        try (TransactionLegacy txn = TransactionLegacy.currentTxn();
+                PreparedStatement pstmt = txn.prepareAutoCloseStatement(sqlUpdateDiskOffering)) {
+            pstmt.setLong(1, diskOfferingId);
+            pstmt.setLong(2, volumeId);
+            pstmt.executeUpdate();
+            txn.commit();
+        } catch (SQLException e) {
+            throw new CloudRuntimeException(e);
+        }
+   }
 }

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

@@ -69,13 +69,16 @@
 import org.apache.cloudstack.storage.command.DettachCommand;
 import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
 import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
 import org.apache.cloudstack.utils.identity.ManagementServerNode;
 import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
 import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.log4j.Logger;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
@@ -247,6 +250,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     private ClusterDetailsDao _clusterDetailsDao;
     @Inject
     private StorageManager storageMgr;
+    @Inject
+    private StoragePoolDetailsDao storagePoolDetailsDao;
 
     protected Gson _gson;
 
@@ -2052,7 +2057,8 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) {
                 updateMissingRootDiskController(vm, vol.getChainInfo());
             }
         }
-
+        DiskOfferingVO newDiskOffering = retrieveAndValidateNewDiskOffering(cmd);
+        validateConditionsToReplaceDiskOfferingOfVolume(vol, newDiskOffering, destPool);
         if (vm != null) {
             // serialize VM operation
             AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
@@ -2062,13 +2068,13 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) {
                 VmWorkJobVO placeHolder = null;
                 placeHolder = createPlaceHolderWork(vm.getId());
                 try {
-                    return orchestrateMigrateVolume(vol.getId(), destPool.getId(), liveMigrateVolume);
+                    return orchestrateMigrateVolume(vol, destPool, liveMigrateVolume, newDiskOffering);
                 } finally {
                     _workJobDao.expunge(placeHolder.getId());
                 }
 
             } else {
-                Outcome<Volume> outcome = migrateVolumeThroughJobQueue(vm.getId(), vol.getId(), destPool.getId(), liveMigrateVolume);
+                Outcome<Volume> outcome = migrateVolumeThroughJobQueue(vm, vol, destPool, liveMigrateVolume, newDiskOffering);
 
                 try {
                     outcome.get();
@@ -2097,21 +2103,97 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) {
             }
         }
 
-        return orchestrateMigrateVolume(vol.getId(), destPool.getId(), liveMigrateVolume);
+        return orchestrateMigrateVolume(vol, destPool, liveMigrateVolume, newDiskOffering);
     }
 
-    private Volume orchestrateMigrateVolume(long volumeId, long destPoolId, boolean liveMigrateVolume) {
-        VolumeVO vol = _volsDao.findById(volumeId);
-        assert (vol != null);
-        StoragePool destPool = (StoragePool)dataStoreMgr.getDataStore(destPoolId, DataStoreRole.Primary);
-        assert (destPool != null);
+    /**
+     * Retrieve the new disk offering UUID that might be sent to replace the current one in the volume being migrated.
+     * If no disk offering UUID is provided we return null. Otherwise, we perform the following checks.
+     * <ul>
+     *  <li>Is the disk offering UUID entered valid? If not, an  {@link InvalidParameterValueException} is thrown;
+     *  <li>If the disk offering was already removed, we thrown an {@link InvalidParameterValueException} is thrown;
+     *  <li>We then check if the user executing the operation has access to the given disk offering.
+     * </ul>
+     * 
+     * If all checks pass, we move forward returning the disk offering object.
+     */
+    private DiskOfferingVO retrieveAndValidateNewDiskOffering(MigrateVolumeCmd cmd) {
+        String newDiskOfferingUuid = cmd.getNewDiskOfferingUuid();
+        if (org.apache.commons.lang.StringUtils.isBlank(newDiskOfferingUuid)) {
+            return null;
+        }
+        DiskOfferingVO newDiskOffering = _diskOfferingDao.findByUuid(newDiskOfferingUuid);
+        if (newDiskOffering == null) {
+            throw new InvalidParameterValueException(String.format("The disk offering informed is not valid [id=%s].", newDiskOfferingUuid));
+        }
+        if (newDiskOffering.getRemoved() != null) {
+            throw new InvalidParameterValueException(String.format("We cannot assign a removed disk offering [id=%s] to a volume. ", newDiskOffering.getUuid()));
+        }
+        Account caller = CallContext.current().getCallingAccount();
+        _accountMgr.checkAccess(caller, newDiskOffering);
+        return newDiskOffering;
+    }
+
+    /**
+     * Performs the validations required for replacing the disk offering while migrating the volume of storage. If no new disk offering is provided, we do not execute any validation.
+     * If a disk offering is informed, we then proceed with the following checks.
+     * <ul>
+     *  <li>We check if the given volume is of ROOT type. We cannot change the disk offering of a ROOT volume. Therefore, we thrown an {@link InvalidParameterValueException}.
+     *  <li>We the disk is being migrated to shared storage and the new disk offering is for local storage (or vice versa), we throw an {@link InvalidParameterValueException}. Bear in mind that we are validating only the new disk offering. If none is provided we can override the current disk offering. This means, placing a volume with shared disk offering in local storage and vice versa.
+     *  <li>We then proceed checking if the tags of the new disk offerings match the tags of the target storage. If they do not match an {@link InvalidParameterValueException} is thrown.
+     * </ul>
+     * 
+     * If all of the above validations pass, we check if the size of the new disk offering is different from the volume. If it is, we log a warning message.
+     */
+    protected void validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume, DiskOfferingVO newDiskOffering, StoragePool destPool) {
+        if (newDiskOffering == null) {
+            return;
+        }
+        if (Volume.Type.ROOT.equals(volume.getVolumeType())) {
+            throw new InvalidParameterValueException(String.format("Cannot change the disk offering of a ROOT volume [id=%s].", volume.getUuid()));
+        }
+        if ((destPool.isShared() && newDiskOffering.getUseLocalStorage()) || destPool.isLocal() && newDiskOffering.isShared()) {
+            throw new InvalidParameterValueException("You cannot move the volume to a shared storage and assing a disk offering for local storage and vice versa.");
+        }
+        String storageTags = getStoragePoolTags(destPool);
+        if (!StringUtils.areTagsEqual(storageTags, newDiskOffering.getTags())) {
+            throw new InvalidParameterValueException(String.format("Target Storage [id=%s] tags [%s] does not match new disk offering [id=%s] tags [%s].", destPool.getUuid(), storageTags,
+                    newDiskOffering.getUuid(), newDiskOffering.getTags()));
+        }
+        if (volume.getSize() != newDiskOffering.getDiskSize()) {
+            DiskOfferingVO oldDiskOffering = this._diskOfferingDao.findById(volume.getDiskOfferingId());
+            s_logger.warn(String.format(
+                    "You are migrating a volume [id=%s] and changing the disk offering[from id=%s to id=%s] to reflect this migration. However, the sizes of the volume and the new disk offering are different.",
+                    volume.getUuid(), oldDiskOffering.getUuid(), newDiskOffering.getUuid()));
+        }
 
+    }
+    
+    /**
+     *  Retrieve the storage pool tags as a {@link String}. If the storage pool does not have tags we return a null value. 
+     */
+    protected String getStoragePoolTags(StoragePool destPool) {
+        List<StoragePoolDetailVO> storagePoolDetails = storagePoolDetailsDao.listDetails(destPool.getId());
+        if (CollectionUtils.isEmpty(storagePoolDetails)) {
+            return null;
+        }
+        String storageTags = "";
+        for (StoragePoolDetailVO storagePoolDetailVO : storagePoolDetails) {
+            storageTags = storageTags + storagePoolDetailVO.getName() + ",";
+        }
+        return storageTags.substring(0, storageTags.length() - 1);
+    }
+
+    private Volume orchestrateMigrateVolume(VolumeVO volume, StoragePool destPool, boolean liveMigrateVolume, DiskOfferingVO newDiskOffering) {
         Volume newVol = null;
         try {
             if (liveMigrateVolume) {
-                newVol = liveMigrateVolume(vol, destPool);
+                newVol = liveMigrateVolume(volume, destPool);
             } else {
-                newVol = _volumeMgr.migrateVolume(vol, destPool);
+                newVol = _volumeMgr.migrateVolume(volume, destPool);
+            }
+            if (newDiskOffering != null) {
+                     _volsDao.updateDiskOffering(newVol.getId(), newDiskOffering.getId());
             }
         } catch (StorageUnavailableException e) {
             s_logger.debug("Failed to migrate volume", e);
@@ -2126,7 +2208,9 @@ private Volume orchestrateMigrateVolume(long volumeId, long destPoolId, boolean
     @DB
     protected Volume liveMigrateVolume(Volume volume, StoragePool destPool) throws StorageUnavailableException {
         VolumeInfo vol = volFactory.getVolume(volume.getId());
-        AsyncCallFuture<VolumeApiResult> future = volService.migrateVolume(vol, (DataStore)destPool);
+
+        DataStore dataStoreTarget = dataStoreMgr.getDataStore(destPool.getId(), DataStoreRole.Primary);
+        AsyncCallFuture<VolumeApiResult> future = volService.migrateVolume(vol, dataStoreTarget);
         try {
             VolumeApiResult result = future.get();
             if (result.isFailed()) {
@@ -3019,14 +3103,10 @@ public Outcome<String> extractVolumeThroughJobQueue(final Long vmId, final long
         return new VmJobVolumeUrlOutcome(workJob);
     }
 
-    public Outcome<Volume> migrateVolumeThroughJobQueue(final Long vmId, final long volumeId,
-            final long destPoolId, final boolean liveMigrate) {
-
-        final CallContext context = CallContext.current();
-        final User callingUser = context.getCallingUser();
-        final Account callingAccount = context.getCallingAccount();
-
-        final VMInstanceVO vm = _vmInstanceDao.findById(vmId);
+    private Outcome<Volume> migrateVolumeThroughJobQueue(VMInstanceVO vm, VolumeVO vol, StoragePool destPool, boolean liveMigrateVolume, DiskOfferingVO newDiskOffering) {
+        CallContext context = CallContext.current();
+        User callingUser = context.getCallingUser();
+        Account callingAccount = context.getCallingAccount();
 
         VmWorkJobVO workJob = new VmWorkJobVO(context.getContextId());
 
@@ -3040,16 +3120,18 @@ public Outcome<Volume> migrateVolumeThroughJobQueue(final Long vmId, final long
         workJob.setVmInstanceId(vm.getId());
         workJob.setRelated(AsyncJobExecutionContext.getOriginJobId());
 
+        Long newDiskOfferingId = newDiskOffering != null ? newDiskOffering.getId() : null;
+
         // save work context info (there are some duplications)
         VmWorkMigrateVolume workInfo = new VmWorkMigrateVolume(callingUser.getId(), callingAccount.getId(), vm.getId(),
-                VolumeApiServiceImpl.VM_WORK_JOB_HANDLER, volumeId, destPoolId, liveMigrate);
+                VolumeApiServiceImpl.VM_WORK_JOB_HANDLER, vol.getId(), destPool.getId(), liveMigrateVolume, newDiskOfferingId);
         workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo));
 
         _jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE, vm.getId());
 
         AsyncJobExecutionContext.getCurrentExecutionContext().joinJob(workJob.getId());
 
-        return new VmJobVolumeOutcome(workJob,volumeId);
+        return new VmJobVolumeOutcome(workJob, vol.getId());
     }
 
     public Outcome<Snapshot> takeVolumeSnapshotThroughJobQueue(final Long vmId, final Long volumeId,
@@ -3117,9 +3199,13 @@ private Pair<JobInfo.Status, String> orchestrateResizeVolume(VmWorkResizeVolume
 
     @ReflectionUse
     private Pair<JobInfo.Status, String> orchestrateMigrateVolume(VmWorkMigrateVolume work) throws Exception {
-        Volume newVol = orchestrateMigrateVolume(work.getVolumeId(), work.getDestPoolId(), work.isLiveMigrate());
-        return new Pair<JobInfo.Status, String>(JobInfo.Status.SUCCEEDED,
-                _jobMgr.marshallResultObject(new Long(newVol.getId())));
+        VolumeVO volume = _volsDao.findById(work.getVolumeId());
+        StoragePoolVO targetStoragePool = _storagePoolDao.findById(work.getDestPoolId());
+        DiskOfferingVO newDiskOffering = _diskOfferingDao.findById(work.getNewDiskOfferingId());
+
+        Volume newVol = orchestrateMigrateVolume(volume, targetStoragePool, work.isLiveMigrate(), newDiskOffering);
+
+        return new Pair<JobInfo.Status, String>(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(newVol.getId()));
     }
 
     @ReflectionUse