[cloudstack] branch master updated: CLOUDSTACK-10004 : On deletion, Vmware volume snapshots are left behind with message 'the snapshot has child, can't delete it on the storage' (#2188)

[bh...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bhaisaab pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/master by this push:
     new 74fe9e3  CLOUDSTACK-10004 : On deletion, Vmware volume snapshots are left behind with message 'the snapshot has child, can't delete it on the storage' (#2188)
74fe9e3 is described below

commit 74fe9e386cfb9dc3611f7d16454a27ea34895813
Author: niteshsarda <ni...@accelerite.com>
AuthorDate: Fri Sep 1 14:34:48 2017 +0530

    CLOUDSTACK-10004 : On deletion, Vmware volume snapshots are left behind with message 'the snapshot has child, can't delete it on the storage' (#2188)
    
    Snapshots are not deleted resulting unexpected storage consumption in case of VMware.
    
    Steps to reproduce this issue :
    
    In VMware setup, create a snapshot of volume say Snap1.
    After successful creation of snapshot Snap1, create new snapshot of same volume say Snap2.snapshots
    While Snap2 is in BackingUp state, delete Snap1.
    Snap1 will disappear from Web UI, but when we check secondary storage, files associated with Snap1 still persists even after cleanup job is performed.
    In snapshot_store_ref table in DB, Snap1 will be in ready state instead of Destroyed.
    Also, in snapshots table, status of Snap1 will be Destroyed but removed column will be null and will never change to the date of snapshot removal.
    Fix for this issue :
    
    In VMware, snapshot chain is not maintained, instead full snapshot is taken every time.
    So, it makes sense not to assign parent snapshot id for the snapshot. In this way, every snapshot will be individual and can be deleted successfully whenever required.
---
 .../storage/image/db/SnapshotDataStoreDaoImpl.java | 68 ++++++++++++++++------
 1 file changed, 51 insertions(+), 17 deletions(-)

diff --git a/engine/storage/src/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java b/engine/storage/src/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java
index 9ffeae0..c3e48b9 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java
@@ -33,6 +33,12 @@ import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.dao.SnapshotDao;
+import javax.inject.Inject;
+import com.cloud.hypervisor.Hypervisor;
+import java.util.ArrayList;
+import com.cloud.utils.db.Filter;
 import javax.naming.ConfigurationException;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
@@ -54,9 +60,14 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
     private SearchBuilder<SnapshotDataStoreVO> volumeIdSearch;
     private SearchBuilder<SnapshotDataStoreVO> volumeSearch;
     private SearchBuilder<SnapshotDataStoreVO> stateSearch;
+    private SearchBuilder<SnapshotDataStoreVO> parentSnapshotSearch;
+    private SearchBuilder<SnapshotVO> snapshotVOSearch;
+
+    public static ArrayList<Hypervisor.HypervisorType> hypervisorsSupportingSnapshotsChaining = new ArrayList<Hypervisor.HypervisorType>();
+
+    @Inject
+    private SnapshotDao _snapshotDao;
 
-    private final String parentSearch = "select store_id, store_role, snapshot_id from cloud.snapshot_store_ref where store_id = ? "
-        + " and store_role = ? and volume_id = ? and state = 'Ready'" + " order by created DESC " + " limit 1";
     private final String findLatestSnapshot = "select store_id, store_role, snapshot_id from cloud.snapshot_store_ref where " +
             " store_role = ? and volume_id = ? and state = 'Ready'" +
             " order by created DESC " +
@@ -128,6 +139,17 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
         stateSearch.and("state", stateSearch.entity().getState(), SearchCriteria.Op.IN);
         stateSearch.done();
 
+        parentSnapshotSearch = createSearchBuilder();
+        parentSnapshotSearch.and("volume_id", parentSnapshotSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
+        parentSnapshotSearch.and("store_id", parentSnapshotSearch.entity().getDataStoreId(), SearchCriteria.Op.EQ);
+        parentSnapshotSearch.and("store_role", parentSnapshotSearch.entity().getRole(), SearchCriteria.Op.EQ);
+        parentSnapshotSearch.and("state", parentSnapshotSearch.entity().getState(), SearchCriteria.Op.EQ);
+        parentSnapshotSearch.done();
+
+        snapshotVOSearch = _snapshotDao.createSearchBuilder();
+        snapshotVOSearch.and("volume_id", snapshotVOSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
+        snapshotVOSearch.done();
+
         return true;
     }
 
@@ -272,22 +294,17 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
     @Override
     @DB
     public SnapshotDataStoreVO findParent(DataStoreRole role, Long storeId, Long volumeId) {
-        TransactionLegacy txn = TransactionLegacy.currentTxn();
-        try (
-                PreparedStatement pstmt = txn.prepareStatement(parentSearch);
-            ){
-            pstmt.setLong(1, storeId);
-            pstmt.setString(2, role.toString());
-            pstmt.setLong(3, volumeId);
-            try (ResultSet rs = pstmt.executeQuery();) {
-                while (rs.next()) {
-                    long sid = rs.getLong(1);
-                    long snid = rs.getLong(3);
-                    return findByStoreSnapshot(role, sid, snid);
-                }
+        if(isSnapshotChainingRequired(volumeId)) {
+            SearchCriteria<SnapshotDataStoreVO> sc = parentSnapshotSearch.create();
+            sc.setParameters("volume_id", volumeId);
+            sc.setParameters("store_role", role.toString());
+            sc.setParameters("state", ObjectInDataStoreStateMachine.State.Ready.name());
+            sc.setParameters("store_id", storeId);
+
+            List<SnapshotDataStoreVO> snapshotList = listBy(sc, new Filter(SnapshotDataStoreVO.class, "created", false, null, null));
+            if (snapshotList != null && snapshotList.size() != 0) {
+                return snapshotList.get(0);
             }
-        } catch (SQLException e) {
-            s_logger.debug("Failed to find parent snapshot: " + e.toString());
         }
         return null;
     }
@@ -419,4 +436,21 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
         sc.setParameters("state", (Object[])states);
         return listBy(sc, null);
     }
+
+    private boolean isSnapshotChainingRequired(long volumeId) {
+
+        hypervisorsSupportingSnapshotsChaining.add(Hypervisor.HypervisorType.XenServer);
+
+        SearchCriteria<SnapshotVO> sc = snapshotVOSearch.create();
+        sc.setParameters("volume_id", volumeId);
+
+        SnapshotVO volSnapshot = _snapshotDao.findOneBy(sc);
+
+        if (volSnapshot != null && hypervisorsSupportingSnapshotsChaining.contains(volSnapshot.getHypervisorType())) {
+            return true;
+        }
+
+        return false;
+    }
+
 }

-- 
To stop receiving notification emails like this one, please contact
['"commits@cloudstack.apache.org" <co...@cloudstack.apache.org>'].