snapshot redesign

1. use full snapshot and delta snapshot, there are maxsnashot/2 of delta snapshot between two full snapshot
2. only support one policy for one volume
3. remove all coleases in secondary storage.

core/src/com/cloud/agent/api/BackupSnapshotCommand.java

@@ -27,7 +27,6 @@ package com.cloud.agent.api;
 public class BackupSnapshotCommand extends SnapshotCommand {
     private String prevSnapshotUuid;
     private String prevBackupUuid;
-    private boolean isFirstSnapshotOfRootVolume;
     private boolean isVolumeInactive;
     private String firstBackupUuid;
     private String vmName;
@@ -56,7 +55,6 @@ public class BackupSnapshotCommand extends SnapshotCommand {
                                  String prevSnapshotUuid,
                                  String prevBackupUuid,
                                  String firstBackupUuid,
-                                 boolean isFirstSnapshotOfRootVolume,
                                  boolean isVolumeInactive,
                                  String vmName) 
     {
@@ -64,7 +62,6 @@ public class BackupSnapshotCommand extends SnapshotCommand {
         this.prevSnapshotUuid = prevSnapshotUuid;
         this.prevBackupUuid = prevBackupUuid;
         this.firstBackupUuid = firstBackupUuid;
-        this.isFirstSnapshotOfRootVolume = isFirstSnapshotOfRootVolume;
         this.isVolumeInactive = isVolumeInactive;
         this.vmName = vmName;
     }
@@ -81,10 +78,6 @@ public class BackupSnapshotCommand extends SnapshotCommand {
         return firstBackupUuid;
     }
     
-    public boolean isFirstSnapshotOfRootVolume() {
-        return isFirstSnapshotOfRootVolume;
-    }
-    
     public boolean isVolumeInactive() {
         return isVolumeInactive;
     }

core/src/com/cloud/agent/api/CreatePrivateTemplateFromVolumeCommand.java

@@ -0,0 +1,79 @@
+/**
+ *  Copyright (C) 2010 Cloud.com, Inc.  All rights reserved.
+ * 
+ * This software is licensed under the GNU General Public License v3 or later.
+ * 
+ * It is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ * 
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ * 
+ */
+
+package com.cloud.agent.api;
+
+public class CreatePrivateTemplateFromVolumeCommand extends SnapshotCommand {
+    private String _volumePath;
+    private String _userSpecifiedName;
+    private String _uniqueName;
+    private long _templateId;
+    private long _accountId;
+    // For XenServer
+    private String _secondaryStorageURL;
+
+
+    public CreatePrivateTemplateFromVolumeCommand() {}
+
+    public CreatePrivateTemplateFromVolumeCommand(String secondaryStorageURL, long templateId, long accountId, String userSpecifiedName, String uniqueName, String volumePath) {
+    	_secondaryStorageURL = secondaryStorageURL;
+    	_templateId = templateId;
+    	_accountId = accountId;
+    	_userSpecifiedName = userSpecifiedName;
+        _uniqueName = uniqueName;
+        _volumePath = volumePath;
+
+    }
+
+    @Override
+    public boolean executeInSequence() { 
+        return false;
+    }
+    
+    public String getSecondaryStorageURL() {
+    	return _secondaryStorageURL;
+    }
+
+    public String getTemplateName() {
+        return _userSpecifiedName;
+    }
+
+    public String getUniqueName() {
+        return _uniqueName;
+    }
+
+    public long getTemplateId() {
+    	return _templateId;
+    }
+    
+    public void setVolumePath(String _volumePath) {
+        this._volumePath = _volumePath;
+    }
+
+    public String getVolumePath() {
+        return _volumePath;
+    }
+
+    public Long getAccountId() {
+    	return _accountId;
+    }
+    
+    public void setTemplateId(long templateId) {
+    	_templateId = templateId;
+    }
+}

core/src/com/cloud/agent/api/CreateVolumeFromSnapshotCommand.java

@@ -22,7 +22,6 @@ package com.cloud.agent.api;
  * This currently assumes that both primary and secondary storage are mounted on the XenServer.  
  */
 public class CreateVolumeFromSnapshotCommand extends SnapshotCommand {
-    private String templatePath;
     
     protected CreateVolumeFromSnapshotCommand() {
         
@@ -51,18 +50,9 @@ public class CreateVolumeFromSnapshotCommand extends SnapshotCommand {
                                            Long   accountId,
                                            Long   volumeId,
                                            String backedUpSnapshotUuid,
-                                           String backedUpSnapshotName,
+                                           String backedUpSnapshotName)
-                                           String templatePath) 
     {
         super(primaryStoragePoolNameLabel, secondaryStoragePoolURL, backedUpSnapshotUuid, backedUpSnapshotName, dcId, accountId, volumeId);
-        this.templatePath = templatePath;
-    }
-
-    /**
-     * @return the templatePath
-     */
-    public String getTemplatePath() {
-        return templatePath;
     }
 
 }

core/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java

@@ -22,7 +22,6 @@ package com.cloud.agent.api;
  * This currently assumes that the secondary storage are mounted on the XenServer.  
  */
 public class DeleteSnapshotBackupCommand extends SnapshotCommand {
-    private String childUUID;
 
     protected DeleteSnapshotBackupCommand() {
         
@@ -59,18 +58,8 @@ public class DeleteSnapshotBackupCommand extends SnapshotCommand {
                                        Long   accountId,
                                        Long   volumeId,
                                        String backupUUID, 
-                                       String backupName,
+                                       String backupName) 
-                                       String childUUID) 
     {
         super(primaryStoragePoolNameLabel, secondaryStoragePoolURL, backupUUID, backupName, dcId, accountId, volumeId);
-        this.childUUID = childUUID;
     }
-
-    /**
-     * @return the childUUID
-     */
-    public String getChildUUID() {
-        return childUUID;
-    }
-
 }

core/src/com/cloud/agent/api/ManageSnapshotCommand.java

@@ -38,19 +38,21 @@ public class ManageSnapshotCommand extends Command {
 
     public ManageSnapshotCommand() {}
 
-    public ManageSnapshotCommand(String commandSwitch, long snapshotId, String path, String snapshotName, String vmName) {
+    public ManageSnapshotCommand(long snapshotId, String volumePath, String preSnapshotPath ,String snapshotName, String vmName) {
-        _commandSwitch = commandSwitch;
+        _commandSwitch = ManageSnapshotCommand.CREATE_SNAPSHOT;
-        if (commandSwitch.equals(ManageSnapshotCommand.CREATE_SNAPSHOT)) {
+        _volumePath = volumePath;
-            _volumePath = path;
+        _snapshotPath = preSnapshotPath;
-        }
-        else if (commandSwitch.equals(ManageSnapshotCommand.DESTROY_SNAPSHOT)) {
-            _snapshotPath = path;
-        }
         _snapshotName = snapshotName;
         _snapshotId = snapshotId;
         _vmName = vmName;
     }
 
+    public ManageSnapshotCommand(long snapshotId, String snapshotPath) {
+        _commandSwitch = ManageSnapshotCommand.DESTROY_SNAPSHOT;
+        _snapshotPath = snapshotPath;
+    }
+    
+    
     @Override
     public boolean executeInSequence() {
         return false;

core/src/com/cloud/agent/api/storage/CreatePrivateTemplateAnswer.java

@@ -38,6 +38,10 @@ public class CreatePrivateTemplateAnswer extends Answer {
         _format = format;
     }
 
+    public CreatePrivateTemplateAnswer(Command cmd, boolean success, String result) {
+        super(cmd, success, result);
+    }
+    
     public String getPath() {
         return _path;
     }

core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java

@@ -67,6 +67,7 @@ import com.cloud.agent.api.CheckVirtualMachineAnswer;
 import com.cloud.agent.api.CheckVirtualMachineCommand;
 import com.cloud.agent.api.Command;
 import com.cloud.agent.api.CreatePrivateTemplateFromSnapshotCommand;
+import com.cloud.agent.api.CreatePrivateTemplateFromVolumeCommand;
 import com.cloud.agent.api.CreateVolumeFromSnapshotAnswer;
 import com.cloud.agent.api.CreateVolumeFromSnapshotCommand;
 import com.cloud.agent.api.DeleteSnapshotBackupAnswer;
@@ -138,7 +139,7 @@ import com.cloud.agent.api.storage.CopyVolumeCommand;
 import com.cloud.agent.api.storage.CreateAnswer;
 import com.cloud.agent.api.storage.CreateCommand;
 import com.cloud.agent.api.storage.CreatePrivateTemplateAnswer;
-import com.cloud.agent.api.storage.CreatePrivateTemplateCommand;
+
 import com.cloud.agent.api.storage.DestroyCommand;
 import com.cloud.agent.api.storage.DownloadAnswer;
 import com.cloud.agent.api.storage.PrimaryStorageDownloadCommand;
@@ -607,8 +608,6 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
             return execute((AttachVolumeCommand) cmd);
         } else if (cmd instanceof AttachIsoCommand) {
             return execute((AttachIsoCommand) cmd);
-        } else if (cmd instanceof ValidateSnapshotCommand) {
-            return execute((ValidateSnapshotCommand) cmd);
         } else if (cmd instanceof ManageSnapshotCommand) {
             return execute((ManageSnapshotCommand) cmd);
         } else if (cmd instanceof BackupSnapshotCommand) {
@@ -619,8 +618,8 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
             return execute((CreateVolumeFromSnapshotCommand) cmd);
         } else if (cmd instanceof DeleteSnapshotsDirCommand) {
             return execute((DeleteSnapshotsDirCommand) cmd);
-        } else if (cmd instanceof CreatePrivateTemplateCommand) {
+        } else if (cmd instanceof CreatePrivateTemplateFromVolumeCommand) {
-            return execute((CreatePrivateTemplateCommand) cmd);
+            return execute((CreatePrivateTemplateFromVolumeCommand) cmd);
         } else if (cmd instanceof CreatePrivateTemplateFromSnapshotCommand) {
             return execute((CreatePrivateTemplateFromSnapshotCommand) cmd);
         } else if (cmd instanceof GetStorageStatsCommand) {
@@ -891,7 +890,6 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
             
             Pair<VM, String> v = createVmFromTemplate(conn, vmSpec, host);
             vm = v.first();
-            String vmUuid = v.second();
             
             for (VolumeTO disk : vmSpec.getDisks()) {
                 createVbd(conn, vmName, vm, disk, disk.getType() == VolumeType.ROOT && vmSpec.getType() != VirtualMachine.Type.User);
@@ -3943,7 +3941,6 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         try {
             Host myself = Host.getByUuid(conn, _host.uuid);
             
-            boolean findsystemvmiso = false;
             Set<SR> srs = SR.getByNameLabel(conn, "XenServer Tools");
             if( srs.size() != 1 ) {
             	throw new CloudRuntimeException("There are " + srs.size() + " SRs with name XenServer Tools");
@@ -4812,7 +4809,7 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
                 throw new Exception("no attached PBD");
             }   
             if (s_logger.isDebugEnabled()) {
-                s_logger.debug(logX(sr, "Created a SR; UUID is " + sr.getUuid(conn)));
+                s_logger.debug(logX(sr, "Created a SR; UUID is " + sr.getUuid(conn) + " device config is " + deviceConfig));
             }
             sr.scan(conn);
             return sr;
@@ -5412,84 +5409,8 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         }
     }
 
-    protected ValidateSnapshotAnswer execute(final ValidateSnapshotCommand cmd) {
-        String primaryStoragePoolNameLabel = cmd.getPrimaryStoragePoolNameLabel();
-        String volumeUuid = cmd.getVolumeUuid(); // Precondition: not null
-        String firstBackupUuid = cmd.getFirstBackupUuid();
-        String previousSnapshotUuid = cmd.getPreviousSnapshotUuid();
-        String templateUuid = cmd.getTemplateUuid();
-
-        // By default assume failure
-        String details = "Could not validate previous snapshot backup UUID " + "because the primary Storage SR could not be created from the name label: "
-                + primaryStoragePoolNameLabel;
-        boolean success = false;
-        String expectedSnapshotBackupUuid = null;
-        String actualSnapshotBackupUuid = null;
-        String actualSnapshotUuid = null;
-
-        Boolean isISCSI = false;
-        String primaryStorageSRUuid = null;
-        Connection conn = getConnection();
-        try {
-            SR primaryStorageSR = getSRByNameLabelandHost(primaryStoragePoolNameLabel);
-
-            if (primaryStorageSR != null) {
-                primaryStorageSRUuid = primaryStorageSR.getUuid(conn);
-                isISCSI = SRType.LVMOISCSI.equals(primaryStorageSR.getType(conn));
-            }
-        } catch (BadServerResponse e) {
-            details += ", reason: " + e.getMessage();
-            s_logger.error(details, e);
-        } catch (XenAPIException e) {
-            details += ", reason: " + e.getMessage();
-            s_logger.error(details, e);
-        } catch (XmlRpcException e) {
-            details += ", reason: " + e.getMessage();
-            s_logger.error(details, e);
-        }
-
-        if (primaryStorageSRUuid != null) {
-            if (templateUuid == null) {
-                templateUuid = "";
-            }
-            if (firstBackupUuid == null) {
-                firstBackupUuid = "";
-            }
-            if (previousSnapshotUuid == null) {
-                previousSnapshotUuid = "";
-            }
-            String result = callHostPlugin("vmopsSnapshot", "validateSnapshot", "primaryStorageSRUuid", primaryStorageSRUuid, "volumeUuid", volumeUuid, "firstBackupUuid", firstBackupUuid,
-                    "previousSnapshotUuid", previousSnapshotUuid, "templateUuid", templateUuid, "isISCSI", isISCSI.toString());
-            if (result == null || result.isEmpty()) {
-                details = "Validating snapshot backup for volume with UUID: " + volumeUuid + " failed because there was an exception in the plugin";
-                // callHostPlugin exception which has been logged already
-            } else {
-                String[] uuids = result.split("#", -1);
-                if (uuids.length >= 3) {
-                    expectedSnapshotBackupUuid = uuids[1];
-                    actualSnapshotBackupUuid = uuids[2];
-                }
-                if (uuids.length >= 4) {
-                    actualSnapshotUuid = uuids[3];
-                } else {
-                    actualSnapshotUuid = "";
-                }
-                if (uuids[0].equals("1")) {
-                    success = true;
-                    details = null;
-                } else {
-                    details = "Previous snapshot backup on the primary storage is invalid. " + "Expected: " + expectedSnapshotBackupUuid + " Actual: " + actualSnapshotBackupUuid;
-                    // success is still false
-                }
-                s_logger.debug("ValidatePreviousSnapshotBackup returned " + " success: " + success + " details: " + details + " expectedSnapshotBackupUuid: "
-                        + expectedSnapshotBackupUuid + " actualSnapshotBackupUuid: " + actualSnapshotBackupUuid + " actualSnapshotUuid: " + actualSnapshotUuid);
-            }
-        }
-
-        return new ValidateSnapshotAnswer(cmd, success, details, expectedSnapshotBackupUuid, actualSnapshotBackupUuid, actualSnapshotUuid);
-    }
-
     protected ManageSnapshotAnswer execute(final ManageSnapshotCommand cmd) {
+      
         long snapshotId = cmd.getSnapshotId();
         String snapshotName = cmd.getSnapshotName();
 
@@ -5510,8 +5431,7 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
             if (cmdSwitch.equals(ManageSnapshotCommand.CREATE_SNAPSHOT)) {
                 // Look up the volume
                 String volumeUUID = cmd.getVolumePath();
-
+                VDI volume = VDI.getByUuid(conn, volumeUUID);
-                VDI volume = getVDIbyUuid(volumeUUID);
 
                 // Create a snapshot
                 VDI snapshot = volume.snapshot(conn, new HashMap<String, String>());
@@ -5519,10 +5439,26 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
                 if (snapshotName != null) {
                     snapshot.setNameLabel(conn, snapshotName);
                 }
-
                 // Determine the UUID of the snapshot
-                VDI.Record vdir = snapshot.getRecord(conn);
+
-                snapshotUUID = vdir.uuid;
+                snapshotUUID = snapshot.getUuid(conn);
+                String preSnapshotUUID = cmd.getSnapshotPath();
+                //check if it is a empty snapshot
+                if( preSnapshotUUID != null) {
+                    SR sr = volume.getSR(conn);
+                    String srUUID = sr.getUuid(conn);
+                    String type = sr.getType(conn);
+                    Boolean isISCSI = SRType.LVMOISCSI.equals(type);
+                    String snapshotParentUUID = getVhdParent(srUUID, snapshotUUID, isISCSI);
+                    
+                    String preSnapshotParentUUID = getVhdParent(srUUID, preSnapshotUUID, isISCSI);
+                    if( snapshotParentUUID != null && snapshotParentUUID.equals(preSnapshotParentUUID)) {
+                        // this is empty snapshot, remove it
+                        snapshot.destroy(conn);
+                        snapshotUUID = preSnapshotUUID;
+                    }
+                    
+                }
                 
                 success = true;
                 details = null;
@@ -5547,74 +5483,63 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         return new ManageSnapshotAnswer(cmd, snapshotId, snapshotUUID, success, details);
     }
 
-    protected CreatePrivateTemplateAnswer execute(final CreatePrivateTemplateCommand cmd) {
+    protected CreatePrivateTemplateAnswer execute(final CreatePrivateTemplateFromVolumeCommand cmd) {
-        String secondaryStorageURL = cmd.getSecondaryStorageURL();
+        String secondaryStoragePoolURL = cmd.getSecondaryStorageURL();
-        String snapshotUUID = cmd.getSnapshotPath();
+        String volumeUUID = cmd.getVolumePath();
+        Long accountId = cmd.getAccountId();
         String userSpecifiedName = cmd.getTemplateName();
+        Long templateId = cmd.getTemplateId();
 
-        SR secondaryStorage = null;
+        String details = null;
-        VDI privateTemplate = null;
+        SR tmpltSR = null;
-        Connection conn = getConnection();
+        boolean result = false;
         try {
-            URI uri = new URI(secondaryStorageURL);
-            String remoteTemplateMountPath = uri.getHost() + ":" + uri.getPath() + "/template/";
-            String templateFolder = cmd.getAccountId() + "/" + cmd.getTemplateId() + "/";
-            String templateDownloadFolder = createTemplateDownloadFolder(remoteTemplateMountPath, templateFolder);
-            String templateInstallFolder = "tmpl/" + templateFolder;
             
-            // Create a SR for the secondary storage download folder
+            URI uri = new URI(secondaryStoragePoolURL);
-            secondaryStorage = createNfsSRbyURI(new URI(secondaryStorageURL + "/template/" + templateDownloadFolder), false);
+            String secondaryStorageMountPath = uri.getHost() + ":" + uri.getPath();
+            String installPath = "template/tmpl/" + accountId + "/" + templateId;
+            if( !createSecondaryStorageFolder(secondaryStorageMountPath, installPath)) {
+                details = " Filed to create folder " + installPath + " in secondary storage";
+                s_logger.warn(details);
+                return new CreatePrivateTemplateAnswer(cmd, false, details);
+            }
+            Connection conn = getConnection();
+            VDI volume = getVDIbyUuid(volumeUUID);
+            // create template SR
+            URI tmpltURI = new URI(secondaryStoragePoolURL + "/" + installPath);
+            tmpltSR = createNfsSRbyURI(tmpltURI, false);
 
-            // Look up the snapshot and copy it to secondary storage
+            // copy volume to template SR
-            VDI snapshot = getVDIbyUuid(snapshotUUID);
+            VDI tmpltVDI = cloudVDIcopy(volume, tmpltSR);
-            privateTemplate = cloudVDIcopy(snapshot, secondaryStorage);
             
             if (userSpecifiedName != null) {
-                privateTemplate.setNameLabel(conn, userSpecifiedName);
+                tmpltVDI.setNameLabel(conn, userSpecifiedName);
             }
 
-            // Determine the template file name and install path
+            String tmpltSrUUID = tmpltSR.getUuid(conn);
-            VDI.Record vdir = privateTemplate.getRecord(conn);
+            String tmpltUUID = tmpltVDI.getUuid(conn);
-            String templateName = vdir.uuid;
+            String tmpltFilename = tmpltUUID + ".vhd";
-            String templateFilename = templateName + ".vhd";
+            long virtualSize = tmpltVDI.getVirtualSize(conn);
-            String installPath = "template/" + templateInstallFolder + templateFilename;
+            long size = tmpltVDI.getPhysicalUtilisation(conn);
-
+            // create the template.properties file
-            // Determine the template's virtual size and then forget the VDI
+            result = postCreatePrivateTemplate(tmpltSrUUID, tmpltFilename, tmpltUUID, userSpecifiedName, null, size, virtualSize, templateId);
-            long virtualSize = privateTemplate.getVirtualSize(conn);
+            if (!result) {
-            // Create the template.properties file in the download folder, move
+                throw new CloudRuntimeException("Could not create the template.properties file on secondary storage dir: " + tmpltURI);
-            // the template and the template.properties file
-            // to the install folder, and then delete the download folder
-            if (!postCreatePrivateTemplate(remoteTemplateMountPath, templateDownloadFolder, templateInstallFolder, templateFilename, templateName, userSpecifiedName, null,
-                    virtualSize, cmd.getTemplateId())) {
-                throw new InternalErrorException("Failed to create the template.properties file.");
             }
-
+            return new CreatePrivateTemplateAnswer(cmd, true, null, installPath, virtualSize, tmpltUUID, ImageFormat.VHD);
-            return new CreatePrivateTemplateAnswer(cmd, true, null, installPath, virtualSize, templateName, ImageFormat.VHD);
         } catch (XenAPIException e) {
-            if (privateTemplate != null) {
+            details = "Creating template from volume " + volumeUUID + " failed due to " + e.getMessage();
-                destroyVDI(privateTemplate);
+            s_logger.error(details, e);
-            }
-
-            s_logger.warn("CreatePrivateTemplate Failed due to " + e.toString(), e);
-            return new CreatePrivateTemplateAnswer(cmd, false, e.toString(), null, 0, null, null);
         } catch (Exception e) {
-            s_logger.warn("CreatePrivateTemplate Failed due to " + e.getMessage(), e);
+            details = "Creating template from volume " + volumeUUID + " failed due to " + e.getMessage();
-            return new CreatePrivateTemplateAnswer(cmd, false, e.getMessage(), null, 0, null, null);
+            s_logger.error(details, e);
         } finally {
             // Remove the secondary storage SR
-            removeSR(secondaryStorage);
+            removeSR(tmpltSR);
         }
+        return new CreatePrivateTemplateAnswer(cmd, result, details);
     }
 
-    private String createTemplateDownloadFolder(String remoteTemplateMountPath, String templateFolder) throws InternalErrorException, URISyntaxException {
-        String templateDownloadFolder = "download/" + _host.uuid + "/" + templateFolder;
-
-        // Create the download folder
-        if (!createSecondaryStorageFolder(remoteTemplateMountPath, templateDownloadFolder)) {
-            throw new InternalErrorException("Failed to create the template download folder.");
-        }
-        return templateDownloadFolder;
-    }
 
     protected CreatePrivateTemplateAnswer execute(final CreatePrivateTemplateFromSnapshotCommand cmd) {
         String primaryStorageNameLabel = cmd.getPrimaryStoragePoolNameLabel();
@@ -5623,59 +5548,61 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         Long volumeId = cmd.getVolumeId();
         String secondaryStoragePoolURL = cmd.getSecondaryStoragePoolURL();
         String backedUpSnapshotUuid = cmd.getSnapshotUuid();
-        String origTemplateInstallPath = cmd.getOrigTemplateInstallPath();
         Long newTemplateId = cmd.getNewTemplateId();
         String userSpecifiedName = cmd.getTemplateName();
 
         // By default, assume failure
-        String details = "Failed to create private template " + newTemplateId + " from snapshot for volume: " + volumeId + " with backupUuid: " + backedUpSnapshotUuid;
+        String details = null;
-        String newTemplatePath = null;
+        SR snapshotSR = null;
-        String templateName = null;
+        SR tmpltSR = null; 
         boolean result = false;
-        long virtualSize = 0;
         try {
             URI uri = new URI(secondaryStoragePoolURL);
-            String remoteTemplateMountPath = uri.getHost() + ":" + uri.getPath() + "/template/";
+            String secondaryStorageMountPath = uri.getHost() + ":" + uri.getPath();
-            String templateFolder = cmd.getAccountId() + "/" + newTemplateId + "/";
+            String installPath = "template/tmpl/" + accountId + "/" + newTemplateId;
-            String templateDownloadFolder = createTemplateDownloadFolder(remoteTemplateMountPath, templateFolder);
+            if( !createSecondaryStorageFolder(secondaryStorageMountPath, installPath)) {
-            String templateInstallFolder = "tmpl/" + templateFolder;
+                details = " Filed to create folder " + installPath + " in secondary storage";
-            // Yes, create a template vhd
+                s_logger.warn(details);
-            Pair<VHDInfo, String> vhdDetails = createVHDFromSnapshot(primaryStorageNameLabel, dcId, accountId, volumeId, secondaryStoragePoolURL, backedUpSnapshotUuid,
+                return new CreatePrivateTemplateAnswer(cmd, false, details);
-                    origTemplateInstallPath, templateDownloadFolder);
-
-            VHDInfo vhdInfo = vhdDetails.first();
-            String failureDetails = vhdDetails.second();
-            if (vhdInfo == null) {
-                if (failureDetails != null) {
-                    details += failureDetails;
             }
-            } else {
+            Connection conn = getConnection();
-                templateName = vhdInfo.getUuid();
+            // create snapshot SR
-                String templateFilename = templateName + ".vhd";
+            URI snapshotURI = new URI(secondaryStoragePoolURL + "/snapshots/" + accountId + "/" + volumeId );
-                String templateInstallPath = templateInstallFolder + "/" + templateFilename;
+            snapshotSR = createNfsSRbyURI(snapshotURI, false);
+            snapshotSR.scan(conn);
+            VDI snapshotVDI = getVDIbyUuid(backedUpSnapshotUuid);
             
-                newTemplatePath = "template" + "/" + templateInstallPath;
+            // create template SR
+            URI tmpltURI = new URI(secondaryStoragePoolURL + "/" + installPath);
+            tmpltSR = createNfsSRbyURI(tmpltURI, false);
+            // copy snapshotVDI to template SR
+            VDI tmpltVDI = cloudVDIcopy(snapshotVDI, tmpltSR);
+            
+            String tmpltSrUUID = tmpltSR.getUuid(conn);
+            String tmpltUUID = tmpltVDI.getUuid(conn);
+            String tmpltFilename = tmpltUUID + ".vhd";
+            long virtualSize = tmpltVDI.getVirtualSize(conn);
+            long size = tmpltVDI.getPhysicalUtilisation(conn);
 
-                virtualSize = vhdInfo.getVirtualSize();
             // create the template.properties file
-                result = postCreatePrivateTemplate(remoteTemplateMountPath, templateDownloadFolder, templateInstallFolder, templateFilename, templateName, userSpecifiedName, null,
+            result = postCreatePrivateTemplate(tmpltSrUUID, tmpltFilename, tmpltUUID, userSpecifiedName, null, size, virtualSize, newTemplateId);
-                        virtualSize, newTemplateId);
             if (!result) {
-                    details += ", reason: Could not create the template.properties file on secondary storage dir: " + templateInstallFolder;
+                throw new CloudRuntimeException("Could not create the template.properties file on secondary storage dir: " + tmpltURI);
-                } else {
-                    // Aaah, success.
-                    details = null;
             } 
             
-            }
+            return new CreatePrivateTemplateAnswer(cmd, true, null, installPath, virtualSize, tmpltUUID, ImageFormat.VHD);
         } catch (XenAPIException e) {
-            details += ", reason: " + e.getMessage();
+            details = "Creating template from snapshot " + backedUpSnapshotUuid + " failed due to " + e.getMessage();
             s_logger.error(details, e);
         } catch (Exception e) {
-            details += ", reason: " + e.getMessage();
+            details = "Creating template from snapshot " + backedUpSnapshotUuid + " failed due to " + e.getMessage();
             s_logger.error(details, e);
+        } finally {
+            // Remove the secondary storage SR
+            removeSR(snapshotSR);
+            removeSR(tmpltSR);
         }
-        return new CreatePrivateTemplateAnswer(cmd, result, details, newTemplatePath, virtualSize, templateName, ImageFormat.VHD);
+        return new CreatePrivateTemplateAnswer(cmd, result, details);
     }
 
     protected BackupSnapshotAnswer execute(final BackupSnapshotCommand cmd) {
@@ -5687,7 +5614,6 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         String snapshotUuid = cmd.getSnapshotUuid(); // not null: Precondition.
         String prevSnapshotUuid = cmd.getPrevSnapshotUuid();
         String prevBackupUuid = cmd.getPrevBackupUuid();
-        boolean isFirstSnapshotOfRootVolume = cmd.isFirstSnapshotOfRootVolume();
         // By default assume failure
         String details = null;
         boolean success = false;
@@ -5704,27 +5630,45 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
             URI uri = new URI(secondaryStoragePoolURL);
             String secondaryStorageMountPath = uri.getHost() + ":" + uri.getPath();
             
-            if (secondaryStorageMountPath == null) {
+
-                details = "Couldn't backup snapshot because the URL passed: " + secondaryStoragePoolURL + " is invalid.";
+            if (prevBackupUuid == null) {
-            } else {
+                // the first snapshot is always a full snapshot
-                boolean gcHappened = true;
+                String folder = "snapshots/" + accountId + "/" + volumeId;
-                if (gcHappened) {
+                if( !createSecondaryStorageFolder(secondaryStorageMountPath, folder)) {
-                    snapshotBackupUuid = backupSnapshot(primaryStorageSRUuid, dcId, accountId, volumeId, secondaryStorageMountPath, snapshotUuid, prevSnapshotUuid, prevBackupUuid,
+                    details = " Filed to create folder " + folder + " in secondary storage";
-                            isFirstSnapshotOfRootVolume, isISCSI);
+                    s_logger.warn(details);
-                    success = (snapshotBackupUuid != null);
+                    return new BackupSnapshotAnswer(cmd, success, details, snapshotBackupUuid);
-                } else {
-                    s_logger.warn("GC hasn't happened yet for previousSnapshotUuid: " + prevSnapshotUuid + ". Will retry again after 1 min");
-                }
                 }
 
-            if (!success) {
+                String snapshotMountpoint = secondaryStoragePoolURL + "/" + folder;
+                SR snapshotSr = null;
+                try {
+                    snapshotSr = createNfsSRbyURI(new URI(snapshotMountpoint), false);
+                    VDI snapshotVdi = getVDIbyUuid(snapshotUuid);
+                    VDI backedVdi = snapshotVdi.copy(conn, snapshotSr);
+                    snapshotBackupUuid = backedVdi.getUuid(conn);
+                    success = true;
+                } finally {
+                    if( snapshotSr != null) {
+                        removeSR(snapshotSr);
+                    }
+                }
+            } else {
+                    snapshotBackupUuid = backupSnapshot(primaryStorageSRUuid, dcId, accountId, volumeId, secondaryStorageMountPath, 
+                            snapshotUuid, prevSnapshotUuid, prevBackupUuid, isISCSI);
+                    success = (snapshotBackupUuid != null);
+            }
+
+            if (success) {
+                details = "Successfully backedUp the snapshotUuid: " + snapshotUuid + " to secondary storage.";
+                
                 // Mark the snapshot as removed in the database.
                 // When the next snapshot is taken, it will be
                 // 1) deleted from the DB 2) The snapshotUuid will be deleted from the primary
                 // 3) the snapshotBackupUuid will be copied to secondary
                 // 4) if possible it will be coalesced with the next snapshot.
 
-            } else if (prevSnapshotUuid != null && !isFirstSnapshotOfRootVolume) {
+                if (prevSnapshotUuid != null) {
                 // Destroy the previous snapshot, if it exists.
                 // We destroy the previous snapshot only if the current snapshot
                 // backup succeeds.
@@ -5732,9 +5676,10 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
                 // so that it doesn't get merged with the
                 // new one
                 // and muddle the vhd chain on the secondary storage.
-                details = "Successfully backedUp the snapshotUuid: " + snapshotUuid + " to secondary storage.";
+
                     destroySnapshotOnPrimaryStorage(prevSnapshotUuid);
                 }
+            }
 
         } catch (XenAPIException e) {
             details = "BackupSnapshot Failed due to " + e.toString();
@@ -5749,93 +5694,44 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
 
     protected CreateVolumeFromSnapshotAnswer execute(final CreateVolumeFromSnapshotCommand cmd) {
         String primaryStorageNameLabel = cmd.getPrimaryStoragePoolNameLabel();
-        Long dcId = cmd.getDataCenterId();
         Long accountId = cmd.getAccountId();
         Long volumeId = cmd.getVolumeId();
         String secondaryStoragePoolURL = cmd.getSecondaryStoragePoolURL();
         String backedUpSnapshotUuid = cmd.getSnapshotUuid();
-        String templatePath = cmd.getTemplatePath();
 
         // By default, assume the command has failed and set the params to be
         // passed to CreateVolumeFromSnapshotAnswer appropriately
         boolean result = false;
         // Generic error message.
-        String details = "Failed to create volume from snapshot for volume: " + volumeId + " with backupUuid: " + backedUpSnapshotUuid;
+        String details = null;
-        String vhdUUID = null;
+        String volumeUUID = null;
-        SR temporarySROnSecondaryStorage = null;
+        SR snapshotSR = null;
-        String mountPointOfTemporaryDirOnSecondaryStorage = null;
+        
+        if (secondaryStoragePoolURL == null) {
+            details += " because the URL passed: " + secondaryStoragePoolURL + " is invalid.";
+            return new CreateVolumeFromSnapshotAnswer(cmd, result, details, volumeUUID);
+        }
         try {
-            VDI vdi = null;
             Connection conn = getConnection();
             SR primaryStorageSR = getSRByNameLabelandHost(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);
             }
+            // Get the absolute path of the snapshot on the secondary storage.
+            URI snapshotURI = new URI(secondaryStoragePoolURL + "/snapshots/" + accountId + "/" + volumeId );
             
-            Boolean isISCSI = SRType.LVMOISCSI.equals(primaryStorageSR.getType(conn));
+            snapshotSR = createNfsSRbyURI(snapshotURI, false);
+            snapshotSR.scan(conn);
+            VDI snapshotVDI = getVDIbyUuid(backedUpSnapshotUuid);
 
-            // Get the absolute path of the template on the secondary storage.
+            VDI volumeVDI = cloudVDIcopy(snapshotVDI, primaryStorageSR);
-            URI uri = new URI(secondaryStoragePoolURL);
-            String secondaryStorageMountPath = uri.getHost() + ":" + uri.getPath();
             
-            if (secondaryStorageMountPath == null) {
+            volumeUUID = volumeVDI.getUuid(conn);
-                details += " because the URL passed: " + secondaryStoragePoolURL + " is invalid.";
-                return new CreateVolumeFromSnapshotAnswer(cmd, result, details, vhdUUID);
-            }
             
-            // Create a volume and not a template
-            String templateDownloadFolder = "";
             
-            VHDInfo vhdInfo = createVHDFromSnapshot(dcId, accountId, volumeId, secondaryStorageMountPath, backedUpSnapshotUuid, templatePath, templateDownloadFolder, isISCSI);
-            if (vhdInfo == null) {
-                details += " because the vmops plugin on XenServer failed at some point";
-            } else {
-                vhdUUID = vhdInfo.getUuid();
-                String tempDirRelativePath = "snapshots" + File.separator + accountId + File.separator + volumeId + "_temp";
-                mountPointOfTemporaryDirOnSecondaryStorage = secondaryStorageMountPath + File.separator + tempDirRelativePath;
-
-                uri = new URI("nfs://" + mountPointOfTemporaryDirOnSecondaryStorage);
-                // No need to check if the SR already exists. It's a temporary
-                // SR destroyed when this method exits.
-                // And two createVolumeFromSnapshot operations cannot proceed at
-                // the same time.
-                temporarySROnSecondaryStorage = createNfsSRbyURI(uri, false);
-                if (temporarySROnSecondaryStorage == null) {
-                    details += "because SR couldn't be created on " + mountPointOfTemporaryDirOnSecondaryStorage;
-                } else {
-                    s_logger.debug("Successfully created temporary SR on secondary storage " + temporarySROnSecondaryStorage.getNameLabel(conn) + "with uuid "
-                            + temporarySROnSecondaryStorage.getUuid(conn) + " and scanned it");
-                    // createNFSSRbyURI also scans the SR and introduces the VDI
-
-                    vdi = getVDIbyUuid(vhdUUID);
-
-                    if (vdi != null) {
-                        s_logger.debug("Successfully created VDI on secondary storage SR " + temporarySROnSecondaryStorage.getNameLabel(conn) + " with uuid " + vhdUUID);
-                        s_logger.debug("Copying VDI: " + vdi.getLocation(conn) + " from secondary to primary");
-                        VDI vdiOnPrimaryStorage = cloudVDIcopy(vdi, primaryStorageSR);
-                        // vdi.copy introduces the vdi into the database. Don't
-                        // need to do a scan on the primary
-                        // storage.
-
-                        if (vdiOnPrimaryStorage != null) {
-                            vhdUUID = vdiOnPrimaryStorage.getUuid(conn);
-                            s_logger.debug("Successfully copied and introduced VDI on primary storage with path " + vdiOnPrimaryStorage.getLocation(conn) + " and uuid " + vhdUUID);
             result = true;
-                            details = null;
 
-                        } else {
-                            details += ". Could not copy the vdi " + vhdUUID + " to primary storage";
-                        }
-
-                        // The VHD on temporary was scanned and introduced as a VDI
-                        // destroy it as we don't need it anymore.
-                        vdi.destroy(conn);
-                    } else {
-                        details += ". Could not scan and introduce vdi with uuid: " + vhdUUID;
-                    }
-                }
-            }
         } catch (XenAPIException e) {
             details += " due to " + e.toString();
             s_logger.warn(details, e);
@@ -5844,13 +5740,8 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
             s_logger.warn(details, e);
         } finally {
             // In all cases, if the temporary SR was created, forget it.
-            if (temporarySROnSecondaryStorage != null) {
+            if (snapshotSR != null) {
-                removeSR(temporarySROnSecondaryStorage);
+                removeSR(snapshotSR);
-                // Delete the temporary directory created.
-                File folderPath = new File(mountPointOfTemporaryDirOnSecondaryStorage);
-                String remoteMountPath = folderPath.getParent();
-                String folder = folderPath.getName();
-                deleteSecondaryStorageFolder(remoteMountPath, folder);
             }
         }
         if (!result) {
@@ -5859,7 +5750,7 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         }
 
         // In all cases return something.
-        return new CreateVolumeFromSnapshotAnswer(cmd, result, details, vhdUUID);
+        return new CreateVolumeFromSnapshotAnswer(cmd, result, details, volumeUUID);
     }
 
     protected DeleteSnapshotBackupAnswer execute(final DeleteSnapshotBackupCommand cmd) {
@@ -5868,31 +5759,10 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         Long volumeId = cmd.getVolumeId();
         String secondaryStoragePoolURL = cmd.getSecondaryStoragePoolURL();
         String backupUUID = cmd.getSnapshotUuid();
-        String childUUID = cmd.getChildUUID();
-        String primaryStorageNameLabel = cmd.getPrimaryStoragePoolNameLabel();
-
         String details = null;
         boolean success = false;
 
-        SR primaryStorageSR = null;
-        Boolean isISCSI = false;
-        try {
-            Connection conn = getConnection();
-            primaryStorageSR = getSRByNameLabelandHost(primaryStorageNameLabel);
-            if (primaryStorageSR == null) {
-                details = "Primary Storage SR could not be created from the name label: " + primaryStorageNameLabel;
-                throw new InternalErrorException(details);
-            }
-            isISCSI = SRType.LVMOISCSI.equals(primaryStorageSR.getType(conn));
-        } catch (XenAPIException e) {
-            details = "Couldn't determine primary SR type " + e.getMessage();
-            s_logger.error(details, e);
-        } catch (Exception e) {
-            details = "Couldn't determine primary SR type " + e.getMessage();
-            s_logger.error(details, e);
-        }
 
-        if (primaryStorageSR != null) {
             URI uri = null;
             try {
                 uri = new URI(secondaryStoragePoolURL);
@@ -5906,14 +5776,13 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
                 if (secondaryStorageMountPath == null) {
                     details = "Couldn't delete snapshot because the URL passed: " + secondaryStoragePoolURL + " is invalid.";
                 } else {
-                    details = deleteSnapshotBackup(dcId, accountId, volumeId, secondaryStorageMountPath, backupUUID, childUUID, isISCSI);
+                    details = deleteSnapshotBackup(dcId, accountId, volumeId, secondaryStorageMountPath, backupUUID);
                     success = (details != null && details.equals("1"));
                     if (success) {
                         s_logger.debug("Successfully deleted snapshot backup " + backupUUID);
                     }
                 }
             }
-        }
         return new DeleteSnapshotBackupAnswer(cmd, success, details);
     }
 
@@ -6134,8 +6003,7 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         return (result != null);
     }
 
-    protected boolean postCreatePrivateTemplate(String remoteTemplateMountPath, String templateDownloadFolder, String templateInstallFolder, String templateFilename,
+    protected boolean postCreatePrivateTemplate(String tmpltSrUUID,String tmpltFilename, String templateName, String templateDescription, String checksum, long size, long virtualSize, long templateId) {
-            String templateName, String templateDescription, String checksum, long virtualSize, long templateId) {
 
         if (templateDescription == null) {
             templateDescription = "";
@@ -6145,23 +6013,18 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
             checksum = "";
         }
 
-        String result = callHostPluginWithTimeOut("vmopsSnapshot", "post_create_private_template", 110*60, "remoteTemplateMountPath", remoteTemplateMountPath, "templateDownloadFolder", templateDownloadFolder,
+        String result = callHostPluginWithTimeOut("vmopsSnapshot", "post_create_private_template", 110*60, "tmpltSrUUID", tmpltSrUUID, "templateFilename", tmpltFilename, "templateName", templateName, "templateDescription", templateDescription,
-                "templateInstallFolder", templateInstallFolder, "templateFilename", templateFilename, "templateName", templateName, "templateDescription", templateDescription,
+                "checksum", checksum, "size", String.valueOf(size), "virtualSize", String.valueOf(virtualSize), "templateId", String.valueOf(templateId));
-                "checksum", checksum, "virtualSize", String.valueOf(virtualSize), "templateId", String.valueOf(templateId));
 
         boolean success = false;
         if (result != null && !result.isEmpty()) {
             // Else, command threw an exception which has already been logged.
 
-            String[] tmp = result.split("#");
+            if (result.equalsIgnoreCase("1")) {
-            String status = tmp[0];
+                s_logger.debug("Successfully created template.properties file on secondary storage for " + tmpltFilename);
-
-            if (status != null && status.equalsIgnoreCase("1")) {
-                s_logger.debug("Successfully created template.properties file on secondary storage dir: " + templateInstallFolder);
                 success = true;
             } else {
-                s_logger.warn("Could not create template.properties file on secondary storage dir: " + templateInstallFolder + " for templateId: " + templateId
+                s_logger.warn("Could not create template.properties file on secondary storage for " + tmpltFilename + " for templateId: " + templateId);
-                        + ". Failed with status " + status);
             }
         }
 
@@ -6170,8 +6033,8 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
 
     // Each argument is put in a separate line for readability.
     // Using more lines does not harm the environment.
-    protected String backupSnapshot(String primaryStorageSRUuid, Long dcId, Long accountId, Long volumeId, String secondaryStorageMountPath, String snapshotUuid,
+    protected String backupSnapshot(String primaryStorageSRUuid, Long dcId, Long accountId, Long volumeId, String secondaryStorageMountPath,
-            String prevSnapshotUuid, String prevBackupUuid, Boolean isFirstSnapshotOfRootVolume, Boolean isISCSI) {
+            String snapshotUuid, String prevSnapshotUuid, String prevBackupUuid, Boolean isISCSI) {
         String backupSnapshotUuid = null;
 
         if (prevSnapshotUuid == null) {
@@ -6185,7 +6048,7 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         // Using more lines does not harm the environment.
         String results = callHostPluginWithTimeOut("vmopsSnapshot", "backupSnapshot", 110*60, "primaryStorageSRUuid", primaryStorageSRUuid, "dcId", dcId.toString(), "accountId", accountId.toString(), "volumeId",
                 volumeId.toString(), "secondaryStorageMountPath", secondaryStorageMountPath, "snapshotUuid", snapshotUuid, "prevSnapshotUuid", prevSnapshotUuid, "prevBackupUuid",
-                prevBackupUuid, "isFirstSnapshotOfRootVolume", isFirstSnapshotOfRootVolume.toString(), "isISCSI", isISCSI.toString());
+                prevBackupUuid, "isISCSI", isISCSI.toString());
 
         if (results == null || results.isEmpty()) {
             // errString is already logged.
@@ -6210,6 +6073,19 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         return backupSnapshotUuid;
     }
     
+    
+    protected String getVhdParent(String primaryStorageSRUuid, String snapshotUuid, Boolean isISCSI) {
+        String parentUuid = callHostPlugin("vmopsSnapshot", "getVhdParent", "primaryStorageSRUuid", primaryStorageSRUuid, 
+                "snapshotUuid", snapshotUuid, "isISCSI", isISCSI.toString());
+
+        if (parentUuid == null || parentUuid.isEmpty()) {
+            s_logger.debug("Unable to get parent of VHD " + snapshotUuid + " in SR " + primaryStorageSRUuid);
+            // errString is already logged.
+            return null;
+        }
+        return parentUuid;
+    }
+
     protected boolean destroySnapshotOnPrimaryStorage(String snapshotUuid) {
         // Precondition snapshotUuid != null
         try {
@@ -6232,11 +6108,11 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         return false;
     }
 
-    protected String deleteSnapshotBackup(Long dcId, Long accountId, Long volumeId, String secondaryStorageMountPath, String backupUUID, String childUUID, Boolean isISCSI) {
+    protected String deleteSnapshotBackup(Long dcId, Long accountId, Long volumeId, String secondaryStorageMountPath, String backupUUID) {
 
         // If anybody modifies the formatting below again, I'll skin them
-        String result = callHostPlugin("vmopsSnapshot", "deleteSnapshotBackup", "backupUUID", backupUUID, "childUUID", childUUID, "dcId", dcId.toString(), "accountId", accountId.toString(),
+        String result = callHostPlugin("vmopsSnapshot", "deleteSnapshotBackup", "backupUUID", backupUUID, "dcId", dcId.toString(), "accountId", accountId.toString(),
-                "volumeId", volumeId.toString(), "secondaryStorageMountPath", secondaryStorageMountPath, "isISCSI", isISCSI.toString());
+                "volumeId", volumeId.toString(), "secondaryStorageMountPath", secondaryStorageMountPath);
 
         return result;
     }
@@ -6249,70 +6125,6 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         return result;
     }
 
-    // If anybody messes up with the formatting, I'll skin them
-    protected Pair<VHDInfo, String> createVHDFromSnapshot(String primaryStorageNameLabel, Long dcId, Long accountId, Long volumeId, String secondaryStoragePoolURL,
-            String backedUpSnapshotUuid, String templatePath, String templateDownloadFolder) throws XenAPIException, IOException, XmlRpcException, InternalErrorException,
-            URISyntaxException {
-        // Return values
-        String details = null;
-        Connection conn = getConnection();
-        SR primaryStorageSR = getSRByNameLabelandHost(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);
-        }
-
-        Boolean isISCSI = SRType.LVMOISCSI.equals(primaryStorageSR.getType(conn));
-
-        // Get the absolute path of the template on the secondary storage.
-        URI uri = new URI(secondaryStoragePoolURL);
-        String secondaryStorageMountPath = uri.getHost() + ":" + uri.getPath();
-        VHDInfo vhdInfo = null;
-        if (secondaryStorageMountPath == null) {
-            details = " because the URL passed: " + secondaryStoragePoolURL + " is invalid.";
-        } else {
-            vhdInfo = createVHDFromSnapshot(dcId, accountId, volumeId, secondaryStorageMountPath, backedUpSnapshotUuid, templatePath, templateDownloadFolder, isISCSI);
-            if (vhdInfo == null) {
-                details = " because the vmops plugin on XenServer failed at some point";
-            }
-        }
-
-        return new Pair<VHDInfo, String>(vhdInfo, details);
-    }
-
-    protected VHDInfo createVHDFromSnapshot(Long dcId, Long accountId, Long volumeId, String secondaryStorageMountPath, String backedUpSnapshotUuid, String templatePath,
-            String templateDownloadFolder, Boolean isISCSI) {
-        String vdiUUID = null;
-
-        String failureString = "Could not create volume from " + backedUpSnapshotUuid;
-        templatePath = (templatePath == null) ? "" : templatePath;
-        String results = callHostPluginWithTimeOut("vmopsSnapshot","createVolumeFromSnapshot", 110*60, "dcId", dcId.toString(), "accountId", accountId.toString(), "volumeId", volumeId.toString(),
-                "secondaryStorageMountPath", secondaryStorageMountPath, "backedUpSnapshotUuid", backedUpSnapshotUuid, "templatePath", templatePath, "templateDownloadFolder",
-                templateDownloadFolder, "isISCSI", isISCSI.toString());
-
-        if (results == null || results.isEmpty()) {
-            // Command threw an exception which has already been logged.
-            return null;
-        }
-        String[] tmp = results.split("#");
-        String status = tmp[0];
-        vdiUUID = tmp[1];
-        Long virtualSizeInMB = 0L;
-        if (tmp.length == 3) {
-            virtualSizeInMB = Long.valueOf(tmp[2]);
-        }
-        // status == "1" if and only if vdiUUID != null
-        // So we don't rely on status value but return vdiUUID as an indicator
-        // of success.
-
-        if (status != null && status.equalsIgnoreCase("1") && vdiUUID != null) {
-            s_logger.debug("Successfully created vhd file with all data on secondary storage : " + vdiUUID);
-        } else {
-            s_logger.debug(failureString + ". Failed with status " + status + " with vdiUuid " + vdiUUID);
-        }
-        return new VHDInfo(vdiUUID, virtualSizeInMB * MB);
-
-    }
 
     @Override
     public boolean start() {
@@ -6417,30 +6229,6 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR
         public String pool;
     }
 
-    private class VHDInfo {
-        private final String uuid;
-        private final Long virtualSize;
-
-        public VHDInfo(String uuid, Long virtualSize) {
-            this.uuid = uuid;
-            this.virtualSize = virtualSize;
-        }
-
-        /**
-         * @return the uuid
-         */
-        public String getUuid() {
-            return uuid;
-        }
-
-        /**
-         * @return the virtualSize
-         */
-        public Long getVirtualSize() {
-            return virtualSize;
-        }
-    }
-
 	protected String getGuestOsType(String stdType) {
 		return CitrixHelper.getGuestOsType(stdType);
 	}

core/src/com/cloud/server/ManagementServer.java

@@ -1716,12 +1716,6 @@ public interface ManagementServer {
            ResourceAllocationException,
            InternalErrorException;
 
-    /**
-     * @param userId    The Id of the user who invoked this operation.
-     * @param volumeId  The volume for which this snapshot is being taken
-     * @return          The properties of the snapshot taken
-     */
-    SnapshotVO createTemplateSnapshot(Long userId, long volumeId);
     
     /**
      * Destroy a snapshot
@@ -1761,7 +1755,7 @@ public interface ManagementServer {
      * @return valid template if success, null otherwise
      * @throws InvalidParameterValueException, ResourceAllocationException
      */
-    VMTemplateVO createPrivateTemplate(VMTemplateVO template, Long userId, long snapshotId, String name, String description) throws InvalidParameterValueException;
+    VMTemplateVO createPrivateTemplate(VMTemplateVO template, Long userId, Long snapshotId, Long volumeId, String name, String description) throws InvalidParameterValueException;
     long createPrivateTemplateAsync(Long userId, long vmId, String name, String description, long guestOSId, Boolean requiresHvm, Integer bits, Boolean passwordEnabled, boolean isPublic, boolean featured, Long snapshotId) throws InvalidParameterValueException, ResourceAllocationException, InternalErrorException;
     
     

core/src/com/cloud/storage/Snapshot.java

@@ -39,7 +39,8 @@ public interface Snapshot {
         Creating,
         CreatedOnPrimary,
         BackingUp,
-        BackedUp;
+        BackedUp,
+        EmptySnapshot;
         
         public String toString() {
             return this.name();

core/src/com/cloud/storage/SnapshotPolicyVO.java

@@ -79,6 +79,10 @@ public class SnapshotPolicyVO {
         return schedule;
     }
     
+    public void setInterval(short interval) {
+        this.interval = interval;
+    }
+
     public void setTimezone(String timezone) {
         this.timezone = timezone;
     }

core/src/com/cloud/storage/SnapshotScheduleVO.java

@@ -77,6 +77,10 @@ public class SnapshotScheduleVO {
         return policyId;
     }
 
+	public void setPolicyId(long policyId) {
+        this.policyId = policyId;
+    }
+
     /**
 	 * @return the scheduledTimestamp
 	 */
@@ -84,6 +88,10 @@ public class SnapshotScheduleVO {
 		return scheduledTimestamp;
 	}
 	
+	public void setScheduledTimestamp(Date scheduledTimestamp) {
+        this.scheduledTimestamp = scheduledTimestamp;
+    }
+
     public Long getAsyncJobId() {
 	    return asyncJobId;
 	}

core/src/com/cloud/storage/SnapshotVO.java

@@ -169,13 +169,12 @@ public class SnapshotVO implements Snapshot {
 		this.prevSnapshotId = prevSnapshotId;
 	}
 
-    public static SnapshotType getSnapshotType(List<Long> policyIds) {
+    public static SnapshotType getSnapshotType(Long policyId) {
-        assert policyIds != null && !policyIds.isEmpty();
+        if (policyId.equals(MANUAL_POLICY_ID)) {
-        SnapshotType snapshotType = SnapshotType.RECURRING;
+            return SnapshotType.MANUAL;
-        if (policyIds.contains(MANUAL_POLICY_ID)) {
+        } else {
-            snapshotType = SnapshotType.MANUAL;
+        	return SnapshotType.RECURRING;
         }
-        return snapshotType;
     }
     
     public static SnapshotType getSnapshotType(String snapshotType) {

core/src/com/cloud/storage/dao/SnapshotPolicyDao.java

@@ -32,4 +32,5 @@ public interface SnapshotPolicyDao extends GenericDao<SnapshotPolicyVO, Long> {
 	List<SnapshotPolicyVO> listByVolumeId(long volumeId, Filter filter);
 	SnapshotPolicyVO findOneByVolumeInterval(long volumeId, short interval);
     List<SnapshotPolicyVO> listActivePolicies();
+    SnapshotPolicyVO findOneByVolume(long volumeId);
 }

core/src/com/cloud/storage/dao/SnapshotPolicyDaoImpl.java

@@ -43,6 +43,14 @@ public class SnapshotPolicyDaoImpl extends GenericDaoBase<SnapshotPolicyVO, Long
 		return findOneIncludingRemovedBy(sc);
 	}
 	
+   @Override
+    public SnapshotPolicyVO findOneByVolume(long volumeId) {
+        SearchCriteria<SnapshotPolicyVO> sc = VolumeIdSearch.create();
+        sc.setParameters("volumeId", volumeId);
+        sc.setParameters("active", true);
+        return findOneBy(sc);
+    }
+	
 	@Override
 	public List<SnapshotPolicyVO> listByVolumeId(long volumeId) {
 		return listByVolumeId(volumeId, null);

core/src/com/cloud/storage/dao/SnapshotScheduleDao.java

@@ -22,6 +22,7 @@ package com.cloud.storage.dao;
 import java.util.Date;
 import java.util.List;
 
+import com.cloud.storage.SnapshotPolicyVO;
 import com.cloud.storage.SnapshotScheduleVO;
 import com.cloud.utils.db.GenericDao;
 
@@ -36,4 +37,6 @@ public interface SnapshotScheduleDao extends GenericDao<SnapshotScheduleVO, Long
 
     SnapshotScheduleVO getCurrentSchedule(Long volumeId, Long policyId, boolean executing);
 
+    SnapshotScheduleVO findOneByVolume(long volumeId);
+
 }

core/src/com/cloud/storage/dao/SnapshotScheduleDaoImpl.java

@@ -26,6 +26,7 @@ import java.util.List;
 
 import javax.ejb.Local;
 
+import com.cloud.storage.SnapshotPolicyVO;
 import com.cloud.storage.SnapshotScheduleVO;
 import com.cloud.utils.db.GenericDaoBase;
 import com.cloud.utils.db.SearchBuilder;
@@ -36,6 +37,7 @@ import com.cloud.utils.db.SearchCriteria;
 public class SnapshotScheduleDaoImpl extends GenericDaoBase<SnapshotScheduleVO, Long> implements SnapshotScheduleDao {
 	protected final SearchBuilder<SnapshotScheduleVO> executableSchedulesSearch;
 	protected final SearchBuilder<SnapshotScheduleVO> coincidingSchedulesSearch;
+    private final SearchBuilder<SnapshotScheduleVO> VolumeIdSearch;
 	// DB constraint: For a given volume and policyId, there will only be one entry in this table.
 	
 	
@@ -52,6 +54,10 @@ public class SnapshotScheduleDaoImpl extends GenericDaoBase<SnapshotScheduleVO,
         coincidingSchedulesSearch.and("asyncJobId", coincidingSchedulesSearch.entity().getAsyncJobId(), SearchCriteria.Op.NULL);
         coincidingSchedulesSearch.done();
         
+        VolumeIdSearch = createSearchBuilder();
+        VolumeIdSearch.and("volumeId", VolumeIdSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
+        VolumeIdSearch.done();
+		
 	}
 	
 	/**
@@ -67,6 +73,13 @@ public class SnapshotScheduleDaoImpl extends GenericDaoBase<SnapshotScheduleVO,
         return listBy(sc);
 	}
 
+	
+    @Override
+    public SnapshotScheduleVO findOneByVolume(long volumeId) {
+        SearchCriteria<SnapshotScheduleVO> sc = VolumeIdSearch.create();
+        sc.setParameters("volumeId", volumeId);
+        return findOneBy(sc);
+    }
 	/**
      * {@inheritDoc} 
      */

scripts/vm/hypervisor/xenserver/vmopsSnapshot

@@ -10,19 +10,12 @@ from util import CommandException
 import vhdutil
 import shutil
 import lvhdutil
-import subprocess
-from lvmcache import LVMCache
-from journaler import Journaler
-from lock import Lock
 import errno
 import subprocess
 import xs_errors
 import cleanup
-import hostvmstats
-import socket
 import stat
 import random
-import tempfile
 
 VHD_UTIL = '/usr/sbin/vhd-util'
 VHD_PREFIX = 'VHD-'
@@ -107,39 +100,31 @@ def delete_secondary_storage_folder(session, args):
         
     return "1"
      
-@echo
-def execute_script(session, args):
-    return ""
-    
 @echo
 def post_create_private_template(session, args):
     local_mount_path = None
-
     try:
-        try:
+        # get local template folder 
-            # Mount the remote templates folder locally
+        sruuid = args["tmpltSrUUID"]
-            remote_mount_path = args["remoteTemplateMountPath"]
+        local_mount_path = os.path.join(SR.MOUNT_BASE, sruuid)
-            local_mount_path = os.path.join(SR.MOUNT_BASE, "template" + str(int(random.random() * 10000)))
-            mount(remote_mount_path, local_mount_path)
-            util.SMlog("Mounted secondary storage template folder")
 
         # Retrieve args
         filename = args["templateFilename"]
         name = args["templateName"]
         description = args["templateDescription"]
         checksum = args["checksum"]
+        size = args["size"]
         virtual_size = args["virtualSize"]
         template_id = args["templateId"]
 
         # Determine the template size
-            template_download_folder = local_mount_path + "/" + args["templateDownloadFolder"]
+        template_install_path = local_mount_path + "/" + filename
-            template_download_path = template_download_folder + filename
+        file_size = os.path.getsize(template_install_path)
-            file_size = os.path.getsize(template_download_path)
         util.SMlog("Got template file_size: " + str(file_size))
             
         # Create the template.properties file
-            template_properties_download_path = template_download_folder + "template.properties"
+        template_properties_install_path = local_mount_path + "/template.properties"
-            f = open(template_properties_download_path, "w")
+        f = open(template_properties_install_path, "w")
         f.write("filename=" + filename + "\n")
         f.write("name=" + filename + "\n")
         f.write("vhd=true\n")
@@ -155,37 +140,15 @@ def post_create_private_template(session, args):
         f.close()
         util.SMlog("Created template.properties file")
            
-            # Create the template install folder if necessary
-            template_install_folder = local_mount_path + "/" + args["templateInstallFolder"]
-            if not os.path.isdir(template_install_folder):
-                current_umask = os.umask(0)
-                os.makedirs(template_install_folder)
-                os.umask(current_umask)
-            
-            # Move the template and the template.properties file to the install folder
-            os.system("mv " + template_download_folder + "/" + filename + " " + template_install_folder)
-            os.system("mv " + template_download_folder + "/template.properties " + template_install_folder)
-            template_install_path = template_install_folder + filename
-            template_properties_install_path = template_install_folder + "template.properties"
-            
         # Set permissions
         permissions = stat.S_IREAD | stat.S_IWRITE | stat.S_IRGRP | stat.S_IWGRP | stat.S_IROTH | stat.S_IWOTH
-            os.chmod(template_install_path, permissions)
         os.chmod(template_properties_install_path, permissions)
         util.SMlog("Set permissions on template and template.properties")
 
-            # Delete the template download folder
-            os.system("rm -rf " + template_download_folder)
     except:
         errMsg = "post_create_private_template failed."
         util.SMlog(errMsg)
         raise xs_errors.XenError(errMsg)
-    finally:
-        if local_mount_path != None:
-            # Unmount the local templates folder
-            umount(local_mount_path)
-            # Remove the local templates folder
-            os.system("rm -rf " + local_mount_path)
 
     return "1" 
   
@@ -237,19 +200,6 @@ def chdir(path):
     util.SMlog("Chdired to " + path)
     return
 
-def coalesce(vhdPath):
-    util.SMlog("Starting to coalesce " + vhdPath + " with its parent")
-    try :
-        cmd = [VHD_UTIL, "coalesce", "-n", vhdPath]
-        txt = util.pread2(cmd)
-    except:
-        errMsg = "Unexpected error while trying to coalesce " + vhdPath + " to its parent"
-        util.SMlog(errMsg)
-        raise xs_errors.XenError(errMsg)
-    util.SMlog("Successfully coalesced " + vhdPath + " with its parent ")
-    
-    return
-
 def scanParent(path):
     # Do a scan for the parent for ISCSI volumes
     # Note that the parent need not be visible on the XenServer
@@ -313,30 +263,14 @@ def rename(originalVHD, newVHD):
         raise xs_errors.XenError(errMsg)
     return
 
-def coalesceToChild(backupVHD, childUUID, isISCSI):
-    # coalesce childVHD with its parent
-    childVHD = getVHD(childUUID, isISCSI)
-    util.SMlog("childVHD: " + childVHD)
-    # check for existence of childVHD
-    isfile(childVHD, False)
-    util.SMlog("childVHD " + childVHD + " exists")
-    # No exception thrown, file exists
-    coalesce(childVHD)
-    
-    # rename the existing backupVHD file to childVHD
-    # childVHD file automatically gets overwritten
-    rename(backupVHD, childVHD)
-
-    # parent of the newly coalesced file still remains the same.
-    # child of childVHD has it's parent name still set to childVHD.
-    # So the VHD chain hasn't been broken.
-    return
-
 def makedirs(path):
     if not os.path.isdir(path):
         try:
             os.makedirs(path)
         except OSError, (errno, strerror):
+            umount(path)
+            if os.path.isdir(path):
+                return
             errMsg = "OSError while creating " + path + " with errno: " + str(errno) + " and strerr: " + strerror
             util.SMlog(errMsg)
             raise xs_errors.XenError(errMsg)
@@ -358,41 +292,16 @@ def mount(remoteDir, localDir):
     return
 
 def umount(localDir):
-    success = False
-    if os.path.isdir(localDir) and os.path.ismount(localDir):
     try: 
         cmd = ['umount', localDir]
         util.pread2(cmd)
     except CommandException:
         errMsg = "CommandException raised while trying to umount " + localDir 
         util.SMlog(errMsg)
-            raise xs_errors.XenError(errMsg)
-        util.SMlog("Successfully unmounted " + localDir)
-        success = True
-    else:
-        util.SMlog("LocalDir: " + localDir + " doesn't exist or is not a mount point")
-
         return
 
-
+    util.SMlog("Successfully unmounted " + localDir)
-def mountTemplatesDir(secondaryStorageMountPath, templatePath):
+    return
-    # Aim is to mount <secondaryStorageMountPath>/<templatePath> on 
-    # SR.MOUNT_BASE/<random_uuid>
-    # It will be unmounted after createVolumeFromSnapshot finishes.
-    # It should be mounted read-only, but don't know how to do that
-    # The random-uuid saves us from conflicts while restoring two different root volumes
-    
-    absoluteTemplatePath = os.path.join(secondaryStorageMountPath, templatePath)
-    absoluteTemplateDir = os.path.dirname(absoluteTemplatePath)
-
-    randomUUID = util.gen_uuid()
-    localTemplateDir = os.path.join(SR.MOUNT_BASE, randomUUID)
-    # create the temp dir
-    makedirs(localTemplateDir)
-    # mount
-    mount(absoluteTemplateDir, localTemplateDir)
-
-    return localTemplateDir
 
 def mountSnapshotsDir(secondaryStorageMountPath, relativeDir, dcId, accountId, instanceId):
     # The aim is to mount secondaryStorageMountPath on 
@@ -445,6 +354,9 @@ def getPrimarySRPath(primaryStorageSRUuid, isISCSI):
     else:
         return os.path.join(SR.MOUNT_BASE, primaryStorageSRUuid)
 
+def getBackupVHD(UUID):
+    return UUID + '.' + SR.DEFAULT_TAP
+
 def getVHD(UUID, isISCSI):
     if isISCSI:
         return VHD_PREFIX + UUID
@@ -529,122 +441,20 @@ def isVolumeAvailable(path):
 
     return (status == "1")  
 
-def getLastSnapshotUuid(volumeUuid, expectedLastSnapshotUuid):
+def getVhdParent(session, args):
-    actualLastSnapshotUuid = ''
+    util.SMlog("getParent with " + str(args))
-    if volumeUuid:
-        cmd = ['xe', 'vdi-param-get', 'uuid=' + volumeUuid, 'param-name=snapshots']
-        stdout = ''
-        try:
-            stdout = util.pread2(cmd)
-        except: #CommandException, (rc, cmdListStr, stderr):
-            #errMsg = "CommandException thrown while executing: " + cmdListStr + " with return code: " + str(rc) + " and stderr: " + stderr
-            errMsg = "Unexpected error while executing cmd: " + str(cmd)
-            util.SMlog(errMsg)
-            raise xs_errors.XenError(errMsg)
-        if stdout:
-            snapshots = stdout.split(';')
-            if len(snapshots) == 1:
-                if snapshots[0] == expectedLastSnapshotUuid:
-                    actualLastSnapshotUuid = expectedLastSnapshotUuid
-            elif len(snapshots) == 2:
-                # We expect only 1 snapshot to be present. If there is another that is unexpected and the last one
-                if (expectedLastSnapshotUuid == snapshots[0].strip()):
-                    actualLastSnapshotUuid = snapshots[1].strip()
-                else:
-                    # it should be equal to snapshots[1]. Else I have no idea.
-                    actualLastSnapshotUuid = snapshots[0].strip()
-            else:
-                # len(snapshots) > 2:
-                errMsg = "Volume: " + volumeUuid + " has more than 2 snapshots: " + str(snapshots)
-                util.SMlog(errMsg)
-                raise xs_errors.XenError(errMsg)
-                
-    return actualLastSnapshotUuid
-    
-@echo
-def validateSnapshot(session, args):
-    util.SMlog("Called validateSnapshot with " + str(args))
     primaryStorageSRUuid      = args['primaryStorageSRUuid']
-    volumeUuid           = args['volumeUuid']
+    snapshotUuid              = args['snapshotUuid']
-    firstBackupUuid      = args['firstBackupUuid']
-    previousSnapshotUuid = args['previousSnapshotUuid']
-    templateUuid         = args['templateUuid']
     isISCSI                   = getIsTrueString(args['isISCSI']) 
 
     primarySRPath = getPrimarySRPath(primaryStorageSRUuid, isISCSI)
     util.SMlog("primarySRPath: " + primarySRPath)
-    volumeVHD    = getVHD(volumeUuid, isISCSI)
-    volumePath   = os.path.join(primarySRPath, volumeVHD)
-    util.SMlog("volumePath: " + volumePath)
 
-    baseCopyUuid = ''
+    baseCopyUuid = getParentOfSnapshot(snapshotUuid, primarySRPath, isISCSI)
-    wasVolumeAvailable = True
-    if isISCSI:
-        wasVolumeAvailable = isVolumeAvailable(volumePath)
-        if not wasVolumeAvailable:
-            # make it available
-            checkVolumeAvailablility(volumePath)
-        baseCopyUuid = scanParent(volumePath)
-    else:
-        baseCopyUuid = getParent(volumePath, isISCSI)
 
-    if baseCopyUuid is None:
+    return  baseCopyUuid
-        # Make it an empty string so that it can be used in the return value
-        baseCopyUuid = ''
 
-    actualSnapshotUuid = getLastSnapshotUuid(volumeUuid, previousSnapshotUuid)
-    expectedActual = firstBackupUuid + "#" + baseCopyUuid + "#" + actualSnapshotUuid
-    if firstBackupUuid:
-        # This is not the first snapshot
-        if baseCopyUuid and (baseCopyUuid == firstBackupUuid):
-            retval = "1#"
-        else:
-            retval = "0#"
-    else: 
-        if templateUuid:
-            # The DB thinks this is the first snapshot of a ROOT DISK
-            # The parent of the volume should be the base template, which is also the parent of the given templateUuid.
-            templateVHD = getVHD(templateUuid, isISCSI)
-            templatePath = os.path.join(primarySRPath, templateVHD)
-            baseTemplateUuid = ''
-            wasTemplateAvailable = True
-            if isISCSI:
-                wasTemplateAvailable = isVolumeAvailable(templatePath)
-                if not wasVolumeAvailable:
-                    # make it available
-                    checkVolumeAvailablility(templatePath)
-                baseTemplateUuid = scanParent(templatePath)
-            else:
-                baseTemplateUuid = getParent(templatePath, isISCSI)
 
-            if baseTemplateUuid is None:
-                # This will never happen.
-                baseTemplateUuid = ''
-
-            expectedActual = baseTemplateUuid + "#" + baseCopyUuid + "#" + actualSnapshotUuid
-            if baseTemplateUuid and (baseCopyUuid == baseTemplateUuid):
-                retval = "1#"
-            else:
-                retval = "0#"
-
-            if isISCSI and not wasTemplateAvailable:
-                manageAvailability(templatePath, '-an')
-
-        else:
-            # The DB thinks this is the first snapshot of a DATA DISK. 
-            # The volume VDI should not have any parent.
-            if not baseCopyUuid:
-                retval = "1#"
-            else:
-                retval = "0#"
-
-    # Set the volume's visibility back to what it was.
-    if isISCSI and not wasVolumeAvailable:
-        manageAvailability(volumePath, '-an')
-
-    return retval + expectedActual
-
-@echo
 def backupSnapshot(session, args):
     util.SMlog("Called backupSnapshot with " + str(args))
     primaryStorageSRUuid      = args['primaryStorageSRUuid']
@@ -655,7 +465,6 @@ def backupSnapshot(session, args):
     snapshotUuid              = args['snapshotUuid']
     prevSnapshotUuid          = args['prevSnapshotUuid']
     prevBackupUuid            = args['prevBackupUuid']
-    isFirstSnapshotOfRootVolume =  getIsTrueString(args['isFirstSnapshotOfRootVolume'])
     isISCSI                   = getIsTrueString(args['isISCSI']) 
 
     primarySRPath = getPrimarySRPath(primaryStorageSRUuid, isISCSI)
@@ -675,28 +484,23 @@ def backupSnapshot(session, args):
     # on it.
     backupsDir = mountSnapshotsDir(secondaryStorageMountPath, "snapshots", dcId, accountId, volumeId)
     util.SMlog("Backups dir " + backupsDir)
-    # chdir to the backupsDir for convenience
-    chdir(backupsDir)
 
     if baseCopyUuid == prevBaseCopyUuid:
         # There has been no change since the last snapshot so no need to backup 
         util.SMlog("There has been no change since the last snapshot with backup: " + prevBaseCopyUuid) 
-        if isFirstSnapshotOfRootVolume:
-            # baseCopyUuid is template. That is *NOT* the backup of any
-            # snapshot. There is no backup. So create an empty dummyVHD representing the empty snapshot.
-            # This will prevent deleteSnapshotBackup and createVolumeFromSnapshot from breaking.
-            prevBaseCopyUuid = createDummyVHD(baseCopyPath, backupsDir, isISCSI)
-            # The backup snapshot is the new dummy VHD created. 
-
         # Set the uuid of the current backup to that of last backup
         txt = "1#" + prevBaseCopyUuid
         return txt
 
     # Check existence of snapshot on primary storage 
     isfile(baseCopyPath, isISCSI)
-    # copy baseCopyPath to backupsDir
+    # copy baseCopyPath to backupsDir with new uuid
-    backupFile = os.path.join(backupsDir, baseCopyVHD)
+    backupUuid = util.gen_uuid()
+    backupVHD = getBackupVHD(backupUuid)  
+    backupFile = os.path.join(backupsDir, backupVHD)
+    util.SMlog("Back up " + baseCopyUuid + " to Secondary Storage as " + backupUuid)
     copyfile(baseCopyPath, backupFile, isISCSI)
+    vhdutil.setHidden(backupFile, False)
     
     # Now set the availability of the snapshotPath and the baseCopyPath to false
     makeUnavailable(snapshotUuid, primarySRPath, isISCSI)
@@ -705,44 +509,18 @@ def backupSnapshot(session, args):
         makeUnavailable(prevSnapshotUuid, primarySRPath, isISCSI)
         makeUnavailable(prevBaseCopyUuid, primarySRPath, isISCSI)
 
-    if isFirstSnapshotOfRootVolume:
-        # First snapshot of the root volume.
-        # It's parent is not null, but the template vhd.
-        # Create a dummy empty vhd and set the parent of backupVHD to it.
-        # This will prevent deleteSnapshotBackup and createVolumeFromSnapshot from breaking
-        prevBackupUuid = createDummyVHD(baseCopyPath, backupsDir, isISCSI)
-
     # Because the primary storage is always scanned, the parent of this base copy is always the first base copy.
     # We don't want that, we want a chain of VHDs each of which is a delta from the previous.
     # So set the parent of the current baseCopyVHD to prevBackupVHD 
     if prevBackupUuid:
         # If there was a previous snapshot
-        prevBackupVHD = getVHD(prevBackupUuid, isISCSI)
+        prevBackupVHD = getBackupVHD(prevBackupUuid)
-        setParent(prevBackupVHD, backupFile)
+        prevBackupFile = os.path.join(backupsDir, prevBackupVHD)
+        setParent(prevBackupFile, backupFile)
 
-    txt = "1#" + baseCopyUuid
+    txt = "1#" + backupUuid
     return txt
 
-def createDummyVHD(baseCopyPath, backupsDir, isISCSI):
-    dummyUUID = ''
-    try:
-        dummyUUID = util.gen_uuid()
-        dummyVHD = getVHD(dummyUUID, isISCSI)
-        if isISCSI:
-            checkVolumeAvailablility(baseCopyPath)
-        cmd = [VHD_UTIL, 'query', '-v', '-n', baseCopyPath]
-        virtualSizeInMB = util.pread2(cmd)
-        util.SMlog("Virtual size of " + baseCopyPath + " is " + virtualSizeInMB)
-        cmd = [VHD_UTIL, 'create', '-n', dummyVHD, '-s', virtualSizeInMB]
-        util.pread2(cmd)
-    except CommandException:
-        errMsg = "Unexpected error while creating a dummy VHD " + dummyVHD + " on " + backupsDir
-        util.SMlog(errMsg)
-        raise xs_errors.XenError(errMsg)
-    util.SMlog("Successfully created a new dummy VHD: " + dummyVHD + " on " + backupsDir)
-
-    return dummyUUID
-
 @echo
 def deleteSnapshotBackup(session, args):
     util.SMlog("Calling deleteSnapshotBackup with " + str(args))
@@ -751,14 +529,12 @@ def deleteSnapshotBackup(session, args):
     volumeId                  = args['volumeId']
     secondaryStorageMountPath = args['secondaryStorageMountPath']
     backupUUID                = args['backupUUID']
-    childUUID                 = args['childUUID']
-    isISCSI                   = getIsTrueString(args['isISCSI'])
 
     backupsDir = mountSnapshotsDir(secondaryStorageMountPath, "snapshots", dcId, accountId, volumeId)
     # chdir to the backupsDir for convenience
     chdir(backupsDir)
 
-    backupVHD = getVHD(backupUUID, isISCSI)
+    backupVHD = getBackupVHD(backupUUID)
     util.SMlog("checking existence of " + backupVHD)
 
     # The backupVHD is on secondary which is NFS and not ISCSI.
@@ -767,10 +543,6 @@ def deleteSnapshotBackup(session, args):
         return "1"
     util.SMlog("backupVHD " + backupVHD + " exists.")
         
-    # Case 1) childUUID exists
-    if childUUID:
-        coalesceToChild(backupVHD, childUUID, isISCSI)
-    else:
     # Just delete the backupVHD
     try:
         os.remove(backupVHD)
@@ -781,197 +553,6 @@ def deleteSnapshotBackup(session, args):
 
     return "1"
    
-@echo
-def createVolumeFromSnapshot(session, args):
-    util.SMlog("Calling createVolumeFromSnapshot with " + str(args))
-    dcId                      = args['dcId']
-    accountId                 = args['accountId']
-    volumeId                  = args['volumeId']
-    secondaryStorageMountPath = args['secondaryStorageMountPath']
-    backedUpSnapshotUuid      = args['backedUpSnapshotUuid']
-    templatePath              = args['templatePath']
-    templateDownloadFolder    = args['templateDownloadFolder']
-    isISCSI                   = getIsTrueString(args['isISCSI'])
-
-    backupsDir = mountSnapshotsDir(secondaryStorageMountPath, "snapshots", dcId, accountId, volumeId)
-    util.SMlog("Backups dir " + backupsDir)
-
-    # chdir to the backupsDir for convenience
-    chdir(backupsDir)
-
-    # Get the parent VHD chain of the backupSnapshotVHD
-    vhdChain = []
-    uuid = backedUpSnapshotUuid
-    while uuid is not None:
-        util.SMlog("Current uuid in parent chain " + uuid)
-        vhd = getVHD(uuid, isISCSI)
-        vhdChain.append(vhd)
-        uuid = getParent(vhd, isISCSI)
-    util.SMlog("successfully created the parent chain " + str(vhdChain))
-
-    destDirParent = ''
-    destDir = ''
-    if templateDownloadFolder:
-        # Copy all the vhds to the final destination templates dir
-        # It is some random directory on the primary created for templates.
-        destDirParent = os.path.join(SR.MOUNT_BASE, "mount" + str(int(random.random() * 1000000)))
-        destDir = os.path.join(destDirParent, templateDownloadFolder)
-        # create the this directory, if it isn't there
-        makedirs(destDir)
-    else:
-        # Copy all the vhds to a temp directory
-        # Create a temp directory
-        destDir = backupsDir + '_temp'
-
-    # Delete the temp directory if it already exists (from a previous createVolumeFromSnapshot)
-    rmtree(destDir)
-    
-    if templateDownloadFolder:
-        # The destDir was created in create_secondary_storage_folder but is not mounted on the primary. Mount it again.
-        remoteMountPoint = os.path.join(secondaryStorageMountPath, "template");
-        remoteMountPoint = os.path.join(remoteMountPoint, templateDownloadFolder)
-        mount(remoteMountPoint, destDir)
-    else:
-        # The parent of the destDir is the snapshots dir and is mounted on the primary. Just create the directory structure.
-        makedirs(destDir)
-
-    # Copy
-    for vhd in vhdChain:
-        vhdPath = os.path.join(backupsDir, vhd)
-        tempFile = os.path.join(destDir, vhd)
-        # We are copying files on secondary storage which is NFS. Set isISCSI to false
-        copyfile(vhdPath, tempFile, False)
-    util.SMlog("Successfully copied all files")
-
-    # coalesce the vhd chains from bottom to top
-    # chdir to destDir for convenience
-    chdir(destDir)
-       
-    # coalesce
-    i = 0
-    finalVhd = vhdChain[0]
-    for vhd in vhdChain:
-        finalVhd = vhdChain[i]
-        last = len(vhdChain) - 1
-        if i == last:
-            # last vhd, has no parent. Don't coalesce
-            break
-        if templatePath and i == (last - 1):
-            # Hack for root disks, the first Vhd is a dummy one. 
-            # Do not coalesce the actual disk with the dummy one.
-            # Instead coalesce it with the templateVHD.
-            break
-
-        i = i + 1
-        # They are arranged from child to parent.
-        util.SMlog("Starting to coalesce " + vhd + " with its parent")
-        try:
-            cmd = [VHD_UTIL, "coalesce", "-n", vhd]
-            txt = util.pread2(cmd)
-        except:
-            errMsg = "Unexpected error while trying to coalesce " + vhd + " to its parent" 
-            util.SMlog(errMsg)
-            raise xs_errors.XenError(errMsg)
-        util.SMlog("Successfully coalesced " + vhd + " with its parent")
-
-        # Remove the child vhd
-        try:
-            os.remove(vhd)
-        except OSError, (errno, strerror):
-            errMsg = "OSError while removing " + vhd + " with errno: " + str(errno) + " and strerr: " + strerror
-            util.SMlog(errMsg)
-            raise xs_errors.XenError(errMsg)
-    
-    util.SMlog("successfully coalesced all vhds to the parent " + finalVhd)
-    finalVhdPath = os.path.join(destDir, finalVhd)
-
-    # This vhd has to be introduced with a new uuid because of the VDI UUID
-    # uniqueness constraint
-    newUUID = ''
-    try:
-        newUUID = util.gen_uuid()
-    except:
-        errMsg = "Unexpected error while trying to generate a uuid"
-        util.SMlog(errMsg)
-        raise xs_errors.XenError(errMsg)
-    util.SMlog("generated a uuid " + newUUID)
-
-    # Now, at the Java layer an NFS SR is created with mount point destDir and scanned. The vhd
-    # file is automatically introduced and a vdi.copy is done to move it to
-    # primary storage.
-    # new vhd file is created on NFS. So it should have NFS naming convention,
-    # set isISCSI to false
-    newVhd = getVHD(newUUID, False)
-    rename(finalVhd, newVhd)
-
-    # For root disk
-    if templatePath:
-        # This will create a vhd on secondary storage destDir with name newVhd
-        mergeTemplateAndSnapshot(secondaryStorageMountPath, templatePath, destDir, newVhd, isISCSI)
-
-    # set the hidden flag of the new VHD to false, so that it doesn't get deleted when the SR scan is done.
-    try:
-        vhdutil.setHidden(newVhd, False)
-    except:
-        errMsg = "Unexpected error while trying to set Hidden flag of " + newVhd
-        util.SMlog(errMsg)
-        raise xs_errors.XenError(errMsg)
-    util.SMlog("Successfully set hidden flag of " + newVhd)
-
-    virtualSizeInMB = 0
-    try:
-        cmd = [VHD_UTIL, 'query', '-v', '-n', newVhd]
-        virtualSizeInMB = util.pread2(cmd)
-        util.SMlog("Virtual size of " + newVhd + " is " + virtualSizeInMB)
-    except:
-        errMsg = "Unexpected error while trying to get the virtual size of " + newVhd
-        util.SMlog(errMsg)
-        raise xs_errors.XenError(errMsg)
-
-    if templateDownloadFolder:
-        # We are done with the destDir on the primary, unmount it and delete the random directory.
-        # Current dir is destDir
-        # cd to something else before unmounting
-        chdir(backupsDir) # as good as anything else
-        # unmount what was mounted.
-        umount(destDir)
-        # Remove the tree starting from the mountXXXX part, just the directories
-        rmtree(destDirParent)
-        # The coalesced data is still available on the secondary.
-
-    return "1#" + newUUID + "#" + virtualSizeInMB
-
-def mergeTemplateAndSnapshot(secondaryStorageMountPath, templatePath, destDir, newVhd, isISCSI):
-    # Mount the template directory present on the secondary to the primary
-    templateDirOnPrimary = mountTemplatesDir(secondaryStorageMountPath, templatePath)
-
-    # Current dir is destDir
-    templateVHD = os.path.basename(templatePath)
-    templatePathOnPrimary = os.path.join(templateDirOnPrimary, templateVHD)
-    templatePathOnTemp = os.path.join(destDir, templateVHD)
-    # Copying from secondary to secondary, so set ISCSI to False
-    copyfile(templatePathOnPrimary, templatePathOnTemp, False)
-    
-    # unmount the temporary directory created for copying the template
-    umount(templateDirOnPrimary)
-
-    # get the dummyVHD which is the parent of the new Vhd
-    dummyUUID = getParent(newVhd, isISCSI)
-    dummyVHD = getVHD(dummyUUID, isISCSI)
-    # set the parent of the newVhd to the templateVHD on secondary
-    setParent(templateVHD, newVhd)
-    # remove the dummyVHD as we don't have any use for it and it wil
-    # lie around after we do an SR scan
-    os.remove(dummyVHD)
-    
-    # coalesce the two VHDs into templateVHD
-    coalesce(newVhd)
-
-    # rename templateVHD to newVhd
-    rename(templateVHD, newVhd)
-
-    return
-    
 def rmtree(path):
     if os.path.isdir(path):
         try:
@@ -1000,5 +581,5 @@ def deleteSnapshotsDir(session, args):
     return "1"
    
 if __name__ == "__main__":
-    XenAPIPlugin.dispatch({"create_secondary_storage_folder":create_secondary_storage_folder, "delete_secondary_storage_folder":delete_secondary_storage_folder, "post_create_private_template":post_create_private_template, "backupSnapshot": backupSnapshot, "deleteSnapshotBackup": deleteSnapshotBackup, "createVolumeFromSnapshot": createVolumeFromSnapshot, "unmountSnapshotsDir": unmountSnapshotsDir, "deleteSnapshotsDir": deleteSnapshotsDir, "validateSnapshot" : validateSnapshot})
+    XenAPIPlugin.dispatch({"getVhdParent":getVhdParent,  "create_secondary_storage_folder":create_secondary_storage_folder, "delete_secondary_storage_folder":delete_secondary_storage_folder, "post_create_private_template":post_create_private_template, "backupSnapshot": backupSnapshot, "deleteSnapshotBackup": deleteSnapshotBackup, "unmountSnapshotsDir": unmountSnapshotsDir, "deleteSnapshotsDir": deleteSnapshotsDir})
 

server/src/com/cloud/async/AsyncJobManagerImpl.java

@@ -91,8 +91,7 @@ public class AsyncJobManagerImpl implements AsyncJobManager {
     
     @Override @DB
     public long submitAsyncJob(AsyncJobVO job, boolean scheduleJobExecutionInContext) {
-    	if(s_logger.isDebugEnabled())
+
-    		s_logger.debug("submit async job-" + job.getId() + ", details: " + job.toString());
     	
     	AsyncJobExecutor executor = getJobExecutor(job);
     	if(executor == null) {
@@ -108,6 +107,9 @@ public class AsyncJobManagerImpl implements AsyncJobManager {
         		executor.setSyncSource(null);
         		executor.setJob(job);
         		scheduleExecution(executor, scheduleJobExecutionInContext);
+                if(s_logger.isDebugEnabled()) {
+                    s_logger.debug("submit async job-" + job.getId() + ", details: " + job.toString());
+                }
         		return job.getId();
         	} catch(Exception e) {
         		s_logger.error("Unexpected exception: ", e);

server/src/com/cloud/async/executor/CreatePrivateTemplateExecutor.java

@@ -116,48 +116,23 @@ public class CreatePrivateTemplateExecutor extends VolumeOperationExecutor {
         
                     	asyncMgr.updateAsyncJobAttachment(job.getId(), "vm_template", template.getId());
                     	asyncMgr.updateAsyncJobStatus(job.getId(), BaseCmd.PROGRESS_INSTANCE_CREATED, template.getId());
-                    	Snapshot snapshot = null;
 
-            	        if (snapshotId == null) {
+                        template = managerServer.createPrivateTemplate(template, param.getUserId(), snapshotId, volumeId, param.getName(), param.getDescription());
-            	            // We are create private template from volume. Create a snapshot, copy the vhd chain of the disk to secondary storage. 
-            	            // For template snapshot, we use a separate snapshot method.
-            	            snapshot = vmMgr.createTemplateSnapshot(param.getUserId(), param.getVolumeId());
-            	        }
-            	        else {
-            	            // We are creating a private template from an already present snapshot. 
-            	            // This snapshot is already backed up on secondary storage.
-            	            snapshot = managerServer.findSnapshotById(snapshotId);
-            	        }
-            	        
-                        if (snapshot == null) {
-                            details += ", reason: Failed to create snapshot for basis of private template";
-                        } else {
-                            param.setSnapshotId(snapshot.getId());
-            		    	
-            				template = managerServer.createPrivateTemplate(template, 
-            				                                               param.getUserId(), 
-            						                                       param.getSnapshotId(), 
-            						                                       param.getName(), 
-            						                                       param.getDescription());
 
                         if (template != null) {
-            					VMTemplateHostVO templateHostRef = managerServer.findTemplateHostRef(template.getId(), volume.getDataCenterId());
+                            VMTemplateHostVO templateHostRef = managerServer.findTemplateHostRef(template.getId(),
+                                    volume.getDataCenterId());
                             jobStatus = AsyncJobResult.STATUS_SUCCEEDED;
                             resultCode = 0;
                             details = null;
-            				    String eventParams = "id="+template.getId()+"\nname=" + template.getName() +"\nsize="+volume.getSize()+ "\ndcId="+volume.getDataCenterId();
+                            String eventParams = "id=" + template.getId() + "\nname=" + template.getName() + "\nsize="
-            				    managerServer.saveEvent(param.getUserId(), param.getAccountId(), EventVO.LEVEL_INFO, EventTypes.EVENT_TEMPLATE_CREATE, "Successfully created Template " +param.getName(), eventParams ,param.getEventId());
+                                    + volume.getSize() + "\ndcId=" + volume.getDataCenterId();
+                            managerServer.saveEvent(param.getUserId(), param.getAccountId(), EventVO.LEVEL_INFO,
+                                    EventTypes.EVENT_TEMPLATE_CREATE, "Successfully created Template "
+                                            + param.getName(), eventParams, param.getEventId());
                             resultObject = composeResultObject(template, templateHostRef, volume.getDataCenterId());
                         }
 
-            				// Irrespective of whether the template was created or not, 
-            				// cleanup the snapshot taken for this template. (If this template is created from a volume and not a snapshot) 
-            				if(snapshotId == null) {
-            				    // Template was created from volume
-            				    // and snapshot is not null.
-            				    managerServer.destroyTemplateSnapshot(param.getUserId(), snapshot.getId());
-            				}
-                        }
                     }
 		        }
 			} catch (InvalidParameterValueException e) {

server/src/com/cloud/async/executor/CreateSnapshotExecutor.java

@@ -33,6 +33,7 @@ import com.cloud.storage.Snapshot;
 import com.cloud.storage.SnapshotVO;
 import com.cloud.storage.VolumeVO;
 import com.cloud.storage.Snapshot.SnapshotType;
+import com.cloud.storage.dao.VolumeDao;
 import com.cloud.storage.snapshot.SnapshotManager;
 import com.cloud.user.Account;
 import com.google.gson.Gson;
@@ -54,8 +55,10 @@ public class CreateSnapshotExecutor extends BaseAsyncJobExecutor {
 		} else {
 	    	SnapshotOperationParam param = gson.fromJson(job.getCmdInfo(), SnapshotOperationParam.class);
 	    	SnapshotManager snapshotManager = asyncMgr.getExecutorContext().getSnapshotMgr();
+	    	VolumeDao volumeDao = asyncMgr.getExecutorContext().getVolumeDao();
 	    	long volumeId = param.getVolumeId();
-	    	List<Long> policyIds = param.getPolicyIds();
+	    	long policyId = param.getPolicyId();
+	    	
 	    	long snapshotId = 0;
 	    	long userId = param.getUserId();
 	    	
@@ -65,9 +68,12 @@ public class CreateSnapshotExecutor extends BaseAsyncJobExecutor {
             int result = AsyncJobResult.STATUS_FAILED;
             int errorCode = BaseCmd.INTERNAL_ERROR;
             Object resultObject = "Failed to create snapshot.";
+            VolumeVO vol = null;
             
 	    	try {
-	    	    SnapshotVO snapshot = snapshotManager.createSnapshot(userId, param.getVolumeId(), param.getPolicyIds());
+	    		vol = volumeDao.acquire(volumeId, 10);
+	    		if( vol != null) {
+		    	    SnapshotVO snapshot = snapshotManager.createSnapshot(userId, volumeId, policyId);
 	
 			    	if (snapshot != null && snapshot.getStatus() == Snapshot.Status.CreatedOnPrimary) {
 					    snapshotId = snapshot.getId();
@@ -82,17 +88,28 @@ public class CreateSnapshotExecutor extends BaseAsyncJobExecutor {
 					        // More specific error
 					        resultObject = "Created snapshot: " + snapshotId + " on primary but failed to backup on secondary";
 					    }
+					} else if (snapshot != null && snapshot.getStatus() == Snapshot.Status.EmptySnapshot) {
+					    resultObject ="There is no change since last snapshot, please use last snapshot";
+		                   s_logger.warn(resultObject);
+					}
+	    		} else {
+	    			resultObject = "Another snapshot is being created for " + volumeId + " try another time ";
+		    		s_logger.warn(resultObject);
 	    		}
 			} catch(Exception e) {
 	    	    resultObject = "Unable to create snapshot: " + e.getMessage();
 	    		s_logger.warn(resultObject, e);
+	    	} finally {
+	    		if( vol != null ){
+	    			volumeDao.release(volumeId);
+	    		}
 	    	}
 
 			// In all cases, ensure that we call completeAsyncJob to the asyncMgr.
 	    	asyncMgr.completeAsyncJob(jobId, result, errorCode, resultObject);
 	    	
 	    	// Cleanup jobs to do after the snapshot has been created.
-	    	snapshotManager.postCreateSnapshot(userId, volumeId, snapshotId, policyIds, backedUp);
+	    	snapshotManager.postCreateSnapshot(userId, volumeId, snapshotId, policyId, backedUp);
 	    	return true;
 		}
 	}

server/src/com/cloud/async/executor/SnapshotOperationParam.java

@@ -25,7 +25,6 @@ public class SnapshotOperationParam {
     private long accountId;
     private long userId;
 	private long snapshotId = 0;
-	private List<Long> policyIds = null;
 	private long policyId = 0;
 	private long volumeId;
 	private String name = null;
@@ -44,11 +43,11 @@ public class SnapshotOperationParam {
 	}
 	
 	// Used to create a snapshot
-    public SnapshotOperationParam(long userId, long accountId, long volumeId, List<Long> policyIds) {
+    public SnapshotOperationParam(long userId, long accountId, long volumeId, long policyId) {
         setUserId(userId);
         setAccountId(accountId);
         setVolumeId(volumeId);
-        this.policyIds = policyIds;
+		this.policyId = policyId;
     }
     
     // Used for CreateVolumeFromSnapshot
@@ -84,10 +83,6 @@ public class SnapshotOperationParam {
 		this.snapshotId = snapshotId;
 	}
 	
-	public List<Long> getPolicyIds() {
-	    return policyIds;
-	}
-	
 	public long getPolicyId() {
 	    return policyId;
 	}

server/src/com/cloud/server/ManagementServerImpl.java

@@ -355,7 +355,6 @@ public class ManagementServerImpl implements ManagementServer {
     private final AsyncJobManager _asyncMgr;
     private final TemplateManager _tmpltMgr;
     private final SnapshotManager _snapMgr;
-    private final SnapshotScheduler _snapshotScheduler;
     private final NetworkGroupManager _networkGroupMgr;
     private final int _purgeDelay;
     private final boolean _directAttachNetworkExternalIpAllocator;
@@ -454,7 +453,6 @@ public class ManagementServerImpl implements ManagementServer {
         _asyncMgr = locator.getManager(AsyncJobManager.class);
         _tmpltMgr = locator.getManager(TemplateManager.class);
         _snapMgr = locator.getManager(SnapshotManager.class);
-        _snapshotScheduler = locator.getManager(SnapshotScheduler.class);
         _networkGroupMgr = locator.getManager(NetworkGroupManager.class);
         _uploadMonitor = locator.getManager(UploadMonitor.class);                
         
@@ -6862,21 +6860,12 @@ public class ManagementServerImpl implements ManagementServer {
                 throw new InvalidParameterValueException("Snapshots of volumes attached to System or router VM are not allowed");
             }
         }
-        
+        long policyId = Snapshot.MANUAL_POLICY_ID;
-    	Long jobId = _snapshotScheduler.scheduleManualSnapshot(userId, volumeId);
+        long jobId = _snapMgr.createSnapshotAsync(userId, volumeId, policyId);
-    	if (jobId == null) {
-    	    throw new InternalErrorException("Snapshot could not be scheduled because there is another snapshot underway for the same volume. " +
-    	    		                         "Please wait for some time.");
-    	}
         
     	return jobId;
     }
 
-    @Override
-    public SnapshotVO createTemplateSnapshot(Long userId, long volumeId) {
-        return _vmMgr.createTemplateSnapshot(userId, volumeId);
-    }
-    
     @Override
     public boolean destroyTemplateSnapshot(Long userId, long snapshotId) {
         return _vmMgr.destroyTemplateSnapshot(userId, snapshotId);
@@ -6963,7 +6952,7 @@ public class ManagementServerImpl implements ManagementServer {
     }
 
     @Override
-    public Snapshot findSnapshotById(long snapshotId) {
+    public SnapshotVO findSnapshotById(long snapshotId) {
         SnapshotVO snapshot = _snapshotDao.findById(snapshotId);
         if (snapshot != null && snapshot.getRemoved() == null && snapshot.getStatus() == Snapshot.Status.BackedUp) {
             return snapshot;
@@ -6974,9 +6963,9 @@ public class ManagementServerImpl implements ManagementServer {
     }
 
     @Override
-    public VMTemplateVO createPrivateTemplate(VMTemplateVO template, Long userId, long snapshotId, String name, String description) throws InvalidParameterValueException {
+    public VMTemplateVO createPrivateTemplate(VMTemplateVO template, Long userId, Long snapshotId, Long volumeId, String name, String description) throws InvalidParameterValueException {
 
-        return _vmMgr.createPrivateTemplate(template, userId, snapshotId, name, description);
+        return _vmMgr.createPrivateTemplate(template, userId, snapshotId, volumeId, name, description);
     }
 
     @Override

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

@@ -490,7 +490,7 @@ public class StorageManagerImpl implements StorageManager {
     }
     
     @DB
-    protected Pair<VolumeVO, String> createVolumeFromSnapshot(long userId, long accountId, String userSpecifiedName, DataCenterVO dc, DiskOfferingVO diskOffering, SnapshotVO snapshot, String templatePath, Long originalVolumeSize, VMTemplateVO template) {
+    protected Pair<VolumeVO, String> createVolumeFromSnapshot(long userId, long accountId, String userSpecifiedName, DataCenterVO dc, SnapshotVO snapshot, long virtualsize) {
         
         VolumeVO createdVolume = null;
         Long volumeId = null;
@@ -506,10 +506,7 @@ public class StorageManagerImpl implements StorageManager {
         volume.setAccountId(accountId);
         volume.setDomainId(account.getDomainId());
         volume.setMirrorState(MirrorState.NOT_MIRRORED);
-        if (diskOffering != null) {
+        volume.setSize(virtualsize);
-            volume.setDiskOfferingId(diskOffering.getId());
-        }
-        volume.setSize(originalVolumeSize);
         volume.setStorageResourceType(Storage.StorageResourceType.STORAGE_POOL);
         volume.setInstanceId(null);
         volume.setUpdated(new Date());
@@ -538,8 +535,7 @@ public class StorageManagerImpl implements StorageManager {
         String volumeUUID = null;
         String details = null;
         
-        DiskProfile dskCh = createDiskCharacteristics(volume, template, dc, diskOffering);
+        DiskProfile dskCh = new DiskProfile(volume.getId(), volume.getVolumeType(), volume.getName(), 0, virtualsize, null, false, false, null);
-        
         
         // Determine what pod to store the volume in
         while ((pod = _agentMgr.findPod(null, null, dc, account.getId(), podsToAvoid)) != null) {
@@ -552,7 +548,7 @@ public class StorageManagerImpl implements StorageManager {
                 
                 // Get the newly created VDI from the snapshot.
                 // This will return a null volumePath if it could not be created
-                Pair<String, String> volumeDetails = createVDIFromSnapshot(userId, snapshot, pool, templatePath);
+                Pair<String, String> volumeDetails = createVDIFromSnapshot(userId, snapshot, pool);
                 volumeUUID = volumeDetails.first();
                 details = volumeDetails.second();
                 
@@ -621,69 +617,18 @@ public class StorageManagerImpl implements StorageManager {
         SnapshotVO snapshot = _snapshotDao.findById(snapshotId); // Precondition: snapshot is not null and not removed.
         Long origVolumeId = snapshot.getVolumeId();
         VolumeVO originalVolume = _volsDao.findById(origVolumeId); // NOTE: Original volume could be destroyed and removed.
-        String templatePath = null;
-        VMTemplateVO template = null;
-        if(originalVolume.getVolumeType().equals(Volume.VolumeType.ROOT)){
-            if(originalVolume.getTemplateId() == null){
-                details = "Null Template Id for Root Volume Id: " + origVolumeId + ". Cannot create volume from snapshot of root disk.";
-                s_logger.error(details);
-            }
-            else {
-                Long templateId = originalVolume.getTemplateId();
-                template = _templateDao.findById(templateId);
-                if(template == null) {
-                    details = "Unable find template id: " + templateId + " to create volume from root disk";
-                    s_logger.error(details);
-                }
-                else if (template.getFormat() != ImageFormat.ISO) {
-                    // For ISOs there is no base template VHD file. The root disk itself is the base template.
-                    // Creating a volume from an ISO Root Disk is the same as creating a volume for a Data Disk.
 
-                    // Absolute crappy way of getting the template path on secondary storage.
-                    // Why is the secondary storage a host? It's just an NFS mount point. Why do we need to look into the templateHostVO?
-                    HostVO secondaryStorageHost = getSecondaryStorageHost(originalVolume.getDataCenterId());
-                    VMTemplateHostVO templateHostVO = _templateHostDao.findByHostTemplate(secondaryStorageHost.getId(), templateId);
-                    if (templateHostVO == null ||
-                        templateHostVO.getDownloadState() != VMTemplateStorageResourceAssoc.Status.DOWNLOADED ||
-                        (templatePath = templateHostVO.getInstallPath()) == null)
-                    {
-                        details = "Template id: " + templateId + " is not present on secondaryStorageHost Id: " + secondaryStorageHost.getId() + ". Can't create volume from ROOT DISK";
-                    }
-                }
-            }
-        }
-        if (details == null) {
         // everything went well till now
         DataCenterVO dc = _dcDao.findById(originalVolume.getDataCenterId());
-            DiskOfferingVO diskOffering = null;
 
-            if (originalVolume.getVolumeType() == VolumeType.DATADISK || originalVolume.getVolumeType() == VolumeType.ROOT) {
+        Pair<VolumeVO, String> volumeDetails = createVolumeFromSnapshot(userId, accountId, volumeName, dc,
-                Long diskOfferingId = originalVolume.getDiskOfferingId();
+                snapshot, originalVolume.getSize());
-                if (diskOfferingId != null) {
-                    diskOffering = _diskOfferingDao.findById(diskOfferingId);
-                }
-            }
-//            else if (originalVolume.getVolumeType() == VolumeType.ROOT) {
-//                // Create a temporary disk offering with the same size as the ROOT DISK
-//                Long rootDiskSize = originalVolume.getSize();
-//                Long rootDiskSizeInMB = rootDiskSize/(1024*1024);
-//                Long sizeInGB = rootDiskSizeInMB/1024;
-//                String name = "Root Disk Offering";
-//                String displayText = "Temporary Disk Offering for Snapshot from Root Disk: " + originalVolume.getId() + "[" + sizeInGB + "GB Disk]";
-//                diskOffering = new DiskOfferingVO(originalVolume.getDomainId(), name, displayText, rootDiskSizeInMB, false, null);
-//            }
-            else {
-                // The code never reaches here.
-                s_logger.error("Original volume must have been a ROOT DISK or a DATA DISK");
-                return null;
-            }
-            Pair<VolumeVO, String> volumeDetails = createVolumeFromSnapshot(userId, accountId, volumeName, dc, diskOffering, snapshot, templatePath, originalVolume.getSize(), template);
         createdVolume = volumeDetails.first();
         if (createdVolume != null) {
             volumeId = createdVolume.getId();
         }
         details = volumeDetails.second();
-        }
+
         
         Transaction txn = Transaction.currentTxn();
         txn.start();
@@ -719,7 +664,7 @@ public class StorageManagerImpl implements StorageManager {
         return createdVolume;
     }
     
-    protected Pair<String, String> createVDIFromSnapshot(long userId, SnapshotVO snapshot, StoragePoolVO pool, String templatePath) {
+    protected Pair<String, String> createVDIFromSnapshot(long userId, SnapshotVO snapshot, StoragePoolVO pool) {
         String vdiUUID = null;
         
         Long volumeId = snapshot.getVolumeId();
@@ -738,8 +683,7 @@ public class StorageManagerImpl implements StorageManager {
                                                 accountId,
                                                 volumeId,
                                                 backedUpSnapshotUuid,
-                                                snapshot.getName(),
+                                                snapshot.getName());
-                                                templatePath);
         
         String basicErrMsg = "Failed to create volume from " + snapshot.getName() + " for volume: " + volume.getId();
         CreateVolumeFromSnapshotAnswer answer = (CreateVolumeFromSnapshotAnswer) sendToHostsOnStoragePool(pool.getId(),

server/src/com/cloud/storage/snapshot/SnapshotManager.java

@@ -47,12 +47,12 @@ public interface SnapshotManager extends Manager {
      * @throws ResourceAllocationException 
      * @throws InvalidParameterValueException 
      */
-    SnapshotVO createSnapshot(long userId, long volumeId, List<Long> policies) throws InvalidParameterValueException, ResourceAllocationException;
+    SnapshotVO createSnapshot(long userId, long volumeId, long policyId) throws InvalidParameterValueException, ResourceAllocationException;
 
     /**
      * Creates a snapshot for the given volume
      */
-    long createSnapshotAsync(long userId, long volumeId, List<Long> policies);
+    long createSnapshotAsync(long userId, long volumeId, long policyId);
 
     /**
      * After successfully creating a snapshot of a volume, copy the snapshot to the secondary storage for 
@@ -75,7 +75,7 @@ public interface SnapshotManager extends Manager {
      * @param policyIds  The list of policyIds to which this snapshot belongs to
      * @param backedUp   If true, the snapshot has been successfully created.
      */
-    void postCreateSnapshot(long userId, long volumeId, long snapshotId, List<Long> policyIds, boolean backedUp);
+    void postCreateSnapshot(long userId, long volumeId, long snapshotId, long policyId, boolean backedUp);
     
     /**
      * Creates a volume from the specified snapshot. A new volume is returned which is not attached to any VM Instance
@@ -158,4 +158,8 @@ public interface SnapshotManager extends Manager {
 
 	ImageFormat getImageFormat(Long volumeId);
 
+    SnapshotPolicyVO getPolicyForVolume(long volumeId);
+
+    boolean destroySnapshotBackUp(long userId, long snapshotId, long policyId);
+
 }

server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java

@@ -25,6 +25,7 @@ import java.util.Map;
 
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
+import javax.persistence.EntityExistsException;
 
 import org.apache.log4j.Logger;
 
@@ -36,8 +37,6 @@ import com.cloud.agent.api.DeleteSnapshotBackupCommand;
 import com.cloud.agent.api.DeleteSnapshotsDirCommand;
 import com.cloud.agent.api.ManageSnapshotAnswer;
 import com.cloud.agent.api.ManageSnapshotCommand;
-import com.cloud.agent.api.ValidateSnapshotAnswer;
-import com.cloud.agent.api.ValidateSnapshotCommand;
 import com.cloud.api.BaseCmd;
 import com.cloud.api.commands.CreateSnapshotCmd;
 import com.cloud.api.commands.CreateVolumeCmd;
@@ -140,9 +139,9 @@ public class SnapshotManagerImpl implements SnapshotManager {
     private final boolean _shouldBeSnapshotCapable = true; // all methods here should be snapshot capable.
 
     @Override @DB
-    public long createSnapshotAsync(long userId, long volumeId, List<Long> policies) {
+    public long createSnapshotAsync(long userId, long volumeId, long policyId) {
         VolumeVO volume = _volsDao.findById(volumeId);
-        SnapshotOperationParam param = new SnapshotOperationParam(userId, volume.getAccountId(), volumeId, policies);
+        SnapshotOperationParam param = new SnapshotOperationParam(userId, volume.getAccountId(), volumeId, policyId);
         Gson gson = GsonHelper.getBuilder().create();
 
         AsyncJobVO job = new AsyncJobVO();
@@ -155,7 +154,7 @@ public class SnapshotManagerImpl implements SnapshotManager {
         return _asyncMgr.submitAsyncJob(job, true);
     }
     
-    private boolean isVolumeDirty(long volumeId, List<Long> policies) {
+    private boolean isVolumeDirty(long volumeId,Long policy) {
         VolumeVO volume = _volsDao.findById(volumeId);
         boolean runSnap = true;
         
@@ -184,16 +183,6 @@ public class SnapshotManagerImpl implements SnapshotManager {
                 }
             }
         }
-        
-        if (!runSnap){
-            if (policies.contains(Snapshot.MANUAL_POLICY_ID)) {
-                // Take a snapshot, but only for the manual policy
-                policies = new ArrayList<Long>();
-                policies.add(Snapshot.MANUAL_POLICY_ID);
-                runSnap = true;
-                s_logger.debug("Volume: "+ volumeId +" is detached/inactive. Executing snapshot for manual policy");
-            }
-        }
         if (volume.getDestroyed() || volume.getRemoved() != null) {
             s_logger.debug("Volume: " + volumeId + " is destroyed/removed. Not taking snapshot");
             runSnap = false;
@@ -214,51 +203,30 @@ public class SnapshotManagerImpl implements SnapshotManager {
         return format;
     }
 
-    private boolean shouldRunSnapshot(long userId, VolumeVO volume, List<Long> policies) throws InvalidParameterValueException, ResourceAllocationException {
+    private boolean shouldRunSnapshot(long userId, VolumeVO volume, long policyId)
-        boolean runSnap = isVolumeDirty(volume.getId(), policies);
+            throws InvalidParameterValueException, ResourceAllocationException {
+        boolean runSnap = isVolumeDirty(volume.getId(), policyId);
 
         ImageFormat format = getImageFormat(volume.getId());
         if (format != null) {
-            if (!(format == ImageFormat.VHD || format == ImageFormat.ISO || format == ImageFormat.QCOW2)) {
+            if (!(format == ImageFormat.VHD || format == ImageFormat.ISO)) {
-                // We only create snapshots for root disks created from templates or ISOs.
+                // We only create snapshots for root disks created from
-                s_logger.error("Currently, a snapshot can be taken from a Root Disk only if it is created from a 1) template in VHD format or 2) from an ISO.");
+                // templates or ISOs.
+                s_logger
+                        .error("Currently, a snapshot can be taken from a Root Disk only if it is created from a 1) template in VHD format or 2) from an ISO.");
                 runSnap = false;
             }
         }
-        
+        /*
-        if (runSnap) {
+         * // Check if the resource limit for snapshots has been exceeded
-            List<Long> policiesToBeRemoved = new ArrayList<Long>();
+         * //UserVO user = _userDao.findById(userId); //AccountVO account =
-            for (Long policyId : policies) {
+         * _accountDao.findById(user.getAccountId()); AccountVO account =
-                // If it's a manual policy then it won't be there in the volume_snap_policy_ref table.
+         * _accountDao.findById(volume.getAccountId()); if
-                // We need to run the snapshot
+         * (_accountMgr.resourceLimitExceeded(account, ResourceType.snapshot)) {
-                if (policyId == Snapshot.MANUAL_POLICY_ID) {
+         * throw newResourceAllocationException(
-                    // Check if the resource limit for snapshots has been exceeded
+         * "The maximum number of snapshots for account " +
-                    //UserVO user = _userDao.findById(userId);
+         * account.getAccountName() + " has been exceeded."); }
-                    //AccountVO account = _accountDao.findById(user.getAccountId());
+         */
-                	AccountVO account = _accountDao.findById(volume.getAccountId());
-                	if (_accountMgr.resourceLimitExceeded(account, ResourceType.snapshot)) {
-                        throw new ResourceAllocationException("The maximum number of snapshots for account " + account.getAccountName() + " has been exceeded.");
-                    }
-                }
-                else {
-                    // Does volume have this policy assigned still
-                    SnapshotPolicyVO volPolicy =  _snapshotPolicyDao.findById(policyId);
-                    if(volPolicy == null || !volPolicy.isActive()) {
-                        // The policy has been removed for the volume. Don't run the snapshot for this policy
-                        s_logger.debug("Policy " + policyId + " has been removed for the volume " + volume.getId() + ". Not running snapshot for this policy");
-                        // Don't remove while iterating
-                        policiesToBeRemoved.add(policyId);
-                    }
-                }
-            }
-            // Remove the unnecessary policies out of the iterator.
-            policies.removeAll(policiesToBeRemoved);
-            if (policies.size() == 0) {
-                // There are no valid policies left for the snapshot. Don't execute it.
-                runSnap = false;
-            }
-        }
-        
         if (!runSnap) {
             s_logger.warn("Snapshot for volume " + volume.getId() + " not created. No policy assigned currently.");
         }
@@ -266,8 +234,9 @@ public class SnapshotManagerImpl implements SnapshotManager {
         return runSnap;
     }
     
+    
     @Override @DB
-    public SnapshotVO createSnapshot(long userId, long volumeId, List<Long> policyIds) throws InvalidParameterValueException, ResourceAllocationException {
+    public SnapshotVO createSnapshot(long userId, long volumeId, long policyId) throws InvalidParameterValueException, ResourceAllocationException {
         // Get the async job id from the context.
         Long jobId = null;
         AsyncJobExecutor asyncExecutor = BaseAsyncJobExecutor.getCurrentExecutor();
@@ -277,43 +246,14 @@ public class SnapshotManagerImpl implements SnapshotManager {
             jobId = job.getId();
         }
         
-        Transaction txn = Transaction.currentTxn();
+        VolumeVO volume = _volsDao.findById(volumeId);
-        txn.start();
-        // set the async_job_id for this in the schedule queue so that it doesn't get scheduled again and block others.
-        // mark each of the coinciding schedules as executing in the job queue.
-        for (Long policyId : policyIds) {
-            SnapshotScheduleVO snapshotSchedule = _snapshotScheduleDao.getCurrentSchedule(volumeId, policyId, false);
-            assert snapshotSchedule != null;
-            snapshotSchedule.setAsyncJobId(jobId);
-            _snapshotScheduleDao.update(snapshotSchedule.getId(), snapshotSchedule);
-        }
-        txn.commit();
 
-        VolumeVO volume = _volsDao.lock(volumeId, true);
+        if (!shouldRunSnapshot(userId, volume, policyId)) {
-        
+            // A null snapshot is interpreted as snapshot creation failed which
-        if (!shouldRunSnapshot(userId, volume, policyIds)) {
+            // is what we want to indicate
-            // A null snapshot is interpreted as snapshot creation failed which is what we want to indicate
             return null;
         }
 
-        // Gets the most recent snapshot taken. Could return 'removed' snapshots too.
-        long lastSnapshotId = _snapshotDao.getLastSnapshot(volumeId, -1);
-        Status snapshotStatus = Status.BackedUp;
-        if (lastSnapshotId != 0) {
-            // There was a previous snapshot.
-            SnapshotVO prevSnapshot = _snapshotDao.findById(lastSnapshotId);
-            snapshotStatus = prevSnapshot.getStatus();
-            if (prevSnapshot.getRemoved() != null && snapshotStatus != Status.BackedUp) {
-                // The snapshot was deleted and it was deleted not manually but because backing up failed.
-                // Try to back it up again.
-                boolean backedUp = backupSnapshotToSecondaryStorage(userId, prevSnapshot);
-                if (!backedUp) {
-                    // If we can't backup this snapshot, there's not much chance that we can't take another one and back it up again.
-                    return null;
-                }
-            }
-        }
-        
         SnapshotVO createdSnapshot = null;
         Long id = null;
 
@@ -328,40 +268,94 @@ public class SnapshotManagerImpl implements SnapshotManager {
         }
         String snapshotName = vmDisplayName + "_" + volume.getName() + "_" + timeString;
 
-        // Create the Snapshot object and save it so we can return it to the user
+        // Create the Snapshot object and save it so we can return it to the
-        SnapshotType snapshotType = SnapshotVO.getSnapshotType(policyIds);
+        // user
-        SnapshotVO snapshotVO = new SnapshotVO(volume.getAccountId(), volume.getId(), null, snapshotName, (short)snapshotType.ordinal(), snapshotType.name());
+        SnapshotType snapshotType = SnapshotVO.getSnapshotType(policyId);
-        txn = Transaction.currentTxn();
+        SnapshotVO snapshotVO = new SnapshotVO(volume.getAccountId(), volume.getId(), null, snapshotName,
-        txn.start();
+                (short) snapshotType.ordinal(), snapshotType.name());
         snapshotVO = _snapshotDao.persist(snapshotVO);
         id = snapshotVO.getId();
         assert id != null;
-        for (Long policyId : policyIds) {
-            // Get the snapshot_schedule table entry for this snapshot and policy id.
-            // Set the snapshotId to retrieve it back later.
-            SnapshotScheduleVO snapshotSchedule = _snapshotScheduleDao.getCurrentSchedule(volumeId, policyId, true);
-            assert snapshotSchedule != null;
-            snapshotSchedule.setSnapshotId(id);
-            _snapshotScheduleDao.update(snapshotSchedule.getId(), snapshotSchedule);
-         
-        }
-        txn.commit();
 
         // Send a ManageSnapshotCommand to the agent
         String vmName = _storageMgr.getVmNameOnVolume(volume);
         
-        ManageSnapshotCommand cmd = new ManageSnapshotCommand(ManageSnapshotCommand.CREATE_SNAPSHOT, id, volume.getPath(), snapshotName, vmName);
+        long  preId = _snapshotDao.getLastSnapshot(volumeId, id);
+        
+        String preSnapshotPath = null;
+        // half of maxsnaps are delta snapshot
+        // when there are half of maxsnaps or presnapshot has not backed up , create a full snapshot
+        SnapshotVO preSnapshotVO = null;
+        if( preId != 0) {
+            preSnapshotVO = _snapshotDao.findById(preId);
+            preSnapshotPath = preSnapshotVO.getPath();
+
+        }
+
+        ManageSnapshotCommand cmd = new ManageSnapshotCommand(id, volume.getPath(), preSnapshotPath, snapshotName, vmName);
         String basicErrMsg = "Failed to create snapshot for volume: " + volume.getId();
-        ManageSnapshotAnswer answer = (ManageSnapshotAnswer) _storageMgr.sendToHostsOnStoragePool(volume.getPoolId(), cmd, basicErrMsg, _totalRetries, _pauseInterval,
+        ManageSnapshotAnswer answer = (ManageSnapshotAnswer) _storageMgr.sendToHostsOnStoragePool(volume.getPoolId(),
-        													_shouldBeSnapshotCapable, volume.getInstanceId());
+                cmd, basicErrMsg, _totalRetries, _pauseInterval, _shouldBeSnapshotCapable, volume.getInstanceId());
-
-        txn = Transaction.currentTxn();
-        txn.start();
-        
         // Update the snapshot in the database
         if ((answer != null) && answer.getResult()) {
             // The snapshot was successfully created
-            createdSnapshot = updateDBOnCreate(id, answer.getSnapshotPath());
+            if( preSnapshotPath != null && preSnapshotPath == answer.getSnapshotPath() ){
+                //empty snapshot
+                s_logger.debug("CreateSnapshot: this is empty snapshot, remove it ");
+                createdSnapshot =  _snapshotDao.findById(id);
+                // delete from the snapshots table
+                _snapshotDao.expunge(id);
+                
+                createdSnapshot.setStatus(Status.EmptySnapshot);
+                
+            } else {
+                long preSnapshotId = 0;
+                if( preSnapshotVO != null && preSnapshotVO.getBackupSnapshotId() != null) {
+                    preSnapshotId = preId;
+                    // default delta snap number is 4
+                    int deltaSnap = 4;
+                    if( policyId != Snapshot.MANUAL_POLICY_ID ) {
+                        SnapshotPolicyVO snapshotPolicy = _snapshotPolicyDao.findById(policyId);
+                        int maxSnap = snapshotPolicy.getMaxSnaps();
+                        deltaSnap = (maxSnap + 1) >> 1;
+                    } else {
+                        // check if there are policy for this volume
+                        SnapshotPolicyVO snapshotPolicy = _snapshotPolicyDao.findOneByVolume(volumeId);
+                        if( snapshotPolicy != null ) {
+                            int maxSnap = snapshotPolicy.getMaxSnaps();
+                            deltaSnap = (maxSnap + 1) >> 1;
+                        }
+                    }
+                    int i;
+                    for (i = 1; i < deltaSnap; i++) {
+                        String prevBackupUuid = preSnapshotVO.getBackupSnapshotId();
+                        // previous snapshot doesn't have backup, create a full snapshot
+                        if (prevBackupUuid == null) {
+                            preSnapshotId = 0;
+                            break;
+                        }
+                        long preSSId = preSnapshotVO.getPrevSnapshotId();
+                        if (preSSId == 0) {
+                            break;
+                        }
+                        preSnapshotVO = _snapshotDao.findById(preSSId);
+                    }
+                    if (i >= deltaSnap) {
+                        preSnapshotId = 0;
+                    }
+                }
+                createdSnapshot = updateDBOnCreate(id, answer.getSnapshotPath(), preSnapshotId);
+                // Get the snapshot_schedule table entry for this snapshot and
+                // policy id.
+                // Set the snapshotId to retrieve it back later.
+                if( policyId != Snapshot.MANUAL_POLICY_ID) {
+                    SnapshotScheduleVO snapshotSchedule = _snapshotScheduleDao.getCurrentSchedule(volumeId, policyId, true);
+                    assert snapshotSchedule != null;
+                    snapshotSchedule.setSnapshotId(id);
+                    _snapshotScheduleDao.update(snapshotSchedule.getId(), snapshotSchedule);
+                }
+            }
+
         } else {
             if (answer != null) {
                 s_logger.error(answer.getDetails());
@@ -378,22 +372,22 @@ public class SnapshotManagerImpl implements SnapshotManager {
             AsyncJobVO job = asyncExecutor.getJob();
 
             if (s_logger.isDebugEnabled())
-                s_logger.debug("CreateSnapshot created a new instance " + id + ", update async job-" + job.getId() + " progress status");
+                s_logger.debug("CreateSnapshot created a new instance " + id + ", update async job-" + job.getId()
+                        + " progress status");
 
             _asyncMgr.updateAsyncJobAttachment(job.getId(), "snapshot", id);
             _asyncMgr.updateAsyncJobStatus(job.getId(), BaseCmd.PROGRESS_INSTANCE_CREATED, id);
         }
-        txn.commit();
 
         return createdSnapshot;
     }
 
-    private SnapshotVO updateDBOnCreate(Long id, String snapshotPath) {
+    private SnapshotVO updateDBOnCreate(Long id, String snapshotPath, long preSnapshotId) {
         SnapshotVO createdSnapshot = _snapshotDao.findById(id);
-        Long volumeId = createdSnapshot.getVolumeId();
+//        Long volumeId = createdSnapshot.getVolumeId();
         createdSnapshot.setPath(snapshotPath);
         createdSnapshot.setStatus(Snapshot.Status.CreatedOnPrimary);
-        createdSnapshot.setPrevSnapshotId(_snapshotDao.getLastSnapshot(volumeId, id));
+        createdSnapshot.setPrevSnapshotId(preSnapshotId);
         _snapshotDao.update(id, createdSnapshot);
         return createdSnapshot;
     }
@@ -409,50 +403,6 @@ public class SnapshotManagerImpl implements SnapshotManager {
         
         switch (status) {
         case Creating:
-            // String snapshotUUID = snapshot.getPath();
-            // Long prevSnapshotId = snapshot.getPrevSnapshotId();
-            // All these will be null.
-
-            // But there the ManageSnapshotCommand might have succeeded on the primary without updating the database
-            // Run validatePreviousSnapshotBackup to check if that is the case.
-            // Depending on the result, complete the remaining part of the createSnapshot method.
-            // Call backupSnapshotToSecondaryStorage, if snapshot was actually taken on primary.
-            s_logger.debug("Management Server crashed before the ManageSnapshotCommand returned. Checking if snapshot was created on primary");
-            Long volumeId = snapshot.getVolumeId();
-            assert volumeId != null;
-            VolumeVO volume = _volsDao.findById(volumeId);
-            // By default, assume failure
-            String actualSnapshotUuid = null;
-            boolean createdOnPrimary = false;
-            
-            ValidateSnapshotAnswer answer = getLastSnapshotDetails(volume, snapshot.getPath());
-            
-            
-            if (answer != null) {
-                if (answer.getResult()) {
-                    // The expected snapshot details on the primary is the same as it would be if this snapshot was never taken at all
-                    // Just delete the entry in the table
-                    _snapshotDao.expunge(id);
-                    // Create an event saying the snapshot failed. ??
-                    // An event is not generated when validatePreviousSnapshotBackup fails. So not generating it here too.
-                }
-                else {
-                    // The answer is different from expected.
-                    // This may be because the snapshot given was actually taken on primary, but DB update didn't happen.
-                    // Now update the DB to denote that snapshot was created on primary and
-                    // fall through to the next case.
-                    actualSnapshotUuid = answer.getActualSnapshotUuid();
-                    if (actualSnapshotUuid != null && !actualSnapshotUuid.isEmpty()) {
-                        s_logger.debug("The snapshot " + id + " was actually created on the primary. Updating the DB record and backing it up to secondary");
-                        updateDBOnCreate(id, actualSnapshotUuid);
-                        createdOnPrimary = true;
-                    }
-                }
-            }
-            
-            if (!createdOnPrimary) {
-                break;
-            }
             // else continue to the next case.
         case CreatedOnPrimary:
             // The snapshot has been created on the primary and the DB has been updated.
@@ -472,9 +422,11 @@ public class SnapshotManagerImpl implements SnapshotManager {
 
     @Override
     @DB
-    public boolean backupSnapshotToSecondaryStorage(long userId, SnapshotVO snapshot) {
+    public boolean backupSnapshotToSecondaryStorage(long userId, SnapshotVO ss) {
-        long id = snapshot.getId();
+        long snapshotId = ss.getId();
-        
+        SnapshotVO snapshot = null;
+        try {
+            snapshot = _snapshotDao.acquire(snapshotId);
             snapshot.setStatus(Snapshot.Status.BackingUp);
             _snapshotDao.update(snapshot.getId(), snapshot);
             
@@ -494,38 +446,13 @@ public class SnapshotManagerImpl implements SnapshotManager {
             String prevSnapshotUuid = null;
             String prevBackupUuid = null;
             
-        boolean isFirstSnapshotOfRootVolume = false;
             long prevSnapshotId = snapshot.getPrevSnapshotId();
             if (prevSnapshotId > 0) {
                 prevSnapshot = _snapshotDao.findById(prevSnapshotId);
                 prevSnapshotUuid = prevSnapshot.getPath();
                 prevBackupUuid = prevSnapshot.getBackupSnapshotId();
-        }
-        else {
-            // This is the first snapshot of the volume.
-            if (volume.getVolumeType() == VolumeType.ROOT && getImageFormat(volumeId) != ImageFormat.ISO && volume.getTemplateId() != null) {
-                isFirstSnapshotOfRootVolume = true;
-                // If the first snapshot of the root volume is empty, it's parent will point to the base template.
-                // So pass the template uuid as the fake previous snapshot.
-                Long templateId = volume.getTemplateId();
-                Long poolId = volume.getPoolId();
-                if (templateId != null && poolId != null) {
-                    VMTemplateStoragePoolVO vmTemplateStoragePoolVO =
-                        _templatePoolDao.findByPoolTemplate(poolId, templateId);
-                    if (vmTemplateStoragePoolVO != null) {
-                        prevSnapshotUuid = vmTemplateStoragePoolVO.getInstallPath();
-                    }
-                    else {
-                        s_logger.warn("Volume id: " + volumeId +
-                                      " in pool id: " + poolId +
-                                      " based off  template id: " + templateId +
-                                      " doesn't have an entry in the template_spool_ref table." +
-                                      " Using null as the template.");
-                    }
-                }
 
             }
-        }
             String firstBackupUuid = volume.getFirstSnapshotBackupUuid();
             boolean isVolumeInactive = _storageMgr.volumeInactive(volume);
             String vmName = _storageMgr.getVmNameOnVolume(volume);
@@ -540,7 +467,6 @@ public class SnapshotManagerImpl implements SnapshotManager {
                                           prevSnapshotUuid,
                                           prevBackupUuid,
                                           firstBackupUuid,
-                                      isFirstSnapshotOfRootVolume,
                                           isVolumeInactive,
                                           vmName);
             
@@ -590,25 +516,7 @@ public class SnapshotManagerImpl implements SnapshotManager {
                         }
                          
                     }
-                // if previous snapshot is marked as Removed, remove it now
-                if(prevSnapshot != null && prevSnapshot.getRemoved() != null) {
-                    s_logger.debug("Snapshot id: " + prevSnapshotId + " was marked as removed. Deleting it from the primary/secondary/DB now.");
-                    // Get the prevSnapshotId of the snapshot to be deleted.
-                    // This will become the prevSnapshotId of the current snapshot
-                    long prevValidSnapshotId = prevSnapshot.getPrevSnapshotId();
-                    String prevValidSnapshotBackupUuid = null;
-                    if (prevValidSnapshotId > 0) {
-                        prevValidSnapshotBackupUuid = _snapshotDao.findById(prevValidSnapshotId).getBackupSnapshotId();
-                    }
 
-                    snapshot.setPrevSnapshotId(prevValidSnapshotId);
-                    _snapshotDao.update(id, snapshot);
-                    
-                    backedUp = destroyLastSnapshot(prevValidSnapshotBackupUuid, prevSnapshot, backedUpSnapshotUuid);
-                    if (!backedUp) {
-                        s_logger.debug("Error while deleting last snapshot id: " + prevSnapshotId + " for volume " + volumeId);
-                    }
-                }
                 }
             }
             else if (answer != null) {
@@ -618,7 +526,7 @@ public class SnapshotManagerImpl implements SnapshotManager {
             Transaction txn = Transaction.currentTxn();
             txn.start();
             
-        SnapshotVO snapshotVO = _snapshotDao.findById(id);
+            SnapshotVO snapshotVO = _snapshotDao.findById(snapshotId);
             snapshotVO.setBackupSnapshotId(backedUpSnapshotUuid);
             if (volume.getFirstSnapshotBackupUuid() == null) {
                 // This is the first ever snapshot taken for the volume.
@@ -639,11 +547,11 @@ public class SnapshotManagerImpl implements SnapshotManager {
             
             if (backedUp) {
                 snapshotVO.setStatus(Snapshot.Status.BackedUp);
-            String eventParams = "id=" + id + "\nssName=" + snapshotName +"\nsize=" + volume.getSize()+"\ndcId=" + volume.getDataCenterId();
+                String eventParams = "id=" + snapshotId + "\nssName=" + snapshotName +"\nsize=" + volume.getSize()+"\ndcId=" + volume.getDataCenterId();
-            event.setDescription("Backed up snapshot id: " + id + " to secondary for volume " + volumeId);
+                event.setDescription("Backed up snapshot id: " + snapshotId + " to secondary for volume " + volumeId);
                 event.setLevel(EventVO.LEVEL_INFO);
                 event.setParameters(eventParams);
-            _snapshotDao.update(id, snapshotVO);
+                _snapshotDao.update(snapshotId, snapshotVO);
             }
             else {
                 // Just mark it as removed in the database. When the next snapshot it taken,
@@ -653,103 +561,48 @@ public class SnapshotManagerImpl implements SnapshotManager {
                 // 2) Create the next Snapshot pretending this is a valid snapshot.
                 // 3) backupSnapshotToSecondaryStorage of the next snapshot
                 // will take care of cleaning up the state of this snapshot
-            _snapshotDao.remove(id);
+                _snapshotDao.remove(snapshotId);
                 event.setLevel(EventVO.LEVEL_ERROR);
-            event.setDescription("Failed to backup snapshot id: " + id + " to secondary for volume " + volumeId);
+                event.setDescription("Failed to backup snapshot id: " + snapshotId + " to secondary for volume " + volumeId);
             }
             // Save the event
             _eventDao.persist(event);
             txn.commit();
             
             return backedUp;
+        } finally {
+            if( snapshot != null ) {
+                _snapshotDao.release(snapshotId);
+            }
+        }
+        
     }
 
     @Override
     @DB
-    public void postCreateSnapshot(long userId, long volumeId, long snapshotId, List<Long> policyIds, boolean backedUp) {
+    public void postCreateSnapshot(long userId, long volumeId, long snapshotId, long policyId, boolean backedUp) {
         // Update the snapshot_policy_ref table with the created snapshot
         // Get the list of policies for this snapshot
         Transaction txn = Transaction.currentTxn();
         txn.start();
-        for (long policyId : policyIds) {
         if (backedUp) {
-                // create an entry in snap_policy_ref table
+            // This is a manual create, so increment the count of snapshots for
-                SnapshotPolicyRefVO snapPolicyRef = new SnapshotPolicyRefVO(snapshotId, volumeId, policyId);
+            // this account
-                _snapPolicyRefDao.persist(snapPolicyRef);
-                
-            	// This is a manual create, so increment the count of snapshots for this account
             if (policyId == Snapshot.MANUAL_POLICY_ID) {
                 Snapshot snapshot = _snapshotDao.findById(snapshotId);
                 _accountMgr.incrementResourceCount(snapshot.getAccountId(), ResourceType.snapshot);
             }
         }
 
-            // Even if the current snapshot failed, we should schedule the next recurring snapshot for this policy.
+        // Even if the current snapshot failed, we should schedule the next
+        // recurring snapshot for this policy.
         if (policyId != Snapshot.MANUAL_POLICY_ID) {
             postCreateRecurringSnapshotForPolicy(userId, volumeId, snapshotId, policyId);
         }
-            else {
-                // Delete the entry from the snapshot_schedule table so that the
-                // next manual snapshot can be taken.
 
-                // Get the schedule of this snapshot
-                SnapshotScheduleVO snapshotSchedule = _snapshotScheduleDao.getCurrentSchedule(volumeId, policyId, true);
-                if (snapshotSchedule != null) {
-                    // We should lock the row before deleting it as it is also being deleted by the scheduler.
-                    _snapshotScheduleDao.expunge(snapshotSchedule.getId());
-                }
-            }
-        }
         txn.commit();
     }
 
-    private ValidateSnapshotAnswer getLastSnapshotDetails(VolumeVO volume, String previousSnapshotUuid) {
-        // Validate the VDI parent structure for the volume on the primary storage
-        Long volumeId = volume.getId();
-        String primaryStoragePoolNameLabel = _storageMgr.getPrimaryStorageNameLabel(volume);
-        String volumeUuid = volume.getPath();
-        Long poolId = volume.getPoolId();
-        String firstSnapshotBackupUuid = volume.getFirstSnapshotBackupUuid();
-        String templateUuid = null;
-        String details = null;
-        if (firstSnapshotBackupUuid == null && volume.getVolumeType() == VolumeType.ROOT) {
-            Long templateId = volume.getTemplateId();
-            VMTemplateVO template = _templateDao.findById(templateId);
-            if(template == null) {
-                details = "Unable find template id: " + templateId + " for root disk volumeId: " + volumeId;
-                s_logger.error(details);
-            }
-            else if (template.getFormat() == ImageFormat.VHD || template.getFormat() == ImageFormat.QCOW2) {
-                // We support creating snapshots of Root Disk created from template only in VHD format.
-                VMTemplateStoragePoolVO templateStoragePoolVO = _templatePoolDao.findByPoolTemplate(volume.getPoolId(), templateId);
-                if (templateStoragePoolVO != null) {
-                    templateUuid = templateStoragePoolVO.getInstallPath();
-                }
-                else {
-                    details = "Template id: " + templateId + " is not present in on the primary storage pool id: " + volume.getPoolId() +
-                              " according to the template_spool_ref table";
-                    s_logger.error(details);
-                }
-            }
-        }
-        ValidateSnapshotAnswer answer = null;
-        if (details == null) {
-            // EverythingBackup is fine until now. Proceed with command.
-            ValidateSnapshotCommand cmd =
-                new ValidateSnapshotCommand(primaryStoragePoolNameLabel, volumeUuid, firstSnapshotBackupUuid, previousSnapshotUuid, templateUuid);
-            String basicErrMsg = "Failed to validate VDI structure for volumeId: " + volume.getId() + " with UUID: " + volumeUuid;
-            
-            answer = (ValidateSnapshotAnswer) _storageMgr.sendToHostsOnStoragePool(poolId,
-                                                                                   cmd,
-                                                                                   basicErrMsg,
-                                                                                   _totalRetries,
-                                                                                   _pauseInterval,
-                                                                                   _shouldBeSnapshotCapable,
-                                                                                   volume.getInstanceId());
-        }
-        
-        return answer;
-    }
     
     private void postCreateRecurringSnapshotForPolicy(long userId, long volumeId, long snapshotId, long policyId) {
         //Use count query
@@ -773,74 +626,33 @@ public class SnapshotManagerImpl implements SnapshotManager {
     @Override @DB
     public boolean deleteSnapshot(long userId, long snapshotId, long policyId) {
         s_logger.debug("Calling deleteSnapshot for snapshotId: " + snapshotId + " and policyId " + policyId);
-        long prevSnapshotId = 0;
+        SnapshotVO lastSnapshot = null;
-        SnapshotVO nextSnapshot = null;
+        _snapshotDao.remove(snapshotId);
-        boolean deleted = true;
+        long lastId = snapshotId;
-        boolean actuallyDelete = false;
+        while( true ) {
-        List<SnapshotPolicyRefVO> snapPolicyRefs = _snapPolicyRefDao.listBySnapshotId(snapshotId);
+            lastSnapshot = _snapshotDao.findNextSnapshot(lastId);
-        // Destroy snapshot if its not part of any policy other than the given one.
+            // prevsnapshotId equal 0, means it is a full snapshot
-        if(snapPolicyRefs.size() == 1 && (snapPolicyRefs.get(0).getPolicyId() == policyId)) {
+            if( lastSnapshot == null || lastSnapshot.getPrevSnapshotId() == 0)
-            SnapshotVO currentSnapshot = _snapshotDao.findById(snapshotId);
+                break;
-            String backupOfSnapshot = currentSnapshot.getBackupSnapshotId();
+            lastId = lastSnapshot.getId();
-            nextSnapshot = _snapshotDao.findNextSnapshot(snapshotId);
-            String backupOfNextSnapshot = null;
-            if (nextSnapshot != null) {
-                backupOfNextSnapshot = nextSnapshot.getBackupSnapshotId();
         }
-             
+        lastSnapshot = _snapshotDao.findById(lastId);
-            prevSnapshotId = currentSnapshot.getPrevSnapshotId();
+        while( lastSnapshot.getRemoved() != null ) {
-            String backupOfPreviousSnapshot = null;
+            String BackupSnapshotId = lastSnapshot.getBackupSnapshotId();
-            if (prevSnapshotId > 0) {
+            if( BackupSnapshotId != null ) {
-                SnapshotVO prevSnapshot = _snapshotDao.findById(prevSnapshotId);
+                if( destroySnapshotBackUp(userId, snapshotId, policyId) ) {
-                backupOfPreviousSnapshot = prevSnapshot.getBackupSnapshotId();
+                    lastSnapshot.setBackupSnapshotId(null);
-            }
+                    _snapshotDao.update(lastId, lastSnapshot);
-            
+                } else {
-            if (backupOfSnapshot != null) {
+                    s_logger.debug("Destroying snapshot backup failed " + lastSnapshot);
-                if (backupOfNextSnapshot != null && backupOfSnapshot.equals(backupOfNextSnapshot)) {
+                    break;
-                    // Both the snapshots point to the same backup VHD file.
-                    // There is no difference in the data between them.
-                    // We don't want to delete the backup of the older snapshot
-                    // as it means that we delete the next snapshot too
-                }
-                else if (backupOfPreviousSnapshot != null && backupOfSnapshot.equals(backupOfPreviousSnapshot)) {
-                    // If we delete the current snapshot, the user will not
-                    // be able to recover from the previous snapshot
-                    // So don't delete anything
-                }
-                else {
-                    actuallyDelete = true;
-                    deleted = destroySnapshot(userId, snapshotId, policyId);
-                }
-                
-                if (!actuallyDelete) {
-                    // Don't actually delete the snapshot backup but delete the entry
-                    // from both snapshots and snapshot_policy_ref table
-                    boolean isLastSnap = (nextSnapshot == null);
-                    postDeleteSnapshot(snapshotId, policyId, isLastSnap);
-                    // create the event
-                    String eventParams = "id=" + snapshotId;
-                    EventVO event = new EventVO();
-                    
-                    event.setUserId(userId);
-                    event.setAccountId((currentSnapshot != null) ? currentSnapshot.getAccountId() : 0);
-                    event.setType(EventTypes.EVENT_SNAPSHOT_DELETE);
-                    event.setDescription("Successfully deleted snapshot " + snapshotId + " for volumeId: " + currentSnapshot.getVolumeId() + " and policyId " + policyId);
-                    event.setParameters(eventParams);
-                    event.setLevel(EventVO.LEVEL_INFO);
-                    _eventDao.persist(event);
                 }
             }
+            postDeleteSnapshot(userId, lastId, policyId);
+            lastId = lastSnapshot.getPrevSnapshotId();
+            lastSnapshot = _snapshotDao.findById(lastId);
         }
-        else {
+        return true;
-            // Just delete the entry from the snapshot_policy_ref table
-            Transaction txn = Transaction.currentTxn();
-            txn.start();
-            _snapPolicyRefDao.removeSnapPolicy(snapshotId, policyId);
-            txn.commit();
-        }
-        
-        return deleted;
-
     }
 
     @Override @DB
@@ -862,19 +674,14 @@ public class SnapshotManagerImpl implements SnapshotManager {
 
     @Override @DB
     public boolean destroySnapshot(long userId, long snapshotId, long policyId) {
+        return true;
+    }
+    @Override @DB
+    public boolean destroySnapshotBackUp(long userId, long snapshotId, long policyId) {
         boolean success = false;
         String details = null;
         SnapshotVO snapshot = _snapshotDao.findById(snapshotId);
-        if (snapshot != null) {
+
-            SnapshotVO nextSnapshot = _snapshotDao.findNextSnapshot(snapshotId);
-            if(nextSnapshot == null){
-                // This is last snapshot.
-                // Destroy this snapshot after creation of next snapshot. Only mark as removed in DB
-                details = "Successfully deleted snapshot " + snapshotId + " for volumeId: " + snapshot.getVolumeId() + " and policyId " + policyId;
-                s_logger.debug("This is last snapshot for volume. Not destroying now: " + snapshot.getId());
-                postDeleteSnapshot(snapshotId, policyId, true);
-                success = true;
-            } else {
         VolumeVO volume = _volsDao.findById(snapshot.getVolumeId());
         String primaryStoragePoolNameLabel = _storageMgr.getPrimaryStorageNameLabel(volume);
         String secondaryStoragePoolUrl = _storageMgr.getSecondaryStorageURL(volume.getDataCenterId());
@@ -883,44 +690,27 @@ public class SnapshotManagerImpl implements SnapshotManager {
         Long volumeId = volume.getId();
 
         String backupOfSnapshot = snapshot.getBackupSnapshotId();
-                String backupOfNextSnapshot = null;
-                if (nextSnapshot != null) {
-                    backupOfNextSnapshot = nextSnapshot.getBackupSnapshotId();
-                }
 
-                DeleteSnapshotBackupCommand cmd =
+        DeleteSnapshotBackupCommand cmd = new DeleteSnapshotBackupCommand(primaryStoragePoolNameLabel,
-                    new DeleteSnapshotBackupCommand(primaryStoragePoolNameLabel,
+                secondaryStoragePoolUrl, dcId, accountId, volumeId, backupOfSnapshot, snapshot.getName());
-                                                    secondaryStoragePoolUrl,
-                                                    dcId,
-                                                    accountId,
-                                                    volumeId,
-                                                    backupOfSnapshot,
-                                                    snapshot.getName(),
-                                                    backupOfNextSnapshot);
 
         details = "Failed to destroy snapshot id:" + snapshotId + " for volume: " + volume.getId();
-                Answer answer = _storageMgr.sendToHostsOnStoragePool(volume.getPoolId(),
+        Answer answer = _storageMgr.sendToHostsOnStoragePool(volume.getPoolId(), cmd, details, _totalRetries,
-                                                                     cmd,
+                _pauseInterval, _shouldBeSnapshotCapable, volume.getInstanceId());
-                                                                     details,
-                                                                     _totalRetries,
-                                                                     _pauseInterval,
-                                                                     _shouldBeSnapshotCapable, volume.getInstanceId());
 
         if ((answer != null) && answer.getResult()) {
             // This is not the last snapshot.
-                    postDeleteSnapshot(snapshotId, policyId, false);
+            postDeleteSnapshot(userId, snapshotId, policyId);
             success = true;
-                    details = "Successfully deleted snapshot " + snapshotId + " for volumeId: " + volumeId + " and policyId " + policyId;
+            details = "Successfully deleted snapshot " + snapshotId + " for volumeId: " + volumeId + " and policyId "
+                    + policyId;
             s_logger.debug(details);
-                }
+        } else if (answer != null) {
-                else if (answer != null) {
             if (answer.getDetails() != null) {
                 details = answer.getDetails();
             }
             s_logger.error(details);
         }
-            }
-        }
 
         // create the event
         String eventParams = "id=" + snapshotId;
@@ -939,28 +729,13 @@ public class SnapshotManagerImpl implements SnapshotManager {
     }
     
     @DB
-    protected void postDeleteSnapshot(long snapshotId, long policyId, boolean isLastSnap) {
+    protected void postDeleteSnapshot(long userId, long snapshotId, long policyId) {
         // Remove the snapshot from the snapshots table and the snap_policy_ref table.
         Transaction txn = Transaction.currentTxn();
         txn.start();
         
         SnapshotVO snapshot = _snapshotDao.findById(snapshotId);
-        if(isLastSnap){
-            _snapshotDao.remove(snapshotId);
-        } else {
         _snapshotDao.expunge(snapshotId);
-            // In the snapshots table,
-            // the last_snapshot_id field of the next snapshot becomes the last_snapshot_id of the deleted snapshot
-            long prevSnapshotId = snapshot.getPrevSnapshotId();
-            SnapshotVO nextSnapshot = _snapshotDao.findNextSnapshot(snapshotId);
-            assert nextSnapshot != null; // That is how lastSnap is decided.
-            nextSnapshot.setPrevSnapshotId(prevSnapshotId);
-            _snapshotDao.update(nextSnapshot.getId(), nextSnapshot);
-
-        }
-        
-        _snapPolicyRefDao.removeSnapPolicy(snapshotId, policyId);
-        
         // If this is a manual delete, decrement the count of snapshots for this account
         if (policyId == Snapshot.MANUAL_POLICY_ID) {
         	_accountMgr.decrementResourceCount(snapshot.getAccountId(), ResourceType.snapshot);
@@ -1075,71 +850,43 @@ public class SnapshotManagerImpl implements SnapshotManager {
         return success;
 	}
     
+    
     @Override
     @DB
     public SnapshotPolicyVO createPolicy(long userId, long accountId, long volumeId, String schedule, short interval, int maxSnaps, String timezone) {
-        Long policyId = null;
+        SnapshotPolicyVO policy = new SnapshotPolicyVO(volumeId, schedule, timezone, interval, maxSnaps);
-        SnapshotPolicyVO policy = getPolicyForVolumeByInterval(volumeId, (interval));
-        
-        Transaction txn = Transaction.currentTxn();
-        txn.start();
         // Create an event
         EventVO event = new EventVO();
-        event.setAccountId(accountId);
+        try{
-        event.setUserId(userId);
+            policy = _snapshotPolicyDao.persist(policy);
-        
+            event.setType(EventTypes.EVENT_SNAPSHOT_POLICY_CREATE);
-        if( policy != null){
+            event.setDescription("Successfully created snapshot policy with Id: "+ policy.getId());
-            s_logger.debug("Policy for specified interval already exists. Updating policy to new schedule");
+        } catch (EntityExistsException e ) {
-            policyId = policy.getId();
+            policy = _snapshotPolicyDao.findOneByVolume(volumeId);
-            
+            try {
-            // By default, assume failure.
+                policy = _snapshotPolicyDao.acquire(policy.getId());
-            event.setType(EventTypes.EVENT_SNAPSHOT_POLICY_UPDATE);
-            event.setDescription("Failed to update schedule for Snapshot policy with id: "+policyId);
-            event.setLevel(EventVO.LEVEL_ERROR);
-            
-            // Check if there are any recurring snapshots being currently executed. Race condition.
-            SnapshotScheduleVO snapshotSchedule = _snapshotScheduleDao.getCurrentSchedule(volumeId, policyId, true);
-            if (snapshotSchedule != null) {
-                Date scheduledTimestamp = snapshotSchedule.getScheduledTimestamp();
-                String dateDisplay = DateUtil.displayDateInTimezone(DateUtil.GMT_TIMEZONE, scheduledTimestamp);
-                s_logger.debug("Cannot update the policy now. Wait until the current snapshot scheduled at " + dateDisplay + " finishes");
-                
-                policyId = null;
-                policy = null;
-            }
-            else {
-                _snapSchedMgr.removeSchedule(volumeId, policyId);
                 policy.setSchedule(schedule);
                 policy.setTimezone(timezone);
+                policy.setInterval(interval);
                 policy.setMaxSnaps(maxSnaps);
                 policy.setActive(true);
-                
+                _snapshotPolicyDao.update(policy.getId(), policy);
-                if(_snapshotPolicyDao.update(policy.getId(), policy)){
+            } finally {
+                if( policy != null) {
+                    _snapshotPolicyDao.release(policy.getId());
+                }
+            }
+            event.setType(EventTypes.EVENT_SNAPSHOT_POLICY_UPDATE);
+            event.setDescription("Successfully updated snapshot policy with Id: "+ policy.getId());
+        }
+        event.setAccountId(accountId);
+        event.setUserId(userId);
         event.setLevel(EventVO.LEVEL_INFO);
-                    event.setDescription("Successfully updated snapshot policy with Id: "+policyId);
-                }
-            }
-        } else {
-            policy = new SnapshotPolicyVO(volumeId, schedule, timezone, interval, maxSnaps);
-            policy = _snapshotPolicyDao.persist(policy);
-            policyId = policy.getId();
-            event.setType(EventTypes.EVENT_SNAPSHOT_POLICY_CREATE);
-            event.setDescription("Successfully created snapshot policy with Id: "+policyId);
-        }
-                
         _eventDao.persist(event);
-        if (policyId != null) {
         _snapSchedMgr.scheduleNextSnapshotJob(policy);
-        }
-        else {
-            s_logger.debug("Failed to update schedule for Snapshot policy with id: " + policyId);
-        }
-        txn.commit();
-        
         return policy;
     }
     
-    
     @Override
     public boolean deletePolicy(long userId, long policyId) {
         SnapshotPolicyVO snapshotPolicy = _snapshotPolicyDao.findById(policyId);
@@ -1202,65 +949,6 @@ public class SnapshotManagerImpl implements SnapshotManager {
     	}
 	}
     
-    private boolean destroyLastSnapshot(String backupOfPreviousSnapshot, SnapshotVO snapshot, String backupOfNextSnapshot) {
-        boolean success = false;
-        long snapshotId = snapshot.getId();
-        if (snapshot != null) {
-            String backupOfSnapshot = snapshot.getBackupSnapshotId();
-            if (backupOfSnapshot != null) {
-                if (backupOfNextSnapshot != null && backupOfSnapshot.equals(backupOfNextSnapshot)) {
-                    // Both the snapshots point to the same backup VHD file.
-                    // There is no difference in the data between them.
-                    // We don't want to delete the backup of the older snapshot
-                    // as it means that we delete the next snapshot too
-                    success = true;
-                    s_logger.debug("Removed snapshot " + snapshotId +
-                                   " is not being destroyed from secondary as " +
-                                   "it is the same as the current snapshot uuid: " + backupOfNextSnapshot);
-                }
-                else if (backupOfPreviousSnapshot != null && backupOfSnapshot.equals(backupOfPreviousSnapshot)) {
-                    // If we delete the current snapshot, the user will not
-                    // be able to recover from the previous snapshot
-                    // So don't delete anything
-                    success = true;
-                    s_logger.debug("Removed snapshot " + snapshotId +
-                                   " is not being destroyed from secondary as " +
-                                   "it is the same as it's previous snapshot with uuid: " + backupOfPreviousSnapshot);
-                } else {
-                    VolumeVO volume = _volsDao.findById(snapshot.getVolumeId());
-                    String primaryStoragePoolNameLabel = _storageMgr.getPrimaryStorageNameLabel(volume);
-                    String secondaryStoragePoolUrl = _storageMgr.getSecondaryStorageURL(volume.getDataCenterId());
-                    Long dcId = volume.getDataCenterId();
-                    Long accountId = volume.getAccountId();
-                    Long volumeId = volume.getId();
-
-                    DeleteSnapshotBackupCommand cmd =
-                        new DeleteSnapshotBackupCommand(primaryStoragePoolNameLabel,
-                                secondaryStoragePoolUrl,
-                                dcId,
-                                accountId,
-                                volumeId,
-                                backupOfSnapshot,
-                                snapshot.getName(),
-                                backupOfNextSnapshot);
-                    String basicErrMsg = "Failed to destroy snapshot id: " + snapshotId + " for volume id: " + volumeId;
-                    Answer answer = _storageMgr.sendToHostsOnStoragePool(volume.getPoolId(), cmd, basicErrMsg, _totalRetries, _pauseInterval, _shouldBeSnapshotCapable, volume.getInstanceId());
-                    
-                    if ((answer != null) && answer.getResult()) {
-                        success = true;
-                        s_logger.debug("Successfully deleted last snapshot: " + snapshotId + " for volume id: " + volumeId);
-                    }
-                    else if (answer != null) {
-                        s_logger.error(answer.getDetails());
-                    }
-                }
-            }
-        }
-        if(success){
-            _snapshotDao.expunge(snapshot.getId());
-        }
-        return success;
-    }
 
     /**
      * {@inheritDoc}
@@ -1290,6 +978,11 @@ public class SnapshotManagerImpl implements SnapshotManager {
 	}
 	
     @Override
+    public SnapshotPolicyVO getPolicyForVolume(long volumeId) {
+        return _snapshotPolicyDao.findOneByVolume(volumeId);
+    }
+        
+	@Override
     public boolean configure(String name, Map<String, Object> params)
     throws ConfigurationException {
         _name = name;

server/src/com/cloud/storage/snapshot/SnapshotScheduler.java

@@ -45,6 +45,4 @@ public interface SnapshotScheduler extends Manager, Scheduler {
      */
     boolean removeSchedule(Long volumeId, Long policyId);
     
-    Long scheduleManualSnapshot(Long userId, Long volumeId);
-    
 }

server/src/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java

@@ -28,6 +28,7 @@ import java.util.TimerTask;
 
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
+import javax.persistence.EntityExistsException;
 
 import org.apache.log4j.Logger;
 
@@ -35,6 +36,7 @@ import com.cloud.async.AsyncJobResult;
 import com.cloud.async.AsyncJobVO;
 import com.cloud.async.dao.AsyncJobDao;
 import com.cloud.configuration.dao.ConfigurationDao;
+import com.cloud.event.EventTypes;
 import com.cloud.storage.Snapshot;
 import com.cloud.storage.SnapshotPolicyVO;
 import com.cloud.storage.SnapshotScheduleVO;
@@ -194,36 +196,6 @@ public class SnapshotSchedulerImpl implements SnapshotScheduler {
         }
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    @DB
-    public Long scheduleManualSnapshot(Long userId, Long volumeId) {
-        // Check if there is another manual snapshot scheduled which hasn't been executed yet.
-        SearchCriteria<SnapshotScheduleVO> sc = _snapshotScheduleDao.createSearchCriteria();
-        sc.addAnd("volumeId", SearchCriteria.Op.EQ, volumeId);
-        sc.addAnd("policyId", SearchCriteria.Op.EQ, Snapshot.MANUAL_POLICY_ID);
-        
-        List<SnapshotScheduleVO> snapshotSchedules = _snapshotScheduleDao.search(sc, null);
-        if (!snapshotSchedules.isEmpty()) {
-            Date scheduledTimestamp = snapshotSchedules.get(0).getScheduledTimestamp();
-            String dateDisplay = DateUtil.displayDateInTimezone(DateUtil.GMT_TIMEZONE, scheduledTimestamp);
-            s_logger.error("Can't execute another manual snapshot for volume: " + volumeId + 
-                           " while another manual snapshot for the same volume is being created/backed up. " +
-                           "The older snapshot was scheduled at " + dateDisplay);
-            return null;
-        }
-        
-        SnapshotScheduleVO snapshotSchedule = new SnapshotScheduleVO(volumeId, Snapshot.MANUAL_POLICY_ID, _currentTimestamp);
-        // There is a race condition here. Two threads enter here. 
-        // Both find that there are no manual snapshots for the same volume scheduled.
-        // Both try to schedule. One fails, which is what we wanted anyway.
-        _snapshotScheduleDao.persist(snapshotSchedule); 
-        List<Long> policyIds = new ArrayList<Long>();
-        policyIds.add(Snapshot.MANUAL_POLICY_ID);
-        return _snapshotManager.createSnapshotAsync(userId, volumeId, policyIds);
-    }
     
     @DB
     protected void scheduleSnapshots() {
@@ -237,71 +209,26 @@ public class SnapshotSchedulerImpl implements SnapshotScheduler {
         // Hence set user id to that of system
         long userId = 1;
 
-        // The volumes which are going to be snapshotted now.
-        // The value contains the list of policies associated with this new snapshot.
-        // There can be more than one policy for a list if different policies coincide for the same volume.
-        Map<Long, List<Long>> listOfVolumesSnapshotted = new HashMap<Long, List<Long>>();
-        Calendar cal = Calendar.getInstance(DateUtil.GMT_TIMEZONE);
-        cal.add(Calendar.MINUTE, -15);
-        //Skip any snapshots older than 15mins
-        Date graceTime = cal.getTime();
-        
         for (SnapshotScheduleVO snapshotToBeExecuted : snapshotsToBeExecuted) {
-            Date scheduleTime = snapshotToBeExecuted.getScheduledTimestamp();
-            if(scheduleTime.before(graceTime)){
-                s_logger.info("Snapshot schedule older than 15mins. Skipping snapshot for volume: "+snapshotToBeExecuted.getVolumeId());
-                scheduleNextSnapshotJob(snapshotToBeExecuted);
-                continue;
-            }
             long policyId = snapshotToBeExecuted.getPolicyId();
             long volumeId = snapshotToBeExecuted.getVolumeId();
-            List<Long> coincidingPolicies = listOfVolumesSnapshotted.get(volumeId);
-            if (coincidingPolicies != null) {
-                s_logger.debug("The snapshot for this volume " + volumeId + " and policy " + policyId + " has already been sent for execution along with " + coincidingPolicies.size() + " policies in total");
-                // This can happen if this coincided with another schedule with a different policy
-                // It would have added all the coinciding policies for the volume to the Map
-                
-                if (coincidingPolicies.contains(snapshotToBeExecuted.getPolicyId())) {
-                    // Don't need to do anything now. The snapshot is already scheduled for execution.
-                    s_logger.debug("coincidingPolicies contains snapshotToBeExecuted id: " + snapshotToBeExecuted.getId() + ". Don't need to do anything now. The snapshot is already scheduled for execution.");
-                }
-                else {
-                    // This will not happen
-                    s_logger.warn("Snapshot Schedule " + snapshotToBeExecuted.getId() +
-                                  " is ready for execution now at timestamp " + _currentTimestamp +
-                                  " but is not coincident with one being executed for volume " + volumeId);
-                    // Add this to the list of policies for the snapshot schedule
-                    coincidingPolicies.add(snapshotToBeExecuted.getPolicyId());
-                    listOfVolumesSnapshotted.put(volumeId, coincidingPolicies);
-                    
-                }
-            }
-            else {
-                coincidingPolicies = new ArrayList<Long>();
-                List<SnapshotScheduleVO> coincidingSchedules = _snapshotScheduleDao.getCoincidingSnapshotSchedules(volumeId, _currentTimestamp);
-
             if (s_logger.isDebugEnabled()) {
                 Date scheduledTimestamp = snapshotToBeExecuted.getScheduledTimestamp();
                 displayTime = DateUtil.displayDateInTimezone(DateUtil.GMT_TIMEZONE, scheduledTimestamp);
+                s_logger.debug("Scheduling 1 snapshot for volume " + volumeId + " for schedule id: "
+                        + snapshotToBeExecuted.getId() + " at " + displayTime);
             }
-
+            long snapshotScheId = snapshotToBeExecuted.getId();
-                Transaction txn = Transaction.currentTxn();
+            SnapshotScheduleVO tmpSnapshotScheduleVO = null;
-                txn.start();
+            try {
-                // There are more snapshots scheduled for this volume at the same time.
+                tmpSnapshotScheduleVO = _snapshotScheduleDao.acquire(snapshotScheId);
-                // Club all the policies together and append them to the coincidingPolicies List
+                long jobId = _snapshotManager.createSnapshotAsync(userId, volumeId, policyId);
-                StringBuilder coincidentSchedules = new StringBuilder();
+                tmpSnapshotScheduleVO.setAsyncJobId(jobId);
-                for (SnapshotScheduleVO coincidingSchedule : coincidingSchedules) {
+                _snapshotScheduleDao.update(snapshotScheId, tmpSnapshotScheduleVO);
-                    coincidingPolicies.add(coincidingSchedule.getPolicyId());
+            } finally {
-                    coincidentSchedules.append(coincidingSchedule.getId() + ", ");
+                if (tmpSnapshotScheduleVO != null) {
+                    _snapshotScheduleDao.release(snapshotScheId);
                 }
-                txn.commit();
-                
-                s_logger.debug("Scheduling 1 snapshot for volume " + volumeId + " for schedule ids: " + coincidentSchedules + " at " + displayTime);
-                long jobId = _snapshotManager.createSnapshotAsync(userId, volumeId, coincidingPolicies);
-				
-                // Add this snapshot to the listOfVolumesSnapshotted
-				// So that the coinciding schedules don't get scheduled again.
-				listOfVolumesSnapshotted.put(volumeId, coincidingPolicies);
 
             }
         }
@@ -328,10 +255,26 @@ public class SnapshotSchedulerImpl implements SnapshotScheduler {
         long policyId = policyInstance.getId();
         Date nextSnapshotTimestamp = getNextScheduledTime(policyId, _currentTimestamp);
         SnapshotScheduleVO snapshotScheduleVO = new SnapshotScheduleVO(policyInstance.getVolumeId(), policyId, nextSnapshotTimestamp);
+        try{
             _snapshotScheduleDao.persist(snapshotScheduleVO);
+        } catch (EntityExistsException e ) {
+            snapshotScheduleVO = _snapshotScheduleDao.findOneByVolume(policyInstance.getVolumeId());
+            try {
+                snapshotScheduleVO = _snapshotScheduleDao.acquire(snapshotScheduleVO.getId());
+                snapshotScheduleVO.setPolicyId(policyId);
+                snapshotScheduleVO.setScheduledTimestamp(nextSnapshotTimestamp);
+                _snapshotScheduleDao.update(snapshotScheduleVO.getId(), snapshotScheduleVO);
+            } finally {
+                if(snapshotScheduleVO != null ) {
+                    _snapshotScheduleDao.release(snapshotScheduleVO.getId());
+                }
+            }
+        }
         return nextSnapshotTimestamp;
     }
     
+ 
+    
     @Override @DB
     public boolean removeSchedule(Long volumeId, Long policyId) {
         // We can only remove schedules which are in the future. Not which are already executed in the past.

server/src/com/cloud/vm/UserVmManager.java

@@ -197,14 +197,8 @@ public interface UserVmManager extends Manager, VirtualMachineManager<UserVmVO>
      * @param description the user give description (aka display text) for the template
      * @return a template if successfully created, null otherwise
      */
-    VMTemplateVO createPrivateTemplate(VMTemplateVO template, Long userId, long snapshotId, String name, String description);
+    VMTemplateVO createPrivateTemplate(VMTemplateVO template, Long userId, Long snapshotId, Long volumeId, String name, String description);
 
-    /**
-     * @param userId    The Id of the user who invoked this operation.
-     * @param volumeId  The volume for which this snapshot is being taken
-     * @return          The properties of the snapshot taken
-     */
-    SnapshotVO createTemplateSnapshot(long userId, long volumeId);
     boolean destroyTemplateSnapshot(Long userId, long snapshotId);
 
     /**

server/src/com/cloud/vm/UserVmManagerImpl.java

@@ -20,7 +20,6 @@ package com.cloud.vm;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.Enumeration;
-import java.util.Formatter;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -48,6 +47,7 @@ import com.cloud.agent.api.CheckVirtualMachineAnswer;
 import com.cloud.agent.api.CheckVirtualMachineCommand;
 import com.cloud.agent.api.Command;
 import com.cloud.agent.api.CreatePrivateTemplateFromSnapshotCommand;
+import com.cloud.agent.api.CreatePrivateTemplateFromVolumeCommand;
 import com.cloud.agent.api.GetVmStatsAnswer;
 import com.cloud.agent.api.GetVmStatsCommand;
 import com.cloud.agent.api.ManageSnapshotAnswer;
@@ -60,7 +60,6 @@ import com.cloud.agent.api.StartCommand;
 import com.cloud.agent.api.StopCommand;
 import com.cloud.agent.api.VmStatsEntry;
 import com.cloud.agent.api.storage.CreatePrivateTemplateAnswer;
-import com.cloud.agent.api.storage.CreatePrivateTemplateCommand;
 import com.cloud.alert.AlertManager;
 import com.cloud.api.BaseCmd;
 import com.cloud.async.AsyncJobExecutor;
@@ -2232,7 +2231,7 @@ public class UserVmManagerImpl implements UserVmManager {
         SnapshotVO snapshot = _snapshotDao.findById(Long.valueOf(snapshotId));
         if (snapshot != null) {
             VolumeVO volume = _volsDao.findById(snapshot.getVolumeId());
-            ManageSnapshotCommand cmd = new ManageSnapshotCommand(ManageSnapshotCommand.DESTROY_SNAPSHOT, snapshotId, snapshot.getPath(), snapshot.getName(), null);
+            ManageSnapshotCommand cmd = new ManageSnapshotCommand(snapshotId, snapshot.getPath());
 
             Answer answer = null;
             String basicErrMsg = "Failed to destroy template snapshot: " + snapshot.getName();
@@ -2252,60 +2251,6 @@ public class UserVmManagerImpl implements UserVmManager {
         return success;
     }
     
-    @Override @DB
-    public SnapshotVO createTemplateSnapshot(long userId, long volumeId) {
-        SnapshotVO createdSnapshot = null;
-        VolumeVO volume = _volsDao.lock(volumeId, true);
-        
-        Long id = null;
-        
-        // Determine the name for this snapshot
-        String timeString = DateUtil.getDateDisplayString(DateUtil.GMT_TIMEZONE, new Date(), DateUtil.YYYYMMDD_FORMAT);
-        String snapshotName = volume.getName() + "_" + timeString;
-        // Create the Snapshot object and save it so we can return it to the user
-        SnapshotType snapshotType = SnapshotType.TEMPLATE;
-        SnapshotVO snapshot = new SnapshotVO(volume.getAccountId(), volume.getId(), null, snapshotName, (short)snapshotType.ordinal(), snapshotType.name());
-        snapshot = _snapshotDao.persist(snapshot);
-        id = snapshot.getId();
-
-        // Send a ManageSnapshotCommand to the agent
-        ManageSnapshotCommand cmd = new ManageSnapshotCommand(ManageSnapshotCommand.CREATE_SNAPSHOT, id, volume.getPath(), snapshotName, null);
-        
-        String basicErrMsg = "Failed to create snapshot for volume: " + volume.getId();
-        // This can be sent to a KVM host too. We are only taking snapshots on primary storage, which doesn't require XenServer.
-        // So shouldBeSnapshotCapable is set to false.
-        ManageSnapshotAnswer answer = (ManageSnapshotAnswer) _storageMgr.sendToHostsOnStoragePool(volume.getPoolId(), cmd, basicErrMsg);
-        
-        // Update the snapshot in the database
-        if ((answer != null) && answer.getResult()) {
-            // The snapshot was successfully created
-            
-            Transaction txn = Transaction.currentTxn();
-            txn.start();
-            createdSnapshot = _snapshotDao.findById(id);
-            createdSnapshot.setPath(answer.getSnapshotPath());
-            createdSnapshot.setStatus(Snapshot.Status.CreatedOnPrimary);
-            _snapshotDao.update(id, createdSnapshot);
-            txn.commit();
-            
-            // Don't Create an event for Template Snapshots for now.
-        } else {
-        	if (answer != null) {
-        		s_logger.error(answer.getDetails());
-        	}
-            // The snapshot was not successfully created
-            Transaction txn = Transaction.currentTxn();
-            txn.start();
-            createdSnapshot = _snapshotDao.findById(id);
-            _snapshotDao.expunge(id);
-            txn.commit();
-            
-            createdSnapshot = null;
-        }
-
-        return createdSnapshot;
-    }
-
     
     @Override
     public void cleanNetworkRules(long userId, long instanceId) {
@@ -2467,72 +2412,76 @@ public class UserVmManagerImpl implements UserVmManager {
     }
 
     @Override @DB
-    public VMTemplateVO createPrivateTemplate(VMTemplateVO template, Long userId, long snapshotId, String name, String description) {
+    public VMTemplateVO createPrivateTemplate(VMTemplateVO template, Long userId, Long snapshotId, Long volumeId, String name, String description) {
+        String uniqueName = getRandomPrivateTemplateName();
         VMTemplateVO privateTemplate = null;
         long templateId = template.getId();
+        VolumeVO volume = null;
+        long zoneId;
+        HostVO secondaryStorageHost;
+        
+        Command cmd = null;
+        if( snapshotId != null ) {
             SnapshotVO snapshot = _snapshotDao.findById(snapshotId);
-        if (snapshot != null) {
+            if( snapshot == null ) {
-        	Long volumeId = snapshot.getVolumeId();
+                throw new CloudRuntimeException("Unable to find snapshot for Id " + snapshotId);
-            VolumeVO volume = _volsDao.findById(volumeId);
+            }
-            StringBuilder userFolder = new StringBuilder();
+            String backupSnapshotUUID = snapshot.getBackupSnapshotId();
-            Formatter userFolderFormat = new Formatter(userFolder);
+            if (backupSnapshotUUID == null) {
-            userFolderFormat.format("u%06d", snapshot.getAccountId());
+                throw new CloudRuntimeException("Unable to create private template from snapshot " + snapshotId + " due to there is no backupSnapshotUUID for this snapshot");
-
+            }
-            String uniqueName = getRandomPrivateTemplateName();
+            Long origVolumeId = snapshot.getVolumeId();
-
+            volume = _volsDao.findById(volumeId);
-            long zoneId = volume.getDataCenterId();
+            zoneId = volume.getDataCenterId();
-            HostVO secondaryStorageHost = _storageMgr.getSecondaryStorageHost(zoneId);
+            secondaryStorageHost = _storageMgr.getSecondaryStorageHost(zoneId);
             String secondaryStorageURL = _storageMgr.getSecondaryStorageURL(zoneId);
 
             if (secondaryStorageHost == null || secondaryStorageURL == null) {
-            	s_logger.warn("Did not find the secondary storage URL in the database.");
+                throw new CloudRuntimeException("Did not find the secondary storage URL in the database for zoneId "
-            	return null;
+                        + zoneId);
             }
 
-            Command cmd = null;
+            // We are creating a private template from a snapshot which has been
-            String backupSnapshotUUID = snapshot.getBackupSnapshotId();
+            // backed up to secondary storage.
-            if (backupSnapshotUUID != null) {
-                // We are creating a private template from a snapshot which has been backed up to secondary storage.
             Long dcId = volume.getDataCenterId();
             Long accountId = volume.getAccountId();
 
             String origTemplateInstallPath = null;
 
-                
-                if (ImageFormat.ISO != _snapshotMgr.getImageFormat(volumeId)) {
-                    Long origTemplateId = volume.getTemplateId();
-                    VMTemplateHostVO vmTemplateHostVO = _templateHostDao.findByHostTemplate(secondaryStorageHost.getId(), origTemplateId);
-                    origTemplateInstallPath = vmTemplateHostVO.getInstallPath();
-                }
-                
             cmd = new CreatePrivateTemplateFromSnapshotCommand(_storageMgr.getPrimaryStorageNameLabel(volume),
-                                                                   secondaryStorageURL,
+                    secondaryStorageURL, dcId, accountId, origVolumeId, backupSnapshotUUID, snapshot.getName(),
-                                                                   dcId,
+                    origTemplateInstallPath, templateId, name);
-                                                                   accountId,
+        } else if( volumeId != null ) {
-                                                                   volumeId,
+            volume = _volsDao.findById(volumeId);
-                                                                   backupSnapshotUUID,
+            if( volume == null ) {
-                                                                   snapshot.getName(),
+                throw new CloudRuntimeException("Unable to find volume for Id " + volumeId);
-                                                                   origTemplateInstallPath,
-                                                                   templateId,
-                                                                   name);
             }
-            else {
+            long instanceId = volume.getInstanceId();
-                cmd = new CreatePrivateTemplateCommand(secondaryStorageURL,
+            VMInstanceVO vm = _vmDao.findById(instanceId);
-                                                       templateId,
+            State vmState = vm.getState();
-                                                       volume.getAccountId(),
+            if( !vmState.equals(State.Stopped) && !vmState.equals(State.Destroyed)) {
-                                                       name,
+                throw new CloudRuntimeException("Please put VM " + vm.getName() + " into Stopped state first");
-                                                       uniqueName,
-                                                       _storageMgr.getPrimaryStorageNameLabel(volume),
-                                                       snapshot.getPath(),
-                                                       snapshot.getName(),
-                                                       userFolder.toString());
             }
             
-            // FIXME: before sending the command, check if there's enough capacity on the storage server to create the template
+            zoneId = volume.getDataCenterId();
+            secondaryStorageHost = _storageMgr.getSecondaryStorageHost(zoneId);
+            String secondaryStorageURL = _storageMgr.getSecondaryStorageURL(zoneId);
+
+            if (secondaryStorageHost == null || secondaryStorageURL == null) {
+                throw new CloudRuntimeException("Did not find the secondary storage URL in the database for zoneId " + zoneId);
+            }
+            cmd = new CreatePrivateTemplateFromVolumeCommand(secondaryStorageURL, templateId, volume.getAccountId(),
+                    name, uniqueName, volume.getPath());
+
+        } else {
+            throw new CloudRuntimeException("Creating private Template need to specify snapshotId or volumeId");
+        }
+        // FIXME: before sending the command, check if there's enough capacity
+        // on the storage server to create the template
 
-            String basicErrMsg = "Failed to create template from snapshot: " + snapshot.getName();
         // This can be sent to a KVM host too.
-            CreatePrivateTemplateAnswer answer = (CreatePrivateTemplateAnswer) _storageMgr.sendToHostsOnStoragePool(volume.getPoolId(), cmd, basicErrMsg);
+        CreatePrivateTemplateAnswer answer = (CreatePrivateTemplateAnswer) _storageMgr.sendToHostsOnStoragePool(volume
+                .getPoolId(), cmd, null);
 
         if ((answer != null) && answer.getResult()) {
 
@@ -2544,12 +2493,14 @@ public class UserVmManagerImpl implements UserVmManager {
             }
 
             if ((origTemplate != null) && !Storage.ImageFormat.ISO.equals(origTemplate.getFormat())) {
-                	// We made a template from a root volume that was cloned from a template
+                // We made a template from a root volume that was cloned from a
+                // template
                 privateTemplate.setFileSystem(origTemplate.getFileSystem());
                 privateTemplate.setRequiresHvm(origTemplate.requiresHvm());
                 privateTemplate.setBits(origTemplate.getBits());
             } else {
-                	// We made a template from a root volume that was not cloned from a template, or a data volume
+                // We made a template from a root volume that was not cloned
+                // from a template, or a data volume
                 privateTemplate.setFileSystem(Storage.FileSystem.Unknown);
                 privateTemplate.setRequiresHvm(true);
                 privateTemplate.setBits(64);
@@ -2564,8 +2515,7 @@ public class UserVmManagerImpl implements UserVmManager {
             ImageFormat format = answer.getImageFormat();
             if (format != null) {
                 privateTemplate.setFormat(format);
-                }
+            } else {
-                else {
                 // This never occurs.
                 // Specify RAW format makes it unusable for snapshots.
                 privateTemplate.setFormat(ImageFormat.RAW);
@@ -2590,10 +2540,9 @@ public class UserVmManagerImpl implements UserVmManager {
 
             // Remove the template record
             _templateDao.remove(templateId);
+            throw new CloudRuntimeException("Creating private Template failed due to " + answer.getDetails());
         }
 
-            
-        }
         return privateTemplate;
     }
     

setup/db/create-schema.sql

@@ -998,7 +998,7 @@ CREATE TABLE  `cloud`.`launch_permission` (
 
 CREATE TABLE `cloud`.`snapshot_policy` (
   `id` bigint unsigned NOT NULL auto_increment,
-  `volume_id` bigint unsigned NOT NULL,
+  `volume_id` bigint unsigned NOT NULL unique,
   `schedule` varchar(100) NOT NULL COMMENT 'schedule time of execution',
   `timezone` varchar(100) NOT NULL COMMENT 'the timezone in which the schedule time is specified',
   `interval` int(4) NOT NULL default 4 COMMENT 'backup schedule, e.g. hourly, daily, etc.',
@@ -1016,12 +1016,11 @@ CREATE TABLE  `cloud`.`snapshot_policy_ref` (
 
 CREATE TABLE  `cloud`.`snapshot_schedule` (
   `id` bigint unsigned NOT NULL auto_increment,
-  `volume_id` bigint unsigned NOT NULL COMMENT 'The volume for which this snapshot is being taken',
+  `volume_id` bigint unsigned NOT NULL unique COMMENT 'The volume for which this snapshot is being taken',
   `policy_id` bigint unsigned NOT NULL COMMENT 'One of the policyIds for which this snapshot was taken',
   `scheduled_timestamp` datetime NOT NULL COMMENT 'Time at which the snapshot was scheduled for execution',
   `async_job_id` bigint unsigned COMMENT 'If this schedule is being executed, it is the id of the create aysnc_job. Before that it is null',
   `snapshot_id` bigint unsigned COMMENT 'If this schedule is being executed, then the corresponding snapshot has this id. Before that it is null',
-  UNIQUE (volume_id, policy_id),
   PRIMARY KEY  (`id`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;