You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by mc...@apache.org on 2013/12/14 03:09:30 UTC

git commit: updated refs/heads/4.3 to 7cd125c

Updated Branches:
  refs/heads/4.3 96678bbbf -> 7cd125ca3


CLOUDSTACK-5446:KVM-Secondary Store down-Even after secondary store is
brought back up after being down for few hours,snapshot jobs do not get
triggered with reason "there is other active snapshot tasks on the
instance to which the volume is attached".


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/7cd125ca
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/7cd125ca
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/7cd125ca

Branch: refs/heads/4.3
Commit: 7cd125ca3905d7d9596bde3a7a4cf19a32a23c9b
Parents: 96678bb
Author: Min Chen <mi...@citrix.com>
Authored: Fri Dec 13 18:08:30 2013 -0800
Committer: Min Chen <mi...@citrix.com>
Committed: Fri Dec 13 18:09:16 2013 -0800

----------------------------------------------------------------------
 .../SnapshotStateMachineManagerImpl.java        |   2 +-
 .../kvm/storage/KVMStorageProcessor.java        | 143 ++++++++++---------
 .../xen/resource/XenServerStorageProcessor.java | 138 ++++++++++--------
 .../storage/snapshot/SnapshotManagerImpl.java   |   2 +-
 4 files changed, 159 insertions(+), 126 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7cd125ca/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotStateMachineManagerImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotStateMachineManagerImpl.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotStateMachineManagerImpl.java
index 555dcb8..a70b078 100644
--- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotStateMachineManagerImpl.java
+++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotStateMachineManagerImpl.java
@@ -44,7 +44,7 @@ public class SnapshotStateMachineManagerImpl implements SnapshotStateMachineMana
         stateMachine.addTransition(Snapshot.State.CreatedOnPrimary, Event.BackupToSecondary, Snapshot.State.BackingUp);
         stateMachine.addTransition(State.CreatedOnPrimary, Event.OperationNotPerformed, State.BackedUp);
         stateMachine.addTransition(Snapshot.State.BackingUp, Event.OperationSucceeded, Snapshot.State.BackedUp);
-        stateMachine.addTransition(Snapshot.State.BackingUp, Event.OperationFailed, Snapshot.State.CreatedOnPrimary);
+        stateMachine.addTransition(Snapshot.State.BackingUp, Event.OperationFailed, Snapshot.State.Error);
         stateMachine.addTransition(Snapshot.State.BackedUp, Event.DestroyRequested, Snapshot.State.Destroying);
         stateMachine.addTransition(Snapshot.State.BackedUp, Event.CopyingRequested, Snapshot.State.Copying);
         stateMachine.addTransition(Snapshot.State.Copying, Event.OperationSucceeded, Snapshot.State.BackedUp);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7cd125ca/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
index 8b8cd9e..3ce9c29 100644
--- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
+++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
@@ -18,6 +18,8 @@
  */
 package com.cloud.hypervisor.kvm.storage;
 
+import static com.cloud.utils.S3Utils.putFile;
+
 import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileNotFoundException;
@@ -35,6 +37,21 @@ import java.util.UUID;
 
 import javax.naming.ConfigurationException;
 
+import org.apache.commons.io.FileUtils;
+import org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo;
+import org.libvirt.DomainSnapshot;
+import org.libvirt.LibvirtException;
+
+import com.ceph.rados.IoCTX;
+import com.ceph.rados.Rados;
+import com.ceph.rados.RadosException;
+import com.ceph.rbd.Rbd;
+import com.ceph.rbd.RbdException;
+import com.ceph.rbd.RbdImage;
+
 import org.apache.cloudstack.storage.command.AttachAnswer;
 import org.apache.cloudstack.storage.command.AttachCommand;
 import org.apache.cloudstack.storage.command.CopyCmdAnswer;
@@ -54,20 +71,7 @@ import org.apache.cloudstack.utils.qemu.QemuImg;
 import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
 import org.apache.cloudstack.utils.qemu.QemuImgException;
 import org.apache.cloudstack.utils.qemu.QemuImgFile;
-import org.apache.commons.io.FileUtils;
-import org.apache.log4j.Logger;
-import org.libvirt.Connect;
-import org.libvirt.Domain;
-import org.libvirt.DomainInfo;
-import org.libvirt.DomainSnapshot;
-import org.libvirt.LibvirtException;
 
