hide the primary storage when a user copies the snapshots

slavkap · slavkap · commit ab3d218cb96d · 2025-06-18T14:35:18.000+03:00
diff --git a/api/src/main/java/com/cloud/storage/VolumeApiService.java b/api/src/main/java/com/cloud/storage/VolumeApiService.java
@@ -113,10 +113,10 @@ public interface VolumeApiService {
 
     Volume detachVolumeFromVM(DetachVolumeCmd cmd);
 
-    Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm, Snapshot.LocationType locationType, boolean asyncBackup, Map<String, String> tags, List<Long> zoneIds, List<Long> poolIds)
+    Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm, Snapshot.LocationType locationType, boolean asyncBackup, Map<String, String> tags, List<Long> zoneIds, List<Long> poolIds, Boolean useStorageReplication)
             throws ResourceAllocationException;
 
-    Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType, List<Long> zoneIds, List<Long> poolIds) throws ResourceAllocationException;
+    Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType, List<Long> zoneIds, Boolean useStorageReplication) throws ResourceAllocationException;
 
     Volume updateVolume(long volumeId, String path, String state, Long storageId,
                         Boolean displayVolume, Boolean deleteProtection,
diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
@@ -480,6 +480,9 @@ public class ApiConstants {
     public static final String SNAPSHOT_POLICY_ID = "snapshotpolicyid";
     public static final String SNAPSHOT_TYPE = "snapshottype";
     public static final String SNAPSHOT_QUIESCEVM = "quiescevm";
+
+    public static final String USE_STORAGE_REPLICATION = "usestoragereplication";
+
     public static final String SOURCE_CIDR_LIST = "sourcecidrlist";
     public static final String SOURCE_ZONE_ID = "sourcezoneid";
     public static final String SSL_VERIFICATION = "sslverification";
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java
@@ -90,10 +90,14 @@ public class CopySnapshotCmd extends BaseAsyncCmd implements UserCmd {
             collectionType = CommandType.UUID,
             entityType = StoragePoolResponse.class,
             required = false,
+            authorized = RoleType.Admin,
             description = "A comma-separated list of IDs of the storage pools in other zones in which the snapshot will be made available. " +
                     "The snapshot will always be made available in the zone in which the volume is present. Currently supported for StorPool only")
     protected List<Long> storagePoolIds;
 
+    @Parameter (name = ApiConstants.USE_STORAGE_REPLICATION, type=CommandType.BOOLEAN, required = false, description = "This parameter enables the option the snapshot to be copied to supported primary storage")
+    protected Boolean useStorageReplication;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -123,6 +127,13 @@ public List<Long> getStoragePoolIds() {
         return storagePoolIds;
     }
 
+    public Boolean useStorageReplication() {
+        if (useStorageReplication == null) {
+            return false;
+        }
+        return useStorageReplication;
+    }
+
     @Override
     public String getEventType() {
         return EventTypes.EVENT_SNAPSHOT_COPY;
@@ -166,7 +177,7 @@ public long getEntityOwnerId() {
     @Override
     public void execute() throws ResourceUnavailableException {
         try {
-            if (destZoneId == null && CollectionUtils.isEmpty(destZoneIds) && CollectionUtils.isEmpty(storagePoolIds))
+            if (destZoneId == null && CollectionUtils.isEmpty(destZoneIds) && useStorageReplication())
                 throw new ServerApiException(ApiErrorCode.PARAM_ERROR,
                         "Either destzoneid or destzoneids parameters have to be specified.");
 
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java
@@ -21,6 +21,7 @@
 import java.util.List;
 import java.util.Map;
 
+import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiCommandResourceType;
 import org.apache.cloudstack.api.ApiConstants;
@@ -104,11 +105,15 @@ public class CreateSnapshotCmd extends BaseAsyncCreateCmd {
             type=CommandType.LIST,
             collectionType = CommandType.UUID,
             entityType = StoragePoolResponse.class,
+            authorized = RoleType.Admin,
             description = "A comma-separated list of IDs of the storage pools in other zones in which the snapshot will be made available. " +
                     "The snapshot will always be made available in the zone in which the volume is present.",
             since = "4.20.0")
     protected List<Long> storagePoolIds;
 
+    @Parameter (name = ApiConstants.USE_STORAGE_REPLICATION, type=CommandType.BOOLEAN, required = false, description = "This parameter enables the option the snapshot to be copied to supported primary storage")
+    protected Boolean useStorageReplication;
+
     private String syncObjectType = BaseAsyncCmd.snapshotHostSyncObject;
 
     // ///////////////////////////////////////////////////
@@ -175,6 +180,13 @@ public List<Long> getStoragePoolIds() {
         return storagePoolIds;
     }
 
+    public Boolean useStorageReplication() {
+        if (useStorageReplication == null) {
+            return false;
+        }
+        return useStorageReplication;
+    }
+
     // ///////////////////////////////////////////////////
     // ///////////// API Implementation///////////////////
     // ///////////////////////////////////////////////////
@@ -223,7 +235,7 @@ public ApiCommandResourceType getApiResourceType() {
 
     @Override
     public void create() throws ResourceAllocationException {
-        Snapshot snapshot = _volumeService.allocSnapshot(getVolumeId(), getPolicyId(), getSnapshotName(), getLocationType(), getZoneIds(), getStoragePoolIds());
+        Snapshot snapshot = _volumeService.allocSnapshot(getVolumeId(), getPolicyId(), getSnapshotName(), getLocationType(), getZoneIds(), useStorageReplication());
         if (snapshot != null) {
             setEntityId(snapshot.getId());
             setEntityUuid(snapshot.getUuid());
@@ -237,7 +249,7 @@ public void execute() {
         Snapshot snapshot;
         try {
             snapshot =
-                _volumeService.takeSnapshot(getVolumeId(), getPolicyId(), getEntityId(), _accountService.getAccount(getEntityOwnerId()), getQuiescevm(), getLocationType(), getAsyncBackup(), getTags(), getZoneIds(), getStoragePoolIds());
+                _volumeService.takeSnapshot(getVolumeId(), getPolicyId(), getEntityId(), _accountService.getAccount(getEntityOwnerId()), getQuiescevm(), getLocationType(), getAsyncBackup(), getTags(), getZoneIds(), getStoragePoolIds(), useStorageReplication());
 
             if (snapshot != null) {
                 SnapshotResponse response = _responseGenerator.createSnapshotResponse(snapshot);
@@ -257,7 +269,7 @@ public void execute() {
         }
     }
 
-    private Snapshot.LocationType getLocationType() {
+    public Snapshot.LocationType getLocationType() {
 
         if (Snapshot.LocationType.values() == null || Snapshot.LocationType.values().length == 0 || locationType == null) {
             return null;
diff --git a/api/src/test/java/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java
@@ -93,7 +93,7 @@ public void testCreateSuccess() {
         Snapshot snapshot = Mockito.mock(Snapshot.class);
         try {
             Mockito.when(volumeApiService.takeSnapshot(nullable(Long.class), nullable(Long.class), isNull(),
-                    nullable(Account.class), nullable(Boolean.class), nullable(Snapshot.LocationType.class), nullable(Boolean.class), nullable(Map.class), nullable(List.class), nullable(List.class))).thenReturn(snapshot);
+                    nullable(Account.class), nullable(Boolean.class), nullable(Snapshot.LocationType.class), nullable(Boolean.class), nullable(Map.class), nullable(List.class), nullable(List.class), Mockito.anyBoolean())).thenReturn(snapshot);
 
         } catch (Exception e) {
             Assert.fail("Received exception when success expected " + e.getMessage());
@@ -126,7 +126,7 @@ public void testCreateFailure() {
 
         try {
                 Mockito.when(volumeApiService.takeSnapshot(nullable(Long.class), nullable(Long.class), nullable(Long.class),
-                        nullable(Account.class), nullable(Boolean.class), nullable(Snapshot.LocationType.class), nullable(Boolean.class), any(), Mockito.anyList(), Mockito.anyList())).thenReturn(null);
+                        nullable(Account.class), nullable(Boolean.class), nullable(Snapshot.LocationType.class), nullable(Boolean.class), any(), Mockito.anyList(), Mockito.anyList(), Mockito.anyBoolean())).thenReturn(null);
         } catch (Exception e) {
             Assert.fail("Received exception when success expected " + e.getMessage());
         }
diff --git a/api/src/test/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmdTest.java
@@ -87,7 +87,12 @@ public void testGetDestZoneIdWithBothParams() {
 
     @Test (expected = ServerApiException.class)
     public void testExecuteWrongNoParams() {
+        UUIDManager uuidManager = Mockito.mock(UUIDManager.class);
+        SnapshotApiService snapshotApiService = Mockito.mock(SnapshotApiService.class);
         final CopySnapshotCmd cmd = new CopySnapshotCmd();
+        cmd._uuidMgr = uuidManager;
+        cmd._snapshotService = snapshotApiService;
+
         try {
             cmd.execute();
         } catch (ResourceUnavailableException e) {
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java
@@ -168,4 +168,7 @@ Pair<List<Long>, Integer> searchForIdsAndCount(Long storagePoolId, String storag
     List<StoragePoolVO> listByIds(List<Long> ids);
 
     List<StoragePoolVO> findStoragePoolsByEmptyStorageAccessGroups(Long dcId, Long podId, Long clusterId, ScopeType scope, HypervisorType hypervisorType);
+
+    List<StoragePoolVO> findPoolsByStorageTypeAndZone(Storage.StoragePoolType storageType, Long zoneId);
+
 }
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
@@ -916,6 +916,14 @@ public List<StoragePoolVO> listByIds(List<Long> ids) {
         return listBy(sc);
     }
 
+    @Override
+    public List<StoragePoolVO> findPoolsByStorageTypeAndZone(Storage.StoragePoolType storageType, Long zoneId) {
+        SearchCriteria<StoragePoolVO> sc = AllFieldSearch.create();
+        sc.setParameters("poolType", storageType);
+        sc.addAnd("dataCenterId", Op.EQ, zoneId);
+        return listBy(sc);
+    }
+
     private SearchCriteria<StoragePoolVO> createStoragePoolSearchCriteria(Long storagePoolId, String storagePoolName,
                                                                           Long zoneId, String path, Long podId, Long clusterId, Long hostId, String address, ScopeType scopeType,
                                                                           StoragePoolStatus status, String keyword, String storageAccessGroup) {
diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java
@@ -364,7 +364,7 @@ private boolean deleteSnapshotFromDbIfNeeded(SnapshotVO snapshotVO, Long zoneId)
                 !Snapshot.State.Destroying.equals(snapshotVO.getState())) {
             throw new InvalidParameterValueException(String.format("Can't delete snapshot %s due to it is in %s Status", snapshotVO, snapshotVO.getState()));
         }
-        List<SnapshotDataStoreVO> storeRefs = _snapshotStoreDao.listBySnapshot(snapshotId, DataStoreRole.Image);
+        List<SnapshotDataStoreVO> storeRefs = _snapshotStoreDao.listBySnapshotAndDataStoreRole(snapshotId, DataStoreRole.Image);
         if (zoneId != null) {
             storeRefs.removeIf(ref -> !zoneId.equals(dataStoreMgr.getStoreZoneId(ref.getDataStoreId(), ref.getRole())));
         }
diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
@@ -18,6 +18,8 @@
 
 import static com.cloud.vm.VmDetailConstants.SSH_PUBLIC_KEY;
 
+
+import com.cloud.cluster.ManagementServerHostPeerJoinVO;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
@@ -185,6 +185,7 @@
 import com.cloud.template.TemplateManager;
 import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
+import com.cloud.user.AccountService;
 import com.cloud.user.ResourceLimitService;
 import com.cloud.user.User;
 import com.cloud.user.VmDiskStatisticsVO;
@@ -363,6 +364,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     @Inject
     EndPointSelector _epSelector;
 
+    @Inject
+    public AccountService _accountService;
+
     protected Gson _gson;
 
     private static final List<HypervisorType> SupportedHypervisorsForVolResize = Arrays.asList(HypervisorType.KVM, HypervisorType.XenServer,
@@ -3773,20 +3777,23 @@ protected Volume liveMigrateVolume(Volume volume, StoragePool destPool) throws S
     @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,
-         Snapshot.LocationType locationType, boolean asyncBackup, Map<String, String> tags, List<Long> zoneIds, List<Long> poolIds)
-            throws ResourceAllocationException {
-        final Snapshot snapshot = takeSnapshotInternal(volumeId, policyId, snapshotId, account, quiescevm, locationType, asyncBackup, zoneIds, poolIds);
+         Snapshot.LocationType locationType, boolean asyncBackup, Map<String, String> tags, List<Long> zoneIds, List<Long> poolIds, Boolean useStorageReplication)
+
+    throws ResourceAllocationException {
+        final Snapshot snapshot = takeSnapshotInternal(volumeId, policyId, snapshotId, account, quiescevm, locationType, asyncBackup, zoneIds, poolIds, useStorageReplication);
         if (snapshot != null && MapUtils.isNotEmpty(tags)) {
             taggedResourceService.createTags(Collections.singletonList(snapshot.getUuid()), ResourceTag.ResourceObjectType.Snapshot, tags, null);
         }
         return snapshot;
     }
 
     private Snapshot takeSnapshotInternal(Long volumeId, Long policyId, Long snapshotId, Account account,
-          boolean quiescevm, Snapshot.LocationType locationType, boolean asyncBackup, List<Long> zoneIds, List<Long> poolIds)
+          boolean quiescevm, Snapshot.LocationType locationType, boolean asyncBackup, List<Long> zoneIds, List<Long> poolIds, Boolean useStorageReplication)
             throws ResourceAllocationException {
         Account caller = CallContext.current().getCallingAccount();
         VolumeInfo volume = volFactory.getVolume(volumeId);
+        poolIds = snapshotHelper.addStoragePoolsForCopyToPrimary(volume, zoneIds, poolIds, useStorageReplication);
+        canCopyOnPrimary(poolIds, volume,CollectionUtils.isEmpty(poolIds));
         if (volume == null) {
             throw new InvalidParameterValueException("Creating snapshot failed due to volume:" + volumeId + " doesn't exist");
         }
@@ -3943,7 +3950,7 @@ private boolean isOperationSupported(VMTemplateVO template, UserVmVO userVm) {
 
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_SNAPSHOT_CREATE, eventDescription = "allocating snapshot", create = true)
-    public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType, List<Long> zoneIds, List<Long> poolIds) throws ResourceAllocationException {
+    public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType, List<Long> zoneIds, Boolean useStorageReplication) throws ResourceAllocationException {
         Account caller = CallContext.current().getCallingAccount();
 
         VolumeInfo volume = volFactory.getVolume(volumeId);
@@ -3992,7 +3999,7 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName,
         if (storagePool == null) {
             throw new InvalidParameterValueException(String.format("Volume: %s please attach this volume to a VM before create snapshot for it", volume.getVolume()));
         }
-        boolean canCopyOnPrimary = canCopyOnPrimary(poolIds, volume, CollectionUtils.isEmpty(poolIds));
+        boolean canCopyOnPrimary = useStorageReplication;
 
         if (CollectionUtils.isNotEmpty(zoneIds)) {
             if (policyId != null && policyId > 0) {
diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java
@@ -61,6 +61,8 @@ public interface SnapshotManager extends Configurable {
     ConfigKey<Integer> snapshotDeltaMax = new ConfigKey<>(Integer.class, "snapshot.delta.max", "Snapshots", "16", "Max delta snapshots between two full snapshots. " +
             "Only valid for KVM and XenServer.", true, ConfigKey.Scope.Global, null);
 
+    public static final ConfigKey<Boolean> UseStorageReplication = new ConfigKey<Boolean>(Boolean.class, "use.storage.replication", "Snapshots", "false", "For snapshot copy to another primary storage in a different zone. Supports only StorPool storage for now", true, ConfigKey.Scope.StoragePool, null);
+
     void deletePoliciesForVolume(Long volumeId);
 
     /**
diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
diff --git a/server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java b/server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java
diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
diff --git a/test/integration/plugins/storpool/test_snapshot_copy_on_primary_storage.py b/test/integration/plugins/storpool/test_snapshot_copy_on_primary_storage.py
diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py

Original file line number	Diff line number	Diff line change
`@@ -168,4 +168,7 @@ Pair<List<Long>, Integer> searchForIdsAndCount(Long storagePoolId, String storag`
`168`	`168`	`List<StoragePoolVO> listByIds(List<Long> ids);`
`169`	`169`
`170`	`170`	`List<StoragePoolVO> findStoragePoolsByEmptyStorageAccessGroups(Long dcId, Long podId, Long clusterId, ScopeType scope, HypervisorType hypervisorType);`
	`171`	`+`
	`172`	`+ List<StoragePoolVO> findPoolsByStorageTypeAndZone(Storage.StoragePoolType storageType, Long zoneId);`
	`173`	`+`
`171`	`174`	`}`
Original file line number	Diff line number	Diff line change
`@@ -364,7 +364,7 @@ private boolean deleteSnapshotFromDbIfNeeded(SnapshotVO snapshotVO, Long zoneId)`
`364`	`364`	`!Snapshot.State.Destroying.equals(snapshotVO.getState())) {`
`365`	`365`	`throw new InvalidParameterValueException(String.format("Can't delete snapshot %s due to it is in %s Status", snapshotVO, snapshotVO.getState()));`
`366`	`366`	`}`
`367`		`- List<SnapshotDataStoreVO> storeRefs = _snapshotStoreDao.listBySnapshot(snapshotId, DataStoreRole.Image);`
	`367`	`+ List<SnapshotDataStoreVO> storeRefs = _snapshotStoreDao.listBySnapshotAndDataStoreRole(snapshotId, DataStoreRole.Image);`
`368`	`368`	`if (zoneId != null) {`
`369`	`369`	`storeRefs.removeIf(ref -> !zoneId.equals(dataStoreMgr.getStoreZoneId(ref.getDataStoreId(), ref.getRole())));`
`370`	`370`	`}`