use list, and add snapshot info prior to take snapshot

Author: sureshanaparti

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java

@@ -19,7 +19,6 @@
 package org.apache.cloudstack.storage.vmsnapshot;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
@@ -112,7 +111,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
         FreezeThawVMAnswer freezeAnswer = null;
         FreezeThawVMCommand thawCmd = null;
         FreezeThawVMAnswer thawAnswer = null;
-        Map<Long, SnapshotInfo> volumeToSnapshotInfoMapForRollback = new HashMap<>();
+        List<SnapshotInfo> snapshotInfoListForRollback = new ArrayList<>();
         long startFreeze = 0;
         try {
             vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshotVO, VMSnapshot.Event.CreateRequested);
@@ -166,7 +165,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
                 logger.info("The virtual machine is frozen");
                 for (VolumeInfo vol : vinfos) {
                     long startSnapshtot = System.nanoTime();
-                    SnapshotInfo snapInfo = createDiskSnapshot(vmSnapshot, volumeToSnapshotInfoMapForRollback, vol);
+                    SnapshotInfo snapInfo = createDiskSnapshot(vmSnapshot, snapshotInfoListForRollback, vol);
 
                     if (snapInfo == null) {
                         thawAnswer = (FreezeThawVMAnswer) agentMgr.send(hostId, thawCmd);
@@ -223,7 +222,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
                 }
             }
             if (!result) {
-                for (SnapshotInfo snapshotInfo : volumeToSnapshotInfoMapForRollback.values()) {
+                for (SnapshotInfo snapshotInfo : snapshotInfoListForRollback) {
                     rollbackDiskSnapshot(snapshotInfo);
                 }
                 try {
@@ -441,7 +440,7 @@ protected void revertDiskSnapshot(VMSnapshot vmSnapshot) {
         }
     }
 
-    protected SnapshotInfo createDiskSnapshot(VMSnapshot vmSnapshot, Map<Long, SnapshotInfo> volumeToSnapshotInfoMapForRollback, VolumeInfo vol) {
+    protected SnapshotInfo createDiskSnapshot(VMSnapshot vmSnapshot, List<SnapshotInfo> snapshotInfoListForRollback, VolumeInfo vol) {
         String snapshotName = vmSnapshot.getId() + "_" + vol.getUuid();
         SnapshotVO snapshot = new SnapshotVO(vol.getDataCenterId(), vol.getAccountId(), vol.getDomainId(), vol.getId(), vol.getDiskOfferingId(),
                               snapshotName, (short) Snapshot.Type.GROUP.ordinal(),  Snapshot.Type.GROUP.name(),  vol.getSize(), vol.getMinIops(),  vol.getMaxIops(), Hypervisor.HypervisorType.KVM, null);
@@ -455,16 +454,14 @@ protected SnapshotInfo createDiskSnapshot(VMSnapshot vmSnapshot, Map<Long, Snaps
         vol.addPayload(setPayload(vol, snapshot, quiescevm));
         SnapshotInfo snapshotInfo = snapshotDataFactory.getSnapshot(snapshot.getId(), vol.getDataStore());
         snapshotInfo.addPayload(vol.getpayload());
-        volumeToSnapshotInfoMapForRollback.put(vol.getId(), snapshotInfo);
+        snapshotInfoListForRollback.add(snapshotInfo);
         SnapshotStrategy snapshotStrategy = storageStrategyFactory.getSnapshotStrategy(snapshotInfo, SnapshotOperation.TAKE);
         if (snapshotStrategy == null) {
             throw new CloudRuntimeException("Could not find strategy for snapshot uuid:" + snapshotInfo.getUuid());
         }
         snapshotInfo = snapshotStrategy.takeSnapshot(snapshotInfo);
         if (snapshotInfo == null) {
             throw new CloudRuntimeException("Failed to create snapshot");
-        } else {
-            volumeToSnapshotInfoMapForRollback.put(vol.getId(), snapshotInfo);
         }
         vmSnapshotDetailsDao.persist(new VMSnapshotDetailsVO(vmSnapshot.getId(), STORAGE_SNAPSHOT, String.valueOf(snapshot.getId()), true));
         snapshotInfo.markBackedUp();

engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java

@@ -24,9 +24,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Date;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.UUID;
 
 import javax.inject.Inject;
@@ -155,7 +153,7 @@ public void setUp() throws Exception {
     @Test
     public void testCreateDiskSnapshotBasedOnStrategy() throws Exception {
         VMSnapshotVO vmSnapshot = Mockito.mock(VMSnapshotVO.class);
-        Map<Long, SnapshotInfo> volumeToSnapshotInfoMapForRollback = new HashMap<>();
+        List<SnapshotInfo> snapshotInfoListForRollback = new ArrayList<>();
         VolumeInfo vol = Mockito.mock(VolumeInfo.class);
         SnapshotInfo snapshotInfo = Mockito.mock(SnapshotInfo.class);
         SnapshotStrategy strategy = Mockito.mock(SnapshotStrategy.class);
@@ -179,7 +177,7 @@ public void testCreateDiskSnapshotBasedOnStrategy() throws Exception {
         VMSnapshotDetailsVO vmDetails = new VMSnapshotDetailsVO(vmSnapshot.getId(), volUuid, String.valueOf(snapshot.getId()), false);
         when(vmSnapshotDetailsDao.persist(any())).thenReturn(vmDetails);
 
-        info =  vmStrategy.createDiskSnapshot(vmSnapshot, volumeToSnapshotInfoMapForRollback, vol);
+        info =  vmStrategy.createDiskSnapshot(vmSnapshot, snapshotInfoListForRollback, vol);
         assertNotNull(info);
     }