-import com.ceph.rados.IoCTX;
-import com.ceph.rados.Rados;
-import com.ceph.rados.RadosException;
-import com.ceph.rbd.Rbd;
-import com.ceph.rbd.RbdException;
-import com.ceph.rbd.RbdImage;
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.storage.PrimaryStorageDownloadAnswer;
 import com.cloud.agent.api.to.DataObjectType;
@@ -96,8 +100,6 @@ import com.cloud.utils.S3Utils;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
 
-import static com.cloud.utils.S3Utils.putFile;
-
 public class KVMStorageProcessor implements StorageProcessor {
     private static final Logger s_logger = Logger.getLogger(KVMStorageProcessor.class);
     private KVMStoragePoolManager storagePoolMgr;
@@ -196,7 +198,7 @@ public class KVMStorageProcessor implements StorageProcessor {
             if (destData instanceof VolumeObjectTO) {
                 VolumeObjectTO volume = (VolumeObjectTO) destData;
                 primaryVol = storagePoolMgr.copyPhysicalDisk(tmplVol, volume.getUuid(),
-                    primaryPool, cmd.getWaitInMillSeconds()); 
+                    primaryPool, cmd.getWaitInMillSeconds());
             } else if (destData instanceof TemplateObjectTO) {
                 TemplateObjectTO destTempl = (TemplateObjectTO) destData;
                 primaryVol = storagePoolMgr.copyPhysicalDisk(tmplVol, destTempl.getUuid(),
@@ -476,7 +478,7 @@ public class KVMStorageProcessor implements StorageProcessor {
 
             KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
             String tmpltPath = secondaryStorage.getLocalPath() + File.separator + templateFolder;
-            this.storageLayer.mkdirs(tmpltPath);
+            storageLayer.mkdirs(tmpltPath);
             String templateName = UUID.randomUUID().toString();
 
             if (primary.getType() != StoragePoolType.RBD) {
@@ -527,14 +529,14 @@ public class KVMStorageProcessor implements StorageProcessor {
             }
 
             Map<String, Object> params = new HashMap<String, Object>();
-            params.put(StorageLayer.InstanceConfigKey, this.storageLayer);
+            params.put(StorageLayer.InstanceConfigKey, storageLayer);
             Processor qcow2Processor = new QCOW2Processor();
 
             qcow2Processor.configure("QCOW2 Processor", params);
 
             FormatInfo info = qcow2Processor.process(tmpltPath, null, templateName);
 
-            TemplateLocation loc = new TemplateLocation(this.storageLayer, tmpltPath);
+            TemplateLocation loc = new TemplateLocation(storageLayer, tmpltPath);
             loc.create(1, true, templateName);
             loc.addFormat(info);
             loc.save();
@@ -656,8 +658,11 @@ public class KVMStorageProcessor implements StorageProcessor {
         String snapshotRelPath = null;
         String vmName = snapshot.getVmName();
         KVMStoragePool secondaryStoragePool = null;
+        Connect conn = null;
+        KVMPhysicalDisk snapshotDisk = null;
+        KVMStoragePool primaryPool = null;
         try {
-            Connect conn = LibvirtConnection.getConnectionByVmName(vmName);
+            conn = LibvirtConnection.getConnectionByVmName(vmName);
 
             secondaryStoragePool = storagePoolMgr.getStoragePoolByURI(secondaryStoragePoolUrl);
 
@@ -665,9 +670,9 @@ public class KVMStorageProcessor implements StorageProcessor {
             snapshotRelPath = destSnapshot.getPath();
 
             snapshotDestPath = ssPmountPath + File.separator + snapshotRelPath;
-            KVMPhysicalDisk snapshotDisk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(),
+            snapshotDisk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(),
                     primaryStore.getUuid(), volumePath);
-            KVMStoragePool primaryPool = snapshotDisk.getPool();
+            primaryPool = snapshotDisk.getPool();
 
             /**
              * RBD snapshots can't be copied using qemu-img, so we have to use
@@ -745,47 +750,6 @@ public class KVMStorageProcessor implements StorageProcessor {
                 }
             }
 
-            /* Delete the snapshot on primary */
-
-            DomainInfo.DomainState state = null;
-            Domain vm = null;
-            if (vmName != null) {
-                try {
-                    vm = this.resource.getDomain(conn, vmName);
-                    state = vm.getInfo().state;
-                } catch (LibvirtException e) {
-                    s_logger.trace("Ignoring libvirt error.", e);
-                }
-            }
-
-            KVMStoragePool primaryStorage = storagePoolMgr.getStoragePool(primaryStore.getPoolType(),
-                    primaryStore.getUuid());
-            if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryStorage.isExternalSnapshot()) {
-                DomainSnapshot snap = vm.snapshotLookupByName(snapshotName);
-                snap.delete(0);
-
-                /*
-                 * libvirt on RHEL6 doesn't handle resume event emitted from
-                 * qemu
-                 */
-                vm = this.resource.getDomain(conn, vmName);
-                state = vm.getInfo().state;
-                if (state == DomainInfo.DomainState.VIR_DOMAIN_PAUSED) {
-                    vm.resume();
-                }
-            } else {
-                if (primaryPool.getType() != StoragePoolType.RBD) {
-                    Script command = new Script(_manageSnapshotPath, _cmdsTimeout, s_logger);
-                    command.add("-d", snapshotDisk.getPath());
-                    command.add("-n", snapshotName);
-                    String result = command.execute();
-                    if (result != null) {
-                        s_logger.debug("Failed to backup snapshot: " + result);
-                        return new CopyCmdAnswer("Failed to backup snapshot: " + result);
-                    }
-                }
-            }
-
             SnapshotObjectTO newSnapshot = new SnapshotObjectTO();
             newSnapshot.setPath(snapshotRelPath + File.separator + snapshotName);
             return new CopyCmdAnswer(newSnapshot);
@@ -796,6 +760,49 @@ public class KVMStorageProcessor implements StorageProcessor {
             s_logger.debug("Failed to backup snapshot: " + e.toString());
             return new CopyCmdAnswer(e.toString());
         } finally {
+            try {
+                /* Delete the snapshot on primary */
+                DomainInfo.DomainState state = null;
+                Domain vm = null;
+                if (vmName != null) {
+                    try {
+                        vm = resource.getDomain(conn, vmName);
+                        state = vm.getInfo().state;
+                    } catch (LibvirtException e) {
+                        s_logger.trace("Ignoring libvirt error.", e);
+                    }
+                }
+
+                KVMStoragePool primaryStorage = storagePoolMgr.getStoragePool(primaryStore.getPoolType(),
+                        primaryStore.getUuid());
+                if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryStorage.isExternalSnapshot()) {
+                    DomainSnapshot snap = vm.snapshotLookupByName(snapshotName);
+                    snap.delete(0);
+
+                    /*
+                     * libvirt on RHEL6 doesn't handle resume event emitted from
+                     * qemu
+                     */
+                    vm = resource.getDomain(conn, vmName);
+                    state = vm.getInfo().state;
+                    if (state == DomainInfo.DomainState.VIR_DOMAIN_PAUSED) {
+                        vm.resume();
+                    }
+                } else {
+                    if (primaryPool.getType() != StoragePoolType.RBD) {
+                        Script command = new Script(_manageSnapshotPath, _cmdsTimeout, s_logger);
+                        command.add("-d", snapshotDisk.getPath());
+                        command.add("-n", snapshotName);
+                        String result = command.execute();
+                        if (result != null) {
+                            s_logger.debug("Failed to delete snapshot on primary: " + result);
+                            // return new CopyCmdAnswer("Failed to backup snapshot: " + result);
+                        }
+                    }
+                }
+            } catch (Exception ex) {
+                s_logger.debug("Failed to delete snapshots on primary", ex);
+            }
             if (secondaryStoragePool != null) {
                 secondaryStoragePool.delete();
             }
@@ -823,12 +830,12 @@ public class KVMStorageProcessor implements StorageProcessor {
             isoXml = iso.toString();
         }
 
-        List<DiskDef> disks = this.resource.getDisks(conn, vmName);
+        List<DiskDef> disks = resource.getDisks(conn, vmName);
         String result = attachOrDetachDevice(conn, true, vmName, isoXml);
         if (result == null && !isAttach) {
             for (DiskDef disk : disks) {
                 if (disk.getDeviceType() == DiskDef.deviceType.CDROM) {
-                    this.resource.cleanupDisk(disk);
+                    resource.cleanupDisk(disk);
                 }
             }
 
@@ -1063,7 +1070,7 @@ public class KVMStorageProcessor implements StorageProcessor {
             Domain vm = null;
             if (vmName != null) {
                 try {
-                    vm = this.resource.getDomain(conn, vmName);
+                    vm = resource.getDomain(conn, vmName);
                     state = vm.getInfo().state;
                 } catch (LibvirtException e) {
                     s_logger.trace("Ignoring libvirt error.", e);
@@ -1086,7 +1093,7 @@ public class KVMStorageProcessor implements StorageProcessor {
                  * libvirt on RHEL6 doesn't handle resume event emitted from
                  * qemu
                  */
-                vm = this.resource.getDomain(conn, vmName);
+                vm = resource.getDomain(conn, vmName);
                 state = vm.getInfo().state;
                 if (state == DomainInfo.DomainState.VIR_DOMAIN_PAUSED) {
                     vm.resume();
@@ -1127,7 +1134,7 @@ public class KVMStorageProcessor implements StorageProcessor {
                     }
                 } else {
                     /* VM is not running, create a snapshot by ourself */
-                    final Script command = new Script(_manageSnapshotPath, this._cmdsTimeout, s_logger);
+                    final Script command = new Script(_manageSnapshotPath, _cmdsTimeout, s_logger);
                     command.add("-c", disk.getPath());
                     command.add("-n", snapshotName);
                     String result = command.execute();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7cd125ca/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java
index 5a19aee..daa2a84 100644
--- a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java
+++ b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java
@@ -22,7 +22,6 @@ package com.cloud.hypervisor.xen.resource;
 import static com.cloud.utils.ReflectUtil.flattenProperties;
 import static com.google.common.collect.Lists.newArrayList;
 
-
 import java.io.File;
 import java.net.URI;
 import java.util.Arrays;
@@ -32,6 +31,22 @@ import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 
+import org.apache.log4j.Logger;
+import org.apache.xmlrpc.XmlRpcException;
+
+import com.xensource.xenapi.Connection;
+import com.xensource.xenapi.Host;
+import com.xensource.xenapi.PBD;
+import com.xensource.xenapi.Pool;
+import com.xensource.xenapi.SR;
+import com.xensource.xenapi.Types;
+import com.xensource.xenapi.Types.BadServerResponse;
+import com.xensource.xenapi.Types.XenAPIException;
+import com.xensource.xenapi.VBD;
+import com.xensource.xenapi.VDI;
+import com.xensource.xenapi.VM;
+import com.xensource.xenapi.VMGuestMetrics;
+
 import org.apache.cloudstack.storage.command.AttachAnswer;
 import org.apache.cloudstack.storage.command.AttachCommand;
 import org.apache.cloudstack.storage.command.AttachPrimaryDataStoreAnswer;
@@ -50,8 +65,6 @@ import org.apache.cloudstack.storage.datastore.protocol.DataStoreProtocol;
 import org.apache.cloudstack.storage.to.SnapshotObjectTO;
 import org.apache.cloudstack.storage.to.TemplateObjectTO;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
-import org.apache.log4j.Logger;
-import org.apache.xmlrpc.XmlRpcException;
 
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.CreateStoragePoolCommand;
@@ -74,18 +87,6 @@ import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.storage.encoding.DecodedDataObject;
 import com.cloud.utils.storage.encoding.DecodedDataStore;
 import com.cloud.utils.storage.encoding.Decoder;
-import com.xensource.xenapi.Connection;
-import com.xensource.xenapi.Host;
-import com.xensource.xenapi.PBD;
-import com.xensource.xenapi.Pool;
-import com.xensource.xenapi.SR;
-import com.xensource.xenapi.Types;
-import com.xensource.xenapi.Types.BadServerResponse;
-import com.xensource.xenapi.Types.XenAPIException;
-import com.xensource.xenapi.VBD;
-import com.xensource.xenapi.VDI;
-import com.xensource.xenapi.VM;
-import com.xensource.xenapi.VMGuestMetrics;
 
 public class XenServerStorageProcessor implements StorageProcessor {
     private static final Logger s_logger = Logger.getLogger(XenServerStorageProcessor.class);
@@ -93,7 +94,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
     private String BaseMountPointOnHost = "/var/run/cloud_mount";
 
     public XenServerStorageProcessor(CitrixResourceBase resource) {
-        this.hypervisorResource = resource;
+        hypervisorResource = resource;
     }
 
     @Override
@@ -117,14 +118,14 @@ public class XenServerStorageProcessor implements StorageProcessor {
 
         String vmName = cmd.getVmName();
         try {
-            Connection conn = this.hypervisorResource.getConnection();
+            Connection conn = hypervisorResource.getConnection();
 
             VBD isoVBD = null;
 
             // Find the VM
-            VM vm = this.hypervisorResource.getVM(conn, vmName);
+            VM vm = hypervisorResource.getVM(conn, vmName);
             // Find the ISO VDI
-            VDI isoVDI = this.hypervisorResource.getIsoVDIByURL(conn, vmName, isoURL);
+            VDI isoVDI = hypervisorResource.getIsoVDIByURL(conn, vmName, isoURL);
 
             // Find the VM's CD-ROM VBD
             Set<VBD> vbds = vm.getVBDs(conn);
@@ -169,7 +170,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
         DataTO data = disk.getData();
 
         try {
-            Connection conn = this.hypervisorResource.getConnection();
+            Connection conn = hypervisorResource.getConnection();
 
             VDI vdi = null;
 
@@ -183,21 +184,21 @@ public class XenServerStorageProcessor implements StorageProcessor {
                 String chapInitiatorSecret = disk.getDetails().get(DiskTO.CHAP_INITIATOR_SECRET);
                 Long volumeSize = Long.parseLong(details.get(DiskTO.VOLUME_SIZE));
 
-                SR sr = this.hypervisorResource.getIscsiSR(conn, iScsiName, storageHost, iScsiName,
+                SR sr = hypervisorResource.getIscsiSR(conn, iScsiName, storageHost, iScsiName,
                             chapInitiatorUsername, chapInitiatorSecret, true);
 
-                vdi = this.hypervisorResource.getVDIbyUuid(conn, data.getPath(), false);
+                vdi = hypervisorResource.getVDIbyUuid(conn, data.getPath(), false);
 
                 if (vdi == null) {
-                    vdi = this.hypervisorResource.createVdi(sr, vdiNameLabel, volumeSize);
+                    vdi = hypervisorResource.createVdi(sr, vdiNameLabel, volumeSize);
                 }
             }
             else {
-                vdi = this.hypervisorResource.mount(conn, null, null, data.getPath());
+                vdi = hypervisorResource.mount(conn, null, null, data.getPath());
             }
 
             // Look up the VM
-            VM vm = this.hypervisorResource.getVM(conn, vmName);
+            VM vm = hypervisorResource.getVM(conn, vmName);
             /* For HVM guest, if no pv driver installed, no attach/detach */
             boolean isHVM;
             if (vm.getPVBootloader(conn).equalsIgnoreCase("")) {
@@ -207,7 +208,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
             }
             VMGuestMetrics vgm = vm.getGuestMetrics(conn);
             boolean pvDrvInstalled = false;
-            if (!this.hypervisorResource.isRefNull(vgm) && vgm.getPVDriversUpToDate(conn)) {
+            if (!hypervisorResource.isRefNull(vgm) && vgm.getPVDriversUpToDate(conn)) {
                 pvDrvInstalled = true;
             }
             if (isHVM && !pvDrvInstalled) {
@@ -223,13 +224,13 @@ public class XenServerStorageProcessor implements StorageProcessor {
                     String msg = "Device 3 is reserved for CD-ROM, choose other device";
                     return new AttachAnswer(msg);
                 }
-                if(this.hypervisorResource.isDeviceUsed(conn, vm, deviceId)) {
+                if(hypervisorResource.isDeviceUsed(conn, vm, deviceId)) {
                     String msg = "Device " + deviceId + " is used in VM " + vmName;
                     return new AttachAnswer(msg);
                 }
                 diskNumber = deviceId.toString();
             } else {
-                diskNumber = this.hypervisorResource.getUnusedDeviceNum(conn, vm);
+                diskNumber = hypervisorResource.getUnusedDeviceNum(conn, vm);
             }
             // Create a new VBD
             VBD.Record vbdr = new VBD.Record();
@@ -281,13 +282,13 @@ public class XenServerStorageProcessor implements StorageProcessor {
         }
 
         try {
-            Connection conn = this.hypervisorResource.getConnection();
+            Connection conn = hypervisorResource.getConnection();
             // Find the VM
-            VM vm = this.hypervisorResource.getVM(conn, cmd.getVmName());
+            VM vm = hypervisorResource.getVM(conn, cmd.getVmName());
             String vmUUID = vm.getUuid(conn);
 
             // Find the ISO VDI
-            VDI isoVDI = this.hypervisorResource.getIsoVDIByURL(conn, cmd.getVmName(), isoURL);
+            VDI isoVDI = hypervisorResource.getIsoVDIByURL(conn, cmd.getVmName(), isoURL);
 
             SR sr = isoVDI.getSR(conn);
 
@@ -310,7 +311,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
             }
 
             if (!sr.getNameLabel(conn).startsWith("XenServer Tools")) {
-                this.hypervisorResource.removeSR(conn, sr);
+                hypervisorResource.removeSR(conn, sr);
             }
 
             return new DettachAnswer(disk);
@@ -332,11 +333,11 @@ public class XenServerStorageProcessor implements StorageProcessor {
         DiskTO disk = cmd.getDisk();
         DataTO data = disk.getData();
         try {
-            Connection conn = this.hypervisorResource.getConnection();
+            Connection conn = hypervisorResource.getConnection();
             // Look up the VDI
-            VDI vdi = this.hypervisorResource.mount(conn, null, null, data.getPath());
+            VDI vdi = hypervisorResource.mount(conn, null, null, data.getPath());
             // Look up the VM
-            VM vm = this.hypervisorResource.getVM(conn, vmName);
+            VM vm = hypervisorResource.getVM(conn, vmName);
             /* For HVM guest, if no pv driver installed, no attach/detach */
             boolean isHVM;
             if (vm.getPVBootloader(conn).equalsIgnoreCase("")) {
@@ -346,7 +347,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
             }
             VMGuestMetrics vgm = vm.getGuestMetrics(conn);
             boolean pvDrvInstalled = false;
-            if (!this.hypervisorResource.isRefNull(vgm) && vgm.getPVDriversUpToDate(conn)) {
+            if (!hypervisorResource.isRefNull(vgm) && vgm.getPVDriversUpToDate(conn)) {
                 pvDrvInstalled = true;
             }
             if (isHVM && !pvDrvInstalled) {
@@ -372,10 +373,10 @@ public class XenServerStorageProcessor implements StorageProcessor {
             // Update the VDI's label to be "detached"
             vdi.setNameLabel(conn, "detached");
 
-            this.hypervisorResource.umount(conn, vdi);
+            hypervisorResource.umount(conn, vdi);
 
             if (cmd.isManaged()) {
-                this.hypervisorResource.handleSrAndVdiDetach(cmd.get_iScsiName());
+                hypervisorResource.handleSrAndVdiDetach(cmd.get_iScsiName());
             }
 
             return new DettachAnswer(disk);
@@ -581,13 +582,13 @@ public class XenServerStorageProcessor implements StorageProcessor {
                     }
                     if (target.equals(dc.get("target")) && targetiqn.equals(dc.get("targetIQN")) && lunid.equals(dc.get("lunid"))) {
                         throw new CloudRuntimeException("There is a SR using the same configuration target:" + dc.get("target") +  ",  targetIQN:"
-                                + dc.get("targetIQN")  + ", lunid:" + dc.get("lunid") + " for pool " + pool.getUuid() + "on host:" + this.hypervisorResource.getHost().uuid);
+                                + dc.get("targetIQN")  + ", lunid:" + dc.get("lunid") + " for pool " + pool.getUuid() + "on host:" + hypervisorResource.getHost().uuid);
                     }
                 }
                 deviceConfig.put("target", target);
                 deviceConfig.put("targetIQN", targetiqn);
 
-                Host host = Host.getByUuid(conn, this.hypervisorResource.getHost().uuid);
+                Host host = Host.getByUuid(conn, hypervisorResource.getHost().uuid);
                 Map<String, String> smConfig = new HashMap<String, String>();
                 String type = SRType.LVMOISCSI.toString();
                 String poolId = Long.toString(pool.getId());
@@ -660,7 +661,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
         }
     }
     protected Answer execute(CreateStoragePoolCommand cmd) {
-        Connection conn = this.hypervisorResource.getConnection();
+        Connection conn = hypervisorResource.getConnection();
         StorageFilerTO pool = cmd.getPool();
         try {
             if (pool.getType() == StoragePoolType.NetworkFilesystem) {
@@ -673,7 +674,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
             }
             return new Answer(cmd, true, "success");
         } catch (Exception e) {
-            String msg = "Catch Exception " + e.getClass().getName() + ", create StoragePool failed due to " + e.toString() + " on host:" + this.hypervisorResource.getHost().uuid + " pool: " + pool.getHost() + pool.getPath();
+            String msg = "Catch Exception " + e.getClass().getName() + ", create StoragePool failed due to " + e.toString() + " on host:" + hypervisorResource.getHost().uuid + " pool: " + pool.getHost() + pool.getPath();
             s_logger.warn(msg, e);
             return new Answer(cmd, false, msg);
         }
@@ -1169,7 +1170,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
             Set<VDI> snapshots = volume.getSnapshots(conn);
             for( VDI snapshot : snapshots ) {
                 try {
-                    if(! snapshot.getUuid(conn).equals(avoidSnapshotUuid)) {
+                    if (!snapshot.getUuid(conn).equals(avoidSnapshotUuid)) {
                         snapshot.destroy(conn);
                     }
                 } catch (Exception e) {
@@ -1190,6 +1191,26 @@ public class XenServerStorageProcessor implements StorageProcessor {
         return false;
     }
 
+    private boolean destroySnapshotOnPrimaryStorage(Connection conn, String lastSnapshotUuid) {
+        try {
+            VDI snapshot = getVDIbyUuid(conn, lastSnapshotUuid);
+            if (snapshot == null) {
+                // since this is just used to cleanup leftover bad snapshots, no need to throw exception
+                s_logger.warn("Could not destroy snapshot " + lastSnapshotUuid + " due to can not find it");
+                return false;
+            }
+            snapshot.destroy(conn);
+            return true;
+        } catch (XenAPIException e) {
+            String msg = "Destroying snapshot: " + lastSnapshotUuid + " failed due to " + e.toString();
+            s_logger.error(msg, e);
+        } catch (Exception e) {
+            String msg = "Destroying snapshot: " + lastSnapshotUuid + " failed due to " + e.toString();
+            s_logger.warn(msg, e);
+        }
+        return false;
+    }
+
     @Override
     public Answer backupSnapshot(CopyCommand cmd) {
         Connection conn = hypervisorResource.getConnection();
@@ -1214,6 +1235,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
         SnapshotObjectTO snapshotTO = (SnapshotObjectTO)srcData;
         SnapshotObjectTO snapshotOnImage = (SnapshotObjectTO)destData;
         String snapshotUuid = snapshotTO.getPath();
+        String volumeUuid = snapshotTO.getVolume().getPath();
 
         String prevBackupUuid = snapshotOnImage.getParentSnapshotPath();
         String prevSnapshotUuid = snapshotTO.getParentSnapshotPath();
@@ -1323,7 +1345,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
                     finalPath = folder + File.separator + snapshotBackupUuid;
                 }
             }
-            String volumeUuid = snapshotTO.getVolume().getPath();
+            // delete primary snapshots with only the last one left
             destroySnapshotOnPrimaryStorageExceptThis(conn, volumeUuid, snapshotUuid);
 
             SnapshotObjectTO newSnapshot = new SnapshotObjectTO();
@@ -1337,9 +1359,13 @@ public class XenServerStorageProcessor implements StorageProcessor {
         } catch (XenAPIException e) {
             details = "BackupSnapshot Failed due to " + e.toString();
             s_logger.warn(details, e);
+            // remove last bad primary snapshot when exception happens
+            destroySnapshotOnPrimaryStorage(conn, snapshotUuid);
         } catch (Exception e) {
             details = "BackupSnapshot Failed due to " + e.getMessage();
             s_logger.warn(details, e);
+            // remove last bad primary snapshot when exception happens
+            destroySnapshotOnPrimaryStorage(conn, snapshotUuid);
         }
 
         return new CopyCmdAnswer(details);
@@ -1347,7 +1373,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
 
     @Override
     public Answer createTemplateFromVolume(CopyCommand cmd) {
-        Connection conn = this.hypervisorResource.getConnection();
+        Connection conn = hypervisorResource.getConnection();
         VolumeObjectTO volume = (VolumeObjectTO)cmd.getSrcTO();
         TemplateObjectTO template = (TemplateObjectTO)cmd.getDestTO();
         NfsTO destStore = (NfsTO)cmd.getDestTO().getDataStore();
@@ -1368,7 +1394,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
             URI uri = new URI(secondaryStoragePoolURL);
             secondaryStorageMountPath = uri.getHost() + ":" + uri.getPath();
             installPath = template.getPath();
-            if( !this.hypervisorResource.createSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath)) {
+            if( !hypervisorResource.createSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath)) {
                 details = " Filed to create folder " + installPath + " in secondary storage";
                 s_logger.warn(details);
                 return new CopyCmdAnswer(details);
@@ -1377,10 +1403,10 @@ public class XenServerStorageProcessor implements StorageProcessor {
             VDI vol = getVDIbyUuid(conn, volumeUUID);
             // create template SR
             URI tmpltURI = new URI(secondaryStoragePoolURL + "/" + installPath);
-            tmpltSR = this.hypervisorResource.createNfsSRbyURI(conn, tmpltURI, false);
+            tmpltSR = hypervisorResource.createNfsSRbyURI(conn, tmpltURI, false);
 
             // copy volume to template SR
-            VDI tmpltVDI = this.hypervisorResource.cloudVDIcopy(conn, vol, tmpltSR, wait);
+            VDI tmpltVDI = hypervisorResource.cloudVDIcopy(conn, vol, tmpltSR, wait);
             // scan makes XenServer pick up VDI physicalSize
             tmpltSR.scan(conn);
             if (userSpecifiedName != null) {
@@ -1393,12 +1419,12 @@ public class XenServerStorageProcessor implements StorageProcessor {
             long physicalSize = tmpltVDI.getPhysicalUtilisation(conn);
             // create the template.properties file
             String templatePath = secondaryStorageMountPath + "/" + installPath;
-            result = this.hypervisorResource.postCreatePrivateTemplate(conn, templatePath, tmpltFilename, tmpltUUID, userSpecifiedName, null, physicalSize, virtualSize, template.getId());
+            result = hypervisorResource.postCreatePrivateTemplate(conn, templatePath, tmpltFilename, tmpltUUID, userSpecifiedName, null, physicalSize, virtualSize, template.getId());
             if (!result) {
                 throw new CloudRuntimeException("Could not create the template.properties file on secondary storage dir: " + tmpltURI);
             }
             installPath = installPath + "/" + tmpltFilename;
-            this.hypervisorResource.removeSR(conn, tmpltSR);
+            hypervisorResource.removeSR(conn, tmpltSR);
             tmpltSR = null;
             TemplateObjectTO newTemplate = new TemplateObjectTO();
             newTemplate.setPath(installPath);
@@ -1410,10 +1436,10 @@ public class XenServerStorageProcessor implements StorageProcessor {
             return answer;
         } catch (Exception e) {
             if (tmpltSR != null) {
-                this.hypervisorResource.removeSR(conn, tmpltSR);
+                hypervisorResource.removeSR(conn, tmpltSR);
             }
             if ( secondaryStorageMountPath != null) {
-                this.hypervisorResource.deleteSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath);
+                hypervisorResource.deleteSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath);
             }
             details = "Creating template from volume " + volumeUUID + " failed due to " + e.toString();
             s_logger.error(details, e);
@@ -1428,7 +1454,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
 
     @Override
     public Answer createVolumeFromSnapshot(CopyCommand cmd) {
-        Connection conn = this.hypervisorResource.getConnection();
+        Connection conn = hypervisorResource.getConnection();
         DataTO srcData = cmd.getSrcTO();
         SnapshotObjectTO snapshot = (SnapshotObjectTO)srcData;
         DataTO destData = cmd.getDestTO();
@@ -1452,7 +1478,7 @@ public class XenServerStorageProcessor implements StorageProcessor {
             return new CopyCmdAnswer(details);
         }
         try {
-            SR primaryStorageSR = this.hypervisorResource.getSRByNameLabelandHost(conn, primaryStorageNameLabel);
+            SR primaryStorageSR = hypervisorResource.getSRByNameLabelandHost(conn, primaryStorageNameLabel);
             if (primaryStorageSR == null) {
                 throw new InternalErrorException("Could not create volume from snapshot because the primary Storage SR could not be created from the name label: "
                         + primaryStorageNameLabel);
@@ -1497,14 +1523,14 @@ public class XenServerStorageProcessor implements StorageProcessor {
         SnapshotObjectTO snapshot = (SnapshotObjectTO)cmd.getData();
         DataStoreTO store = snapshot.getDataStore();
         if (store.getRole() == DataStoreRole.Primary) {
-            Connection conn = this.hypervisorResource.getConnection();
+            Connection conn = hypervisorResource.getConnection();
             VDI snapshotVdi = getVDIbyUuid(conn, snapshot.getPath());
             if (snapshotVdi == null) {
                 return new Answer(null);
             }
             String errMsg = null;
             try {
-                this.deleteVDI(conn, snapshotVdi);
+                deleteVDI(conn, snapshotVdi);
             } catch (BadServerResponse e) {
                 s_logger.debug("delete snapshot failed:" + e.toString());
                 errMsg = e.toString();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7cd125ca/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
index 4cabd55..70fca7b 100755
--- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
+++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
@@ -916,7 +916,7 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
                         || userVm.getHypervisorType() == HypervisorType.KVM) {
                     List<SnapshotVO> activeSnapshots = _snapshotDao.listByInstanceId(volume.getInstanceId(),
                             Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp);
-                    if (activeSnapshots.size() > 1) {
+                    if (activeSnapshots.size() > 0) {
                         throw new CloudRuntimeException(
                                 "There is other active snapshot tasks on the instance to which the volume is attached, please try again later");
                     }