[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4385: vmware: vm migration improvements

[GitBox <gi...@apache.org>]

sureshanaparti commented on a change in pull request #4385:
URL: https://github.com/apache/cloudstack/pull/4385#discussion_r564464767



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -109,15 +122,43 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
+        if (getHostId() == null && getStorageId() == null) {
+            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
+        }
 
-        Host destinationHost = _resourceService.getHost(getHostId());
-        if (destinationHost == null) {
-            throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+        if (getHostId() != null && getStorageId() != null) {
+            throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
+        }
+
+        Host destinationHost = null;
+        if (getHostId() != null) {
+            destinationHost = _resourceService.getHost(getHostId());
+            if (destinationHost == null) {
+                throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+            }
+            if (destinationHost.getType() != Host.Type.Routing) {
+                throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+            }
+            CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+        }
+
+        // OfflineMigration performed when this parameter is specified
+        StoragePool destStoragePool = null;
+        if (getStorageId() != null) {
+            destStoragePool = _storageService.getStoragePool(getStorageId());
+            if (destStoragePool == null) {
+                throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
+            }
+            CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
         }
         try {
-            CallContext.current().setEventDetails("VM Id: " + this._uuidMgr.getUuid(VirtualMachine.class, getVirtualMachineId()) + " to host Id: " + this._uuidMgr.getUuid(Host.class, getHostId()));
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
-            VirtualMachine migratedVm = _userVmService.migrateVirtualMachine(getVirtualMachineId(), destinationHost);
+            VirtualMachine migratedVm = null;
+            if (getHostId() != null) {
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
+            } else if (getStorageId() != null) {
+                migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);

Review comment:
       @shwstppr possible to optimise multiple checks for _getHostId()_ and _getStorageId()_ not null here ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org