Merge pull request #1941 from anshul1886/CLOUDSTACK-8663

CLOUDSTACK-8663: Fixed various issues to allow VM snapshots and volumesnapshots to exist together

Reverting VM to disk only snapshot in Xenserver corrupts VM

Stale NFS secondary storage on XS leads to volume creation failure from snapshot

Fixed various concerns raised in #672

* pr/1941:
  CLOUDSTACK-8663: Fixed various issues to allow VM snapshots and volume snapshots to exist together

Signed-off-by: Rajani Karuturi <rajani.karuturi@accelerite.com>

Author: karuturi

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java

@@ -29,6 +29,8 @@
 import java.net.URL;
 import java.net.URLConnection;
 import java.nio.charset.Charset;
+
+import org.apache.commons.collections.MapUtils;
 import org.joda.time.Duration;
 import java.util.ArrayList;
 import java.util.Date;
@@ -1415,6 +1417,10 @@ public VM createWorkingVM(final Connection conn, final String vmName, final Stri
             vbdr.userdevice = "autodetect";
             vbdr.mode = Types.VbdMode.RW;
             vbdr.type = Types.VbdType.DISK;
+            Long deviceId = volumeTO.getDeviceId();
+            if (deviceId != null && (!isDeviceUsed(conn, vm, deviceId) || deviceId > 3)) {
+                vbdr.userdevice = deviceId.toString();
+            }
             VBD.create(conn, vbdr);
         }
         return vm;
@@ -4350,6 +4356,30 @@ public void rebootVM(final Connection conn, final VM vm, final String vmName) th
         }
     }
 
+    protected void skipOrRemoveSR(Connection conn, SR sr) {
+        if (sr == null) {
+            return;
+        }
+        if (s_logger.isDebugEnabled()) {
+            s_logger.debug(logX(sr, "Removing SR"));
+        }
+        try {
+            Set<VDI> vdis = sr.getVDIs(conn);
+            for (VDI vdi : vdis) {
+                if (MapUtils.isEmpty(vdi.getCurrentOperations(conn))) {
+                    continue;
+                }
+                return;
+            }
+            removeSR(conn, sr);
+            return;
+        } catch (XenAPIException | XmlRpcException e) {
+            s_logger.warn(logX(sr, "Unable to get current opertions " + e.toString()), e);
+        }
+        String msg = "Remove SR failed";
+        s_logger.warn(msg);
+    }
+
     public void removeSR(final Connection conn, final SR sr) {
         if (sr == null) {
             return;

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java

@@ -1199,7 +1199,7 @@ protected boolean destroySnapshotOnPrimaryStorageExceptThis(final Connection con
             final Set<VDI> snapshots = volume.getSnapshots(conn);
             for (final VDI snapshot : snapshots) {
                 try {
-                    if (!snapshot.getUuid(conn).equals(avoidSnapshotUuid) && snapshot.getSnapshotTime(conn).before(avoidSnapshot.getSnapshotTime(conn))) {
+                    if (!snapshot.getUuid(conn).equals(avoidSnapshotUuid) && snapshot.getSnapshotTime(conn).before(avoidSnapshot.getSnapshotTime(conn)) && snapshot.getVBDs(conn).isEmpty()) {
                         snapshot.destroy(conn);
                     }
                 } catch (final Exception e) {

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java

@@ -738,7 +738,7 @@ public Answer createVolumeFromSnapshot(final CopyCommand cmd) {
             s_logger.warn(details, e);
         } finally {
             if (srcSr != null) {
-                hypervisorResource.removeSR(conn, srcSr);
+                hypervisorResource.skipOrRemoveSR(conn, srcSr);
             }
             if (!result && destVdi != null) {
                 try {

server/src/com/cloud/storage/VolumeApiServiceImpl.java

@@ -2146,13 +2146,6 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName)
             throw new InvalidParameterValueException("Can't find zone by id " + volume.getDataCenterId());
         }
 
-        if (volume.getInstanceId() != null) {
-            // Check that Vm to which this volume is attached does not have VM Snapshots
-            if (_vmSnapshotDao.findByVm(volume.getInstanceId()).size() > 0) {
-                throw new InvalidParameterValueException("Volume snapshot is not allowed, please detach it from VM with VM Snapshots");
-            }
-        }
-
         if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) {
             throw new PermissionDeniedException("Cannot perform this operation, Zone is currently disabled: " + zone.getName());
         }

server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java

@@ -263,11 +263,6 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc
             throw new InvalidParameterValueException("Creating VM snapshot failed due to VM:" + vmId + " is a system VM or does not exist");
         }
 
-        if (_snapshotDao.listByInstanceId(vmId, Snapshot.State.BackedUp).size() > 0) {
-            throw new InvalidParameterValueException(
-                    "VM snapshot for this VM is not allowed. This VM has volumes attached which has snapshots, please remove all snapshots before taking VM snapshot");
-        }
-
         // VM snapshot with memory is not supported for VGPU Vms
         if (snapshotMemory && _serviceOfferingDetailsDao.findDetail(userVmVo.getServiceOfferingId(), GPU.Keys.vgpuType.toString()) != null) {
             throw new InvalidParameterValueException("VM snapshot with MEMORY is not supported for vGPU enabled VMs.");