DaanHoogland commented on code in PR #9478:
URL: https://github.com/apache/cloudstack/pull/9478#discussion_r1819250296
##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java:
##########
@@ -177,6 +107,74 @@
import com.cloud.vm.dao.UserVmCloneSettingDao;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.UserVmDetailsDao;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy;
+import org.apache.cloudstack.api.command.admin.vm.MigrateVMCmd;
+import org.apache.cloudstack.api.command.admin.volume.MigrateVolumeCmdByAdmin;
+import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd;
+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.DataStoreDriver;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
+import
org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver;
+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.StoragePoolAllocator;
+import
org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService;
+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.ConfigDepot;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.jobs.AsyncJobManager;
+import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
+import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
+import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
+import org.apache.cloudstack.secret.PassphraseVO;
+import org.apache.cloudstack.secret.dao.PassphraseDao;
+import org.apache.cloudstack.snapshot.SnapshotHelper;
+import org.apache.cloudstack.storage.command.CommandResult;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
+import
org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+import org.apache.commons.collections.CollectionUtils;
Review Comment:
```suggestion
import org.apache.commons.collections.CollectionUtils;
```
##########
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java:
##########
@@ -782,4 +781,35 @@ public AsyncCallFuture<CreateCmdResult>
queryCopySnapshot(SnapshotInfo snapshot)
ep.sendMessageAsync(cmd, caller);
return future;
}
+
+ public AsyncCallFuture<SnapshotResult> copySnapshot(SnapshotInfo
sourceSnapshot, SnapshotInfo destSnapshot, SnapshotStrategy strategy) {
+ try {
+ if (destSnapshot.getStatus() ==
ObjectInDataStoreStateMachine.State.Allocated) {
+ destSnapshot.processEvent(Event.CreateOnlyRequested);
+ } else if (sourceSnapshot.getStatus() ==
ObjectInDataStoreStateMachine.State.Ready) {
+ destSnapshot.processEvent(Event.CopyRequested);
+ } else {
+ logger.info(String.format("Cannot copy snapshot to another
storage in different zone. It's not in the right state %s",
sourceSnapshot.getStatus()));
+ sourceSnapshot.processEvent(Event.OperationFailed);
+ throw new CloudRuntimeException(String.format("Cannot copy
snapshot to another storage in different zone. It's not in the right state %s",
sourceSnapshot.getStatus()));
+ }
+ } catch (Exception e) {
+ logger.debug("Failed to change snapshot state: " + e.toString());
+ sourceSnapshot.processEvent(Event.OperationFailed);
+ throw new CloudRuntimeException(e);
+ }
+
+ AsyncCallFuture<SnapshotResult> future = new
AsyncCallFuture<SnapshotResult>();
+ try {
+ CopySnapshotContext<CommandResult> context = new
CopySnapshotContext<>(null, sourceSnapshot, destSnapshot, future);
+ AsyncCallbackDispatcher<SnapshotServiceImpl, CreateCmdResult>
caller = AsyncCallbackDispatcher.create(this);
+
caller.setCallback(caller.getTarget().copySnapshotZoneAsyncCallback(null,
null)).setContext(context);
+ strategy.copySnapshot(sourceSnapshot, destSnapshot, caller);
+ } catch (Exception e) {
+ logger.debug("Failed to take snapshot: " + destSnapshot.getId(),
e);
+ destSnapshot.processEvent(Event.OperationFailed);
+ throw new CloudRuntimeException("Failed to copy snapshot" +
destSnapshot.getId());
+ }
Review Comment:
aren't these a `prepareSomeThing()` and a `planSomeThing()` methoid packed
in one method body?
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -122,9 +71,60 @@
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.VirtualMachineManager;
import com.cloud.vm.dao.VMInstanceDao;
+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.CreateCmdResult;
+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.ObjectInDataStoreStateMachine;
+import
org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
+import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
+import org.apache.cloudstack.storage.RemoteHostEndPoint;
+import org.apache.cloudstack.storage.command.CommandResult;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.command.CreateObjectAnswer;
+import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
+import org.apache.cloudstack.storage.datastore.api.StorPoolSnapshotDef;
+import org.apache.cloudstack.storage.datastore.api.StorPoolVolumeDef;
+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.StoragePoolDetailVO;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
+import org.apache.cloudstack.storage.datastore.util.StorPoolHelper;
+import org.apache.cloudstack.storage.datastore.util.StorPoolUtil;
+import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpApiResponse;
+import
org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpConnectionDesc;
+import org.apache.cloudstack.storage.snapshot.StorPoolConfigurationManager;
+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.cloudstack.storage.volume.VolumeObject;
+import org.apache.commons.collections4.CollectionUtils;
Review Comment:
```suggestion
import org.apache.commons.collections4.CollectionUtils;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java:
##########
@@ -16,47 +16,55 @@
// under the License.
package org.apache.cloudstack.storage.snapshot;
-import java.util.ArrayList;
-import java.util.List;
-
-import javax.inject.Inject;
-
+import com.cloud.api.query.dao.SnapshotJoinDao;
+import com.cloud.api.query.vo.SnapshotJoinVO;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.hypervisor.kvm.storage.StorPoolStorageAdaptor;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.Storage;
+import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.dao.SnapshotDetailsDao;
+import com.cloud.storage.dao.SnapshotDetailsVO;
+import com.cloud.storage.dao.SnapshotZoneDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.NoTransitionException;
+import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
+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.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.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.StrategyPriority;
+import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
+import org.apache.cloudstack.storage.command.CreateObjectAnswer;
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.StoragePoolDetailVO;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.datastore.util.StorPoolHelper;
import org.apache.cloudstack.storage.datastore.util.StorPoolUtil;
import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpApiResponse;
import
org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpConnectionDesc;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
Review Comment:
```suggestion
import org.apache.cloudstack.storage.to.SnapshotObjectTO;
```
##########
server/src/main/java/com/cloud/api/ApiDBUtils.java:
##########
@@ -361,6 +279,86 @@
import com.cloud.vm.dao.VMInstanceDao;
import com.cloud.vm.snapshot.VMSnapshot;
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import org.apache.cloudstack.acl.Role;
Review Comment:
```suggestion
import org.apache.cloudstack.acl.Role;
```
##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java:
##########
@@ -177,6 +107,74 @@
import com.cloud.vm.dao.UserVmCloneSettingDao;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.UserVmDetailsDao;
+import org.apache.cloudstack.api.ApiCommandResourceType;
Review Comment:
```suggestion
import org.apache.cloudstack.api.ApiCommandResourceType;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/collector/StorPoolAbandonObjectsCollector.java:
##########
@@ -50,14 +31,42 @@
import com.cloud.utils.db.TransactionStatus;
import com.google.gson.JsonArray;
import com.google.gson.JsonObject;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+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.util.StorPoolHelper;
+import org.apache.cloudstack.storage.datastore.util.StorPoolUtil;
+import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpApiResponse;
+import
org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpConnectionDesc;
+import org.apache.commons.collections.CollectionUtils;
Review Comment:
```suggestion
import org.apache.commons.collections.CollectionUtils;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/collector/StorPoolAbandonObjectsCollector.java:
##########
@@ -322,4 +340,76 @@ private Map<String,String>
getStorPoolNamesAndCsTag(JsonArray arr) {
}
return map;
}
+
+ class StorPoolSnapshotRecoveryCheck extends ManagedContextRunnable {
+
+ @Override
+ protected void runInContext() {
Review Comment:
i'd disect this a little further as well.
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/collector/StorPoolAbandonObjectsCollector.java:
##########
@@ -50,14 +31,42 @@
import com.cloud.utils.db.TransactionStatus;
import com.google.gson.JsonArray;
import com.google.gson.JsonObject;
+import org.apache.cloudstack.framework.config.ConfigKey;
Review Comment:
```suggestion
import org.apache.cloudstack.framework.config.ConfigKey;
```
##########
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java:
##########
@@ -17,11 +17,24 @@
package org.apache.cloudstack.storage.snapshot;
-import java.util.List;
-import java.util.concurrent.ExecutionException;
-
-import javax.inject.Inject;
-
+import com.cloud.agent.api.Answer;
+import com.cloud.configuration.Config;
+import com.cloud.dc.DataCenter;
+import com.cloud.event.EventTypes;
+import com.cloud.event.UsageEventUtils;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.storage.CreateSnapshotPayload;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.dao.SnapshotDetailsDao;
+import com.cloud.storage.template.TemplateConstants;
+import com.cloud.utils.db.Transaction;
+import com.cloud.utils.db.TransactionCallbackNoReturn;
+import com.cloud.utils.db.TransactionStatus;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.NoTransitionException;
Review Comment:
```suggestion
import com.cloud.utils.fsm.NoTransitionException;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -122,9 +71,60 @@
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.VirtualMachineManager;
import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
Review Comment:
```suggestion
import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
```
##########
engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java:
##########
@@ -46,6 +28,22 @@
import com.cloud.utils.db.SearchCriteria;
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.commons.collections.CollectionUtils;
Review Comment:
```suggestion
import org.apache.commons.collections.CollectionUtils;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/collector/StorPoolAbandonObjectsCollector.java:
##########
@@ -102,6 +116,10 @@ private void init() {
_volumeTagsUpdateExecutor.scheduleAtFixedRate(new
StorPoolSnapshotsTagsUpdate(),
snapshotCheckupTagsInterval.value(),
snapshotCheckupTagsInterval.value(), TimeUnit.SECONDS);
}
+ if (snapshotRecoveryFromRemoteCheck.value() > 0) {
+ snapshotRecoveryCheckExecutor.scheduleAtFixedRate(new
StorPoolSnapshotRecoveryCheck(),
+ snapshotRecoveryFromRemoteCheck.value(),
snapshotRecoveryFromRemoteCheck.value(), TimeUnit.SECONDS);
+ }
Review Comment:
extra method?
##########
server/src/main/java/com/cloud/template/TemplateManagerImpl.java:
##########
@@ -220,6 +125,99 @@
import com.google.common.base.Joiner;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
+import org.apache.cloudstack.acl.SecurityChecker.AccessType;
Review Comment:
```suggestion
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolHelper.java:
##########
@@ -64,6 +45,26 @@
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
+import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpApiResponse;
+import org.apache.cloudstack.storage.snapshot.StorPoolConfigurationManager;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.commons.collections4.CollectionUtils;
Review Comment:
```suggestion
import org.apache.commons.collections4.CollectionUtils;
```
##########
server/src/main/java/com/cloud/api/query/dao/SnapshotJoinDaoImpl.java:
##########
@@ -49,6 +32,21 @@
import com.cloud.utils.db.SearchBuilder;
import com.cloud.utils.db.SearchCriteria;
import com.cloud.vm.VMInstanceVO;
+import org.apache.cloudstack.annotation.AnnotationService;
Review Comment:
```suggestion
import org.apache.cloudstack.annotation.AnnotationService;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolHelper.java:
##########
@@ -64,6 +45,26 @@
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
Review Comment:
```suggestion
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -618,88 +644,77 @@ public void copyAsync(DataObject srcData, DataObject
dstData, AsyncCompletionCal
try {
if (srcType == DataObjectType.SNAPSHOT && dstType ==
DataObjectType.VOLUME) {
SnapshotInfo sinfo = (SnapshotInfo)srcData;
- final String snapshotName =
StorPoolHelper.getSnapshotName(srcData.getId(), srcData.getUuid(),
snapshotDataStoreDao, snapshotDetailsDao);
-
VolumeInfo vinfo = (VolumeInfo)dstData;
final String volumeName = vinfo.getUuid();
final Long size = vinfo.getSize();
SpConnectionDesc conn =
StorPoolUtil.getSpConnection(vinfo.getDataStore().getUuid(),
vinfo.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
- SpApiResponse resp = StorPoolUtil.volumeCreate(volumeName,
snapshotName, size, null, null, "volume", sinfo.getBaseVolume().getMaxIops(),
conn);
- if (resp.getError() == null) {
- updateStoragePool(dstData.getDataStore().getId(), size);
Review Comment:
not going these next lines with comments ;)
##########
server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java:
##########
@@ -107,6 +56,56 @@
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.ExtractSnapshotCmd;
Review Comment:
```suggestion
import org.apache.cloudstack.api.command.user.snapshot.ExtractSnapshotCmd;
```
##########
server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java:
##########
@@ -62,6 +38,29 @@
import com.cloud.user.ResourceLimitService;
import com.cloud.user.dao.AccountDao;
import com.cloud.utils.Pair;
+import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
+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.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.SnapshotService;
+import org.apache.cloudstack.framework.async.AsyncCallFuture;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
+import org.junit.Assert;
Review Comment:
```suggestion
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
import org.junit.Assert;
```
##########
server/src/main/java/com/cloud/api/query/dao/SnapshotJoinDao.java:
##########
@@ -38,4 +37,6 @@ public interface SnapshotJoinDao extends
GenericDao<SnapshotJoinVO, Long> {
List<SnapshotJoinVO> searchBySnapshotStorePair(String... pairs);
List<SnapshotJoinVO> findByDistinctIds(Long zoneId, Long... ids);
+
+ List<SnapshotJoinVO> listBySnapshotIdAndZoneId(Long zoneId, Long id);
Review Comment:
see below
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -514,6 +516,15 @@ public void deleteAsync(DataStore dataStore, DataObject
data, AsyncCompletionCal
} catch (Exception e) {
err = String.format("Could not delete volume due to %s",
e.getMessage());
}
+ } else if (data.getType() == DataObjectType.SNAPSHOT) {
+ SnapshotInfo snapshot = (SnapshotInfo) data;
+ SpConnectionDesc conn =
StorPoolUtil.getSpConnection(snapshot.getDataStore().getUuid(),
snapshot.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
+ String name =
StorPoolStorageAdaptor.getVolumeNameFromPath(snapshot.getPath(), true);
+ SpApiResponse resp = StorPoolUtil.snapshotDelete(name, conn);
+ if (resp.getError() != null) {
+ err = String.format("Failed to clean-up Storpool snapshot %s.
Error: %s", name, resp.getError());
+ StorPoolUtil.spLog(err);
+ }
Review Comment:
new method `handleSnapShotDeletion()` ?
##########
engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java:
##########
@@ -46,6 +28,22 @@
import com.cloud.utils.db.SearchCriteria;
import com.cloud.utils.db.TransactionLegacy;
import com.cloud.utils.db.UpdateBuilder;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataObjectInStore;
Review Comment:
```suggestion
import org.apache.cloudstack.engine.subsystem.api.storage.DataObjectInStore;
```
##########
server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java:
##########
@@ -107,6 +56,56 @@
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.ExtractSnapshotCmd;
+import org.apache.cloudstack.context.CallContext;
+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.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.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.framework.config.ConfigKey;
+import org.apache.cloudstack.snapshot.SnapshotHelper;
+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.cloudstack.storage.image.datastore.ImageStoreEntity;
+import org.junit.After;
Review Comment:
```suggestion
import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
import org.junit.After;
```
##########
server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java:
##########
@@ -19,15 +19,17 @@
package org.apache.cloudstack.snapshot;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.stream.Collectors;
-
-import javax.inject.Inject;
-
+import com.cloud.api.query.dao.SnapshotJoinDao;
+import com.cloud.api.query.vo.SnapshotJoinVO;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.Storage.StoragePoolType;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.utils.exception.CloudRuntimeException;
Review Comment:
```suggestion
import com.cloud.utils.exception.CloudRuntimeException;
```
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -235,6 +131,109 @@
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonParseException;
+import org.apache.cloudstack.api.ApiConstants;
Review Comment:
```suggestion
import org.apache.cloudstack.api.ApiConstants;
```
##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -344,6 +194,154 @@
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.UserVmDetailsDao;
import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.acl.ControlledEntity;
Review Comment:
```suggestion
import org.apache.cloudstack.acl.ControlledEntity;
```
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -235,6 +131,109 @@
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonParseException;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.InternalIdentity;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.volume.AssignVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
+import
org.apache.cloudstack.api.command.user.volume.ChangeOfferingForVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.CheckAndRepairVolumeCmd;
+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.backup.Backup;
+import org.apache.cloudstack.backup.BackupManager;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.direct.download.DirectDownloadHelper;
+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.DataStoreCapabilities;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver;
+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.ObjectInDataStoreStateMachine;
+import
org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver;
+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.SnapshotInfo;
+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.Configurable;
+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.resourcedetail.DiskOfferingDetailVO;
+import org.apache.cloudstack.resourcedetail.SnapshotPolicyDetailVO;
+import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
+import org.apache.cloudstack.resourcedetail.dao.SnapshotPolicyDetailsDao;
+import org.apache.cloudstack.snapshot.SnapshotHelper;
+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.ImageStoreDao;
+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.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.jsinterpreter.TagAsRuleHelper;
+import
org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
+import org.apache.commons.collections.CollectionUtils;
Review Comment:
```suggestion
import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
import org.apache.commons.collections.CollectionUtils;
```
##########
server/src/main/java/com/cloud/template/TemplateManagerImpl.java:
##########
@@ -1671,18 +1671,33 @@ public VirtualMachineTemplate
createPrivateTemplate(CreateTemplateCmd command) t
AsyncCallFuture<TemplateApiResult> future = null;
if (snapshotId != null) {
- DataStoreRole dataStoreRole =
snapshotHelper.getDataStoreRole(snapshot);
- kvmSnapshotOnlyInPrimaryStorage =
snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole);
+ DataStoreRole dataStoreRole =
snapshotHelper.getDataStoreRole(snapshot, zoneId);
+ kvmSnapshotOnlyInPrimaryStorage =
snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole,
zoneId);
snapInfo =
_snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId);
- if (dataStoreRole == DataStoreRole.Image ||
kvmSnapshotOnlyInPrimaryStorage) {
+ boolean skipCopyToSecondary = false;
+ boolean keepOnPrimary =
snapshotHelper.isStorPoolStorage(snapInfo);
+ if (kvmSnapshotOnlyInPrimaryStorage && keepOnPrimary) {
+ skipCopyToSecondary = true;
+ }
+ if (dataStoreRole == DataStoreRole.Image ||
!skipCopyToSecondary) {
snapInfo =
snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo,
dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage);
_accountMgr.checkAccess(caller, null, true, snapInfo);
DataStore snapStore = snapInfo.getDataStore();
if (snapStore != null) {
store = snapStore; // pick snapshot image store to
create template
}
+ } else if (keepOnPrimary) {
+ ImageStoreVO imageStore =
_imgStoreDao.findOneByZoneAndProtocol(zoneId, "nfs");
+ if (imageStore == null) {
+ throw new CloudRuntimeException(String.format("Could
not find an NFS secondary storage pool on zone %s to use as a temporary
location " +
+ "for instance conversion", zoneId));
+ }
+ DataStore dataStore =
_dataStoreMgr.getDataStore(imageStore.getId(), DataStoreRole.Image);
+ if (dataStore != null) {
+ store = dataStore;
+ }
Review Comment:
factor out?
##########
server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java:
##########
@@ -1987,6 +2103,61 @@ public Snapshot copySnapshot(CopySnapshotCmd cmd) throws
StorageUnavailableExcep
}
}
+ private boolean canCopyOnPrimary(List<Long> poolIds, Snapshot snapshot) {
+ List<Long> poolsToBeRemoved = new ArrayList<>();
+ for (Long poolId : poolIds) {
+ PrimaryDataStore dataStore = (PrimaryDataStore)
dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
+ if (dataStore == null) {
+ poolsToBeRemoved.add(poolId);
+ continue;
+ }
+ SnapshotInfo snapshotInfo =
snapshotFactory.getSnapshot(snapshot.getId(), poolId, DataStoreRole.Primary);
+ if (snapshotInfo != null) {
+ logger.debug(String.format("Snapshot [%s] already exist on
pool [%s]", snapshot.getUuid(), dataStore.getName()));
+ continue;
+ }
+
+ VolumeVO volume = _volsDao.findById(snapshot.getVolumeId());
+ if (volume == null) {
+ poolsToBeRemoved.add(poolId);
+ continue;
+ }
+ if (!dataStore.getDriver().getCapabilities()
+
.containsKey(DataStoreCapabilities.CAN_COPY_SNAPSHOT_BETWEEN_ZONES_AND_SAME_POOL_TYPE.toString())
+ && dataStore.getPoolType() != volume.getPoolType()) {
+ poolsToBeRemoved.add(poolId);
+ logger.debug(String.format("The %s does not support copy to
%s between zones", dataStore.getPoolType(), volume.getPoolType()));
+ }
+ }
+ poolIds.removeAll(poolsToBeRemoved);
+ if (CollectionUtils.isEmpty(poolIds)) {
+ return false;
+ }
+ return true;
+ }
Review Comment:
this can be further distilled into smaller methods. Surely some of those can
than be reused for the actual copy method.
##########
server/src/main/java/com/cloud/api/ApiResponseHelper.java:
##########
@@ -16,39 +16,210 @@
// under the License.
package com.cloud.api;
-import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
-
-import java.security.cert.Certificate;
-import java.security.cert.CertificateException;
-import java.text.DecimalFormat;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Calendar;
-import java.util.Date;
-import java.util.EnumSet;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
-import java.util.TimeZone;
-import java.util.function.Consumer;
-import java.util.stream.Collectors;
-
-import javax.inject.Inject;
-
+import com.cloud.agent.api.VgpuTypesInfo;
+import com.cloud.api.query.ViewResponseHelper;
+import com.cloud.api.query.dao.UserVmJoinDao;
+import com.cloud.api.query.vo.AccountJoinVO;
+import com.cloud.api.query.vo.AsyncJobJoinVO;
+import com.cloud.api.query.vo.ControlledViewEntity;
+import com.cloud.api.query.vo.DataCenterJoinVO;
+import com.cloud.api.query.vo.DiskOfferingJoinVO;
+import com.cloud.api.query.vo.DomainRouterJoinVO;
+import com.cloud.api.query.vo.EventJoinVO;
+import com.cloud.api.query.vo.HostJoinVO;
+import com.cloud.api.query.vo.ImageStoreJoinVO;
+import com.cloud.api.query.vo.InstanceGroupJoinVO;
+import com.cloud.api.query.vo.NetworkOfferingJoinVO;
+import com.cloud.api.query.vo.ProjectAccountJoinVO;
+import com.cloud.api.query.vo.ProjectInvitationJoinVO;
+import com.cloud.api.query.vo.ProjectJoinVO;
+import com.cloud.api.query.vo.ResourceTagJoinVO;
+import com.cloud.api.query.vo.SecurityGroupJoinVO;
+import com.cloud.api.query.vo.ServiceOfferingJoinVO;
+import com.cloud.api.query.vo.StoragePoolJoinVO;
+import com.cloud.api.query.vo.TemplateJoinVO;
+import com.cloud.api.query.vo.UserAccountJoinVO;
+import com.cloud.api.query.vo.UserVmJoinVO;
+import com.cloud.api.query.vo.VolumeJoinVO;
+import com.cloud.api.query.vo.VpcOfferingJoinVO;
+import com.cloud.api.response.ApiResponseSerializer;
import com.cloud.bgp.ASNumber;
import com.cloud.bgp.ASNumberRange;
+import com.cloud.capacity.Capacity;
+import com.cloud.capacity.CapacityVO;
+import com.cloud.capacity.dao.CapacityDaoImpl.SummedCapacity;
+import com.cloud.configuration.ConfigurationManager;
+import com.cloud.configuration.Resource.ResourceOwnerType;
+import com.cloud.configuration.Resource.ResourceType;
+import com.cloud.configuration.ResourceCount;
+import com.cloud.configuration.ResourceLimit;
import com.cloud.dc.ASNumberRangeVO;
import com.cloud.dc.ASNumberVO;
+import com.cloud.dc.ClusterDetailsDao;
+import com.cloud.dc.ClusterVO;
+import com.cloud.dc.DataCenter;
+import com.cloud.dc.DataCenterGuestIpv6Prefix;
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.HostPodVO;
+import com.cloud.dc.Pod;
+import com.cloud.dc.StorageNetworkIpRange;
+import com.cloud.dc.Vlan;
+import com.cloud.dc.Vlan.VlanType;
import com.cloud.dc.VlanDetailsVO;
+import com.cloud.dc.VlanVO;
import com.cloud.dc.dao.ASNumberDao;
import com.cloud.dc.dao.ASNumberRangeDao;
import com.cloud.dc.dao.VlanDetailsDao;
+import com.cloud.domain.Domain;
+import com.cloud.domain.DomainVO;
+import com.cloud.event.Event;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.PermissionDeniedException;
+import com.cloud.gpu.GPU;
+import com.cloud.host.ControlState;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
import com.cloud.hypervisor.Hypervisor;
+import com.cloud.hypervisor.HypervisorCapabilities;
+import com.cloud.network.GuestVlan;
+import com.cloud.network.GuestVlanRange;
+import com.cloud.network.IpAddress;
+import com.cloud.network.Ipv6Service;
+import com.cloud.network.Network;
+import com.cloud.network.Network.Capability;
+import com.cloud.network.Network.Provider;
+import com.cloud.network.Network.Service;
+import com.cloud.network.NetworkModel;
+import com.cloud.network.NetworkPermission;
+import com.cloud.network.NetworkProfile;
+import com.cloud.network.Networks.BroadcastDomainType;
+import com.cloud.network.Networks.IsolationType;
+import com.cloud.network.Networks.TrafficType;
+import com.cloud.network.OvsProvider;
+import com.cloud.network.PhysicalNetwork;
+import com.cloud.network.PhysicalNetworkServiceProvider;
+import com.cloud.network.PhysicalNetworkTrafficType;
+import com.cloud.network.PublicIpQuarantine;
+import com.cloud.network.RemoteAccessVpn;
+import com.cloud.network.RouterHealthCheckResult;
+import com.cloud.network.Site2SiteCustomerGateway;
+import com.cloud.network.Site2SiteVpnConnection;
+import com.cloud.network.Site2SiteVpnGateway;
+import com.cloud.network.VirtualRouterProvider;
+import com.cloud.network.VpnUser;
+import com.cloud.network.VpnUserVO;
+import com.cloud.network.as.AutoScalePolicy;
+import com.cloud.network.as.AutoScaleVmGroup;
+import com.cloud.network.as.AutoScaleVmProfile;
+import com.cloud.network.as.AutoScaleVmProfileVO;
+import com.cloud.network.as.Condition;
+import com.cloud.network.as.ConditionVO;
+import com.cloud.network.as.Counter;
+import com.cloud.network.dao.FirewallRulesDao;
+import com.cloud.network.dao.IPAddressDao;
+import com.cloud.network.dao.IPAddressVO;
+import com.cloud.network.dao.LoadBalancerVO;
+import com.cloud.network.dao.NetworkDao;
+import com.cloud.network.dao.NetworkDetailVO;
+import com.cloud.network.dao.NetworkDetailsDao;
+import com.cloud.network.dao.NetworkServiceMapDao;
+import com.cloud.network.dao.NetworkVO;
+import com.cloud.network.dao.PhysicalNetworkVO;
+import com.cloud.network.router.VirtualRouter;
+import com.cloud.network.rules.FirewallRule;
+import com.cloud.network.rules.HealthCheckPolicy;
+import com.cloud.network.rules.LoadBalancer;
+import com.cloud.network.rules.LoadBalancerContainer.Scheme;
+import com.cloud.network.rules.PortForwardingRule;
+import com.cloud.network.rules.PortForwardingRuleVO;
+import com.cloud.network.rules.StaticNatRule;
+import com.cloud.network.rules.StickinessPolicy;
+import com.cloud.network.security.SecurityGroup;
+import com.cloud.network.security.SecurityGroupVO;
+import com.cloud.network.security.SecurityRule;
+import com.cloud.network.security.SecurityRule.SecurityRuleType;
+import com.cloud.network.vpc.NetworkACL;
+import com.cloud.network.vpc.NetworkACLItem;
+import com.cloud.network.vpc.PrivateGateway;
+import com.cloud.network.vpc.StaticRoute;
+import com.cloud.network.vpc.Vpc;
+import com.cloud.network.vpc.VpcOffering;
+import com.cloud.network.vpc.VpcVO;
+import com.cloud.network.vpc.dao.VpcOfferingDao;
+import com.cloud.offering.DiskOffering;
+import com.cloud.offering.NetworkOffering;
+import com.cloud.offering.NetworkOffering.Detail;
+import com.cloud.offering.ServiceOffering;
+import com.cloud.offerings.NetworkOfferingVO;
+import com.cloud.offerings.dao.NetworkOfferingDao;
+import com.cloud.org.Cluster;
+import com.cloud.projects.Project;
+import com.cloud.projects.ProjectAccount;
+import com.cloud.projects.ProjectInvitation;
+import com.cloud.region.ha.GlobalLoadBalancerRule;
+import com.cloud.resource.RollingMaintenanceManager;
+import com.cloud.server.ResourceIcon;
+import com.cloud.server.ResourceTag;
+import com.cloud.server.ResourceTag.ResourceObjectType;
+import com.cloud.service.ServiceOfferingVO;
import com.cloud.storage.BucketVO;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.DiskOfferingVO;
+import com.cloud.storage.GuestOS;
+import com.cloud.storage.GuestOSCategoryVO;
+import com.cloud.storage.GuestOSHypervisor;
+import com.cloud.storage.GuestOsCategory;
+import com.cloud.storage.ImageStore;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.Upload;
+import com.cloud.storage.VMTemplateVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.GuestOSCategoryDao;
+import com.cloud.storage.dao.GuestOSDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.storage.snapshot.SnapshotPolicy;
+import com.cloud.storage.snapshot.SnapshotSchedule;
+import com.cloud.tags.dao.ResourceTagDao;
+import com.cloud.template.VirtualMachineTemplate;
+import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
+import com.cloud.user.SSHKeyPair;
+import com.cloud.user.User;
+import com.cloud.user.UserAccount;
+import com.cloud.user.UserData;
+import com.cloud.user.UserStatisticsVO;
+import com.cloud.user.dao.UserDataDao;
+import com.cloud.user.dao.UserStatisticsDao;
+import com.cloud.uservm.UserVm;
+import com.cloud.utils.Pair;
+import com.cloud.utils.crypt.DBEncryptionUtil;
+import com.cloud.utils.db.EntityManager;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.db.SearchCriteria.Op;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.Dhcp;
+import com.cloud.utils.net.Ip;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.security.CertificateHelper;
+import com.cloud.vm.ConsoleProxyVO;
+import com.cloud.vm.InstanceGroup;
+import com.cloud.vm.Nic;
+import com.cloud.vm.NicExtraDhcpOptionVO;
+import com.cloud.vm.NicProfile;
+import com.cloud.vm.NicSecondaryIp;
+import com.cloud.vm.NicVO;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.VirtualMachine.Type;
+import com.cloud.vm.dao.NicExtraDhcpOptionDao;
+import com.cloud.vm.dao.NicSecondaryIpVO;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
Review Comment:
```suggestion
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java:
##########
@@ -16,47 +16,55 @@
// under the License.
package org.apache.cloudstack.storage.snapshot;
-import java.util.ArrayList;
-import java.util.List;
-
-import javax.inject.Inject;
-
+import com.cloud.api.query.dao.SnapshotJoinDao;
+import com.cloud.api.query.vo.SnapshotJoinVO;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.hypervisor.kvm.storage.StorPoolStorageAdaptor;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.Storage;
+import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.dao.SnapshotDetailsDao;
+import com.cloud.storage.dao.SnapshotDetailsVO;
+import com.cloud.storage.dao.SnapshotZoneDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.NoTransitionException;
+import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
Review Comment:
```suggestion
import com.cloud.utils.fsm.NoTransitionException;
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/motion/StorPoolDataMotionStrategy.java:
##########
@@ -55,48 +84,18 @@
import org.apache.cloudstack.storage.datastore.util.StorPoolUtil;
import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpApiResponse;
import
org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpConnectionDesc;
-import org.apache.cloudstack.storage.snapshot.StorPoolConfigurationManager;
import org.apache.cloudstack.storage.to.SnapshotObjectTO;
import org.apache.cloudstack.storage.to.TemplateObjectTO;
import org.apache.commons.collections.MapUtils;
Review Comment:
```suggestion
import org.apache.commons.collections.MapUtils;
```
##########
server/src/main/java/com/cloud/template/TemplateManagerImpl.java:
##########
@@ -220,6 +125,99 @@
import com.google.common.base.Joiner;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
+import org.apache.cloudstack.acl.SecurityChecker.AccessType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.BaseListTemplateOrIsoPermissionsCmd;
+import org.apache.cloudstack.api.BaseUpdateTemplateOrIsoCmd;
+import org.apache.cloudstack.api.BaseUpdateTemplateOrIsoPermissionsCmd;
+import org.apache.cloudstack.api.command.user.iso.DeleteIsoCmd;
+import org.apache.cloudstack.api.command.user.iso.ExtractIsoCmd;
+import org.apache.cloudstack.api.command.user.iso.GetUploadParamsForIsoCmd;
+import org.apache.cloudstack.api.command.user.iso.ListIsoPermissionsCmd;
+import org.apache.cloudstack.api.command.user.iso.RegisterIsoCmd;
+import org.apache.cloudstack.api.command.user.iso.UpdateIsoCmd;
+import org.apache.cloudstack.api.command.user.iso.UpdateIsoPermissionsCmd;
+import org.apache.cloudstack.api.command.user.template.CopyTemplateCmd;
+import org.apache.cloudstack.api.command.user.template.CreateTemplateCmd;
+import org.apache.cloudstack.api.command.user.template.DeleteTemplateCmd;
+import org.apache.cloudstack.api.command.user.template.ExtractTemplateCmd;
+import
org.apache.cloudstack.api.command.user.template.GetUploadParamsForTemplateCmd;
+import
org.apache.cloudstack.api.command.user.template.ListTemplatePermissionsCmd;
+import org.apache.cloudstack.api.command.user.template.RegisterTemplateCmd;
+import org.apache.cloudstack.api.command.user.template.RegisterVnfTemplateCmd;
+import org.apache.cloudstack.api.command.user.template.UpdateTemplateCmd;
+import
org.apache.cloudstack.api.command.user.template.UpdateTemplatePermissionsCmd;
+import org.apache.cloudstack.api.command.user.template.UpdateVnfTemplateCmd;
+import
org.apache.cloudstack.api.command.user.userdata.LinkUserDataToTemplateCmd;
+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.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.EndPointSelector;
+import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
+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.StorageCacheManager;
+import
org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService;
+import
org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult;
+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.async.AsyncCallFuture;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.messagebus.MessageBus;
+import org.apache.cloudstack.framework.messagebus.PublishScope;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.secstorage.dao.SecondaryStorageHeuristicDao;
+import org.apache.cloudstack.secstorage.heuristics.HeuristicType;
+import org.apache.cloudstack.snapshot.SnapshotHelper;
+import org.apache.cloudstack.storage.command.AttachCommand;
+import org.apache.cloudstack.storage.command.CommandResult;
+import org.apache.cloudstack.storage.command.DettachCommand;
+import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
+import org.apache.cloudstack.storage.heuristics.HeuristicRuleHelper;
+import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
+import org.apache.cloudstack.storage.template.VnfTemplateManager;
+import org.apache.cloudstack.storage.template.VnfTemplateUtils;
+import org.apache.cloudstack.storage.to.TemplateObjectTO;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
+import org.apache.commons.collections.CollectionUtils;
Review Comment:
```suggestion
import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
import org.apache.commons.collections.CollectionUtils;
```
##########
server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java:
##########
@@ -174,6 +108,73 @@
import com.cloud.vm.snapshot.VMSnapshotVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
+import org.apache.cloudstack.acl.SecurityChecker;
+import org.apache.cloudstack.annotation.AnnotationService;
+import org.apache.cloudstack.annotation.dao.AnnotationDao;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.command.user.snapshot.CopySnapshotCmd;
+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.ExtractSnapshotCmd;
+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.CreateCmdResult;
+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.ObjectInDataStoreStateMachine;
+import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
+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.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.async.AsyncCallFuture;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.resourcedetail.SnapshotPolicyDetailVO;
+import org.apache.cloudstack.resourcedetail.dao.SnapshotPolicyDetailsDao;
+import org.apache.cloudstack.snapshot.SnapshotHelper;
+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.cloudstack.storage.image.datastore.ImageStoreEntity;
+import
org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+import org.apache.commons.collections.CollectionUtils;
Review Comment:
```suggestion
import
org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
import org.apache.commons.collections.CollectionUtils;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java:
##########
@@ -105,70 +117,118 @@ public SnapshotInfo backupSnapshot(SnapshotInfo
snapshotInfo) {
public boolean deleteSnapshot(Long snapshotId, Long zoneId) {
final SnapshotVO snapshotVO = _snapshotDao.findById(snapshotId);
- VolumeVO volume =
_volumeDao.findByIdIncludingRemoved(snapshotVO.getVolumeId());
String name = StorPoolHelper.getSnapshotName(snapshotId,
snapshotVO.getUuid(), _snapshotStoreDao, _snapshotDetailsDao);
boolean res = false;
// clean-up snapshot from Storpool storage pools
- StoragePoolVO storage =
_primaryDataStoreDao.findById(volume.getPoolId());
- if
(storage.getStorageProviderName().equals(StorPoolUtil.SP_PROVIDER_NAME)) {
- try {
- SpConnectionDesc conn =
StorPoolUtil.getSpConnection(storage.getUuid(), storage.getId(),
storagePoolDetailsDao, _primaryDataStoreDao);
- SpApiResponse resp = StorPoolUtil.snapshotDelete(name, conn);
- if (resp.getError() != null) {
- final String err = String.format("Failed to clean-up
Storpool snapshot %s. Error: %s", name, resp.getError());
- StorPoolUtil.spLog(err);
- markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId, resp);
- throw new CloudRuntimeException(err);
- } else {
- res = deleteSnapshotFromDbIfNeeded(snapshotVO, zoneId);
-
StorPoolUtil.spLog("StorpoolSnapshotStrategy.deleteSnapshot: executed
successfully=%s, snapshot uuid=%s, name=%s", res, snapshotVO.getUuid(), name);
+ List<SnapshotDataStoreVO> snapshotDataStoreVOS =
_snapshotStoreDao.listBySnapshot(snapshotId, DataStoreRole.Primary);
+ List<SnapshotJoinVO> snapshotJoinVOList =
snapshotJoinDao.listBySnapshotIdAndZoneId(zoneId, snapshotId);
+ try {
+ for (SnapshotJoinVO snapshot: snapshotJoinVOList) {
+ if (State.Destroyed.equals(snapshot.getStatus())) {
+ continue;
+ }
+ if (snapshot.getStoreRole().isImageStore()) {
+ continue;
}
- } catch (Exception e) {
- String errMsg = String.format("Cannot delete snapshot due to
%s", e.getMessage());
- throw new CloudRuntimeException(errMsg);
+ StoragePoolVO storage =
_primaryDataStoreDao.findById(snapshot.getStoreId());
+ if (zoneId != null) {
+ if (!zoneId.equals(snapshot.getDataCenterId())) {
+ continue;
+ }
+ res = deleteSnapshot(snapshotId, zoneId, snapshotVO, name,
storage);
+ break;
+ }
+ res = deleteSnapshot(snapshotId, zoneId, snapshotVO, name,
storage);
}
+ } catch (Exception e) {
+ String errMsg = String.format("Cannot delete snapshot due to %s",
e.getMessage());
+ throw new CloudRuntimeException(errMsg);
+ }
+
+ snapshotDataStoreVOS = _snapshotStoreDao.listBySnapshotId(snapshotId);
+ boolean areAllSnapshotsDestroyed =
snapshotDataStoreVOS.stream().allMatch(v ->
v.getState().equals(State.Destroyed) || v.getState().equals(State.Destroying));
+ if (areAllSnapshotsDestroyed) {
+ updateSnapshotToDestroyed(snapshotVO);
+ return true;
}
+ return res;
+ }
+
+ private boolean deleteSnapshot(Long snapshotId, Long zoneId, SnapshotVO
snapshotVO, String name, StoragePoolVO storage) {
+ boolean res = false;
+ SpConnectionDesc conn =
StorPoolUtil.getSpConnection(storage.getUuid(), storage.getId(),
storagePoolDetailsDao, _primaryDataStoreDao);
+ SpApiResponse resp = StorPoolUtil.snapshotDelete(name, conn);
+ List<SnapshotInfo> snapshotInfos =
snapshotDataFactory.getSnapshots(snapshotId, zoneId);
+ processResult(snapshotInfos,
ObjectInDataStoreStateMachine.Event.DestroyRequested);
+ if (resp.getError() != null) {
+ if (resp.getError().getDescr().contains("still exported")) {
+ processResult(snapshotInfos, Event.OperationFailed);
+ throw new CloudRuntimeException(String.format("The snapshot
[%s] was exported to another cluster. [%s]", name, resp.getError()));
+ }
+ final String err = String.format("Failed to clean-up Storpool
snapshot %s. Error: %s", name, resp.getError());
+ StorPoolUtil.spLog(err);
+ if (resp.getError().getName().equals("objectDoesNotExist")) {
+ return true;
+ }
+ } else {
+ res = deleteSnapshotFromDbIfNeeded(snapshotVO, zoneId);
+ StorPoolUtil.spLog("StorpoolSnapshotStrategy.deleteSnapshot:
executed successfully=%s, snapshot uuid=%s, name=%s", res,
snapshotVO.getUuid(), name);
+ }
+ if (res) {
+ processResult(snapshotInfos, Event.OperationSuccessed);
+ cleanUpDestroyedRecords(snapshotId);
+ } else {
+ processResult(snapshotInfos, Event.OperationFailed);
+ }
return res;
}
- private void markSnapshotAsDestroyedIfAlreadyRemoved(Long snapshotId,
SpApiResponse resp) {
- if (resp.getError().getName().equals("objectDoesNotExist")) {
- SnapshotDataStoreVO snapshotOnPrimary =
_snapshotStoreDao.findBySourceSnapshot(snapshotId, DataStoreRole.Primary);
- if (snapshotOnPrimary != null) {
- snapshotOnPrimary.setState(State.Destroyed);
- _snapshotStoreDao.update(snapshotOnPrimary.getId(),
snapshotOnPrimary);
+ private void cleanUpDestroyedRecords(Long snapshotId) {
+ List<SnapshotDataStoreVO> snapshots =
_snapshotStoreDao.listBySnapshotId(snapshotId);
+ for (SnapshotDataStoreVO snapshot : snapshots) {
+ if (snapshot.getInstallPath().contains("/dev/storpool-byid") &&
State.Destroyed.equals(snapshot.getState())) {
+ _snapshotStoreDao.remove(snapshot.getId());
}
}
}
+ private void processResult(List<SnapshotInfo> snapshotInfos,
ObjectInDataStoreStateMachine.Event event) {
+ for (SnapshotInfo snapshot : snapshotInfos) {
+ SnapshotObject snapshotObject = (SnapshotObject) snapshot;
+ snapshotObject.processEvent(event);
+ }
+ }
Review Comment:
```suggestion
}
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/motion/StorPoolDataMotionStrategy.java:
##########
@@ -19,13 +19,42 @@
package org.apache.cloudstack.storage.motion;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-import javax.inject.Inject;
-
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.Command;
+import com.cloud.agent.api.MigrateAnswer;
+import com.cloud.agent.api.MigrateCommand;
+import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo;
+import com.cloud.agent.api.ModifyTargetsAnswer;
+import com.cloud.agent.api.ModifyTargetsCommand;
+import com.cloud.agent.api.PrepareForMigrationCommand;
+import com.cloud.agent.api.storage.StorPoolBackupTemplateFromSnapshotCommand;
+import com.cloud.agent.api.to.DataObjectType;
+import com.cloud.agent.api.to.VirtualMachineTO;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Storage.ImageFormat;
+import com.cloud.storage.StorageManager;
+import com.cloud.storage.VMTemplateDetailVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.GuestOSCategoryDao;
+import com.cloud.storage.dao.GuestOSDao;
+import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.dao.SnapshotDetailsDao;
+import com.cloud.storage.dao.SnapshotDetailsVO;
+import com.cloud.storage.dao.VMTemplateDetailsDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.VirtualMachineManager;
+import com.cloud.vm.dao.VMInstanceDao;
Review Comment:
```suggestion
import com.cloud.vm.dao.VMInstanceDao;
```
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java:
##########
@@ -372,4 +400,78 @@ public boolean revertSnapshot(SnapshotInfo snapshot) {
@Override
public void postSnapshotCreation(SnapshotInfo snapshot) {
}
+
+ @Override
+ public void copySnapshot(DataObject snapshot, DataObject snapshotDest,
AsyncCompletionCallback<CreateCmdResult> callback) {
Review Comment:
should this be a 70 line method?
##########
server/src/main/java/com/cloud/api/query/dao/SnapshotJoinDao.java:
##########
@@ -17,16 +17,15 @@
package com.cloud.api.query.dao;
-import java.util.List;
-
-import org.apache.cloudstack.api.ResponseObject;
-import org.apache.cloudstack.api.response.SnapshotResponse;
-
import com.cloud.api.query.vo.SnapshotJoinVO;
import com.cloud.utils.Pair;
import com.cloud.utils.db.Filter;
import com.cloud.utils.db.GenericDao;
import com.cloud.utils.db.SearchCriteria;
+import org.apache.cloudstack.api.ResponseObject;
Review Comment:
```suggestion
import org.apache.cloudstack.api.ResponseObject;
```
##########
server/src/main/java/com/cloud/api/query/dao/SnapshotJoinDaoImpl.java:
##########
@@ -258,4 +263,16 @@ public List<SnapshotJoinVO> findByDistinctIds(Long zoneId,
Long... ids) {
sc.setParameters("idsIN", ids);
return searchIncludingRemoved(sc, searchFilter, null, false);
}
+
+ public List<SnapshotJoinVO> listBySnapshotIdAndZoneId(Long zoneId, Long
id) {
Review Comment:
the name suggests
```suggestion
public List<SnapshotJoinVO> listBySnapshotIdAndZoneId(Long id, Long
zoneId) {
```
maybe change the name or the param order?
##########
server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java:
##########
@@ -62,6 +38,29 @@
import com.cloud.user.ResourceLimitService;
import com.cloud.user.dao.AccountDao;
import com.cloud.utils.Pair;
+import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
Review Comment:
```suggestion
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
```
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -3671,6 +3670,11 @@ private Snapshot takeSnapshotInternal(Long volumeId,
Long policyId, Long snapsho
}
List<SnapshotPolicyDetailVO> details =
snapshotPolicyDetailsDao.findDetails(policyId, ApiConstants.ZONE_ID);
zoneIds = details.stream().map(d ->
Long.valueOf(d.getValue())).collect(Collectors.toList());
+ if (CollectionUtils.isNotEmpty(poolIds)) {
+ throw new InvalidParameterValueException(String.format("%s can
not be specified for snapshots linked with snapshot policy",
ApiConstants.STORAGE_ID_LIST));
+ }
+ List<SnapshotPolicyDetailVO> poolDetails =
snapshotPolicyDetailsDao.findDetails(policyId, ApiConstants.STORAGE_ID);
+ poolIds = poolDetails.stream().map(d ->
Long.valueOf(d.getValue())).collect(Collectors.toList());
Review Comment:
new method?
##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -344,6 +194,154 @@
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.UserVmDetailsDao;
import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.acl.ControlledEntity;
+import org.apache.cloudstack.acl.ControlledEntity.ACLType;
+import org.apache.cloudstack.acl.SecurityChecker;
+import org.apache.cloudstack.affinity.AffinityGroupDomainMapVO;
+import org.apache.cloudstack.affinity.AffinityGroupResponse;
+import org.apache.cloudstack.affinity.AffinityGroupVMMapVO;
+import org.apache.cloudstack.affinity.dao.AffinityGroupDomainMapDao;
+import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseListProjectAndAccountResourcesCmd;
+import org.apache.cloudstack.api.InternalIdentity;
+import org.apache.cloudstack.api.ResourceDetail;
+import org.apache.cloudstack.api.ResponseGenerator;
+import org.apache.cloudstack.api.ResponseObject.ResponseView;
+import org.apache.cloudstack.api.command.admin.account.ListAccountsCmdByAdmin;
+import org.apache.cloudstack.api.command.admin.domain.ListDomainsCmd;
+import org.apache.cloudstack.api.command.admin.domain.ListDomainsCmdByAdmin;
+import org.apache.cloudstack.api.command.admin.host.ListHostTagsCmd;
+import org.apache.cloudstack.api.command.admin.host.ListHostsCmd;
+import org.apache.cloudstack.api.command.admin.internallb.ListInternalLBVMsCmd;
+import org.apache.cloudstack.api.command.admin.iso.ListIsosCmdByAdmin;
+import org.apache.cloudstack.api.command.admin.management.ListMgmtsCmd;
+import
org.apache.cloudstack.api.command.admin.resource.icon.ListResourceIconCmd;
+import
org.apache.cloudstack.api.command.admin.router.GetRouterHealthCheckResultsCmd;
+import org.apache.cloudstack.api.command.admin.router.ListRoutersCmd;
+import
org.apache.cloudstack.api.command.admin.snapshot.ListSnapshotsCmdByAdmin;
+import org.apache.cloudstack.api.command.admin.storage.ListImageStoresCmd;
+import
org.apache.cloudstack.api.command.admin.storage.ListObjectStoragePoolsCmd;
+import
org.apache.cloudstack.api.command.admin.storage.ListSecondaryStagingStoresCmd;
+import org.apache.cloudstack.api.command.admin.storage.ListStoragePoolsCmd;
+import org.apache.cloudstack.api.command.admin.storage.ListStorageTagsCmd;
+import
org.apache.cloudstack.api.command.admin.storage.heuristics.ListSecondaryStorageSelectorsCmd;
+import
org.apache.cloudstack.api.command.admin.template.ListTemplatesCmdByAdmin;
+import org.apache.cloudstack.api.command.admin.user.ListUsersCmd;
+import
org.apache.cloudstack.api.command.admin.vm.ListAffectedVmsForStorageScopeChangeCmd;
+import org.apache.cloudstack.api.command.admin.zone.ListZonesCmdByAdmin;
+import org.apache.cloudstack.api.command.user.account.ListAccountsCmd;
+import org.apache.cloudstack.api.command.user.account.ListProjectAccountsCmd;
+import org.apache.cloudstack.api.command.user.address.ListQuarantinedIpsCmd;
+import
org.apache.cloudstack.api.command.user.affinitygroup.ListAffinityGroupsCmd;
+import org.apache.cloudstack.api.command.user.bucket.ListBucketsCmd;
+import org.apache.cloudstack.api.command.user.event.ListEventsCmd;
+import org.apache.cloudstack.api.command.user.iso.ListIsosCmd;
+import org.apache.cloudstack.api.command.user.job.ListAsyncJobsCmd;
+import org.apache.cloudstack.api.command.user.offering.ListDiskOfferingsCmd;
+import org.apache.cloudstack.api.command.user.offering.ListServiceOfferingsCmd;
+import
org.apache.cloudstack.api.command.user.project.ListProjectInvitationsCmd;
+import org.apache.cloudstack.api.command.user.project.ListProjectsCmd;
+import org.apache.cloudstack.api.command.user.resource.ListDetailOptionsCmd;
+import
org.apache.cloudstack.api.command.user.securitygroup.ListSecurityGroupsCmd;
+import org.apache.cloudstack.api.command.user.snapshot.CopySnapshotCmd;
+import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotsCmd;
+import org.apache.cloudstack.api.command.user.tag.ListTagsCmd;
+import org.apache.cloudstack.api.command.user.template.ListTemplatesCmd;
+import org.apache.cloudstack.api.command.user.template.ListVnfTemplatesCmd;
+import org.apache.cloudstack.api.command.user.vm.ListVMsCmd;
+import org.apache.cloudstack.api.command.user.vmgroup.ListVMGroupsCmd;
+import org.apache.cloudstack.api.command.user.volume.ListResourceDetailsCmd;
+import org.apache.cloudstack.api.command.user.volume.ListVolumesCmd;
+import org.apache.cloudstack.api.command.user.zone.ListZonesCmd;
+import org.apache.cloudstack.api.response.AccountResponse;
+import org.apache.cloudstack.api.response.AsyncJobResponse;
+import org.apache.cloudstack.api.response.BucketResponse;
+import org.apache.cloudstack.api.response.DetailOptionsResponse;
+import org.apache.cloudstack.api.response.DiskOfferingResponse;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.DomainRouterResponse;
+import org.apache.cloudstack.api.response.EventResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.HostTagResponse;
+import org.apache.cloudstack.api.response.ImageStoreResponse;
+import org.apache.cloudstack.api.response.InstanceGroupResponse;
+import org.apache.cloudstack.api.response.IpQuarantineResponse;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.ManagementServerResponse;
+import org.apache.cloudstack.api.response.ObjectStoreResponse;
+import org.apache.cloudstack.api.response.ProjectAccountResponse;
+import org.apache.cloudstack.api.response.ProjectInvitationResponse;
+import org.apache.cloudstack.api.response.ProjectResponse;
+import org.apache.cloudstack.api.response.ResourceDetailResponse;
+import org.apache.cloudstack.api.response.ResourceIconResponse;
+import org.apache.cloudstack.api.response.ResourceTagResponse;
+import org.apache.cloudstack.api.response.RouterHealthCheckResultResponse;
+import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
+import org.apache.cloudstack.api.response.SecurityGroupResponse;
+import org.apache.cloudstack.api.response.ServiceOfferingResponse;
+import org.apache.cloudstack.api.response.SnapshotResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.StorageTagResponse;
+import org.apache.cloudstack.api.response.TemplateResponse;
+import org.apache.cloudstack.api.response.UserResponse;
+import org.apache.cloudstack.api.response.UserVmResponse;
+import org.apache.cloudstack.api.response.VirtualMachineResponse;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.backup.BackupOfferingVO;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+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.DataStoreDriver;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateState;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
+import org.apache.cloudstack.outofbandmanagement.OutOfBandManagementVO;
+import org.apache.cloudstack.outofbandmanagement.dao.OutOfBandManagementDao;
+import org.apache.cloudstack.query.QueryService;
+import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
+import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
+import org.apache.cloudstack.secstorage.HeuristicVO;
+import org.apache.cloudstack.secstorage.dao.SecondaryStorageHeuristicDao;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+import org.apache.cloudstack.storage.datastore.db.ObjectStoreDao;
+import org.apache.cloudstack.storage.datastore.db.ObjectStoreVO;
+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.StoragePoolDetailVO;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
+import org.apache.commons.collections.CollectionUtils;
Review Comment:
```suggestion
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
import org.apache.commons.collections.CollectionUtils;
```
##########
server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java:
##########
@@ -174,6 +108,73 @@
import com.cloud.vm.snapshot.VMSnapshotVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
+import org.apache.cloudstack.acl.SecurityChecker;
Review Comment:
```suggestion
import org.apache.cloudstack.acl.SecurityChecker;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
