shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595912257
##########
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 yes, we do pass empty volume-pool mapping to the method
but later in the code, we generate volume-pool mapping when needed (such as
inter-cluster migration with cluster-scoped pools)
Moving back to 4.15 code will definitely allow systemvm migration with
changes from this PR but inter-cluster migration of systemvms would not work.
----------------------------------------------------------------
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
