Support Backup of Snapshots for Managed Storage

This PR adds an ability to Pass a new parameter, locationType,
    to the “createSnapshot” API command. Depending on the locationType,
    we decide where the snapshot should go in case of managed storage.

    There are two possible values for the locationType param

    1) `Standard`: The standard operation for managed storage is to
    keep the snapshot on the device. For non-managed storage, this will
    be to upload it to secondary storage. This option will be the
    default.

    2) `Archive`: Applicable only to managed storage. This will
    keep the snapshot on the secondary storage. For non-managed
    storage, this will result in an error.

    The reason for implementing this feature is to avoid a single
    point of failure for primary storage. Right now in case of managed
    storage, if the primary storage goes down, there is no easy way
    to recover data as all snapshots are also stored on the primary.
    This features allows us to mitigate that risk.

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

@@ -16,14 +16,13 @@
 // under the License.
 package com.cloud.storage;
 
-import java.util.Date;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
-
+import com.cloud.utils.fsm.StateObject;
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.api.Identity;
 import org.apache.cloudstack.api.InternalIdentity;
 
-import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import java.util.Date;
-import com.cloud.utils.fsm.StateObject;
 
 public interface Snapshot extends ControlledEntity, Identity, InternalIdentity, StateObject<Snapshot.State> {
     public enum Type {
@@ -67,6 +66,10 @@ public interface Snapshot extends ControlledEntity, Identity, InternalIdentity,
         CreateRequested, OperationNotPerformed, BackupToSecondary, BackedupToSecondary, DestroyRequested, CopyingRequested, OperationSucceeded, OperationFailed
     }
 
+    enum LocationType {
+        PRIMARY, SECONDARY
+    }
+
     public static final long MANUAL_POLICY_ID = 0L;
 
     @Override
@@ -89,4 +92,5 @@ public interface Snapshot extends ControlledEntity, Identity, InternalIdentity,
 
     short getsnapshotType();
 
+    LocationType getLocationType(); // This type is in reference to the location where the snapshot resides (ex. primary storage, archive on secondary storage, etc.)
 }

api/src/com/cloud/storage/VolumeApiService.java

@@ -70,8 +70,6 @@ public interface VolumeApiService {
     /**
      * Uploads the volume to secondary storage
      *
-     * @param UploadVolumeCmdByAdmin cmd
-     *
      * @return Volume object
      */
     Volume uploadVolume(UploadVolumeCmd cmd)    throws ResourceAllocationException;
@@ -82,11 +80,11 @@ public interface VolumeApiService {
 
     Volume attachVolumeToVM(AttachVolumeCmd command);
 
-    Volume detachVolumeFromVM(DetachVolumeCmd cmmd);
+    Volume detachVolumeFromVM(DetachVolumeCmd cmd);
 
-    Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm) throws ResourceAllocationException;
+    Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm, Snapshot.LocationType locationType) throws ResourceAllocationException;
 
-    Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName) throws ResourceAllocationException;
+    Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType) throws ResourceAllocationException;
 
     Volume updateVolume(long volumeId, String path, String state, Long storageId, Boolean displayVolume, String customId, long owner, String chainInfo);
 

api/src/com/cloud/storage/snapshot/SnapshotApiService.java

@@ -86,7 +86,7 @@ public interface SnapshotApiService {
 
     boolean deleteSnapshotPolicies(DeleteSnapshotPoliciesCmd cmd);
 
-    Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName) throws ResourceAllocationException;
+    Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType) throws ResourceAllocationException;
 
     /**
      * Create a snapshot of a volume

api/src/org/apache/cloudstack/api/ApiConstants.java

@@ -131,6 +131,7 @@ public class ApiConstants {
     public static final String INTERNAL_DNS1 = "internaldns1";
     public static final String INTERNAL_DNS2 = "internaldns2";
     public static final String INTERVAL_TYPE = "intervaltype";
+    public static final String LOCATION_TYPE = "locationtype";
     public static final String IOPS_READ_RATE = "iopsreadrate";
     public static final String IOPS_WRITE_RATE = "iopswriterate";
     public static final String IP_ADDRESS = "ipaddress";

api/src/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java

@@ -16,8 +16,15 @@
 // under the License.
 package org.apache.cloudstack.api.command.user.snapshot;
 
-import org.apache.log4j.Logger;
+import com.cloud.event.EventTypes;
-
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.PermissionDeniedException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.projects.Project;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.Volume;
+import com.cloud.user.Account;
+import com.cloud.utils.exception.CloudRuntimeException;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiCommandJobType;
 import org.apache.cloudstack.api.ApiConstants;
@@ -30,16 +37,7 @@ import org.apache.cloudstack.api.response.DomainResponse;
 import org.apache.cloudstack.api.response.SnapshotPolicyResponse;
 import org.apache.cloudstack.api.response.SnapshotResponse;
 import org.apache.cloudstack.api.response.VolumeResponse;
-import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
-
-import com.cloud.event.EventTypes;
-import com.cloud.exception.InvalidParameterValueException;
-import com.cloud.exception.PermissionDeniedException;
-import com.cloud.exception.ResourceAllocationException;
-import com.cloud.projects.Project;
-import com.cloud.storage.Snapshot;
-import com.cloud.storage.Volume;
-import com.cloud.user.Account;
 
 @APICommand(name = "createSnapshot", description = "Creates an instant snapshot of a volume.", responseObject = SnapshotResponse.class, entityType = {Snapshot.class},
         requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
@@ -74,6 +72,10 @@ public class CreateSnapshotCmd extends BaseAsyncCreateCmd {
     @Parameter(name = ApiConstants.SNAPSHOT_QUIESCEVM, type = CommandType.BOOLEAN, required = false, description = "quiesce vm if true")
     private Boolean quiescevm;
 
+    @Parameter(name = ApiConstants.LOCATION_TYPE, type = CommandType.STRING, required = false, description = "Currently applicable only for managed storage. " +
+            "Valid location types: 'primary', 'secondary'. Default = 'primary'.")
+    private String locationType;
+
     @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "the name of the snapshot")
     private String snapshotName;
 
@@ -108,7 +110,7 @@ public class CreateSnapshotCmd extends BaseAsyncCreateCmd {
     }
 
     public String getVolumeUuid() {
-        Volume volume = (Volume)this._entityMgr.findById(Volume.class, getVolumeId());
+        Volume volume = _entityMgr.findById(Volume.class, getVolumeId());
         if (volume == null) {
             throw new InvalidParameterValueException("Unable to find volume's UUID");
         }
@@ -184,7 +186,7 @@ public class CreateSnapshotCmd extends BaseAsyncCreateCmd {
 
     @Override
     public void create() throws ResourceAllocationException {
-        Snapshot snapshot = _volumeService.allocSnapshot(getVolumeId(), getPolicyId(), getSnapshotName());
+        Snapshot snapshot = _volumeService.allocSnapshot(getVolumeId(), getPolicyId(), getSnapshotName(), getLocationType());
         if (snapshot != null) {
             setEntityId(snapshot.getId());
             setEntityUuid(snapshot.getUuid());
@@ -195,21 +197,37 @@ public class CreateSnapshotCmd extends BaseAsyncCreateCmd {
 
     @Override
     public void execute() {
-        s_logger.info("VOLSS: createSnapshotCmd starts:" + System.currentTimeMillis());
-        CallContext.current().setEventDetails("Volume Id: " + getVolumeUuid());
         Snapshot snapshot;
         try {
             snapshot =
-                _volumeService.takeSnapshot(getVolumeId(), getPolicyId(), getEntityId(), _accountService.getAccount(getEntityOwnerId()), getQuiescevm());
+                _volumeService.takeSnapshot(getVolumeId(), getPolicyId(), getEntityId(), _accountService.getAccount(getEntityOwnerId()), getQuiescevm(), getLocationType());
+
             if (snapshot != null) {
                 SnapshotResponse response = _responseGenerator.createSnapshotResponse(snapshot);
                 response.setResponseName(getCommandName());
                 setResponseObject(response);
             } else {
-                throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create snapshot due to an internal error creating snapshot for volume " + volumeId);
+                throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create snapshot due to an internal error creating snapshot for volume " + getVolumeId());
             }
         } catch (Exception e) {
-            throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create snapshot due to an internal error creating snapshot for volume " + volumeId);
+            throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create snapshot due to an internal error creating snapshot for volume " + getVolumeId());
+        }
+    }
+
+    private Snapshot.LocationType getLocationType() {
+
+        if (Snapshot.LocationType.values() == null || Snapshot.LocationType.values().length == 0 || locationType == null) {
+            return null;
+        }
+
+        try {
+            String lType = locationType.trim().toUpperCase();
+            return Snapshot.LocationType.valueOf(lType);
+        } catch (IllegalArgumentException e) {
+            String errMesg = "Invalid locationType " + locationType + "Specified for volume " + getVolumeId()
+                        + " Valid values are: primary,secondary ";
+            s_logger.warn(errMesg);
+            throw  new CloudRuntimeException(errMesg);
         }
     }
 

api/src/org/apache/cloudstack/api/response/SnapshotResponse.java

@@ -16,17 +16,15 @@
 // under the License.
 package org.apache.cloudstack.api.response;
 
-import java.util.Date;
+import com.cloud.serializer.Param;
-import java.util.List;
+import com.cloud.storage.Snapshot;
-
 import com.google.gson.annotations.SerializedName;
-
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.BaseResponse;
 import org.apache.cloudstack.api.EntityReference;
 
-import com.cloud.serializer.Param;
+import java.util.Date;
-import com.cloud.storage.Snapshot;
+import java.util.List;
 
 @EntityReference(value = Snapshot.class)
 public class SnapshotResponse extends BaseResponse implements ControlledEntityResponse {
@@ -82,6 +80,10 @@ public class SnapshotResponse extends BaseResponse implements ControlledEntityRe
     @Param(description = "valid types are hourly, daily, weekly, monthy, template, and none.")
     private String intervalType;
 
+    @SerializedName(ApiConstants.LOCATION_TYPE)
+    @Param(description = "valid location types are primary and secondary.")
+    private String locationType;
+
     @SerializedName(ApiConstants.STATE)
     @Param(description = "the state of the snapshot. BackedUp means that snapshot is ready to be used; Creating - the snapshot is being allocated on the primary storage; BackingUp - the snapshot is being backed up on secondary storage")
     private Snapshot.State state;
@@ -166,6 +168,10 @@ public class SnapshotResponse extends BaseResponse implements ControlledEntityRe
         this.intervalType = intervalType;
     }
 
+    public void setLocationType(String locationType) {
+        this.locationType = locationType;
+    }
+
     public void setState(Snapshot.State state) {
         this.state = state;
     }

api/test/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java

@@ -0,0 +1,132 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.test;
+
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.VolumeApiService;
+import com.cloud.user.Account;
+import com.cloud.user.AccountService;
+import junit.framework.TestCase;
+import org.apache.cloudstack.api.ResponseGenerator;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotCmd;
+import org.apache.cloudstack.api.response.SnapshotResponse;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.Mockito;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.isNull;
+
+public class CreateSnapshotCmdTest extends TestCase {
+
+    private CreateSnapshotCmd createSnapshotCmd;
+    private ResponseGenerator responseGenerator;
+
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
+    @Override
+    @Before
+    public void setUp() {
+
+        createSnapshotCmd = new CreateSnapshotCmd() {
+
+            @Override
+            public String getCommandName() {
+                return "createsnapshotresponse";
+            }
+
+            @Override
+            public Long getVolumeId(){
+                return 1L;
+            }
+
+            @Override
+            public long getEntityOwnerId(){
+                return 1L;
+            }
+        };
+
+    }
+
+    @Test
+    public void testCreateSuccess() {
+
+        AccountService accountService = Mockito.mock(AccountService.class);
+        Account account = Mockito.mock(Account.class);
+        Mockito.when(accountService.getAccount(anyLong())).thenReturn(account);
+
+        VolumeApiService volumeApiService = Mockito.mock(VolumeApiService.class);
+        Snapshot snapshot = Mockito.mock(Snapshot.class);
+        try {
+
+            Mockito.when(volumeApiService.takeSnapshot(anyLong(), anyLong(), anyLong(),
+                    any(Account.class), anyBoolean(), isNull(Snapshot.LocationType.class))).thenReturn(snapshot);
+
+        } catch (Exception e) {
+            Assert.fail("Received exception when success expected " + e.getMessage());
+        }
+
+        responseGenerator = Mockito.mock(ResponseGenerator.class);
+        SnapshotResponse snapshotResponse = Mockito.mock(SnapshotResponse.class);
+        Mockito.when(responseGenerator.createSnapshotResponse(snapshot)).thenReturn(snapshotResponse);
+        Mockito.doNothing().when(snapshotResponse).setAccountName(anyString());
+
+        createSnapshotCmd._accountService = accountService;
+        createSnapshotCmd._responseGenerator = responseGenerator;
+        createSnapshotCmd._volumeService = volumeApiService;
+
+        try {
+            createSnapshotCmd.execute();
+        } catch (Exception e) {
+            Assert.fail("Received exception when success expected " + e.getMessage());
+        }
+    }
+
+    @Test
+    public void testCreateFailure() {
+
+        AccountService accountService = Mockito.mock(AccountService.class);
+        Account account = Mockito.mock(Account.class);
+        Mockito.when(accountService.getAccount(anyLong())).thenReturn(account);
+
+        VolumeApiService volumeApiService = Mockito.mock(VolumeApiService.class);
+
+        try {
+                Mockito.when(volumeApiService.takeSnapshot(anyLong(), anyLong(), anyLong(),
+                        any(Account.class), anyBoolean(), isNull(Snapshot.LocationType.class))).thenReturn(null);
+        } catch (Exception e) {
+            Assert.fail("Received exception when success expected " + e.getMessage());
+        }
+
+        createSnapshotCmd._accountService = accountService;
+        createSnapshotCmd._volumeService = volumeApiService;
+
+        try {
+            createSnapshotCmd.execute();
+        } catch (ServerApiException exception) {
+            Assert.assertEquals("Failed to create snapshot due to an internal error creating snapshot for volume 1", exception.getDescription());
+        }
+    }
+}

core/src/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java

@@ -19,13 +19,12 @@
 
 package org.apache.cloudstack.storage.to;
 
-import java.util.Map;
-
-import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
-
 import com.cloud.agent.api.to.DataStoreTO;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.Storage.StoragePoolType;
+import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
+
+import java.util.Map;
 
 public class PrimaryDataStoreTO implements DataStoreTO {
     public static final String MANAGED = PrimaryDataStore.MANAGED;
@@ -52,6 +51,7 @@ public class PrimaryDataStoreTO implements DataStoreTO {
     private Map<String, String> details;
     private static final String pathSeparator = "/";
     private Boolean fullCloneFlag;
+    private final boolean isManaged;
 
     public PrimaryDataStoreTO(PrimaryDataStore dataStore) {
         this.uuid = dataStore.getUuid();
@@ -63,6 +63,7 @@ public class PrimaryDataStoreTO implements DataStoreTO {
         this.setPort(dataStore.getPort());
         this.url = dataStore.getUri();
         this.details = dataStore.getDetails();
+        this.isManaged = dataStore.isManaged();
     }
 
     public long getId() {
@@ -153,4 +154,8 @@ public class PrimaryDataStoreTO implements DataStoreTO {
     public void setFullCloneFlag(Boolean fullCloneFlag) {
         this.fullCloneFlag = fullCloneFlag;
     }
+
+    public boolean isManaged() {
+        return isManaged;
+    }
 }

engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotResult.java

@@ -21,21 +21,21 @@ import org.apache.cloudstack.storage.command.CommandResult;
 import com.cloud.agent.api.Answer;
 
 public class SnapshotResult extends CommandResult {
-    private SnapshotInfo snashot;
+    private SnapshotInfo snapshot;
     private Answer answer;
 
     public SnapshotResult(SnapshotInfo snapshot, Answer answer) {
         super();
-        this.setSnashot(snapshot);
+        this.setSnapshot(snapshot);
         this.setAnswer(answer);
     }
 
-    public SnapshotInfo getSnashot() {
+    public SnapshotInfo getSnapshot() {
-        return snashot;
+        return snapshot;
     }
 
-    public void setSnashot(SnapshotInfo snashot) {
+    public void setSnapshot(SnapshotInfo snapshot) {
-        this.snashot = snashot;
+        this.snapshot = snapshot;
     }
 
     public Answer getAnswer() {

engine/components-api/src/com/cloud/vm/VmWorkTakeVolumeSnapshot.java

@@ -16,6 +16,8 @@
 // under the License.
 package com.cloud.vm;
 
+import com.cloud.storage.Snapshot;
+
 public class VmWorkTakeVolumeSnapshot extends VmWork {
 
     private static final long serialVersionUID = 341816293003023823L;
@@ -24,14 +26,16 @@ public class VmWorkTakeVolumeSnapshot extends VmWork {
     private Long policyId;
     private Long snapshotId;
     private boolean quiesceVm;
+    private Snapshot.LocationType locationType;
 
     public VmWorkTakeVolumeSnapshot(long userId, long accountId, long vmId, String handlerName,
-            Long volumeId, Long policyId, Long snapshotId, boolean quiesceVm) {
+            Long volumeId, Long policyId, Long snapshotId, boolean quiesceVm, Snapshot.LocationType locationType) {
         super(userId, accountId, vmId, handlerName);
         this.volumeId = volumeId;
         this.policyId = policyId;
         this.snapshotId = snapshotId;
         this.quiesceVm = quiesceVm;
+        this.locationType = locationType;
     }
 
     public Long getVolumeId() {
@@ -49,4 +53,6 @@ public class VmWorkTakeVolumeSnapshot extends VmWork {
     public boolean isQuiesceVm() {
         return quiesceVm;
     }
+
+    public Snapshot.LocationType getLocationType() { return locationType; }
 }

engine/schema/src/com/cloud/storage/SnapshotVO.java

@@ -16,8 +16,9 @@
 // under the License.
 package com.cloud.storage;
 
-import java.util.Date;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import java.util.UUID;
+import com.cloud.utils.db.GenericDao;
+import com.google.gson.annotations.Expose;
 
 import javax.persistence.Column;
 import javax.persistence.Entity;
@@ -27,11 +28,8 @@ import javax.persistence.GeneratedValue;
 import javax.persistence.GenerationType;
 import javax.persistence.Id;
 import javax.persistence.Table;
-
+import java.util.Date;
-import com.google.gson.annotations.Expose;
+import java.util.UUID;
-
-import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import com.cloud.utils.db.GenericDao;
 
 @Entity
 @Table(name = "snapshots")
@@ -69,6 +67,11 @@ public class SnapshotVO implements Snapshot {
     @Column(name = "snapshot_type")
     short snapshotType;
 
+    @Expose
+    @Column(name = "location_type", updatable = true, nullable = true)
+    @Enumerated(value = EnumType.STRING)
+    private LocationType locationType;
+
     @Column(name = "type_description")
     String typeDescription;
 
@@ -103,7 +106,7 @@ public class SnapshotVO implements Snapshot {
     }
 
     public SnapshotVO(long dcId, long accountId, long domainId, Long volumeId, Long diskOfferingId, String name, short snapshotType, String typeDescription, long size,
-            Long minIops, Long maxIops, HypervisorType hypervisorType) {
+            Long minIops, Long maxIops, HypervisorType hypervisorType, LocationType locationType) {
         dataCenterId = dcId;
         this.accountId = accountId;
         this.domainId = domainId;
@@ -119,6 +122,7 @@ public class SnapshotVO implements Snapshot {
         this.hypervisorType = hypervisorType;
         version = "2.2";
         uuid = UUID.randomUUID().toString();
+        this.locationType = locationType;
     }
 
     @Override
@@ -171,6 +175,15 @@ public class SnapshotVO implements Snapshot {
         return Type.values()[snapshotType];
     }
 
+    @Override
+    public LocationType getLocationType() {
+        return locationType;
+    }
+
+    public void setLocationType(LocationType locationType) {
+        this.locationType = locationType;
+    }
+
     @Override
     public HypervisorType getHypervisorType() {
         return hypervisorType;

engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java

@@ -18,53 +18,17 @@
  */
 package org.apache.cloudstack.storage.motion;
 
