DaanHoogland commented on code in PR #12949:
URL: https://github.com/apache/cloudstack/pull/12949#discussion_r3028137372
##########
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java:
##########
@@ -448,6 +455,7 @@ protected SnapshotInfo createDiskSnapshot(VMSnapshot
vmSnapshot, List<SnapshotIn
vol.addPayload(setPayload(vol, snapshot, quiescevm));
SnapshotInfo snapshotInfo =
snapshotDataFactory.getSnapshot(snapshot.getId(), vol.getDataStore());
snapshotInfo.addPayload(vol.getpayload());
+ volumeToSnapshotInfoMapForRollback.put(vol.getId(), snapshotInfo);
Review Comment:
so it might be added twice right? so why the else at line 466?
##########
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java:
##########
@@ -222,7 +223,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
}
}
if (!result) {
- for (SnapshotInfo snapshotInfo : forRollback) {
+ for (SnapshotInfo snapshotInfo :
volumeToSnapshotInfoMapForRollback.values()) {
Review Comment:
this seems the only usage, why not a HashSet?
--
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]
