* Fix worker VM placement algorithm (#9)

* Add VM disk consolidation for OfflineVmware VM migrations

* Fix several typos encountered

* Improve garbage collection for failed volume migrations

Author: Dingane Hlaluku

api/src/com/cloud/hypervisor/HypervisorGuru.java

@@ -91,7 +91,7 @@ public interface HypervisorGuru extends Adapter {
      *
      * @param vm the stopped vm to migrate
      * @param destination the primary storage pool to migrate to
-     * @return a list of commands to perform for a succesful migration
+     * @return a list of commands to perform for a successful migration
      */
     List<Command> finalizeMigrate(VirtualMachine vm, StoragePool destination);
 }

api/src/com/cloud/storage/Volume.java

@@ -51,7 +51,7 @@ enum State {
         NotUploaded("The volume entry is just created in DB, not yet uploaded"),
         UploadInProgress("Volume upload is in progress"),
         UploadError("Volume upload encountered some error"),
-        UploadAbandoned("Volume upload is abandoned since the upload was never initiated within a specificed time");
+        UploadAbandoned("Volume upload is abandoned since the upload was never initiated within a specified time");
 
         String _description;
 

api/src/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java

@@ -44,7 +44,7 @@
 
 @APICommand(name = "migrateVirtualMachine",
             description = "Attempts Migration of a VM to a different host or Root volume of the vm to a different storage pool",
-        responseObject = UserVmResponse.class, entityType = {VirtualMachine.class},
+            responseObject = UserVmResponse.class, entityType = {VirtualMachine.class},
             requestHasSensitiveInfo = false,
             responseHasSensitiveInfo = true)
 public class MigrateVMCmd extends BaseAsyncCmd {
@@ -149,6 +149,7 @@ public void execute() {
             CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
         }
 
+        // OfflineMigration performed when this parameter is specified
         StoragePool destStoragePool = null;
         if (getStoragePoolId() != null) {
             destStoragePool = _storageService.getStoragePool(getStoragePoolId());

api/src/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java

@@ -46,7 +46,7 @@
 
 @APICommand(name = "migrateVirtualMachineWithVolume",
             description = "Attempts Migration of a VM with its volumes to a different host",
-        responseObject = UserVmResponse.class, entityType = {VirtualMachine.class},
+            responseObject = UserVmResponse.class, entityType = {VirtualMachine.class},
             requestHasSensitiveInfo = false,
             responseHasSensitiveInfo = true)
 public class MigrateVirtualMachineWithVolumeCmd extends BaseAsyncCmd {

core/src/com/cloud/agent/api/MigrateVmToPoolCommand.java

@@ -23,8 +23,8 @@
 import java.util.Collection;
 
 /**
- * used to tell the agent to migrate a vm to a different primare storage pool.
- * It is for now only mplemented on Vmware and is supposed to work irrespective of whether the VM is started or not.
+ * used to tell the agent to migrate a vm to a different primary storage pool.
+ * It is for now only implemented on Vmware and is supposed to work irrespective of whether the VM is started or not.
  *
  */
 public class MigrateVmToPoolCommand extends Command {
@@ -39,8 +39,8 @@ protected MigrateVmToPoolCommand() {
     /**
      *
      * @param vmName the name of the VM to migrate
-     * @param volumes used to suplly feedback on vmware generated names
-     * @param destinationPool the primare storage pool to migrate the VM to
+     * @param volumes used to supply feedback on vmware generated names
+     * @param destinationPool the primary storage pool to migrate the VM to
      * @param executeInSequence
      */
     public MigrateVmToPoolCommand(String vmName, Collection<VolumeTO> volumes, String destinationPool, boolean executeInSequence) {

engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -1813,7 +1813,7 @@ private void preStorageMigrationStateCheck(StoragePool destPool, VMInstanceVO vm
     private void checkDestinationForTags(StoragePool destPool, VMInstanceVO vm) {
         List<VolumeVO> vols = _volsDao.findUsableVolumesForInstance(vm.getId());
 
-        // OfflineVmwareMigration: itterate over volumes
+        // OfflineVmwareMigration: iterate over volumes
         // OfflineVmwareMigration: get disk offering
         List<String> storageTags = storageMgr.getStoragePoolTagList(destPool.getId());
         for(Volume vol : vols) {
@@ -1856,7 +1856,7 @@ private Answer[] attemptHypervisorMigration(StoragePool destPool, VMInstanceVO v
         List<Command> commandsToSend = hvGuru.finalizeMigrate(vm, destPool);
 
         Long hostId = vm.getHostId();
-        // OfflineVmwareMigration: probaby this is null when vm is stopped
+        // OfflineVmwareMigration: probably this is null when vm is stopped
         if(hostId == null) {
             hostId = vm.getLastHostId();
             if (s_logger.isDebugEnabled()) {
@@ -1971,8 +1971,8 @@ private void afterStorageMigrationVmwareVMcleanup(StoragePool destPool, VMInstan
         }
     }
 
-    // OfflineVmwareMigration: on port forward refator this to be done in two
-    // OfflineVmwareMigration: command creation in the guru.migrat method
+    // OfflineVmwareMigration: on port forward refactor this to be done in two
+    // OfflineVmwareMigration: command creation in the guru.migrate method
     // OfflineVmwareMigration: sending up in the attemptHypevisorMigration with execute in sequence (responsibility of the guru)
     private void removeStaleVmFromSource(VMInstanceVO vm, HostVO srcHost) {
         s_logger.debug("Since VM's storage was successfully migrated across VMware Datacenters, unregistering VM: " + vm.getInstanceName() +

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

@@ -955,6 +955,13 @@ private void checkConcurrentJobsPerDatastoreThreshhold(final StoragePool destPoo
     @DB
     public Volume migrateVolume(Volume volume, StoragePool destPool) throws StorageUnavailableException {
         VolumeInfo vol = volFactory.getVolume(volume.getId());
+        if (vol == null){
+            throw new CloudRuntimeException("Migrate volume failed because volume object of volume " + volume.getName()+ "is null");
+        }
+
+        if (destPool == null) {
+            throw new CloudRuntimeException("Migrate volume failed because destination storage pool is not available!!");
+        }
 
         checkConcurrentJobsPerDatastoreThreshhold(destPool);
 

engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java

@@ -18,12 +18,17 @@
  */
 package org.apache.cloudstack.storage.motion;
 
+import java.util.Date;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
 import javax.inject.Inject;
 
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.VolumeDao;
+import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
@@ -40,10 +45,15 @@
 import com.cloud.utils.StringUtils;
 import com.cloud.utils.exception.CloudRuntimeException;
 
+
 @Component
 public class DataMotionServiceImpl implements DataMotionService {
+    private static final Logger LOGGER = Logger.getLogger(DataMotionServiceImpl.class);
+
     @Inject
     StorageStrategyFactory storageStrategyFactory;
+    @Inject
+    VolumeDao volDao;
 
     @Override
     public void copyAsync(DataObject srcData, DataObject destData, Host destHost, AsyncCompletionCallback<CopyCommandResult> callback) {
@@ -61,13 +71,33 @@ public void copyAsync(DataObject srcData, DataObject destData, Host destHost, As
 
         DataMotionStrategy strategy = storageStrategyFactory.getDataMotionStrategy(srcData, destData);
         if (strategy == null) {
+            // OfflineVmware volume migration
+            // Cleanup volumes from target and reset the state of volume at source
+            cleanUpVolumesForFailedMigrations(srcData, destData);
             throw new CloudRuntimeException("Can't find strategy to move data. " + "Source: " + srcData.getType().name() + " '" + srcData.getUuid() + ", Destination: " +
-                destData.getType().name() + " '" + destData.getUuid() + "'");
+                    destData.getType().name() + " '" + destData.getUuid() + "'");
         }
 
         strategy.copyAsync(srcData, destData, destHost, callback);
     }
 
+    /**
+     * Offline Vmware volume migration
+     * Cleanup volumes after failed migrations and reset state of source volume
+     *
+     * @param srcData
+     * @param destData
+     */
+    private void cleanUpVolumesForFailedMigrations(DataObject srcData, DataObject destData) {
+        VolumeVO destinationVO = volDao.findById(destData.getId());
+        VolumeVO sourceVO = volDao.findById(srcData.getId());
+        sourceVO.setState(Volume.State.Ready);
+        volDao.update(sourceVO.getId(), sourceVO);
+        destinationVO.setState(Volume.State.Expunged);
+        destinationVO.setRemoved(new Date());
+        volDao.update(destinationVO.getId(), destinationVO);
+    }
+
     @Override
     public void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback<CopyCommandResult> callback) {
         copyAsync(srcData, destData, null, callback);
@@ -84,7 +114,7 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeMap, VirtualMachineTO vmT
             }
 
             throw new CloudRuntimeException("Can't find strategy to move data. " + "Source Host: " + srcHost.getName() + ", Destination Host: " + destHost.getName() +
-                ", Volume UUIDs: " + StringUtils.join(volumeIds, ","));
+                    ", Volume UUIDs: " + StringUtils.join(volumeIds, ","));
         }
 
         strategy.copyAsync(volumeMap, vmTo, srcHost, destHost, callback);