-import java.util.ArrayList;
+import com.cloud.agent.AgentManager;
-import java.util.Collections;
+import com.cloud.agent.api.Answer;
-import java.util.HashMap;
+import com.cloud.agent.api.to.DataStoreTO;
-import java.util.List;
+import com.cloud.agent.api.to.DataTO;
-import java.util.Map;
+import com.cloud.agent.api.to.DiskTO;
-import java.util.Random;
+import com.cloud.agent.api.to.NfsTO;
-import java.util.concurrent.ExecutionException;
+import com.cloud.agent.api.to.VirtualMachineTO;
-
+import com.cloud.configuration.Config;
-import javax.inject.Inject;
-
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.exception.AgentUnavailableException;
 import com.cloud.exception.OperationTimedoutException;
-
-import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
-import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
-import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService.VolumeApiResult;
-import org.apache.cloudstack.framework.async.AsyncCallFuture;
-import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.storage.command.CopyCmdAnswer;
-import org.apache.cloudstack.storage.command.CopyCommand;
-import org.apache.cloudstack.storage.command.ResignatureAnswer;
-import org.apache.cloudstack.storage.command.ResignatureCommand;
-import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.apache.cloudstack.storage.to.VolumeObjectTO;
-import org.apache.commons.lang.StringUtils;
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
-
-import com.cloud.agent.AgentManager;
-import com.cloud.agent.api.to.DiskTO;
-import com.cloud.agent.api.to.VirtualMachineTO;
-import com.cloud.configuration.Config;
 import com.cloud.host.Host;
 import com.cloud.host.HostVO;
 import com.cloud.host.dao.HostDao;
@@ -75,6 +39,7 @@ import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.DiskOfferingVO;
 import com.cloud.storage.SnapshotVO;
 import com.cloud.storage.Storage.ImageFormat;
+import com.cloud.storage.Volume;
 import com.cloud.storage.VolumeDetailVO;
 import com.cloud.storage.VolumeVO;
 import com.cloud.storage.dao.DiskOfferingDao;
@@ -86,8 +51,58 @@ import com.cloud.storage.dao.VolumeDetailsDao;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.VirtualMachineManager;
-
 import com.google.common.base.Preconditions;
+import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
+import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
+import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService.VolumeApiResult;
+import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
+import org.apache.cloudstack.framework.async.AsyncCallFuture;
+import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.command.CopyCommand;
+import org.apache.cloudstack.storage.command.DeleteCommand;
+import org.apache.cloudstack.storage.command.DettachAnswer;
+import org.apache.cloudstack.storage.command.DettachCommand;
+import org.apache.cloudstack.storage.command.ResignatureAnswer;
+import org.apache.cloudstack.storage.command.ResignatureCommand;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import javax.inject.Inject;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 @Component
 public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
@@ -109,7 +124,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
     @Inject private VolumeDataFactory _volumeDataFactory;
     @Inject private VolumeDetailsDao volumeDetailsDao;
     @Inject private VolumeService _volumeService;
