shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595843031
##########
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:
@weizhouapache For user VMs we have two different APIs,
migrateVirtualMachine and migrateVirtualMachineWithVolume. Depending on the
need for storage migration requirement appropriate API can be used.
While working on #4385 choice was to add a new API migrateSystemVmWithVolume
or using `migrateVirtualMachineWithVolume` method while code making the
decision if volume needs to be migrated.
MigrateVMCmd can use the `migrateVirtualMachineWithVolume` method but it
might need some additional checks and testing.
----------------------------------------------------------------
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:
[email protected]
