From 94776fbfd030528feccacdfc2d1ed85a38858f3f Mon Sep 17 00:00:00 2001
From: Khosrow Moossavi <372575+khos2ow@users.noreply.github.com>
Date: Tue, 6 Feb 2018 14:09:40 -0500
Subject: [PATCH] CLOUDSTACK-10222: Clean snaphosts from primary storage when
 taking new (#2398)

When user creates a snapshot (manual or recurring), snapshot remains on
the primary storage, even if the snapshot is transferred successfully to
secondary storage. This is causing issues because XenServer can only hold
a limited number of snapshots in its VDI chain, preventing the user from
creating new snapshots after some time, when too many old snapshots are
present on the primary storage.
---
 .../resource/XenServerStorageProcessor.java     |  2 +-
 .../resource/Xenserver625StorageProcessor.java  | 17 +++++++++--------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java
index 257c6a23e41..b575fdb4f02 100644
--- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java
+++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java
@@ -1071,7 +1071,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
         return false;
     }
 
-    private boolean destroySnapshotOnPrimaryStorage(final Connection conn, final String lastSnapshotUuid) {
+    protected boolean destroySnapshotOnPrimaryStorage(final Connection conn, final String lastSnapshotUuid) {
         try {
             final VDI snapshot = getVDIbyUuid(conn, lastSnapshotUuid);
             if (snapshot == null) {
diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java
index ca58e4a4154..dea1752abd7 100644
--- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java
+++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java
@@ -559,12 +559,12 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
                     physicalSize = Long.parseLong(tmp[1]);
                     finalPath = folder + File.separator + snapshotBackupUuid + ".vhd";
                 }
-
-                final String volumeUuid = snapshotTO.getVolume().getPath();
-
-                destroySnapshotOnPrimaryStorageExceptThis(conn, volumeUuid, snapshotUuid);
             }
 
+            // remove every snapshot except this one from primary storage
+            final String volumeUuid = snapshotTO.getVolume().getPath();
+            destroySnapshotOnPrimaryStorageExceptThis(conn, volumeUuid, snapshotUuid);
+
             final SnapshotObjectTO newSnapshot = new SnapshotObjectTO();
             newSnapshot.setPath(finalPath);
             newSnapshot.setPhysicalSize(physicalSize);
@@ -577,12 +577,13 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
             s_logger.info("New snapshot physical utilization: "+physicalSize);
 
             return new CopyCmdAnswer(newSnapshot);
-        } catch (final Types.XenAPIException e) {
-            details = "BackupSnapshot Failed due to " + e.toString();
-            s_logger.warn(details, e);
         } catch (final Exception e) {
-            details = "BackupSnapshot Failed due to " + e.getMessage();
+            final String reason = e instanceof Types.XenAPIException ? e.toString() : e.getMessage();
+            details = "BackupSnapshot Failed due to " + reason;
             s_logger.warn(details, e);
+
+            // remove last bad primary snapshot when exception happens
+            destroySnapshotOnPrimaryStorage(conn, snapshotUuid);
         }
 
         return new CopyCmdAnswer(details);