[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r237216601
##
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/volume/MigrateVolumeCmd.java
##
@@ -120,4 +120,19 @@ public void execute() {
}
}
+@Override
+public String getSyncObjType() {
+if (getSyncObjId() != null) {
+return BaseAsyncCmd.migrationSyncObject;
+}
+return null;
+}
+
+@Override
+public Long getSyncObjId() {
+if (getStoragePoolId() != null) {
Review comment:
I did not get your explanation. When we extend the class and then override
the method, the whole implementation of `getSyncObjId` is changed. I do not
know how a possible method override can require this code here.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r237217694
##
File path:
engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java
##
@@ -25,11 +25,28 @@
import com.cloud.agent.api.to.VirtualMachineTO;
import com.cloud.host.Host;
+/**
+ * interface to query how to move data around and to commision the moving
+ */
public interface DataMotionStrategy {
+/**
+ * reports whether this instance can do a move from source to destination
Review comment:
here as well.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r237219175
##
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##
@@ -1988,92 +1977,249 @@ public void storageMigration(final String vmUuid,
final StoragePool destPool) {
private void orchestrateStorageMigration(final String vmUuid, final
StoragePool destPool) {
final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
-if (destPool == null) {
-throw new CloudRuntimeException("Unable to migrate vm: missing
destination storage pool");
-}
+preStorageMigrationStateCheck(destPool, vm);
try {
-stateTransitTo(vm, VirtualMachine.Event.StorageMigrationRequested,
null);
-} catch (final NoTransitionException e) {
-s_logger.debug("Unable to migrate vm: " + e.toString());
-throw new CloudRuntimeException("Unable to migrate vm: " +
e.toString());
+if(s_logger.isDebugEnabled()) {
+s_logger.debug(
+String.format("Offline migration of %s vm %s with
volumes",
+vm.getHypervisorType().toString(),
+vm.getInstanceName()));
+}
+
+migrateThroughHypervisorOrStorage(destPool, vm);
+
+} catch (ConcurrentOperationException
+| InsufficientCapacityException // possibly
InsufficientVirtualNetworkCapacityException or
InsufficientAddressCapacityException
+| StorageUnavailableException e) {
+String msg = String.format("Failed to migrate vm: %s" ,
vm.getUuid(), e);
+s_logger.debug(msg);
+throw new CloudRuntimeException(msg);
Review comment:
what about re throwing the exception? So, we do not loose the initial stack
trace.
I mean, you can send the original exception in the new
`CloudRuntimeException`
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r237218861
##
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##
@@ -1976,92 +1965,244 @@ public void storageMigration(final String vmUuid,
final StoragePool destPool) {
private void orchestrateStorageMigration(final String vmUuid, final
StoragePool destPool) {
final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
+preStorageMigrationStateCheck(destPool, vm);
+
+try {
+if(s_logger.isDebugEnabled()) {
Review comment:
@mike-tutkowski the current log4j we use does not allow that. However,
log4j-2 does. I am trying to make time to finish the upgrade.
@dhlaluku even though I think that using the check
`s_logger.isDebugEnabled()` is foolish in our context, you do not need to
remove just because of me. I understand @DaanHoogland perspective.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline migration URL: https://github.com/apache/cloudstack/pull/2848#discussion_r237215236 ## File path: api/src/main/java/com/cloud/hypervisor/HypervisorGuru.java ## @@ -84,4 +85,13 @@ List finalizeExpungeVolumes(VirtualMachine vm); Map getClusterSettings(long vmId); + +/** + * will generate commands to migrate a vm to a pool. For now this will ony work for stopped VMs on Vmware. Review comment: Can you also capitalize the first letter. `Will`? This is an automated message from the Apache Git Service. To respond to the message, please log on 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 With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline migration URL: https://github.com/apache/cloudstack/pull/2848#discussion_r237217584 ## File path: engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java ## @@ -25,11 +25,28 @@ import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.host.Host; +/** + * interface to query how to move data around and to commision the moving Review comment: Maybe it is my OCD, but can you capitalize the first letter here? `Interface`? This is an automated message from the Apache Git Service. To respond to the message, please log on 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 With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r237217011
##
File path: core/src/main/java/com/cloud/agent/api/MigrateVmToPoolCommand.java
##
@@ -0,0 +1,70 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+package com.cloud.agent.api;
+
+import com.cloud.agent.api.to.VolumeTO;
+
+import java.util.Collection;
+
+/**
+ * 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 {
+private Collection volumes;
+private String vmName;
+private String destinationPool;
+private boolean executeInSequence = false;
+
+protected MigrateVmToPoolCommand() {
+}
+
+/**
+ *
+ * @param vmName the name of the VM to migrate
+ * @param volumes used to supply feedback on vmware generated names
+ * @param destinationPool the primary storage pool to migrate the VM to
+ * @param executeInSequence
Review comment:
Good question. I do not know how it would be better now. Maybe it would have
an impact if we generated the javadoc for the code, but we do not do that...
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline migration URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233212655 ## File path: engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java ## @@ -25,11 +25,28 @@ import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.host.Host; +/** + * interface to query how to move data around and to commision the moving Review comment: I am loving this new approach that you are adopting to document the code you introduce in ACS This is an automated message from the Apache Git Service. To respond to the message, please log on 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 With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233214319
##
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##
@@ -1976,92 +1965,244 @@ public void storageMigration(final String vmUuid,
final StoragePool destPool) {
private void orchestrateStorageMigration(final String vmUuid, final
StoragePool destPool) {
final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
+preStorageMigrationStateCheck(destPool, vm);
+
+try {
+if(s_logger.isDebugEnabled()) {
+s_logger.debug(
+String.format("Offline migration of %s vm %s with
volumes",
+vm.getHypervisorType().toString(),
+vm.getInstanceName()));
+}
+
+migrateThroughHypervisorOrStorage(destPool, vm);
+
+} catch (ConcurrentOperationException
+| InsufficientCapacityException // possibly
InsufficientVirtualNetworkCapacityException or
InsufficientAddressCapacityException
+| StorageUnavailableException e) {
+s_logger.debug("Failed to migration: " + e.toString());
+throw new CloudRuntimeException("Failed to migration: " +
e.toString());
Review comment:
This will break the stack trace. So, what about `throw new
CloudRuntimeException(Sring.format ("Failed to migrate VM: %s ", vm.getUuid()),
e);` instead?
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233214502
##
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##
@@ -1976,92 +1965,244 @@ public void storageMigration(final String vmUuid,
final StoragePool destPool) {
private void orchestrateStorageMigration(final String vmUuid, final
StoragePool destPool) {
final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
+preStorageMigrationStateCheck(destPool, vm);
+
+try {
+if(s_logger.isDebugEnabled()) {
+s_logger.debug(
+String.format("Offline migration of %s vm %s with
volumes",
+vm.getHypervisorType().toString(),
+vm.getInstanceName()));
+}
+
+migrateThroughHypervisorOrStorage(destPool, vm);
+
+} catch (ConcurrentOperationException
+| InsufficientCapacityException // possibly
InsufficientVirtualNetworkCapacityException or
InsufficientAddressCapacityException
+| StorageUnavailableException e) {
+s_logger.debug("Failed to migration: " + e.toString());
+throw new CloudRuntimeException("Failed to migration: " +
e.toString());
+} finally {
+try {
+stateTransitTo(vm, Event.AgentReportStopped, null);
+} catch (final NoTransitionException e) {
+s_logger.debug("Failed to change vm state: " + e.toString());
+throw new CloudRuntimeException("Failed to change vm state: "
+ e.toString());
+}
+}
+}
+
+private Answer[] attemptHypervisorMigration(StoragePool destPool,
VMInstanceVO vm) {
+boolean migrationResult = false;
+final HypervisorGuru hvGuru =
_hvGuruMgr.getGuru(vm.getHypervisorType());
+// OfflineVmwareMigration: in case of vmware call vcenter to do it for
us.
Review comment:
instead of commenting here, what about a proper Java doc?
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233215111
##
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##
@@ -1976,92 +1965,244 @@ public void storageMigration(final String vmUuid,
final StoragePool destPool) {
private void orchestrateStorageMigration(final String vmUuid, final
StoragePool destPool) {
final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
+preStorageMigrationStateCheck(destPool, vm);
+
+try {
+if(s_logger.isDebugEnabled()) {
+s_logger.debug(
+String.format("Offline migration of %s vm %s with
volumes",
+vm.getHypervisorType().toString(),
+vm.getInstanceName()));
+}
+
+migrateThroughHypervisorOrStorage(destPool, vm);
+
+} catch (ConcurrentOperationException
+| InsufficientCapacityException // possibly
InsufficientVirtualNetworkCapacityException or
InsufficientAddressCapacityException
+| StorageUnavailableException e) {
+s_logger.debug("Failed to migration: " + e.toString());
+throw new CloudRuntimeException("Failed to migration: " +
e.toString());
+} finally {
+try {
+stateTransitTo(vm, Event.AgentReportStopped, null);
+} catch (final NoTransitionException e) {
+s_logger.debug("Failed to change vm state: " + e.toString());
+throw new CloudRuntimeException("Failed to change vm state: "
+ e.toString());
+}
+}
+}
+
+private Answer[] attemptHypervisorMigration(StoragePool destPool,
VMInstanceVO vm) {
+boolean migrationResult = false;
+final HypervisorGuru hvGuru =
_hvGuruMgr.getGuru(vm.getHypervisorType());
+// OfflineVmwareMigration: in case of vmware call vcenter to do it for
us.
+// OfflineVmwareMigration: should we check the proximity of source and
destination
+// OfflineVmwareMigration: if we are in the same
cluster/datacentre/pool or whatever?
+// OfflineVmwareMigration: we are checking on success to optionally
delete an old vm if we are not
+List commandsToSend = hvGuru.finalizeMigrate(vm, destPool);
+
+Long hostId = vm.getHostId();
+// OfflineVmwareMigration: probaby this is null when vm is stopped
+if(hostId == null) {
+hostId = vm.getLastHostId();
+if (s_logger.isDebugEnabled()) {
+s_logger.debug(String.format("host id is null, using last host
id %d", hostId) );
+}
+}
+
+if(CollectionUtils.isNotEmpty(commandsToSend)) {
+Commands commandsContainer = new Commands(commandsToSend.get(0));
Review comment:
are you only sending the first command?
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233213044
##
File path:
engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java
##
@@ -25,11 +25,28 @@
import com.cloud.agent.api.to.VirtualMachineTO;
import com.cloud.host.Host;
+/**
+ * interface to query how to move data around and to commision the moving
+ */
public interface DataMotionStrategy {
+/**
+ * reports whether this instance can do a move from source to destination
+ * @param srcData object to move
+ * @param destData location to move it to
+ * @return the expertise level with which this instance knows how to
handle the move
+ */
StrategyPriority canHandle(DataObject srcData, DataObject destData);
StrategyPriority canHandle(Map volumeMap, Host
srcHost, Host destHost);
+/**
+ * copy the source volume to its destination (on a host if not null)
Review comment:
what happens if the host is null?
Is it possible to be null when we get to this part of the code?
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233213563
##
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##
@@ -1976,92 +1965,244 @@ public void storageMigration(final String vmUuid,
final StoragePool destPool) {
private void orchestrateStorageMigration(final String vmUuid, final
StoragePool destPool) {
final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
+preStorageMigrationStateCheck(destPool, vm);
+
+try {
+if(s_logger.isDebugEnabled()) {
Review comment:
do you really need to check if debug log level is enabled? You can simply
log it, and let the logging framework to manage it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233214602
##
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##
@@ -1976,92 +1965,244 @@ public void storageMigration(final String vmUuid,
final StoragePool destPool) {
private void orchestrateStorageMigration(final String vmUuid, final
StoragePool destPool) {
final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
+preStorageMigrationStateCheck(destPool, vm);
+
+try {
+if(s_logger.isDebugEnabled()) {
+s_logger.debug(
+String.format("Offline migration of %s vm %s with
volumes",
+vm.getHypervisorType().toString(),
+vm.getInstanceName()));
+}
+
+migrateThroughHypervisorOrStorage(destPool, vm);
+
+} catch (ConcurrentOperationException
+| InsufficientCapacityException // possibly
InsufficientVirtualNetworkCapacityException or
InsufficientAddressCapacityException
+| StorageUnavailableException e) {
+s_logger.debug("Failed to migration: " + e.toString());
+throw new CloudRuntimeException("Failed to migration: " +
e.toString());
+} finally {
+try {
+stateTransitTo(vm, Event.AgentReportStopped, null);
+} catch (final NoTransitionException e) {
+s_logger.debug("Failed to change vm state: " + e.toString());
+throw new CloudRuntimeException("Failed to change vm state: "
+ e.toString());
+}
+}
+}
+
+private Answer[] attemptHypervisorMigration(StoragePool destPool,
VMInstanceVO vm) {
+boolean migrationResult = false;
+final HypervisorGuru hvGuru =
_hvGuruMgr.getGuru(vm.getHypervisorType());
+// OfflineVmwareMigration: in case of vmware call vcenter to do it for
us.
+// OfflineVmwareMigration: should we check the proximity of source and
destination
+// OfflineVmwareMigration: if we are in the same
cluster/datacentre/pool or whatever?
+// OfflineVmwareMigration: we are checking on success to optionally
delete an old vm if we are not
+List commandsToSend = hvGuru.finalizeMigrate(vm, destPool);
+
+Long hostId = vm.getHostId();
+// OfflineVmwareMigration: probaby this is null when vm is stopped
+if(hostId == null) {
+hostId = vm.getLastHostId();
+if (s_logger.isDebugEnabled()) {
Review comment:
do we really need this?
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233214364
##
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##
@@ -1976,92 +1965,244 @@ public void storageMigration(final String vmUuid,
final StoragePool destPool) {
private void orchestrateStorageMigration(final String vmUuid, final
StoragePool destPool) {
final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
+preStorageMigrationStateCheck(destPool, vm);
+
+try {
+if(s_logger.isDebugEnabled()) {
+s_logger.debug(
+String.format("Offline migration of %s vm %s with
volumes",
+vm.getHypervisorType().toString(),
+vm.getInstanceName()));
+}
+
+migrateThroughHypervisorOrStorage(destPool, vm);
+
+} catch (ConcurrentOperationException
+| InsufficientCapacityException // possibly
InsufficientVirtualNetworkCapacityException or
InsufficientAddressCapacityException
+| StorageUnavailableException e) {
+s_logger.debug("Failed to migration: " + e.toString());
+throw new CloudRuntimeException("Failed to migration: " +
e.toString());
+} finally {
+try {
+stateTransitTo(vm, Event.AgentReportStopped, null);
+} catch (final NoTransitionException e) {
+s_logger.debug("Failed to change vm state: " + e.toString());
+throw new CloudRuntimeException("Failed to change vm state: "
+ e.toString());
Review comment:
The same thing here
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233214939
##
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##
@@ -1976,92 +1965,244 @@ public void storageMigration(final String vmUuid,
final StoragePool destPool) {
private void orchestrateStorageMigration(final String vmUuid, final
StoragePool destPool) {
final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
+preStorageMigrationStateCheck(destPool, vm);
+
+try {
+if(s_logger.isDebugEnabled()) {
+s_logger.debug(
+String.format("Offline migration of %s vm %s with
volumes",
+vm.getHypervisorType().toString(),
+vm.getInstanceName()));
+}
+
+migrateThroughHypervisorOrStorage(destPool, vm);
+
+} catch (ConcurrentOperationException
+| InsufficientCapacityException // possibly
InsufficientVirtualNetworkCapacityException or
InsufficientAddressCapacityException
+| StorageUnavailableException e) {
+s_logger.debug("Failed to migration: " + e.toString());
+throw new CloudRuntimeException("Failed to migration: " +
e.toString());
+} finally {
+try {
+stateTransitTo(vm, Event.AgentReportStopped, null);
+} catch (final NoTransitionException e) {
+s_logger.debug("Failed to change vm state: " + e.toString());
+throw new CloudRuntimeException("Failed to change vm state: "
+ e.toString());
+}
+}
+}
+
+private Answer[] attemptHypervisorMigration(StoragePool destPool,
VMInstanceVO vm) {
+boolean migrationResult = false;
+final HypervisorGuru hvGuru =
_hvGuruMgr.getGuru(vm.getHypervisorType());
+// OfflineVmwareMigration: in case of vmware call vcenter to do it for
us.
+// OfflineVmwareMigration: should we check the proximity of source and
destination
+// OfflineVmwareMigration: if we are in the same
cluster/datacentre/pool or whatever?
+// OfflineVmwareMigration: we are checking on success to optionally
delete an old vm if we are not
+List commandsToSend = hvGuru.finalizeMigrate(vm, destPool);
+
+Long hostId = vm.getHostId();
+// OfflineVmwareMigration: probaby this is null when vm is stopped
+if(hostId == null) {
+hostId = vm.getLastHostId();
+if (s_logger.isDebugEnabled()) {
+s_logger.debug(String.format("host id is null, using last host
id %d", hostId) );
+}
+}
+
+if(CollectionUtils.isNotEmpty(commandsToSend)) {
+Commands commandsContainer = new Commands(commandsToSend.get(0));
+try {
+// OfflineVmwareMigration: change to the call back variety?
+// OfflineVmwareMigration: getting a Long seq to be filled
with _agentMgr.send(hostId, commandsContainer, this)
+Answer[] answers = _agentMgr.send(hostId, commandsContainer);
Review comment:
`return _agentMgr.send(hostId, commandsContainer);`
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233212308
##
File path: core/src/main/java/com/cloud/agent/api/UnregisterVMCommand.java
##
@@ -22,14 +22,19 @@
public class UnregisterVMCommand extends Command {
String vmName;
boolean cleanupVmFiles = false;
+boolean executeInSequence = false;
Review comment:
The default is `false`. No need to declare it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline migration URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233212888 ## File path: engine/components-api/src/main/java/com/cloud/storage/StorageManager.java ## @@ -106,7 +106,14 @@ * @param poolId * @return comma separated list of tags */ -public String getStoragePoolTags(long poolId); +String getStoragePoolTags(long poolId); + +/** + * Returns a list of Strings with tags for the specified storage pool + * @param poolId Review comment: If you do not have anything to say about a parameter, you do not need to declare it here. This is an automated message from the Apache Git Service. To respond to the message, please log on 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 With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233211404
##
File path:
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java
##
@@ -192,4 +187,25 @@ public void execute() {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR,
e.getMessage());
}
}
+
+@Override
+public String getSyncObjType() {
Review comment:
Why did you need to implemente these `getSyncObjType` and `getSyncObjId`?
Could you explain this a little bit further?
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233212190
##
File path: core/src/main/java/com/cloud/agent/api/MigrateVmToPoolCommand.java
##
@@ -0,0 +1,70 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+package com.cloud.agent.api;
+
+import com.cloud.agent.api.to.VolumeTO;
+
+import java.util.Collection;
+
+/**
+ * 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 {
+private Collection volumes;
+private String vmName;
+private String destinationPool;
+private boolean executeInSequence = false;
+
+protected MigrateVmToPoolCommand() {
+}
+
+/**
+ *
+ * @param vmName the name of the VM to migrate
+ * @param volumes used to supply feedback on vmware generated names
+ * @param destinationPool the primary storage pool to migrate the VM to
+ * @param executeInSequence
Review comment:
if you do not have anything further to talk about a parameter, I think that
you do not need to list it here.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2848: Vmware offline migration
rafaelweingartner commented on a change in pull request #2848: Vmware offline
migration
URL: https://github.com/apache/cloudstack/pull/2848#discussion_r233211822
##
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/volume/MigrateVolumeCmd.java
##
@@ -120,4 +120,19 @@ public void execute() {
}
}
+@Override
+public String getSyncObjType() {
+if (getSyncObjId() != null) {
+return BaseAsyncCmd.migrationSyncObject;
+}
+return null;
+}
+
+@Override
+public Long getSyncObjId() {
+if (getStoragePoolId() != null) {
Review comment:
If `getStoragePoolId()` != null, then return `getStoragePoolId()`, otherwise
return null
You can simple call `return getStoragePoolId()`. The result is the same.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
