Github user jburwell commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1600#discussion_r77075736
  
    --- 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 --
    
    Throwing an exception here will end processing of this ``finally`` block, 
and skipping the ``_volumeService.revokeAccess`` call at the end of the method. 
 What would be the impact of skipping that call in the event an exception is 
thrown here?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to