Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77220737
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -328,15 +412,64 @@ private void
handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
copyCommand.setOptions(srcDetails);
copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
- }
- catch (CloudRuntimeException | AgentUnavailableException |
OperationTimedoutException ex) {
+
+ if (needCache) {
+
+ // If cached storage was needed (in case of object
store as secondary
+ // storage), at this point, the data has been copied
from the primary
+ // to the NFS cache by the hypervisor. We now invoke
another copy
+ // command to copy this data from cache to secondary
storage. We
+ // then cleanup the cache
+
+ destOnStore.processEvent(Event.OperationSuccessed,
copyCmdAnswer);
+
+ CopyCommand cmd = new CopyCommand(destOnStore.getTO(),
destData.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+ EndPoint ep = selector.select(destOnStore, destData);
+
+ if (ep == null) {
+ errMsg = "No remote endpoint to send command,
check if host or ssvm is down?";
+ LOGGER.error(errMsg);
+ copyCmdAnswer = new CopyCmdAnswer(errMsg);
+ } else {
+ copyCmdAnswer = (CopyCmdAnswer)
ep.sendMessage(cmd);
+ }
+
+ // clean up snapshot copied to staging
+ performCleanupCacheStorage(destOnStore);
+ }
+
+
+ } catch (CloudRuntimeException | AgentUnavailableException |
OperationTimedoutException ex) {
String msg = "Failed to create template from snapshot
(Snapshot ID = " + snapshotInfo.getId() + ") : ";
LOGGER.warn(msg, ex);
throw new CloudRuntimeException(msg + ex.getMessage());
- }
- finally {
+
+ } finally {
+
+ // detach and remove access after the volume is created
+ SnapshotObjectTO snapshotObjectTO = (SnapshotObjectTO)
snapshotInfo.getTO();
+ DiskTO disk = new DiskTO(snapshotObjectTO, null,
snapshotInfo.getPath(), Volume.Type.UNKNOWN);
+ DettachCommand cmd = new DettachCommand(disk, null);
+ StoragePoolVO storagePool =
_storagePoolDao.findById(srcDataStore.getId());
+
+ cmd.setManaged(true);
+ cmd.setStorageHost(storagePool.getHostAddress());
+ cmd.setStoragePort(storagePool.getPort());
+ cmd.set_iScsiName(getProperty(snapshotInfo.getId(),
DiskTO.IQN));
+
+ try {
+ DettachAnswer dettachAnswer = (DettachAnswer)
_agentMgr.send(hostVO.getId(), cmd);
+
+ if (!dettachAnswer.getResult()) {
+ throw new CloudRuntimeException("Error detaching
volume:" + dettachAnswer.getDetails());
--- End diff --
I lost track of the catch block below. Why are we throwing an exception to
catch a few lines later? It appears to be flow of control using exceptions is
a code anti-pattern.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
