weizhouapache commented on a change in pull request #4378: URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595808483
########## File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java ########## @@ -122,34 +122,34 @@ public String getEventDescription() { @Override public void execute() { - if (getHostId() == null && getStorageId() == null) { - throw new InvalidParameterValueException("Either hostId or storageId must be specified"); - } - if (getHostId() != null && getStorageId() != null) { throw new InvalidParameterValueException("Only one of hostId and storageId can be specified"); } + try { //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer VirtualMachine migratedVm = null; - if (getHostId() != null) { - Host 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()); - migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>()); - } else if (getStorageId() != null) { + if (getStorageId() != null) { // OfflineMigration performed when this parameter is specified StoragePool 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()); migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool); + } else { + 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()); + migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>()); Review comment: @shwstppr I will test it. ---------------------------------------------------------------- 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: us...@infra.apache.org