-
+    @Inject private StorageCacheManager cacheMgr;
+    @Inject private EndPointSelector selector;
     @Override
     public StrategyPriority canHandle(DataObject srcData, DataObject destData) {
         if (srcData instanceof SnapshotInfo) {
@@ -180,9 +196,9 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
 
             boolean canHandleSrc = canHandle(srcData);
 
-            if (canHandleSrc && destData instanceof TemplateInfo &&
+            if (canHandleSrc && (destData instanceof TemplateInfo || destData instanceof SnapshotInfo) &&
                     (destData.getDataStore().getRole() == DataStoreRole.Image || destData.getDataStore().getRole() == DataStoreRole.ImageCache)) {
-                handleCreateTemplateFromSnapshot(snapshotInfo, (TemplateInfo)destData, callback);
+                handleCopyDataToSecondaryStorage(snapshotInfo, destData, callback);
 
                 return;
             }
@@ -207,16 +223,12 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
                     }
                 }
 
-                if (canHandleSrc) {
+                if (canHandleDest) {
-                    String errMsg = "This operation is not supported (DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT " +
+                    handleCreateVolumeFromSnapshotOnSecondaryStorage(snapshotInfo, volumeInfo, callback);
-                            "not supported by destination storage plug-in). " + getDestDataStoreMsg(destData);
+                    return;
-
-                    LOGGER.warn(errMsg);
-
-                    throw new UnsupportedOperationException(errMsg);
                 }
 
-                if (canHandleDest) {
+                if (canHandleSrc) {
                     String errMsg = "This operation is not supported (DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT " +
                             "not supported by source storage plug-in). " + getSrcDataStoreMsg(srcData);
 
@@ -280,7 +292,72 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
         return Boolean.parseBoolean(property);
     }
 
-    private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, TemplateInfo templateInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+    protected boolean needCacheStorage(DataObject srcData, DataObject destData) {
+        DataTO srcTO = srcData.getTO();
+        DataStoreTO srcStoreTO = srcTO.getDataStore();
+        DataTO destTO = destData.getTO();
+        DataStoreTO destStoreTO = destTO.getDataStore();
+
+        // both snapshot and volume are on primary datastore. No need for a cache storage as
+        // hypervisor will copy directly
+        if (srcStoreTO instanceof PrimaryDataStoreTO && destStoreTO instanceof PrimaryDataStoreTO) {
+            return false;
+        }
+
+        if (srcStoreTO instanceof NfsTO || srcStoreTO.getRole() == DataStoreRole.ImageCache) {
+            return false;
+        }
+
+
+        if (destStoreTO instanceof NfsTO || destStoreTO.getRole() == DataStoreRole.ImageCache) {
+            return false;
+        }
+        if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("needCacheStorage true, dest at " + destTO.getPath() + " dest role " + destStoreTO.getRole().toString() + srcTO.getPath() + " src role " +
+                srcStoreTO.getRole().toString());
+        }
+        return true;
+    }
+
+    private Scope pickCacheScopeForCopy(DataObject srcData, DataObject destData) {
+        Scope srcScope = srcData.getDataStore().getScope();
+        Scope destScope = destData.getDataStore().getScope();
+
+        Scope selectedScope = null;
+        if (srcScope.getScopeId() != null) {
+            selectedScope = getZoneScope(srcScope);
+        } else if (destScope.getScopeId() != null) {
+            selectedScope = getZoneScope(destScope);
+        } else {
+            LOGGER.warn("Cannot find a zone-wide scope for movement that needs a cache storage");
+        }
+        return selectedScope;
+    }
+
+    private Scope getZoneScope(Scope scope) {
+        ZoneScope zoneScope;
+        if (scope instanceof ClusterScope) {
+            ClusterScope clusterScope = (ClusterScope)scope;
+            zoneScope = new ZoneScope(clusterScope.getZoneId());
+        } else if (scope instanceof HostScope) {
+            HostScope hostScope = (HostScope)scope;
+            zoneScope = new ZoneScope(hostScope.getZoneId());
+        } else {
+            zoneScope = (ZoneScope)scope;
+        }
+        return zoneScope;
+    }
+
+    /**
+     * This function is responsible for copying a volume from the managed store to a secondary store. This is used in two cases
+     * 1) When creating a template from a snapshot
+     * 2) When createSnapshot is called with location=SECONDARY
+     *
+     * @param snapshotInfo Source snapshot
+     * @param destData destination (can be template or snapshot)
+     * @param callback callback for async
+     */
+    private void handleCopyDataToSecondaryStorage(SnapshotInfo snapshotInfo, DataObject destData, AsyncCompletionCallback<CopyCommandResult> callback) {
         try {
             snapshotInfo.processEvent(Event.CopyingRequested);
         }
@@ -292,6 +369,16 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
 
         boolean usingBackendSnapshot = usingBackendSnapshotFor(snapshotInfo);
         boolean computeClusterSupportsResign = clusterDao.getSupportsResigning(hostVO.getClusterId());
+        boolean needCache = needCacheStorage(snapshotInfo, destData);
+
+        DataObject destOnStore = destData;
+
+        if (needCache) {
+            // creates an object in the DB for data to be cached
+            Scope selectedScope = pickCacheScopeForCopy(snapshotInfo, destData);
+            destOnStore = cacheMgr.getCacheObject(snapshotInfo, selectedScope);
+            destOnStore.processEvent(Event.CreateOnlyRequested);
+        }
 
         if (usingBackendSnapshot && !computeClusterSupportsResign) {
             String noSupportForResignErrMsg = "Unable to locate an applicable host with which to perform a resignature operation : Cluster ID = " + hostVO.getClusterId();
@@ -310,16 +397,15 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
 
             String value = _configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
             int primaryStorageDownloadWait = NumbersUtil.parseInt(value, Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-            CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+            CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(), destOnStore.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
 
             String errMsg = null;
-
             CopyCmdAnswer copyCmdAnswer = null;
 
             try {
                 // If we are using a back-end snapshot, then we should still have access to it from the hosts in the cluster that hostVO is in
                 // (because we passed in true as the third parameter to createVolumeFromSnapshot above).
-                if (usingBackendSnapshot == false) {
+                if (!usingBackendSnapshot) {
                     _volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
                 }
 
@@ -328,21 +414,46 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
                 copyCommand.setOptions(srcDetails);
 
                 copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
-            }
+
-            catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) {
+                if (needCache) {
+
+                    // If cached storage was needed (in case of object store as secondary
+                    // storage), at this point, the data has been copied from the primary
+                    // to the NFS cache by the hypervisor. We now invoke another copy
+                    // command to copy this data from cache to secondary storage. We
+                    // then cleanup the cache
+
+                    destOnStore.processEvent(Event.OperationSuccessed, copyCmdAnswer);
+
+                    CopyCommand cmd = new CopyCommand(destOnStore.getTO(), destData.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+                    EndPoint ep = selector.select(destOnStore, destData);
+
+                    if (ep == null) {
+                        errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
+                        LOGGER.error(errMsg);
+                        copyCmdAnswer = new CopyCmdAnswer(errMsg);
+                    } else {
+                        copyCmdAnswer = (CopyCmdAnswer) ep.sendMessage(cmd);
+                    }
+
+                    // clean up snapshot copied to staging
+                    performCleanupCacheStorage(destOnStore);
+                }
+
+
+            } catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) {
                 String msg = "Failed to create template from snapshot (Snapshot ID = " + snapshotInfo.getId() + ") : ";
 
                 LOGGER.warn(msg, ex);
 
                 throw new CloudRuntimeException(msg + ex.getMessage());
-            }
+
-            finally {
+            } finally {
-                try {
+
-                    _volumeService.revokeAccess(snapshotInfo, hostVO, srcDataStore);
+                // detach and remove access after the volume is created
-                }
+                SnapshotObjectTO snapshotObjectTO = (SnapshotObjectTO) snapshotInfo.getTO();
-                catch (Exception ex) {
+                DiskTO disk = new DiskTO(snapshotObjectTO, null, snapshotInfo.getPath(), Volume.Type.UNKNOWN);
-                    LOGGER.warn("Error revoking access to snapshot (Snapshot ID = " + snapshotInfo.getId() + "): " + ex.getMessage(), ex);
+                detachManagedStoreVolume(snapshotInfo, hostVO, getProperty(snapshotInfo.getId(), DiskTO.IQN), srcDataStore.getId(), disk);
-                }
 
                 if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
                     if (copyCmdAnswer != null && !StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
@@ -379,6 +490,141 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
         }
     }
 
+    /**
+     * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+     * @param snapshotInfo snapshot on secondary
+     * @param volumeInfo volume to be created on the storage
+     * @param callback  for async
+     */
+    private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+        // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+        DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+        SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+        DataStore destDataStore = volumeInfo.getDataStore();
+
+        // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+        _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+        CopyCmdAnswer copyCmdAnswer = null;
+        String errMsg = null;
+
+        HostVO hostVO = null;
+        try {
+
+            //create a volume on the storage
+            AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+            VolumeApiResult result = future.get();
+
+            if (result.isFailed()) {
+                LOGGER.error("Failed to create a volume: " + result.getResult());
+                throw new CloudRuntimeException(result.getResult());
+            }
+
+            volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
+
+            volumeInfo.processEvent(Event.MigrationRequested);
+
+            volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
+
+            hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+            //copy the volume from secondary via the hypervisor
+            copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo, hostVO);
+
+            if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+                if (copyCmdAnswer != null && !StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+                    errMsg = copyCmdAnswer.getDetails();
+                }
+                else {
+                    errMsg = "Unable to create volume from snapshot";
+                }
+            }
+        }
+        catch (Exception ex) {
+            errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy operation failed in 'StorageSystemDataMotionStrategy.handleCreateVolumeFromSnapshotBothOnStorageSystem'";
+        } finally {
+
+
+            DiskTO disk = new DiskTO(volumeInfo.getTO(), volumeInfo.getDeviceId(), volumeInfo.getPath(),volumeInfo.getVolumeType());
+            long storagePoolId = volumeInfo.getPoolId();
+            detachManagedStoreVolume(volumeInfo, hostVO, volumeInfo.get_iScsiName(), storagePoolId, disk);
+        }
+
+        CopyCommandResult result = new CopyCommandResult(null, copyCmdAnswer);
+
+        result.setResult(errMsg);
+
+        callback.complete(result);
+
+
+    }
+
+    /**
+     * Detaches a managed volume from a host
+     *  @param dataObject Volume to be detached
+     * @param hostVO    Host where the volume is currently attached
+     * @param storagePoolId Storage where volume resides
+     * @param disk  Object which stores disk attributes to send to the agents
+     */
+    private void detachManagedStoreVolume(DataObject dataObject, HostVO hostVO, String iqn, long storagePoolId, DiskTO disk) {
+
+        DataStore destDataStore = dataObject.getDataStore();
+        DettachCommand cmd = new DettachCommand(disk, null);
+        StoragePoolVO storagePool = _storagePoolDao.findById(storagePoolId);
+
+        cmd.setManaged(true);
+        cmd.setStorageHost(storagePool.getHostAddress());
+        cmd.setStoragePort(storagePool.getPort());
+        cmd.set_iScsiName(iqn);
+
+        try {
+            DettachAnswer dettachAnswer = (DettachAnswer) _agentMgr.send(hostVO.getId(), cmd);
+
+            if (!dettachAnswer.getResult()) {
+                LOGGER.warn("Error detaching DataObject:" + dettachAnswer.getDetails());
+            }
+
+        } catch (Exception e) {
+            LOGGER.warn("Error detaching DataObject " + dataObject.getId() + " Error: " + e.getMessage());
+        }
+
+
+        try {
+            _volumeService.revokeAccess(dataObject, hostVO, destDataStore);
+        } catch (Exception e) {
+            LOGGER.warn("Error revoking access to DataObject (DataObject ID = " + dataObject.getId() + "): " + e.getMessage(), e);
+        }
+    }
+
+    private void performCleanupCacheStorage(DataObject destOnStore) {
+        destOnStore.processEvent(Event.DestroyRequested);
+
+        DeleteCommand deleteCommand = new DeleteCommand(destOnStore.getTO());
+        EndPoint ep = selector.select(destOnStore);
+        try {
+            if (ep == null) {
+                LOGGER.warn("Unable to cleanup staging NFS because no endpoint was found " +
+                "Object: " + destOnStore.getType() + " ID: " + destOnStore.getId());
+
+                destOnStore.processEvent(Event.OperationFailed);
+            } else {
+                Answer deleteAnswer = ep.sendMessage(deleteCommand);
+                if (deleteAnswer != null && deleteAnswer.getResult()) {
+                    LOGGER.warn("Unable to cleanup staging NFS " + deleteAnswer.getDetails() +
+                    "Object: " + destOnStore.getType() + " ID: " + destOnStore.getId());
+                    destOnStore.processEvent(Event.OperationFailed);
+                }
+            }
+
+            destOnStore.processEvent(Event.OperationSuccessed);
+        } catch (Exception e) {
+            destOnStore.processEvent(Event.OperationFailed);
+            LOGGER.warn("Unable to clean up staging cache Exception " + e.getMessage() +
+                    "Object: " + destOnStore.getType() + " ID: " + destOnStore.getId());
+        }
+    }
+
     /**
      * Clones a template present on the storage to a new volume and resignatures it.
      *
@@ -418,7 +664,9 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
             volumeDetail = volumeDetailsDao.persist(volumeDetail);
 
             AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
-            VolumeApiResult result = future.get();
+            int storagePoolMaxWaitSeconds = NumbersUtil.parseInt(_configDao.getValue(Config.StoragePoolMaxWaitSeconds.key()), 3600);
+
+            VolumeApiResult result = future.get(storagePoolMaxWaitSeconds, TimeUnit.SECONDS);
 
             if (volumeDetail != null) {
                 volumeDetailsDao.remove(volumeDetail.getId());
@@ -426,14 +674,11 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
 
             if (result.isFailed()) {
                 LOGGER.warn("Failed to create a volume: " + result.getResult());
-
                 throw new CloudRuntimeException(result.getResult());
             }
 
             volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
-
             volumeInfo.processEvent(Event.MigrationRequested);
-
             volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
 
             copyCmdAnswer = performResignature(volumeInfo, hostVO);
@@ -441,12 +686,11 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
             if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
                 if (copyCmdAnswer != null && !StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
                     throw new CloudRuntimeException(copyCmdAnswer.getDetails());
-                }
+                } else {
-                else {
                     throw new CloudRuntimeException("Unable to create a volume from a template");
                 }
             }
-        } catch (InterruptedException | ExecutionException ex) {
+        } catch (InterruptedException | ExecutionException | TimeoutException ex ) {
             volumeInfo.getDataStore().getDriver().deleteAsync(volumeInfo.getDataStore(), volumeInfo, null);
 
             throw new CloudRuntimeException("Create volume from template (ID = " + templateInfo.getId() + ") failed " + ex.getMessage());
@@ -574,8 +818,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
         if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
             if (copyCmdAnswer != null && !StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
                 throw new CloudRuntimeException(copyCmdAnswer.getDetails());
-            }
+            } else {
-            else {
                 throw new CloudRuntimeException("Unable to create volume from snapshot");
             }
         }
@@ -802,14 +1045,45 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
         return new CopyCmdAnswer(newVolume);
     }
 
+    protected DataObject cacheSnapshotChain(SnapshotInfo snapshot, Scope scope) {
+        DataObject leafData = null;
+        DataStore store = cacheMgr.getCacheStorage(snapshot, scope);
+        while (snapshot != null) {
+            DataObject cacheData = cacheMgr.createCacheObject(snapshot, store);
+            if (leafData == null) {
+                leafData = cacheData;
+            }
+            snapshot = snapshot.getParent();
+        }
+        return leafData;
+    }
+
+    /**
+     * Copies data from secondary storage to a primary volume
+     * @param volumeInfo The primary volume
+     * @param snapshotInfo  destination of the copy
+     * @param hostVO the host used to copy the data
+     * @return result of the copy
+     */
     private CopyCmdAnswer performCopyOfVdi(VolumeInfo volumeInfo, SnapshotInfo snapshotInfo, HostVO hostVO) {
         String value = _configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
         int primaryStorageDownloadWait = NumbersUtil.parseInt(value, Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-        CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(), volumeInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
 
+        DataObject srcData = snapshotInfo;
         CopyCmdAnswer copyCmdAnswer = null;
+        DataObject cacheData = null;
+        boolean needCacheStorage = needCacheStorage(snapshotInfo, volumeInfo);
+
+        if (needCacheStorage) {
+            cacheData = cacheSnapshotChain(snapshotInfo, new ZoneScope(volumeInfo.getDataCenterId()));
+            srcData = cacheData;
+
+        }
+
+        CopyCommand copyCommand = new CopyCommand(srcData.getTO(), volumeInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
 
         try {
+
             _volumeService.grantAccess(snapshotInfo, hostVO, snapshotInfo.getDataStore());
             _volumeService.grantAccess(volumeInfo, hostVO, volumeInfo.getDataStore());
 
@@ -833,6 +1107,10 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
         finally {
             _volumeService.revokeAccess(snapshotInfo, hostVO, snapshotInfo.getDataStore());
             _volumeService.revokeAccess(volumeInfo, hostVO, volumeInfo.getDataStore());
+
+            if (needCacheStorage && copyCmdAnswer != null && copyCmdAnswer.getResult()) {
+                performCleanupCacheStorage(cacheData);
+            }
         }
 
         return copyCmdAnswer;

engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java

@@ -238,6 +238,9 @@ public class SnapshotObject implements SnapshotInfo {
         return snapshot.getRecurringType();
     }
 
+    @Override
+    public LocationType getLocationType() { return snapshot.getLocationType(); }
+
     @Override
     public State getState() {
         return snapshot.getState();

engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java

@@ -284,7 +284,7 @@ public class SnapshotServiceImpl implements SnapshotService {
             if (res.isFailed()) {
                 throw new CloudRuntimeException(res.getResult());
             }
-            SnapshotInfo destSnapshot = res.getSnashot();
+            SnapshotInfo destSnapshot = res.getSnapshot();
             return destSnapshot;
         } catch (InterruptedException e) {
             s_logger.debug("failed copy snapshot", e);

engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotStrategyBase.java

@@ -28,7 +28,7 @@ public abstract class SnapshotStrategyBase implements SnapshotStrategy {
 
     @Override
     public SnapshotInfo takeSnapshot(SnapshotInfo snapshot) {
-        return snapshotSvr.takeSnapshot(snapshot).getSnashot();
+        return snapshotSvr.takeSnapshot(snapshot).getSnapshot();
     }
 
     @Override

engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java

@@ -16,36 +16,6 @@
 // under the License.
 package org.apache.cloudstack.storage.snapshot;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Random;
-
-import javax.inject.Inject;
-
-import org.apache.log4j.Logger;
-import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
-import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult;
-import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
-import org.apache.cloudstack.storage.command.SnapshotAndCopyAnswer;
-import org.apache.cloudstack.storage.command.SnapshotAndCopyCommand;
-import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-
-import org.springframework.stereotype.Component;
-
-import com.google.common.base.Optional;
-
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.to.DiskTO;
 import com.cloud.dc.dao.ClusterDao;
@@ -57,6 +27,7 @@ import com.cloud.org.Cluster;
 import com.cloud.org.Grouping.AllocationState;
 import com.cloud.resource.ResourceState;
 import com.cloud.server.ManagementService;
+import com.cloud.storage.CreateSnapshotPayload;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.Snapshot;
 import com.cloud.storage.SnapshotVO;
@@ -72,6 +43,33 @@ import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.fsm.NoTransitionException;
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.dao.VMInstanceDao;
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
+import org.apache.cloudstack.storage.command.SnapshotAndCopyAnswer;
+import org.apache.cloudstack.storage.command.SnapshotAndCopyCommand;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import javax.inject.Inject;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
 
 @Component
 public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase {
@@ -92,7 +90,34 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase {
 
     @Override
     public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
-        return snapshotInfo;
+
+        Preconditions.checkArgument(snapshotInfo != null, "backupSnapshot expects a valid snapshot");
+
+        if (snapshotInfo.getLocationType() != Snapshot.LocationType.SECONDARY) {
+
+            markAsBackedUp((SnapshotObject)snapshotInfo);
+            return snapshotInfo;
+        }
+
+        // At this point the snapshot is either taken as a native
+        // snapshot on the storage or exists as a volume on the storage (clone).
+        // If archive flag is passed, we will copy this snapshot to secondary
+        // storage and delete it from the primary storage.
+
+        HostVO host = getHost(snapshotInfo.getVolumeId());
+        boolean canStorageSystemCreateVolumeFromSnapshot = canStorageSystemCreateVolumeFromSnapshot(snapshotInfo.getBaseVolume().getPoolId());
+        boolean computeClusterSupportsResign = clusterDao.getSupportsResigning(host.getClusterId());
+
+        if (!canStorageSystemCreateVolumeFromSnapshot || !computeClusterSupportsResign) {
+            String mesg = "Cannot archive snapshot: Either canStorageSystemCreateVolumeFromSnapshot or " +
+                    "computeClusterSupportsResign were false.  ";
+
+            s_logger.warn(mesg);
+            throw new CloudRuntimeException(mesg);
+        }
+
+        return snapshotSvr.backupSnapshot(snapshotInfo);
+
     }
 
     @Override
@@ -113,6 +138,19 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase {
             throw new InvalidParameterValueException("Unable to delete snapshotshot " + snapshotId + " because it is in the following state: " + snapshotVO.getState());
         }
 
+        return cleanupSnapshotOnPrimaryStore(snapshotId);
+    }
+
+    /**
+     * Cleans up a snapshot which was taken on a primary store. This function
+     * removes
+     *
+     * @param snapshotId: ID of snapshot that needs to be removed
+     * @return true if snapshot is removed, false otherwise
+     */
+
+    private boolean cleanupSnapshotOnPrimaryStore(long snapshotId) {
+
         SnapshotObject snapshotObj = (SnapshotObject)snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
 
         if (snapshotObj == null) {
@@ -153,7 +191,6 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase {
 
             return false;
         }
-
         return true;
     }
 
@@ -178,6 +215,8 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase {
         }
 
         SnapshotResult result = null;
+        SnapshotInfo snapshotOnPrimary = null;
+        SnapshotInfo backedUpSnapshot = null;
 
         try {
             volumeInfo.stateTransit(Volume.Event.SnapshotRequested);
@@ -212,20 +251,46 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase {
                 performSnapshotAndCopyOnHostSide(volumeInfo, snapshotInfo);
             }
 
-            markAsBackedUp((SnapshotObject)result.getSnashot());
+            snapshotOnPrimary = result.getSnapshot();
+            backedUpSnapshot = backupSnapshot(snapshotOnPrimary);
+
+            updateLocationTypeInDb(backedUpSnapshot);
+
         }
         finally {
             if (result != null && result.isSuccess()) {
                 volumeInfo.stateTransit(Volume.Event.OperationSucceeded);
-            }
+
-            else {
+                if (snapshotOnPrimary != null && snapshotInfo.getLocationType() == Snapshot.LocationType.SECONDARY) {
+                    // cleanup the snapshot on primary
+                    try {
+                        snapshotSvr.deleteSnapshot(snapshotOnPrimary);
+                    } catch (Exception e) {
+                        s_logger.warn("Failed to clean up snapshot on primary Id:" + snapshotOnPrimary.getId() + " "
+                                + e.getMessage());
+                    }
+                }
+            } else {
                 volumeInfo.stateTransit(Volume.Event.OperationFailed);
             }
-
-            snapshotDao.releaseFromLockTable(snapshotInfo.getId());
         }
 
-        return snapshotInfo;
+        snapshotDao.releaseFromLockTable(snapshotInfo.getId());
+        return backedUpSnapshot;
+    }
+
+    private void updateLocationTypeInDb(SnapshotInfo snapshotInfo) {
+        Object objPayload = snapshotInfo.getPayload();
+
+        if (objPayload instanceof CreateSnapshotPayload) {
+            CreateSnapshotPayload payload = (CreateSnapshotPayload)objPayload;
+
+            SnapshotVO snapshot = snapshotDao.findById(snapshotInfo.getId());
+
+            snapshot.setLocationType(payload.getLocationType());
+
+            snapshotDao.update(snapshotInfo.getId(), snapshot);
+        }
     }
 
     private boolean canStorageSystemCreateVolumeFromSnapshot(long storagePoolId) {
@@ -519,6 +584,13 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase {
 
         DataStore dataStore = dataStoreMgr.getDataStore(storagePoolId, DataStoreRole.Primary);
 
+        Snapshot.LocationType locationType = snapshot.getLocationType();
+
+        //If the snapshot exisits on Secondary Storage, we can't delete it.
+        if (SnapshotOperation.DELETE.equals(op) && Snapshot.LocationType.SECONDARY.equals(locationType)) {
+            return StrategyPriority.CANT_HANDLE;
+        }
+
         if (dataStore != null) {
             Map<String, String> mapCapabilities = dataStore.getDriver().getCapabilities();
 

engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java

@@ -290,7 +290,7 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
 
     @Override
     public boolean revertSnapshot(SnapshotInfo snapshot) {
-        if (canHandle(snapshot,SnapshotOperation.REVERT) == StrategyPriority.CANT_HANDLE) {
+        if (canHandle(snapshot, SnapshotOperation.REVERT) == StrategyPriority.CANT_HANDLE) {
             throw new CloudRuntimeException("Reverting not supported. Create a template or volume based on the snapshot instead.");
         }
 
@@ -372,7 +372,7 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
                 }
             }
 
-            snapshot = result.getSnashot();
+            snapshot = result.getSnapshot();
             DataStore primaryStore = snapshot.getDataStore();
 
             SnapshotInfo backupedSnapshot = backupSnapshot(snapshot);

engine/storage/src/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java

@@ -16,25 +16,6 @@
 // under the License.
 package org.apache.cloudstack.storage.image.db;
 
-import java.sql.PreparedStatement;
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.util.Date;
-import java.util.List;
-import java.util.Map;
-
-import javax.naming.ConfigurationException;
-
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
-
-import org.apache.cloudstack.engine.subsystem.api.storage.DataObjectInStore;
-import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
-import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
-import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.State;
-import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
-
 import com.cloud.storage.DataStoreRole;
 import com.cloud.utils.db.DB;
 import com.cloud.utils.db.GenericDaoBase;
@@ -43,6 +24,22 @@ import com.cloud.utils.db.SearchCriteria;
 import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.db.TransactionLegacy;
 import com.cloud.utils.db.UpdateBuilder;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataObjectInStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.State;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import javax.naming.ConfigurationException;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Date;
+import java.util.List;
+import java.util.Map;
 
 @Component
 public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO, Long> implements SnapshotDataStoreDao {
@@ -103,6 +100,7 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
         snapshotSearch = createSearchBuilder();
         snapshotSearch.and("snapshot_id", snapshotSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ);
         snapshotSearch.and("store_role", snapshotSearch.entity().getRole(), SearchCriteria.Op.EQ);
+        snapshotSearch.and("state", snapshotSearch.entity().getState(), SearchCriteria.Op.EQ);
         snapshotSearch.done();
 
         storeSnapshotSearch = createSearchBuilder();
@@ -294,6 +292,7 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
         SearchCriteria<SnapshotDataStoreVO> sc = snapshotSearch.create();
         sc.setParameters("snapshot_id", snapshotId);
         sc.setParameters("store_role", role);
+        sc.setParameters("state", State.Ready);
         return findOneBy(sc);
     }
 

engine/storage/src/org/apache/cloudstack/storage/snapshot/SnapshotEntityImpl.java

@@ -184,6 +184,11 @@ public class SnapshotEntityImpl implements SnapshotEntity {
         return null;
     }
 
+    @Override
+    public LocationType getLocationType() {
+        return null;
+    }
+
     @Override
     public Class<?> getEntityType() {
         return Snapshot.class;

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

@@ -16,50 +16,6 @@
 // under the License.
 package com.cloud.hypervisor.xenserver.resource;
 
-import java.io.BufferedReader;
-import java.io.BufferedWriter;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.InputStreamReader;
-import java.io.OutputStreamWriter;
-import java.net.MalformedURLException;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.net.URL;
-import java.net.URLConnection;
-import java.nio.charset.Charset;
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Properties;
-import java.util.Queue;
-import java.util.Random;
-import java.util.Set;
-import java.util.UUID;
-import java.util.concurrent.TimeoutException;
-
-import javax.naming.ConfigurationException;
-import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.ParserConfigurationException;
-
-import org.apache.cloudstack.storage.to.TemplateObjectTO;
-import org.apache.cloudstack.storage.to.VolumeObjectTO;
-import org.apache.commons.io.FileUtils;
-import org.apache.log4j.Logger;
-import org.apache.xmlrpc.XmlRpcException;
-import org.w3c.dom.Document;
-import org.w3c.dom.Node;
-import org.w3c.dom.NodeList;
-import org.xml.sax.InputSource;
-import org.xml.sax.SAXException;
-
 import com.cloud.agent.IAgentControl;
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.Command;
@@ -152,6 +108,48 @@ import com.xensource.xenapi.VIF;
 import com.xensource.xenapi.VLAN;
 import com.xensource.xenapi.VM;
 import com.xensource.xenapi.XenAPIObject;
+import org.apache.cloudstack.storage.to.TemplateObjectTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.commons.io.FileUtils;
+import org.apache.log4j.Logger;
+import org.apache.xmlrpc.XmlRpcException;
+import org.w3c.dom.Document;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+
+import javax.naming.ConfigurationException;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.net.URLConnection;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Queue;
+import java.util.Random;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.TimeoutException;
 
 /**
  * CitrixResourceBase encapsulates the calls to the XenServer Xapi process to
@@ -2288,11 +2286,22 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
 
     public SR getIscsiSR(final Connection conn, final String srNameLabel, final String target, String path, final String chapInitiatorUsername,
             final String chapInitiatorPassword, final boolean ignoreIntroduceException) {
-        return getIscsiSR(conn, srNameLabel, target, path, chapInitiatorUsername, chapInitiatorPassword, false, ignoreIntroduceException);
+
+        return getIscsiSR(conn, srNameLabel, target, path, chapInitiatorUsername,
+                chapInitiatorPassword, false, SRType.LVMOISCSI.toString(),
+                ignoreIntroduceException);
     }
 
     public SR getIscsiSR(final Connection conn, final String srNameLabel, final String target, String path, final String chapInitiatorUsername,
             final String chapInitiatorPassword, final boolean resignature, final boolean ignoreIntroduceException) {
+
+        return getIscsiSR(conn, srNameLabel, target, path, chapInitiatorUsername,
+                chapInitiatorPassword, resignature, SRType.LVMOISCSI.toString(),
+                ignoreIntroduceException);
+    }
+
+    public SR getIscsiSR(final Connection conn, final String srNameLabel, final String target, String path, final String chapInitiatorUsername,
+            final String chapInitiatorPassword, final boolean resignature, final  String srType, final boolean ignoreIntroduceException) {
         synchronized (srNameLabel.intern()) {
             final Map<String, String> deviceConfig = new HashMap<String, String>();
             try {
@@ -2310,9 +2319,141 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
                 final String lunid = tmp[2].trim();
                 String scsiid = "";
 
+                //Throws an exception if SR already exists and is attached
+                checkIfIscsiSrExisits(conn, srNameLabel, target, targetiqn, lunid);
+
+                // We now know the SR is not attached to the XenServer. We probe the
+                // LUN to see if an SR was already exists on it, if so, we just
+                // attach it or else we create a brand new SR
+
+                deviceConfig.put("target", target);
+                deviceConfig.put("targetIQN", targetiqn);
+
+                if (StringUtils.isNotBlank(chapInitiatorUsername) && StringUtils.isNotBlank(chapInitiatorPassword)) {
+                    deviceConfig.put("chapuser", chapInitiatorUsername);
+                    deviceConfig.put("chappassword", chapInitiatorPassword);
+                }
+
+                final Host host = Host.getByUuid(conn, _host.getUuid());
+                final Map<String, String> smConfig = new HashMap<String, String>();
+                SR sr = null;
+                String pooluuid = null;
+
+                if (SRType.LVMOISCSI.equals(srType)) {
+                    scsiid = probeScisiId(conn, host, deviceConfig, srType, srNameLabel, lunid, smConfig);
+                    deviceConfig.put("SCSIid", scsiid);
+
+                    String result = SR.probe(conn, host, deviceConfig, srType, smConfig);
+                    if (result.indexOf("<UUID>") != -1) {
+                        pooluuid = result.substring(result.indexOf("<UUID>") + 6, result.indexOf("</UUID>")).trim();
+                    }
+                }
+
+                if (pooluuid == null || pooluuid.length() != 36) {
+                    sr = SR.create(conn, host, deviceConfig, new Long(0), srNameLabel, srNameLabel, srType, "user", true, smConfig);
+                }
+                else {
+                    if (resignature) {
+                        // We resignature the SR for managed storage if needed. At the end of this
+                        // we have an SR which is ready to be attached. For VHDoISCSI SR,
+                        // we don't need to resignature
+                        pooluuid = resignatureIscsiSr(conn, host, deviceConfig, srNameLabel, smConfig);
+                    }
+                    sr = introduceAndPlugIscsiSr(conn, pooluuid, srNameLabel, srType, smConfig, deviceConfig, ignoreIntroduceException);
+                }
+
+                sr.scan(conn);
+                return sr;
+
+            } catch (final XenAPIException e) {
+                final String msg = "Unable to create Iscsi SR  " + deviceConfig + " due to  " + e.toString();
+                s_logger.warn(msg, e);
+                throw new CloudRuntimeException(msg, e);
+            } catch (final Exception e) {
+                final String msg = "Unable to create Iscsi SR  " + deviceConfig + " due to  " + e.getMessage();
+                s_logger.warn(msg, e);
+                throw new CloudRuntimeException(msg, e);
+            }
+        }
+    }
+
+    private SR introduceAndPlugIscsiSr(Connection conn, String pooluuid, String srNameLabel, String type, Map<String, String> smConfig, Map<String, String> deviceConfig, boolean ignoreIntroduceException) throws XmlRpcException, XenAPIException {
+        SR sr = null;
+        try {
+            sr = SR.introduce(conn, pooluuid, srNameLabel, srNameLabel, type, "user", true, smConfig);
+        } catch (final XenAPIException ex) {
+            if (ignoreIntroduceException) {
+                return sr;
+            }
+
+            throw ex;
+        }
+
+        final Set<Host> setHosts = Host.getAll(conn);
+
+        if (setHosts == null) {
+            final String msg = "Unable to create iSCSI SR " + deviceConfig + " due to hosts not available.";
+
+            s_logger.warn(msg);
+
+            throw new CloudRuntimeException(msg);
+        }
+
+        for (final Host currentHost : setHosts) {
+            final PBD.Record rec = new PBD.Record();
+
+            rec.deviceConfig = deviceConfig;
+            rec.host = currentHost;
+            rec.SR = sr;
+
+            final PBD pbd = PBD.create(conn, rec);
+
+            pbd.plug(conn);
+        }
+
+        return sr;
+    }
+
+    private String resignatureIscsiSr(Connection conn, Host host, Map<String, String> deviceConfig, String srNameLabel, Map<String, String> smConfig) throws XmlRpcException, XenAPIException {
+
+        String pooluuid;
+        try {
+            SR.create(conn, host, deviceConfig, new Long(0), srNameLabel, srNameLabel, SRType.RELVMOISCSI.toString(),
+                        "user", true, smConfig);
+
+            // The successful outcome of SR.create (right above) is to throw an exception of type XenAPIException (with expected
+            // toString() text) after resigning the metadata (we indicated to perform a resign by passing in SRType.RELVMOISCSI.toString()).
+            // That being the case, if this CloudRuntimeException statement is executed, there appears to have been some kind
+            // of failure in the execution of the above SR.create (resign) method.
+            throw new CloudRuntimeException("Problem resigning the metadata");
+        }
+        catch (XenAPIException ex) {
+            String msg = ex.toString();
+
+            if (!msg.contains("successfully resigned")) {
+                throw ex;
+            }
+
+            String type = SRType.LVMOISCSI.toString();
+            String result = SR.probe(conn, host, deviceConfig, type, smConfig);
+
+            pooluuid = null;
+
+            if (result.indexOf("<UUID>") != -1) {
+                pooluuid = result.substring(result.indexOf("<UUID>") + 6, result.indexOf("</UUID>")).trim();
+            }
+
+            if (pooluuid == null || pooluuid.length() != 36) {
+                throw new CloudRuntimeException("Non-existent or invalid SR UUID");
+            }
+        }
+        return pooluuid;
+    }
+
+    private void checkIfIscsiSrExisits(Connection conn, String srNameLabel, String target, String targetiqn, String lunid) throws XenAPIException, XmlRpcException {
                 final Set<SR> srs = SR.getByNameLabel(conn, srNameLabel);
                 for (final SR sr : srs) {
-                    if (!SRType.LVMOISCSI.equals(sr.getType(conn))) {
+                    if (!(SRType.LVMOISCSI.equals(sr.getType(conn)))) {
                         continue;
                     }
                     final Set<PBD> pbds = sr.getPBDs(conn);
@@ -2338,142 +2479,46 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
                                 + ", lunid:" + dc.get("lunid") + " for pool " + srNameLabel + "on host:" + _host.getUuid());
                     }
                 }
-                deviceConfig.put("target", target);
-                deviceConfig.put("targetIQN", targetiqn);
 
-                if (StringUtils.isNotBlank(chapInitiatorUsername) && StringUtils.isNotBlank(chapInitiatorPassword)) {
+    }
-                    deviceConfig.put("chapuser", chapInitiatorUsername);
-                    deviceConfig.put("chappassword", chapInitiatorPassword);
-                }
 
-                final Host host = Host.getByUuid(conn, _host.getUuid());
+    private String probeScisiId(Connection conn, Host host, Map<String, String> deviceConfig, String type, String srNameLabel, String lunid, Map<String, String> smConfig) throws XenAPIException, XmlRpcException {
-                final Map<String, String> smConfig = new HashMap<String, String>();
+        SR sr = null;
-                final String type = SRType.LVMOISCSI.toString();
+        String scsiid = null;
-                SR sr = null;
+
-                try {
+        try {
-                    sr = SR.create(conn, host, deviceConfig, new Long(0), srNameLabel, srNameLabel, type, "user", true, smConfig);
+            sr = SR.create(conn, host, deviceConfig, new Long(0), srNameLabel, srNameLabel, type, "user", true, smConfig);
-                } catch (final XenAPIException e) {
+        } catch (final XenAPIException e) {
-                    final String errmsg = e.toString();
+            final String errmsg = e.toString();
-                    if (errmsg.contains("SR_BACKEND_FAILURE_107")) {
+            if (errmsg.contains("SR_BACKEND_FAILURE_107")) {
-                        final String lun[] = errmsg.split("<LUN>");
+                final String lun[] = errmsg.split("<LUN>");
-                        boolean found = false;
+                boolean found = false;
-                        for (int i = 1; i < lun.length; i++) {
+                for (int i = 1; i < lun.length; i++) {
-                            final int blunindex = lun[i].indexOf("<LUNid>") + 7;
+                    final int blunindex = lun[i].indexOf("<LUNid>") + 7;
-                            final int elunindex = lun[i].indexOf("</LUNid>");
+                    final int elunindex = lun[i].indexOf("</LUNid>");
-                            String ilun = lun[i].substring(blunindex, elunindex);
+                    String ilun = lun[i].substring(blunindex, elunindex);
-                            ilun = ilun.trim();
+                    ilun = ilun.trim();
-                            if (ilun.equals(lunid)) {
+                    if (ilun.equals(lunid)) {
-                                final int bscsiindex = lun[i].indexOf("<SCSIid>") + 8;
+                        final int bscsiindex = lun[i].indexOf("<SCSIid>") + 8;
-                                final int escsiindex = lun[i].indexOf("</SCSIid>");
+                        final int escsiindex = lun[i].indexOf("</SCSIid>");
-                                scsiid = lun[i].substring(bscsiindex, escsiindex);
+                        scsiid = lun[i].substring(bscsiindex, escsiindex);
-                                scsiid = scsiid.trim();
+                        scsiid = scsiid.trim();
-                                found = true;
+                        found = true;
-                                break;
+                        break;
-                            }
-                        }
-                        if (!found) {
-                            final String msg = "can not find LUN " + lunid + " in " + errmsg;
-                            s_logger.warn(msg);
-                            throw new CloudRuntimeException(msg);
-                        }
-                    } else {
-                        final String msg = "Unable to create Iscsi SR  " + deviceConfig + " due to  " + e.toString();
-                        s_logger.warn(msg, e);
-                        throw new CloudRuntimeException(msg, e);
                     }
                 }
-
+                if (!found) {
-                deviceConfig.put("SCSIid", scsiid);
+                    final String msg = "can not find LUN " + lunid + " in " + errmsg;
-
+                    s_logger.warn(msg);
-                String result = SR.probe(conn, host, deviceConfig, type, smConfig);
+                    throw new CloudRuntimeException(msg);
-
-                String pooluuid = null;
-
-                if (result.indexOf("<UUID>") != -1) {
-                    pooluuid = result.substring(result.indexOf("<UUID>") + 6, result.indexOf("</UUID>")).trim();
                 }
-
+            } else {
-                if (pooluuid == null || pooluuid.length() != 36) {
-                    sr = SR.create(conn, host, deviceConfig, new Long(0), srNameLabel, srNameLabel, type, "user", true, smConfig);
-                }
-                else {
-                    if (resignature) {
-                        try {
-                            SR.create(conn, host, deviceConfig, new Long(0), srNameLabel, srNameLabel, SRType.RELVMOISCSI.toString(), "user", true, smConfig);
-
-                            // The successful outcome of SR.create (right above) is to throw an exception of type XenAPIException (with expected
-                            // toString() text) after resigning the metadata (we indicated to perform a resign by passing in SRType.RELVMOISCSI.toString()).
-                            // That being the case, if this CloudRuntimeException statement is executed, there appears to have been some kind
-                            // of failure in the execution of the above SR.create (resign) method.
-                            throw new CloudRuntimeException("Problem resigning the metadata");
-                        }
-                        catch (XenAPIException ex) {
-                            String msg = ex.toString();
-
-                            if (!msg.contains("successfully resigned")) {
-                                throw ex;
-                            }
-
-                            result = SR.probe(conn, host, deviceConfig, type, smConfig);
-
-                            pooluuid = null;
-
-                            if (result.indexOf("<UUID>") != -1) {
-                                pooluuid = result.substring(result.indexOf("<UUID>") + 6, result.indexOf("</UUID>")).trim();
-                            }
-
-                            if (pooluuid == null || pooluuid.length() != 36) {
-                                throw new CloudRuntimeException("Non-existent or invalid SR UUID");
-                            }
-                        }
-                    }
-
-                    try {
-                        sr = SR.introduce(conn, pooluuid, srNameLabel, srNameLabel, type, "user", true, smConfig);
-                    } catch (final XenAPIException ex) {
-                        if (ignoreIntroduceException) {
-                            return sr;
-                        }
-
-                        throw ex;
-                    }
-
-                    final Set<Host> setHosts = Host.getAll(conn);
-
-                    if (setHosts == null) {
-                        final String msg = "Unable to create iSCSI SR " + deviceConfig + " due to hosts not available.";
-
-                        s_logger.warn(msg);
-
-                        throw new CloudRuntimeException(msg);
-                    }
-
-                    for (final Host currentHost : setHosts) {
-                        final PBD.Record rec = new PBD.Record();
-
-                        rec.deviceConfig = deviceConfig;
-                        rec.host = currentHost;
-                        rec.SR = sr;
-
-                        final PBD pbd = PBD.create(conn, rec);
-
-                        pbd.plug(conn);
-                    }
-                }
-
-                sr.scan(conn);
-
-                return sr;
-            } catch (final XenAPIException e) {
                 final String msg = "Unable to create Iscsi SR  " + deviceConfig + " due to  " + e.toString();
                 s_logger.warn(msg, e);
                 throw new CloudRuntimeException(msg, e);
-            } catch (final Exception e) {
-                final String msg = "Unable to create Iscsi SR  " + deviceConfig + " due to  " + e.getMessage();
-                s_logger.warn(msg, e);
-                throw new CloudRuntimeException(msg, e);
             }
         }
+        return scsiid;
     }
 
     public SR getISOSRbyVmName(final Connection conn, final String vmName) {
@@ -4082,7 +4127,8 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
 
             return getNfsSR(conn, poolid, namelable, storageHost, mountpoint, volumedesc);
         } else {
-            return getIscsiSR(conn, iScsiName, storageHost, iScsiName, chapInitiatorUsername, chapInitiatorSecret, true);
+            return getIscsiSR(conn, iScsiName, storageHost, iScsiName,
+                    chapInitiatorUsername, chapInitiatorSecret, false, SRType.LVMOISCSI.toString(), true);
         }
     }
 

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

@@ -18,18 +18,38 @@
  */
 package com.cloud.hypervisor.xenserver.resource;
 
-import static com.cloud.utils.ReflectUtil.flattenProperties;
+import com.cloud.agent.api.Answer;
-import static com.google.common.collect.Lists.newArrayList;
+import com.cloud.agent.api.to.DataObjectType;
-
+import com.cloud.agent.api.to.DataStoreTO;
-import java.io.File;
+import com.cloud.agent.api.to.DataTO;
-import java.net.URI;
+import com.cloud.agent.api.to.DiskTO;
-import java.util.Arrays;
+import com.cloud.agent.api.to.NfsTO;
-import java.util.HashMap;
+import com.cloud.agent.api.to.S3TO;
-import java.util.List;
+import com.cloud.agent.api.to.StorageFilerTO;
-import java.util.Map;
+import com.cloud.agent.api.to.SwiftTO;
-import java.util.Set;
+import com.cloud.exception.InternalErrorException;
-import java.util.UUID;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
-
+import com.cloud.hypervisor.xenserver.resource.CitrixResourceBase.SRType;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Storage;
+import com.cloud.storage.Storage.ImageFormat;
+import com.cloud.storage.resource.StorageProcessor;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.storage.S3.ClientOptions;
+import com.cloud.utils.storage.encoding.DecodedDataObject;
+import com.cloud.utils.storage.encoding.DecodedDataStore;
+import com.cloud.utils.storage.encoding.Decoder;
+import com.xensource.xenapi.Connection;
+import com.xensource.xenapi.Host;
+import com.xensource.xenapi.PBD;
+import com.xensource.xenapi.SR;
+import com.xensource.xenapi.Types;
+import com.xensource.xenapi.Types.BadServerResponse;
+import com.xensource.xenapi.Types.VmPowerState;
+import com.xensource.xenapi.Types.XenAPIException;
+import com.xensource.xenapi.VBD;
+import com.xensource.xenapi.VDI;
+import com.xensource.xenapi.VM;
 import org.apache.cloudstack.storage.command.AttachAnswer;
 import org.apache.cloudstack.storage.command.AttachCommand;
 import org.apache.cloudstack.storage.command.AttachPrimaryDataStoreAnswer;
@@ -56,38 +76,17 @@ import org.apache.cloudstack.storage.to.VolumeObjectTO;
 import org.apache.log4j.Logger;
 import org.apache.xmlrpc.XmlRpcException;
 
-import com.cloud.agent.api.Answer;
+import java.io.File;
-import com.cloud.agent.api.to.DataObjectType;
+import java.net.URI;
-import com.cloud.agent.api.to.DataStoreTO;
+import java.util.Arrays;
-import com.cloud.agent.api.to.DataTO;
+import java.util.HashMap;
-import com.cloud.agent.api.to.DiskTO;
+import java.util.List;
-import com.cloud.agent.api.to.NfsTO;
+import java.util.Map;
-import com.cloud.agent.api.to.S3TO;
+import java.util.Set;
-import com.cloud.agent.api.to.StorageFilerTO;
+import java.util.UUID;
-import com.cloud.agent.api.to.SwiftTO;
+
-import com.cloud.exception.InternalErrorException;
+import static com.cloud.utils.ReflectUtil.flattenProperties;
-import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import static com.google.common.collect.Lists.newArrayList;
-import com.cloud.hypervisor.xenserver.resource.CitrixResourceBase.SRType;
-import com.cloud.storage.DataStoreRole;
-import com.cloud.storage.Storage;
-import com.cloud.storage.Storage.ImageFormat;
-import com.cloud.storage.resource.StorageProcessor;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.storage.encoding.DecodedDataObject;
-import com.cloud.utils.storage.encoding.DecodedDataStore;
-import com.cloud.utils.storage.encoding.Decoder;
-import com.cloud.utils.storage.S3.ClientOptions;
-import com.xensource.xenapi.Connection;
-import com.xensource.xenapi.Host;
-import com.xensource.xenapi.PBD;
-import com.xensource.xenapi.SR;
-import com.xensource.xenapi.Types;
-import com.xensource.xenapi.Types.BadServerResponse;
-import com.xensource.xenapi.Types.VmPowerState;
-import com.xensource.xenapi.Types.XenAPIException;
-import com.xensource.xenapi.VBD;
-import com.xensource.xenapi.VDI;
-import com.xensource.xenapi.VM;
 
 public class XenServerStorageProcessor implements StorageProcessor {
     private static final Logger s_logger = Logger.getLogger(XenServerStorageProcessor.class);

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

@@ -18,25 +18,6 @@
  */
 package com.cloud.hypervisor.xenserver.resource;
 
-import java.io.File;
-import java.net.URI;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.UUID;
-
-import org.apache.cloudstack.storage.command.CopyCmdAnswer;
-import org.apache.cloudstack.storage.command.CopyCommand;
-import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
-import org.apache.cloudstack.storage.to.SnapshotObjectTO;
-import org.apache.cloudstack.storage.to.TemplateObjectTO;
-import org.apache.cloudstack.storage.to.VolumeObjectTO;
-import org.apache.commons.lang.StringUtils;
-import org.apache.log4j.Logger;
-import org.apache.xmlrpc.XmlRpcException;
-
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.to.DataObjectType;
 import com.cloud.agent.api.to.DataStoreTO;
@@ -58,6 +39,24 @@ import com.xensource.xenapi.Types;
 import com.xensource.xenapi.Types.BadServerResponse;
 import com.xensource.xenapi.Types.XenAPIException;
 import com.xensource.xenapi.VDI;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.command.CopyCommand;
+import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+import org.apache.cloudstack.storage.to.TemplateObjectTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+import org.apache.xmlrpc.XmlRpcException;
+
+import java.io.File;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
 
 public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
     private static final Logger s_logger = Logger.getLogger(XenServerStorageProcessor.class);
@@ -424,7 +423,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
 
         final SnapshotObjectTO snapshotTO = (SnapshotObjectTO) srcData;
         final SnapshotObjectTO snapshotOnImage = (SnapshotObjectTO) destData;
-        final String snapshotUuid = snapshotTO.getPath();
+        String snapshotUuid = snapshotTO.getPath();
 
         final String prevBackupUuid = snapshotOnImage.getParentSnapshotPath();
         final String prevSnapshotUuid = snapshotTO.getParentSnapshotPath();
@@ -432,10 +431,35 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
         // By default assume failure
         String details = null;
         String snapshotBackupUuid = null;
-        final boolean fullbackup = Boolean.parseBoolean(options.get("fullSnapshot"));
+        boolean fullbackup = Boolean.parseBoolean(options.get("fullSnapshot"));
         Long physicalSize = null;
         try {
-            final SR primaryStorageSR = hypervisorResource.getSRByNameLabelandHost(conn, primaryStorageNameLabel);
+
+            SR primaryStorageSR = null;
+            if (primaryStore.isManaged()) {
+                fullbackup = true; // currently, managed storage only supports full backup
+
+                final Map<String, String> srcDetails = cmd.getOptions();
+
+                final String iScsiName = srcDetails.get(DiskTO.IQN);
+                final String storageHost = srcDetails.get(DiskTO.STORAGE_HOST);
+                final String chapInitiatorUsername = srcDetails.get(DiskTO.CHAP_INITIATOR_USERNAME);
+                final String chapInitiatorSecret = srcDetails.get(DiskTO.CHAP_INITIATOR_SECRET);
+                final String srType = CitrixResourceBase.SRType.LVMOISCSI.toString();
+
+                primaryStorageSR = hypervisorResource.getIscsiSR(conn, iScsiName, storageHost, iScsiName,
+                        chapInitiatorUsername, chapInitiatorSecret, false, srType, true);
+
+                final VDI srcVdi = primaryStorageSR.getVDIs(conn).iterator().next();
+                if (srcVdi == null) {
+                    throw new InternalErrorException("Could not Find a VDI on the SR: " + primaryStorageSR.getNameLabel(conn));
+                }
+                snapshotUuid = srcVdi.getUuid(conn);
+
+            } else {
+                primaryStorageSR = hypervisorResource.getSRByNameLabelandHost(conn, primaryStorageNameLabel);
+            }
+
             if (primaryStorageSR == null) {
                 throw new InternalErrorException("Could not backup snapshot because the primary Storage SR could not be created from the name label: " + primaryStorageNameLabel);
             }
@@ -443,7 +467,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
             final Boolean isISCSI = IsISCSI(primaryStorageSR.getType(conn));
 
             final VDI snapshotVdi = getVDIbyUuid(conn, snapshotUuid);
-            final String snapshotPaUuid = null;
+            final String snapshotPaUuid = snapshotVdi.getUuid(conn);
 
             final URI uri = new URI(secondaryStorageUrl);
             final String secondaryStorageMountPath = uri.getHost() + ":" + uri.getPath();
@@ -505,7 +529,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
                         // finalPath = folder + File.separator +
                         // snapshotBackupUuid;
                     } else {
-                        finalPath = folder + File.separator + snapshotBackupUuid;
+                        finalPath = folder + File.separator + snapshotBackupUuid + ".vhd";
                     }
 
                 } finally {
@@ -538,7 +562,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
                     final String[] tmp = result.split("#");
                     snapshotBackupUuid = tmp[0];
                     physicalSize = Long.parseLong(tmp[1]);
-                    finalPath = folder + File.separator + snapshotBackupUuid;
+                    finalPath = folder + File.separator + snapshotBackupUuid + ".vhd";
                 }
             }
             final String volumeUuid = snapshotTO.getVolume().getPath();
@@ -696,11 +720,29 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
         SR srcSr = null;
         VDI destVdi = null;
         try {
-            final SR primaryStorageSR = hypervisorResource.getSRByNameLabelandHost(conn, primaryStorageNameLabel);
+            SR primaryStorageSR;
+
+            if (pool.isManaged()) {
+                Map<String,String> destDetails = cmd.getOptions2();
+
+                final String iScsiName = destDetails.get(DiskTO.IQN);
+                final String storageHost = destDetails.get(DiskTO.STORAGE_HOST);
+                final String chapInitiatorUsername = destDetails.get(DiskTO.CHAP_INITIATOR_USERNAME);
+                final String chapInitiatorSecret = destDetails.get(DiskTO.CHAP_INITIATOR_SECRET);
+                final String srType = CitrixResourceBase.SRType.LVMOISCSI.toString();
+
+                primaryStorageSR = hypervisorResource.getIscsiSR(conn, iScsiName, storageHost, iScsiName,
+                        chapInitiatorUsername, chapInitiatorSecret, false, srType, true);
+
+            } else {
+                primaryStorageSR = hypervisorResource.getSRByNameLabelandHost(conn, primaryStorageNameLabel);
+            }
+
             if (primaryStorageSR == null) {
                 throw new InternalErrorException("Could not create volume from snapshot because the primary Storage SR could not be created from the name label: "
                         + primaryStorageNameLabel);
             }
+
             final String nameLabel = "cloud-" + UUID.randomUUID().toString();
             destVdi = createVdi(conn, nameLabel, primaryStorageSR, volume.getSize());
             volumeUUID = destVdi.getUuid(conn);
@@ -1041,11 +1083,12 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
         }
 
         NfsTO destStore = null;
+        PrimaryDataStoreTO srcStore = null;
         URI destUri = null;
 
         try {
+            srcStore = (PrimaryDataStoreTO) snapshotObjTO.getDataStore();
             destStore = (NfsTO) templateObjTO.getDataStore();
-
             destUri = new URI(destStore.getUrl());
         } catch (final Exception ex) {
             s_logger.debug("Invalid URI", ex);
@@ -1068,8 +1111,11 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor {
             final String storageHost = srcDetails.get(DiskTO.STORAGE_HOST);
             final String chapInitiatorUsername = srcDetails.get(DiskTO.CHAP_INITIATOR_USERNAME);
             final String chapInitiatorSecret = srcDetails.get(DiskTO.CHAP_INITIATOR_SECRET);
+            String srType = null;
 
-            srcSr = hypervisorResource.getIscsiSR(conn, iScsiName, storageHost, iScsiName, chapInitiatorUsername, chapInitiatorSecret, true);
+            srType = CitrixResourceBase.SRType.LVMOISCSI.toString();
+
+            srcSr = hypervisorResource.getIscsiSR(conn, iScsiName, storageHost, iScsiName, chapInitiatorUsername, chapInitiatorSecret, false, srType, true);
 
             final String destNfsPath = destUri.getHost() + ":" + destUri.getPath();
             final String localDir = "/var/cloud_mount/" + UUID.nameUUIDFromBytes(destNfsPath.getBytes());

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixResizeVolumeCommandWrapper.java

@@ -77,7 +77,7 @@ public final class CitrixResizeVolumeCommandWrapper extends CommandWrapper<Resiz
             Set<PBD> allPbds = new HashSet<>();
 
             for (SR sr : srs) {
-                if (!CitrixResourceBase.SRType.LVMOISCSI.equals(sr.getType(conn))) {
+                if (!(CitrixResourceBase.SRType.LVMOISCSI.equals(sr.getType(conn))))  {
                     continue;
                 }
 

server/src/com/cloud/api/ApiDBUtils.java

@@ -883,6 +883,12 @@ public class ApiDBUtils {
         return snapshot.getRecurringType().name();
     }
 
+    public static String getSnapshotLocationType(long snapshotId) {
+        SnapshotVO snapshot = s_snapshotDao.findById(snapshotId);
+
+        return snapshot.getLocationType() != null ? snapshot.getLocationType().name() : null;
+    }
+
     public static String getStoragePoolTags(long poolId) {
         return s_storageMgr.getStoragePoolTags(poolId);
     }

server/src/com/cloud/api/ApiResponseHelper.java

@@ -486,6 +486,7 @@ public class ApiResponseHelper implements ResponseGenerator {
         snapshotResponse.setName(snapshot.getName());
         snapshotResponse.setIntervalType(ApiDBUtils.getSnapshotIntervalTypes(snapshot.getId()));
         snapshotResponse.setState(snapshot.getState());
+        snapshotResponse.setLocationType(ApiDBUtils.getSnapshotLocationType(snapshot.getId()));
 
         SnapshotInfo snapshotInfo = null;
 

server/src/com/cloud/configuration/Config.java

@@ -16,15 +16,6 @@
 // under the License.
 package com.cloud.configuration;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.StringTokenizer;
-
-import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
-import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
-import org.apache.cloudstack.framework.config.ConfigKey;
-
 import com.cloud.agent.AgentManager;
 import com.cloud.consoleproxy.ConsoleProxyManager;
 import com.cloud.ha.HighAvailabilityManager;
@@ -38,6 +29,14 @@ import com.cloud.storage.snapshot.SnapshotManager;
 import com.cloud.template.TemplateManager;
 import com.cloud.vm.UserVmManager;
 import com.cloud.vm.snapshot.VMSnapshotManager;
+import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
+import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
+import org.apache.cloudstack.framework.config.ConfigKey;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.StringTokenizer;
 
 public enum Config {
 

server/src/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -219,6 +219,7 @@ import com.cloud.vm.dao.NicIpAliasDao;
 import com.cloud.vm.dao.NicIpAliasVO;
 import com.cloud.vm.dao.NicSecondaryIpDao;
 import com.cloud.vm.dao.VMInstanceDao;
+
 import com.google.common.base.Preconditions;
 
 public class ConfigurationManagerImpl extends ManagerBase implements ConfigurationManager, ConfigurationService, Configurable {

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

@@ -23,6 +23,7 @@ public class CreateSnapshotPayload {
     private Long snapshotId;
     private Account account;
     private boolean quiescevm;
+    private Snapshot.LocationType locationType;
 
     public Long getSnapshotPolicyId() {
         return snapshotPolicyId;
@@ -48,12 +49,13 @@ public class CreateSnapshotPayload {
         this.account = account;
     }
 
-    public void setQuiescevm(boolean quiescevm) {
+    public void setQuiescevm(boolean quiescevm) { this.quiescevm = quiescevm; }
-        this.quiescevm = quiescevm;
-    }
 
     public boolean getQuiescevm() {
         return this.quiescevm;
     }
 
+    public Snapshot.LocationType getLocationType() { return this.locationType; }
+
+    public void setLocationType(Snapshot.LocationType locationType) { this.locationType = locationType; }
 }

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

@@ -16,76 +16,6 @@
 // under the License.
 package com.cloud.storage;
 
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
-import java.util.concurrent.ExecutionException;
-
-import javax.inject.Inject;
-
-import com.cloud.utils.EncryptionUtil;
-import com.cloud.utils.db.TransactionCallbackWithException;
-import com.google.common.base.Strings;
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
-import com.google.gson.JsonParseException;
-
-import org.apache.cloudstack.api.command.user.volume.GetUploadParamsForVolumeCmd;
-import org.apache.cloudstack.api.response.GetUploadParamsResponse;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
-import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
-import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
-import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
-import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
-import org.apache.log4j.Logger;
-import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.ExtractVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.UploadVolumeCmd;
-import org.apache.cloudstack.context.CallContext;
-import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
-import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
-import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
-import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
-import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService.VolumeApiResult;
-import org.apache.cloudstack.framework.async.AsyncCallFuture;
-import org.apache.cloudstack.framework.config.ConfigKey;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.framework.jobs.AsyncJob;
-import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext;
-import org.apache.cloudstack.framework.jobs.AsyncJobManager;
-import org.apache.cloudstack.framework.jobs.Outcome;
-import org.apache.cloudstack.framework.jobs.dao.VmWorkJobDao;
-import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
-import org.apache.cloudstack.framework.jobs.impl.OutcomeImpl;
-import org.apache.cloudstack.framework.jobs.impl.VmWorkJobVO;
-import org.apache.cloudstack.jobs.JobInfo;
-import org.apache.cloudstack.storage.command.AttachAnswer;
-import org.apache.cloudstack.storage.command.AttachCommand;
-import org.apache.cloudstack.storage.command.DettachCommand;
-import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
-import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
-import org.apache.cloudstack.utils.identity.ManagementServerNode;
-
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.to.DataTO;
@@ -134,6 +64,7 @@ import com.cloud.user.VmDiskStatisticsVO;
 import com.cloud.user.dao.AccountDao;
 import com.cloud.user.dao.VmDiskStatisticsDao;
 import com.cloud.utils.DateUtil;
+import com.cloud.utils.EncryptionUtil;
 import com.cloud.utils.EnumUtils;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.Pair;
@@ -146,6 +77,7 @@ import com.cloud.utils.db.DB;
 import com.cloud.utils.db.EntityManager;
 import com.cloud.utils.db.Transaction;
 import com.cloud.utils.db.TransactionCallback;
+import com.cloud.utils.db.TransactionCallbackWithException;
 import com.cloud.utils.db.TransactionStatus;
 import com.cloud.utils.db.UUIDManager;
 import com.cloud.utils.exception.CloudRuntimeException;
@@ -172,10 +104,74 @@ import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.VMInstanceDao;
 import com.cloud.vm.snapshot.VMSnapshotVO;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
-
+import com.google.common.base.Strings;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.gson.JsonParseException;
+import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.ExtractVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.GetUploadParamsForVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.UploadVolumeCmd;
+import org.apache.cloudstack.api.response.GetUploadParamsResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
+import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
+import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
+import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService.VolumeApiResult;
+import org.apache.cloudstack.framework.async.AsyncCallFuture;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.jobs.AsyncJob;
+import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext;
+import org.apache.cloudstack.framework.jobs.AsyncJobManager;
+import org.apache.cloudstack.framework.jobs.Outcome;
+import org.apache.cloudstack.framework.jobs.dao.VmWorkJobDao;
+import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
+import org.apache.cloudstack.framework.jobs.impl.OutcomeImpl;
+import org.apache.cloudstack.framework.jobs.impl.VmWorkJobVO;
+import org.apache.cloudstack.jobs.JobInfo;
+import org.apache.cloudstack.storage.command.AttachAnswer;
+import org.apache.cloudstack.storage.command.AttachCommand;
+import org.apache.cloudstack.storage.command.DettachCommand;
+import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
+import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
+import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
+import org.apache.log4j.Logger;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 
+import javax.inject.Inject;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.ExecutionException;
+
 public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiService, VmWorkJobHandler {
     private final static Logger s_logger = Logger.getLogger(VolumeApiServiceImpl.class);
     public static final String VM_WORK_JOB_HANDLER = VolumeApiServiceImpl.class.getSimpleName();
@@ -219,7 +215,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     @Inject
     AccountDao _accountDao;
     @Inject
-    final DataCenterDao _dcDao = null;
+    DataCenterDao _dcDao = null;
     @Inject
     VMTemplateDao _templateDao;
     @Inject
@@ -481,7 +477,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 // decrement it
                 _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.volume);
                 //url can be null incase of postupload
-                if(url!=null) {
+                if (url != null) {
                     _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.secondary_storage, UriUtils.getRemoteSize(url));
                 }
 
@@ -2047,8 +2043,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_SNAPSHOT_CREATE, eventDescription = "taking snapshot", async = true)
-    public Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm) throws ResourceAllocationException {
+    public Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm, Snapshot.LocationType locationType)
-
+            throws ResourceAllocationException {
         VolumeInfo volume = volFactory.getVolume(volumeId);
         if (volume == null) {
             throw new InvalidParameterValueException("Creating snapshot failed due to volume:" + volumeId + " doesn't exist");
@@ -2058,6 +2054,11 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
             throw new InvalidParameterValueException("VolumeId: " + volumeId + " is not in " + Volume.State.Ready + " state but " + volume.getState() + ". Cannot take snapshot.");
         }
 
+        StoragePoolVO storagePoolVO = _storagePoolDao.findById(volume.getPoolId());
+        if (storagePoolVO.isManaged() && locationType == null) {
+            locationType = Snapshot.LocationType.PRIMARY;
+        }
+
         VMInstanceVO vm = null;
         if (volume.getInstanceId() != null)
             vm = _vmInstanceDao.findById(volume.getInstanceId());
@@ -2071,13 +2072,13 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 VmWorkJobVO placeHolder = null;
                 placeHolder = createPlaceHolderWork(vm.getId());
                 try {
-                    return orchestrateTakeVolumeSnapshot(volumeId, policyId, snapshotId, account, quiescevm);
+                    return orchestrateTakeVolumeSnapshot(volumeId, policyId, snapshotId, account, quiescevm, locationType);
                 } finally {
                     _workJobDao.expunge(placeHolder.getId());
                 }
 
             } else {
-                Outcome<Snapshot> outcome = takeVolumeSnapshotThroughJobQueue(vm.getId(), volumeId, policyId, snapshotId, account.getId(), quiescevm);
+                Outcome<Snapshot> outcome = takeVolumeSnapshotThroughJobQueue(vm.getId(), volumeId, policyId, snapshotId, account.getId(), quiescevm, locationType);
 
                 try {
                     outcome.get();
@@ -2110,7 +2111,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         }
     }
 
-    private Snapshot orchestrateTakeVolumeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm)
+    private Snapshot orchestrateTakeVolumeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account,
+                                                   boolean quiescevm, Snapshot.LocationType locationType)
             throws ResourceAllocationException {
 
         VolumeInfo volume = volFactory.getVolume(volumeId);
@@ -2124,17 +2126,21 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         }
 
         CreateSnapshotPayload payload = new CreateSnapshotPayload();
+
         payload.setSnapshotId(snapshotId);
         payload.setSnapshotPolicyId(policyId);
         payload.setAccount(account);
         payload.setQuiescevm(quiescevm);
+        payload.setLocationType(locationType);
+
         volume.addPayload(payload);
+
         return volService.takeSnapshot(volume);
     }
 
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_SNAPSHOT_CREATE, eventDescription = "allocating snapshot", create = true)
-    public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName) throws ResourceAllocationException {
+    public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType) throws ResourceAllocationException {
         Account caller = CallContext.current().getCallingAccount();
 
         VolumeInfo volume = volFactory.getVolume(volumeId);
@@ -2172,12 +2178,21 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
             }
         }
 
+        StoragePoolVO storagePoolVO = _storagePoolDao.findById(volume.getPoolId());
+        if (!storagePoolVO.isManaged() && locationType != null) {
+            throw new InvalidParameterValueException("VolumeId: " + volumeId + " LocationType is supported only for managed storage");
+        }
+
+        if (storagePoolVO.isManaged() && locationType == null) {
+            locationType = Snapshot.LocationType.PRIMARY;
+        }
+
         StoragePool storagePool = (StoragePool)volume.getDataStore();
         if (storagePool == null) {
             throw new InvalidParameterValueException("VolumeId: " + volumeId + " please attach this volume to a VM before create snapshot for it");
         }
 
-        return snapshotMgr.allocSnapshot(volumeId, policyId, snapshotName);
+        return snapshotMgr.allocSnapshot(volumeId, policyId, snapshotName, locationType);
     }
 
     @Override
@@ -2851,7 +2866,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     }
 
     public Outcome<Snapshot> takeVolumeSnapshotThroughJobQueue(final Long vmId, final Long volumeId,
-            final Long policyId, final Long snapshotId, final Long accountId, final boolean quiesceVm) {
+            final Long policyId, final Long snapshotId, final Long accountId, final boolean quiesceVm, final Snapshot.LocationType locationType) {
 
         final CallContext context = CallContext.current();
         final User callingUser = context.getCallingUser();
@@ -2874,7 +2889,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         // save work context info (there are some duplications)
         VmWorkTakeVolumeSnapshot workInfo = new VmWorkTakeVolumeSnapshot(
                 callingUser.getId(), accountId != null ? accountId : callingAccount.getId(), vm.getId(),
-                VolumeApiServiceImpl.VM_WORK_JOB_HANDLER, volumeId, policyId, snapshotId, quiesceVm);
+                VolumeApiServiceImpl.VM_WORK_JOB_HANDLER, volumeId, policyId, snapshotId, quiesceVm, locationType);
         workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo));
 
         _jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE, vm.getId());
@@ -2924,7 +2939,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     private Pair<JobInfo.Status, String> orchestrateTakeVolumeSnapshot(VmWorkTakeVolumeSnapshot work) throws Exception {
         Account account = _accountDao.findById(work.getAccountId());
         orchestrateTakeVolumeSnapshot(work.getVolumeId(), work.getPolicyId(), work.getSnapshotId(),
-                account, work.isQuiesceVm());
+                account, work.isQuiesceVm(), work.getLocationType());
         return new Pair<JobInfo.Status, String>(JobInfo.Status.SUCCEEDED,
                 _jobMgr.marshallResultObject(work.getSnapshotId()));
     }

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

@@ -16,44 +16,6 @@
 // under the License.
 package com.cloud.storage.snapshot;
 
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.List;
-import java.util.Map;
-import java.util.TimeZone;
-
-import javax.inject.Inject;
-import javax.naming.ConfigurationException;
-
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
-
-import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotPolicyCmd;
-import org.apache.cloudstack.api.command.user.snapshot.DeleteSnapshotPoliciesCmd;
-import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotPoliciesCmd;
-import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotsCmd;
-import org.apache.cloudstack.api.command.user.snapshot.UpdateSnapshotPolicyCmd;
-import org.apache.cloudstack.context.CallContext;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
-import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
-import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
-import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.Command;
 import com.cloud.agent.api.DeleteSnapshotsDirCommand;
@@ -128,6 +90,41 @@ import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.snapshot.VMSnapshot;
 import com.cloud.vm.snapshot.VMSnapshotVO;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotPolicyCmd;
+import org.apache.cloudstack.api.command.user.snapshot.DeleteSnapshotPoliciesCmd;
+import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotPoliciesCmd;
+import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotsCmd;
+import org.apache.cloudstack.api.command.user.snapshot.UpdateSnapshotPolicyCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
+import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+import java.util.Map;
+import java.util.TimeZone;
 
 @Component
 public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implements SnapshotManager, SnapshotApiService {
@@ -999,6 +996,9 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
     @DB
     public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationException {
         CreateSnapshotPayload payload = (CreateSnapshotPayload)volume.getpayload();
+
+        updateSnapshotPayload(volume.getPoolId(), payload);
+
         Long snapshotId = payload.getSnapshotId();
         Account snapshotOwner = payload.getAccount();
         SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId, volume.getDataStore());
@@ -1036,6 +1036,21 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
         return snapshot;
     }
 
+    private void updateSnapshotPayload(long storagePoolId, CreateSnapshotPayload payload) {
+        StoragePoolVO storagePoolVO = _storagePoolDao.findById(storagePoolId);
+
+        if (storagePoolVO.isManaged()) {
+            Snapshot.LocationType locationType = payload.getLocationType();
+
+            if (locationType == null) {
+                payload.setLocationType(Snapshot.LocationType.PRIMARY);
+            }
+        }
+        else {
+            payload.setLocationType(null);
+        }
+    }
+
     private static DataStoreRole getDataStoreRole(Snapshot snapshot, SnapshotDataStoreDao snapshotStoreDao, DataStoreManager dataStoreMgr) {
         SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findBySnapshot(snapshot.getId(), DataStoreRole.Primary);
 
@@ -1145,7 +1160,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
     }
 
     @Override
-    public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName) throws ResourceAllocationException {
+    public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType) throws ResourceAllocationException {
         Account caller = CallContext.current().getCallingAccount();
         VolumeInfo volume = volFactory.getVolume(volumeId);
         supportedByHypervisor(volume);
@@ -1196,7 +1211,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
 
         SnapshotVO snapshotVO =
             new SnapshotVO(volume.getDataCenterId(), volume.getAccountId(), volume.getDomainId(), volume.getId(), volume.getDiskOfferingId(), snapshotName,
-                (short)snapshotType.ordinal(), snapshotType.name(), volume.getSize(), volume.getMinIops(), volume.getMaxIops(), hypervisorType);
+                (short)snapshotType.ordinal(), snapshotType.name(), volume.getSize(), volume.getMinIops(), volume.getMaxIops(), hypervisorType, locationType);
 
         SnapshotVO snapshot = _snapshotDao.persist(snapshotVO);
         if (snapshot == null) {

server/test/com/cloud/storage/VolumeApiServiceImplTest.java

@@ -16,39 +16,32 @@
 // under the License.
 package com.cloud.storage;
 
-import static org.mockito.Matchers.any;
+import com.cloud.dc.DataCenterVO;
-import static org.mockito.Matchers.anyLong;
+import com.cloud.dc.dao.DataCenterDao;
-import static org.mockito.Matchers.anyString;
+import com.cloud.exception.InvalidParameterValueException;
-import static org.mockito.Matchers.eq;
+import com.cloud.exception.ResourceAllocationException;
-import static org.mockito.Mockito.doNothing;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import static org.mockito.Mockito.times;
+import com.cloud.org.Grouping;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import java.lang.reflect.Field;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.UUID;
-
-import javax.inject.Inject;
-
 import com.cloud.serializer.GsonHelper;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
+import com.cloud.user.AccountVO;
 import com.cloud.user.User;
+import com.cloud.user.UserVO;
+import com.cloud.utils.db.TransactionLegacy;
 import com.cloud.vm.UserVmManager;
+import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
 import junit.framework.Assert;
-import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
-import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.MockitoAnnotations;
-
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
+import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
@@ -61,22 +54,29 @@ import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDao;
 import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
 
-import com.cloud.exception.InvalidParameterValueException;
+import javax.inject.Inject;
-import com.cloud.exception.ResourceAllocationException;
+import java.lang.reflect.Field;
-import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import java.util.ArrayList;
-import com.cloud.storage.dao.VolumeDao;
+import java.util.List;
-import com.cloud.user.Account;
+import java.util.UUID;
-import com.cloud.user.AccountManager;
+
-import com.cloud.user.AccountVO;
+import static org.mockito.Matchers.any;
-import com.cloud.user.UserVO;
+import static org.mockito.Matchers.anyLong;
-import com.cloud.utils.db.TransactionLegacy;
+import static org.mockito.Matchers.anyString;
-import com.cloud.vm.UserVmVO;
+import static org.mockito.Matchers.eq;
-import com.cloud.vm.VirtualMachine.State;
+import static org.mockito.Mockito.doNothing;
-import com.cloud.vm.dao.UserVmDao;
+import static org.mockito.Mockito.times;
-import com.cloud.vm.dao.VMInstanceDao;
+import static org.mockito.Mockito.verify;
-import com.cloud.vm.snapshot.VMSnapshotVO;
+import static org.mockito.Mockito.when;
-import com.cloud.vm.snapshot.dao.VMSnapshotDao;
 
 public class VolumeApiServiceImplTest {
     @Inject
@@ -110,6 +110,8 @@ public class VolumeApiServiceImplTest {
     CreateVolumeCmd createVol;
     @Mock
     UserVmManager _userVmMgr;
+    @Mock
+    DataCenterDao _dcDao;
 
     DetachVolumeCmd detachCmd = new DetachVolumeCmd();
     Class<?> _detachCmdClass = detachCmd.getClass();
@@ -128,6 +130,7 @@ public class VolumeApiServiceImplTest {
         _svc.volFactory = _volFactory;
         _svc.volService = volService;
         _svc._userVmMgr = _userVmMgr;
+        _svc._dcDao = _dcDao;
         _svc._gson = GsonHelper.getGsonLogger();
 
         // mock caller context
@@ -180,6 +183,7 @@ public class VolumeApiServiceImplTest {
             when(_svc._volsDao.findById(3L)).thenReturn(volumeOfStoppeHyperVVm);
 
             StoragePoolVO unmanagedPool = new StoragePoolVO();
+
             when(_svc._storagePoolDao.findById(1L)).thenReturn(unmanagedPool);
 
             // volume of managed pool id=4
@@ -203,6 +207,9 @@ public class VolumeApiServiceImplTest {
             when(correctRootVolume.getDataCenterId()).thenReturn(1L);
             when(correctRootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT);
             when(correctRootVolume.getInstanceId()).thenReturn(null);
+            when(correctRootVolume.getState()).thenReturn(Volume.State.Ready);
+            when(correctRootVolume.getTemplateId()).thenReturn(null);
+            when(correctRootVolume.getPoolId()).thenReturn(1L);
             when(_svc.volFactory.getVolume(6L)).thenReturn(correctRootVolume);
 
             VolumeVO correctRootVolumeVO = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", Storage.ProvisioningType.THIN, 1, null,
@@ -255,6 +262,11 @@ public class VolumeApiServiceImplTest {
             when(_svc._vmSnapshotDao.findByVm(any(Long.class))).thenReturn(new ArrayList<VMSnapshotVO>());
             when(_svc._vmInstanceDao.findById(any(Long.class))).thenReturn(stoppedVm);
 
+            DataCenterVO enabledZone = Mockito.mock(DataCenterVO.class);
+            when(enabledZone.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled);
+
+            when(_svc._dcDao.findById(anyLong())).thenReturn(enabledZone);
+
         } finally {
             txn.close("runVolumeDaoImplTest");
         }
@@ -358,7 +370,8 @@ public class VolumeApiServiceImplTest {
     public void testTakeSnapshotF1() throws ResourceAllocationException {
         when(_volFactory.getVolume(anyLong())).thenReturn(volumeInfoMock);
         when(volumeInfoMock.getState()).thenReturn(Volume.State.Allocated);
-        _svc.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false);
+        when(volumeInfoMock.getPoolId()).thenReturn(1L);
+        _svc.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false, null);
     }
 
     @Test
@@ -366,8 +379,9 @@ public class VolumeApiServiceImplTest {
         when(_volFactory.getVolume(anyLong())).thenReturn(volumeInfoMock);
         when(volumeInfoMock.getState()).thenReturn(Volume.State.Ready);
         when(volumeInfoMock.getInstanceId()).thenReturn(null);
+        when(volumeInfoMock.getPoolId()).thenReturn(1L);
         when (volService.takeSnapshot(Mockito.any(VolumeInfo.class))).thenReturn(snapshotInfoMock);
-        _svc.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false);
+        _svc.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false, null);
     }
 
     @Test
@@ -408,6 +422,23 @@ public class VolumeApiServiceImplTest {
         verify(_svc._userVmMgr, times(1)).persistDeviceBusInfo(any(UserVmVO.class), eq("scsi"));
     }
 
+    @Test
+    /**
+     * Setting locationType for a non-managed storage should give an error
+     */
+    public void testAllocSnapshotNonManagedStorageArchive() {
+        try {
+            _svc.allocSnapshot(6L, 1L, "test", Snapshot.LocationType.SECONDARY);
+        } catch (InvalidParameterValueException e) {
+            Assert.assertEquals(e.getMessage(), "VolumeId: 6 LocationType is supported only for managed storage");
+            return;
+        } catch (ResourceAllocationException e) {
+            Assert.fail("Unexpected excepiton " + e.getMessage());
+        }
+
+        Assert.fail("Expected Exception for archive in non-managed storage");
+    }
+
     @After
     public void tearDown() {
         CallContext.unregister();

server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java

@@ -16,15 +16,34 @@
 // under the License.
 package com.cloud.storage.snapshot;
 
-import static org.mockito.Matchers.any;
+import com.cloud.configuration.Resource.ResourceType;
-import static org.mockito.Matchers.anyLong;
+import com.cloud.exception.InvalidParameterValueException;
-import static org.mockito.Mockito.doNothing;
+import com.cloud.exception.ResourceAllocationException;
-import static org.mockito.Mockito.mock;
+import com.cloud.hypervisor.Hypervisor;
-import static org.mockito.Mockito.when;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
-
+import com.cloud.resource.ResourceManager;
-import java.util.List;
+import com.cloud.storage.DataStoreRole;
-import java.util.UUID;
+import com.cloud.storage.ScopeType;
-
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.Storage.ImageFormat;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
+import com.cloud.user.AccountVO;
+import com.cloud.user.ResourceLimitService;
+import com.cloud.user.User;
+import com.cloud.user.UserVO;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.context.CallContext;
@@ -47,34 +66,14 @@ import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 import org.mockito.Spy;
 
-import com.cloud.configuration.Resource.ResourceType;
+import java.util.List;
-import com.cloud.exception.InvalidParameterValueException;
+import java.util.UUID;
-import com.cloud.exception.ResourceAllocationException;
+
-import com.cloud.hypervisor.Hypervisor;
+import static org.mockito.Matchers.any;
-import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import static org.mockito.Matchers.anyLong;
-import com.cloud.resource.ResourceManager;
+import static org.mockito.Mockito.doNothing;
-import com.cloud.storage.DataStoreRole;
+import static org.mockito.Mockito.mock;
-import com.cloud.storage.ScopeType;
+import static org.mockito.Mockito.when;
-import com.cloud.storage.Snapshot;
-import com.cloud.storage.SnapshotVO;
-import com.cloud.storage.Volume;
-import com.cloud.storage.VolumeVO;
-import com.cloud.storage.Storage.ImageFormat;
-import com.cloud.storage.dao.SnapshotDao;
-import com.cloud.storage.dao.VolumeDao;
-import com.cloud.user.Account;
-import com.cloud.user.AccountManager;
-import com.cloud.user.AccountVO;
-import com.cloud.user.ResourceLimitService;
-import com.cloud.user.User;
-import com.cloud.user.UserVO;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.vm.UserVmVO;
-import com.cloud.vm.VirtualMachine.State;
-import com.cloud.vm.dao.UserVmDao;
-import com.cloud.vm.snapshot.VMSnapshot;
-import com.cloud.vm.snapshot.VMSnapshotVO;
-import com.cloud.vm.snapshot.dao.VMSnapshotDao;
 
 public class SnapshotManagerTest {
     @Spy
@@ -182,7 +181,7 @@ public class SnapshotManagerTest {
     public void testAllocSnapshotF1() throws ResourceAllocationException {
         when(_vmDao.findById(anyLong())).thenReturn(vmMock);
         when(vmMock.getState()).thenReturn(State.Destroyed);
-        _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null);
+        _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null, null);
     }
 
     // active snapshots
@@ -197,7 +196,7 @@ public class SnapshotManagerTest {
         List<SnapshotVO> mockList = mock(List.class);
         when(mockList.size()).thenReturn(1);
         when(_snapshotDao.listByInstanceId(TEST_VM_ID, Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp)).thenReturn(mockList);
-        _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null);
+        _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null, null);
     }
 
     // active vm snapshots
@@ -215,7 +214,7 @@ public class SnapshotManagerTest {
         List<VMSnapshotVO> mockList2 = mock(List.class);
         when(mockList2.size()).thenReturn(1);
         when(_vmSnapshotDao.listByInstanceId(TEST_VM_ID, VMSnapshot.State.Creating, VMSnapshot.State.Reverting, VMSnapshot.State.Expunging)).thenReturn(mockList2);
-        _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null);
+        _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null, null);
     }
 
     // successful test
@@ -234,7 +233,7 @@ public class SnapshotManagerTest {
         when(mockList2.size()).thenReturn(0);
         when(_vmSnapshotDao.listByInstanceId(TEST_VM_ID, VMSnapshot.State.Creating, VMSnapshot.State.Reverting, VMSnapshot.State.Expunging)).thenReturn(mockList2);
         when(_snapshotDao.persist(any(SnapshotVO.class))).thenReturn(snapshotMock);
-        _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null);
+        _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null, null);
     }
 
     @Test

services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java

@@ -16,91 +16,7 @@
 // under the License.
 package org.apache.cloudstack.storage.resource;
 
-import static com.cloud.utils.storage.S3.S3Utils.putFile;
-import static com.cloud.utils.StringUtils.join;
-import static java.lang.String.format;
-import static java.util.Arrays.asList;
-import static org.apache.commons.lang.StringUtils.substringAfterLast;
-
-import java.io.BufferedReader;
-import java.io.BufferedWriter;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
-import java.io.FileReader;
-import java.io.FileWriter;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.UnsupportedEncodingException;
-import java.math.BigInteger;
-import java.net.InetAddress;
-import java.net.URI;
-import java.net.UnknownHostException;
-import java.security.MessageDigest;
-import java.security.NoSuchAlgorithmException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
-
-import javax.naming.ConfigurationException;
-
-import com.cloud.exception.InvalidParameterValueException;
-import com.cloud.storage.Storage;
-import com.cloud.storage.template.TemplateConstants;
-import com.cloud.utils.EncryptionUtil;
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
-import io.netty.bootstrap.ServerBootstrap;
-import io.netty.channel.Channel;
-import io.netty.channel.ChannelInitializer;
-import io.netty.channel.ChannelPipeline;
-import io.netty.channel.EventLoopGroup;
-import io.netty.channel.nio.NioEventLoopGroup;
-import io.netty.channel.socket.SocketChannel;
-import io.netty.channel.socket.nio.NioServerSocketChannel;
-import io.netty.handler.codec.http.HttpContentCompressor;
-import io.netty.handler.codec.http.HttpRequestDecoder;
-import io.netty.handler.codec.http.HttpResponseEncoder;
-import io.netty.handler.logging.LogLevel;
-import io.netty.handler.logging.LoggingHandler;
-import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
-import org.apache.cloudstack.storage.template.UploadEntity;
-import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
-import org.apache.commons.codec.digest.DigestUtils;
-import org.apache.commons.io.FileUtils;
-import org.apache.commons.io.FilenameUtils;
-import org.apache.commons.lang.StringUtils;
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
-import org.apache.http.NameValuePair;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.utils.URLEncodedUtils;
-import org.apache.http.impl.client.DefaultHttpClient;
-import org.apache.log4j.Logger;
-
 import com.amazonaws.services.s3.model.S3ObjectSummary;
-
-import org.apache.cloudstack.framework.security.keystore.KeystoreManager;
-import org.apache.cloudstack.storage.command.CopyCmdAnswer;
-import org.apache.cloudstack.storage.command.CopyCommand;
-import org.apache.cloudstack.storage.command.DeleteCommand;
-import org.apache.cloudstack.storage.command.DownloadCommand;
-import org.apache.cloudstack.storage.command.DownloadProgressCommand;
-import org.apache.cloudstack.storage.command.UploadStatusAnswer;
-import org.apache.cloudstack.storage.command.UploadStatusAnswer.UploadStatus;
-import org.apache.cloudstack.storage.command.UploadStatusCommand;
-import org.apache.cloudstack.storage.template.DownloadManager;
-import org.apache.cloudstack.storage.template.DownloadManagerImpl;
-import org.apache.cloudstack.storage.template.DownloadManagerImpl.ZfsPathParser;
-import org.apache.cloudstack.storage.template.UploadManager;
-import org.apache.cloudstack.storage.template.UploadManagerImpl;
-import org.apache.cloudstack.storage.to.SnapshotObjectTO;
-import org.apache.cloudstack.storage.to.TemplateObjectTO;
-import org.apache.cloudstack.storage.to.VolumeObjectTO;
-
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.CheckHealthAnswer;
 import com.cloud.agent.api.CheckHealthCommand;
@@ -135,11 +51,13 @@ import com.cloud.agent.api.to.NfsTO;
 import com.cloud.agent.api.to.S3TO;
 import com.cloud.agent.api.to.SwiftTO;
 import com.cloud.exception.InternalErrorException;
+import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.host.Host;
 import com.cloud.host.Host.Type;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.resource.ServerResourceBase;
 import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Storage;
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.StorageLayer;
 import com.cloud.storage.VMTemplateStorageResourceAssoc;
@@ -149,21 +67,99 @@ import com.cloud.storage.template.Processor.FormatInfo;
 import com.cloud.storage.template.QCOW2Processor;
 import com.cloud.storage.template.RawImageProcessor;
 import com.cloud.storage.template.TARProcessor;
+import com.cloud.storage.template.TemplateConstants;
 import com.cloud.storage.template.TemplateLocation;
 import com.cloud.storage.template.TemplateProp;
 import com.cloud.storage.template.VhdProcessor;
 import com.cloud.storage.template.VmdkProcessor;
+import com.cloud.utils.EncryptionUtil;
 import com.cloud.utils.NumbersUtil;
-import com.cloud.utils.storage.S3.S3Utils;
 import com.cloud.utils.SwiftUtil;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.net.NetUtils;
 import com.cloud.utils.script.OutputInterpreter;
 import com.cloud.utils.script.Script;
+import com.cloud.utils.storage.S3.S3Utils;
 import com.cloud.vm.SecondaryStorageVm;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.Channel;
+import io.netty.channel.ChannelInitializer;
+import io.netty.channel.ChannelPipeline;
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.nio.NioEventLoopGroup;
+import io.netty.channel.socket.SocketChannel;
+import io.netty.channel.socket.nio.NioServerSocketChannel;
+import io.netty.handler.codec.http.HttpContentCompressor;
+import io.netty.handler.codec.http.HttpRequestDecoder;
+import io.netty.handler.codec.http.HttpResponseEncoder;
+import io.netty.handler.logging.LogLevel;
+import io.netty.handler.logging.LoggingHandler;
+import org.apache.cloudstack.framework.security.keystore.KeystoreManager;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.command.CopyCommand;
+import org.apache.cloudstack.storage.command.DeleteCommand;
+import org.apache.cloudstack.storage.command.DownloadCommand;
+import org.apache.cloudstack.storage.command.DownloadProgressCommand;
+import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.storage.command.UploadStatusAnswer;
+import org.apache.cloudstack.storage.command.UploadStatusAnswer.UploadStatus;
+import org.apache.cloudstack.storage.command.UploadStatusCommand;
+import org.apache.cloudstack.storage.template.DownloadManager;
+import org.apache.cloudstack.storage.template.DownloadManagerImpl;
+import org.apache.cloudstack.storage.template.DownloadManagerImpl.ZfsPathParser;
+import org.apache.cloudstack.storage.template.UploadEntity;
+import org.apache.cloudstack.storage.template.UploadManager;
+import org.apache.cloudstack.storage.template.UploadManagerImpl;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+import org.apache.cloudstack.storage.to.TemplateObjectTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpResponse;
+import org.apache.http.NameValuePair;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.utils.URLEncodedUtils;
+import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.log4j.Logger;
 import org.joda.time.DateTime;
 import org.joda.time.format.ISODateTimeFormat;
 
+import javax.naming.ConfigurationException;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UnsupportedEncodingException;
+import java.math.BigInteger;
+import java.net.InetAddress;
+import java.net.URI;
+import java.net.UnknownHostException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+import static com.cloud.utils.StringUtils.join;
+import static com.cloud.utils.storage.S3.S3Utils.putFile;
+import static java.lang.String.format;
+import static java.util.Arrays.asList;
+import static org.apache.commons.lang.StringUtils.substringAfterLast;
+
 public class NfsSecondaryStorageResource extends ServerResourceBase implements SecondaryStorageResource {
 
     public static final Logger s_logger = Logger.getLogger(NfsSecondaryStorageResource.class);
@@ -583,8 +579,11 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         String filePath = getRootDir(nfsPath, nfsVersion) + File.separator + path;
         File f = new File(filePath);
         if (!f.exists()) {
-            _storage.mkdirs(filePath);
+            f = findFile(filePath);
-            f = new File(filePath);
+            if (f == null) {
+                _storage.mkdirs(filePath);
+                f = new File(filePath);
+            }
         }
         return f;
     }
@@ -660,6 +659,8 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
 
         if (destDataStore instanceof S3TO) {
             return copyFromNfsToS3(cmd);
+        } else if (destDataStore instanceof SwiftTO) {
+            return copyFromNfsToSwift(cmd);
         } else {
             return new CopyCmdAnswer("unsupported ");
         }
@@ -874,6 +875,28 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
 
     }
 
+    protected File findFile(String path) {
+
+        File srcFile = _storage.getFile(path);
+        if (!srcFile.exists()) {
+            srcFile = _storage.getFile(path + ".qcow2");
+            if (!srcFile.exists()) {
+                srcFile = _storage.getFile(path + ".vhd");
+                if (!srcFile.exists()) {
+                    srcFile = _storage.getFile(path + ".ova");
+                    if (!srcFile.exists()) {
+                        srcFile = _storage.getFile(path + ".vmdk");
+                        if (!srcFile.exists()) {
+                            return null;
+                        }
+                    }
+                }
+            }
+        }
+
+        return srcFile;
+    }
+
     protected Answer copyFromNfsToS3(CopyCommand cmd) {
         final DataTO srcData = cmd.getSrcTO();
         final DataTO destData = cmd.getDestTO();
@@ -891,23 +914,9 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
             }
 
             final String bucket = s3.getBucketName();
-            File srcFile = _storage.getFile(templatePath);
+            File srcFile = findFile(templatePath);
-            // guard the case where templatePath does not have file extension, since we are not completely sure
+            if (srcFile == null) {
-            // about hypervisor, so we check each extension
+                return new CopyCmdAnswer("Can't find src file:" + templatePath);
-            if (!srcFile.exists()) {
-                srcFile = _storage.getFile(templatePath + ".qcow2");
-                if (!srcFile.exists()) {
-                    srcFile = _storage.getFile(templatePath + ".vhd");
-                    if (!srcFile.exists()) {
-                        srcFile = _storage.getFile(templatePath + ".ova");
-                        if (!srcFile.exists()) {
-                            srcFile = _storage.getFile(templatePath + ".vmdk");
-                            if (!srcFile.exists()) {
-                                return new CopyCmdAnswer("Can't find src file:" + templatePath);
-                            }
-                        }
-                    }
-                }
             }
 
             ImageFormat format = getTemplateFormat(srcFile.getName());
@@ -1018,10 +1027,15 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         File srcFile = getFile(srcData.getPath(), srcStore.getUrl(), _nfsVersion);
 
         SwiftTO swift = (SwiftTO)destDataStore;
+        long pathId = destData.getId();
 
         try {
 
-            String containerName = SwiftUtil.getContainerName(destData.getObjectType().toString(), destData.getId());
+            if (destData instanceof SnapshotObjectTO) {
+                pathId = ((SnapshotObjectTO) destData).getVolume().getId();
+            }
+
+            String containerName = SwiftUtil.getContainerName(destData.getObjectType().toString(), pathId);
             String swiftPath = SwiftUtil.putObject(swift, srcFile, containerName, srcFile.getName());
 
 
@@ -1041,7 +1055,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
                 retObj = newVol;
             } else if (destData.getObjectType() == DataObjectType.SNAPSHOT) {
                 SnapshotObjectTO newSnapshot = new SnapshotObjectTO();
-                newSnapshot.setPath(containerName);
+                newSnapshot.setPath(containerName + File.separator + srcFile.getName());
                 retObj = newSnapshot;
             }
 
@@ -2404,7 +2418,6 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
      * @param uri
      *            crresponding to the remote device. Will throw for unsupported
      *            scheme.
-     * @param imgStoreId
      * @param nfsVersion NFS version to use in mount command
      * @return name of folder in _parent that device was mounted.
      * @throws UnknownHostException

setup/db/db/schema-481to490.sql

@@ -548,3 +548,5 @@ INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervis
 
 ALTER TABLE `cloud`.`image_store_details` CHANGE COLUMN `value` `value` VARCHAR(255) NULL DEFAULT NULL COMMENT 'value of the detail', ADD COLUMN `display` tinyint(1) NOT 
 NULL DEFAULT '1' COMMENT 'True if the detail can be displayed to the end user' AFTER `value`;
+
+ALTER TABLE `snapshots` ADD COLUMN `location_type` VARCHAR(32) COMMENT 'Location of snapshot (ex. Primary)';

test/integration/plugins/solidfire/TestSnapshots.py

@@ -1636,7 +1636,7 @@ class TestSnapshots(cloudstackTestCase):
         vol_snap = Snapshot.create(
             self.apiClient,
             volume_id=volume_id_for_snapshot,
-            locationtype=2
+            locationtype="secondary"
         )
 
         self._wait_for_snapshot_state(vol_snap.id, Snapshot.BACKED_UP)

tools/marvin/marvin/lib/base.py

@@ -968,7 +968,8 @@ class Volume:
         cmd.name = "-".join([services["diskname"], random_gen()])
         cmd.snapshotid = snapshot_id
         cmd.zoneid = services["zoneid"]
-        cmd.size = services["size"]
+        if "size" in services:
+            cmd.size = services["size"]
         if services["ispublic"]:
             cmd.ispublic = services["ispublic"]
         else:
@@ -1093,7 +1094,7 @@ class Snapshot:
 
     @classmethod
     def create(cls, apiclient, volume_id, account=None,
-               domainid=None, projectid=None):
+               domainid=None, projectid=None, locationtype=None):
         """Create Snapshot"""
         cmd = createSnapshot.createSnapshotCmd()
         cmd.volumeid = volume_id
@@ -1103,6 +1104,8 @@ class Snapshot:
             cmd.domainid = domainid
         if projectid:
             cmd.projectid = projectid
+        if locationtype:
+            cmd.locationtype = locationtype
         return Snapshot(apiclient.createSnapshot(cmd).__dict__)
 
     def delete(self, apiclient):