rafaelweingartner commented on a change in pull request #2298: CLOUDSTACK-9620:
Enhancements for managed storage
URL: https://github.com/apache/cloudstack/pull/2298#discussion_r161185599
##########
File path:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
##########
@@ -684,40 +1237,154 @@ private void
handleCreateVolumeFromSnapshotBothOnStorageSystem(SnapshotInfo snap
}
volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-
volumeInfo.processEvent(Event.MigrationRequested);
-
volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
- if (useCloning) {
- copyCmdAnswer = performResignature(volumeInfo, hostVO);
+ if
(HypervisorType.XenServer.equals(snapshotInfo.getHypervisorType()) ||
HypervisorType.VMware.equals(snapshotInfo.getHypervisorType())) {
+ if (useCloning) {
+ Map<String, String> extraDetails = null;
+
+ if
(HypervisorType.VMware.equals(snapshotInfo.getHypervisorType())) {
+ extraDetails = new HashMap<>();
+
+ String extraDetailsVmdk =
getSnapshotProperty(snapshotInfo.getId(), DiskTO.VMDK);
+
+ extraDetails.put(DiskTO.VMDK, extraDetailsVmdk);
+ }
+
+ copyCmdAnswer = performResignature(volumeInfo, hostVO,
extraDetails);
+
+ // If using VMware, have the host rescan its software HBA
if dynamic discovery is in use.
+ if
(HypervisorType.VMware.equals(snapshotInfo.getHypervisorType())) {
+ disconnectHostFromVolume(hostVO,
volumeInfo.getPoolId(), volumeInfo.get_iScsiName());
+ }
+ } else {
+ // asking for a XenServer host here so we don't always
prefer to use XenServer hosts that support resigning
+ // even when we don't need those hosts to do this kind of
copy work
+ hostVO = getHost(snapshotInfo.getDataCenterId(),
snapshotInfo.getHypervisorType(), false);
+
+ copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo,
hostVO);
+ }
+
+ if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+ if (copyCmdAnswer != null &&
!StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+ throw new
CloudRuntimeException(copyCmdAnswer.getDetails());
+ } else {
+ throw new CloudRuntimeException("Unable to create
volume from snapshot");
+ }
+ }
+ }
+ else if
(HypervisorType.KVM.equals(snapshotInfo.getHypervisorType())) {
+ VolumeObjectTO newVolume = new VolumeObjectTO();
+
+ newVolume.setSize(volumeInfo.getSize());
+ newVolume.setPath(volumeInfo.get_iScsiName());
+ newVolume.setFormat(volumeInfo.getFormat());
+
+ copyCmdAnswer = new CopyCmdAnswer(newVolume);
}
else {
- // asking for a XenServer host here so we don't always prefer
to use XenServer hosts that support resigning
- // even when we don't need those hosts to do this kind of copy
work
- hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+ throw new CloudRuntimeException("Unsupported hypervisor type");
+ }
+ }
+ catch (Exception ex) {
+ errMsg = "Copy operation failed in
'StorageSystemDataMotionStrategy.handleCreateVolumeFromSnapshotBothOnStorageSystem':
" +
+ ex.getMessage();
- copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo,
hostVO);
+ throw new CloudRuntimeException(errMsg);
+ }
+ finally {
+ if (copyCmdAnswer == null) {
+ copyCmdAnswer = new CopyCmdAnswer(errMsg);
}
+ CopyCommandResult result = new CopyCommandResult(null,
copyCmdAnswer);
+
+ result.setResult(errMsg);
+
+ callback.complete(result);
+ }
+ }
+
+ private void handleCreateVolumeFromVolumeOnSecondaryStorage(VolumeInfo
srcVolumeInfo, VolumeInfo destVolumeInfo,
+ long
dataCenterId, HypervisorType hypervisorType,
+
AsyncCompletionCallback<CopyCommandResult> callback) {
+ String errMsg = null;
+ CopyCmdAnswer copyCmdAnswer = null;
+
+ try {
+ // create a volume on the storage
+
destVolumeInfo.getDataStore().getDriver().createAsync(destVolumeInfo.getDataStore(),
destVolumeInfo, null);
+
+ destVolumeInfo =
_volumeDataFactory.getVolume(destVolumeInfo.getId(),
destVolumeInfo.getDataStore());
+
+ HostVO hostVO = getHost(dataCenterId, hypervisorType, false);
+
+ // copy the volume from secondary via the hypervisor
+ copyCmdAnswer = copyImageToVolume(srcVolumeInfo, destVolumeInfo,
hostVO);
+
if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
if (copyCmdAnswer != null &&
!StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
- errMsg = copyCmdAnswer.getDetails();
+ throw new
CloudRuntimeException(copyCmdAnswer.getDetails());
}
else {
- errMsg = "Unable to create volume from snapshot";
+ throw new CloudRuntimeException("Unable to create volume
from volume");
}
}
}
catch (Exception ex) {
- errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy
operation failed in
'StorageSystemDataMotionStrategy.handleCreateVolumeFromSnapshotBothOnStorageSystem'";
+ errMsg = "Copy operation failed in
'StorageSystemDataMotionStrategy.handleCreateVolumeFromVolumeOnSecondaryStorage':
" +
+ ex.getMessage();
+
+ throw new CloudRuntimeException(errMsg);
}
+ finally {
+ if (copyCmdAnswer == null) {
+ copyCmdAnswer = new CopyCmdAnswer(errMsg);
+ }
- CopyCommandResult result = new CopyCommandResult(null, copyCmdAnswer);
+ CopyCommandResult result = new CopyCommandResult(null,
copyCmdAnswer);
- result.setResult(errMsg);
+ result.setResult(errMsg);
- callback.complete(result);
+ callback.complete(result);
+ }
+ }
+
+ private CopyCmdAnswer copyImageToVolume(DataObject srcDataObject,
VolumeInfo destVolumeInfo, HostVO hostVO) {
+ String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+ int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+
+ CopyCommand copyCommand = new CopyCommand(srcDataObject.getTO(),
destVolumeInfo.getTO(), primaryStorageDownloadWait,
+ VirtualMachineManager.ExecuteInSequence.value());
+
+ CopyCmdAnswer copyCmdAnswer;
+
+ try {
+ _volumeService.grantAccess(destVolumeInfo, hostVO,
destVolumeInfo.getDataStore());
+
+ Map<String, String> destDetails = getVolumeDetails(destVolumeInfo);
+
+ copyCommand.setOptions2(destDetails);
+
+ copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
+ }
+ catch (CloudRuntimeException | AgentUnavailableException |
OperationTimedoutException ex) {
+ String msg = "Failed to copy image : ";
+
+ LOGGER.warn(msg, ex);
+
+ throw new CloudRuntimeException(msg + ex.getMessage());
Review comment:
I like the idea of switching from checked to unchecked exception, but I
would prefer using throw new CloudRuntimeException(ex), because this would
give me the full stack to the cause of the problem.
If we re-throw an exception, we do not need to log it right away...
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
