rajiv-jain-netapp commented on code in PR #13053:
URL: https://github.com/apache/cloudstack/pull/13053#discussion_r3193145093
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java:
##########
@@ -146,21 +150,13 @@ protected Answer
takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotComma
}
} catch (LibvirtException | QemuImgException e) {
logger.error("Exception while creating disk-only VM snapshot for
VM [{}]. Deleting leftover deltas.", vmName, e);
- for (VolumeObjectTO volumeObjectTO : volumeObjectTos) {
- Pair<Long, String> volSizeAndNewPath =
mapVolumeToSnapshotSizeAndNewVolumePath.get(volumeObjectTO.getUuid());
- PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO)
volumeObjectTO.getDataStore();
- KVMStoragePool kvmStoragePool =
storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(),
primaryDataStoreTO.getUuid());
-
- if (volSizeAndNewPath == null) {
- continue;
- }
- try {
-
Files.deleteIfExists(Path.of(kvmStoragePool.getLocalPathFor(volSizeAndNewPath.second())));
- } catch (IOException ex) {
- logger.warn("Tried to delete leftover snapshot at [{}]
failed.", volSizeAndNewPath.second(), ex);
- }
- }
+ cleanupLeftoverDeltas(volumeObjectTos,
mapVolumeToSnapshotSizeAndNewVolumePath, storagePoolMgr);
return new Answer(cmd, e);
+ } catch (Exception e) {
+ logger.error("Unexpected exception while creating disk-only VM
snapshot for VM [{}]. Deleting leftover deltas.", vmName, e);
+ cleanupLeftoverDeltas(volumeObjectTos,
mapVolumeToSnapshotSizeAndNewVolumePath, storagePoolMgr);
+ return new CreateDiskOnlyVmSnapshotAnswer(cmd, false,
+ String.format("Creation of disk-only VM snapshot for VM
[%s] failed due to %s.", vmName, e.getMessage()), null);
Review Comment:
@winterhazel, I added this logic separately after encountering failures in
negative scenarios where the UI did not surface any error details. With this
change, users are now notified of the failure reason when the exception falls
outside the scope of the existing catch block. This is expected to improve
overall user experience. Please let me know if you still feel this should be
removed.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
