You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ga...@apache.org on 2020/03/16 13:17:59 UTC
[cloudstack] branch snapshot-deletion-issues created (now 814af6a)
This is an automated email from the ASF dual-hosted git repository.
gabriel pushed a change to branch snapshot-deletion-issues
in repository https://gitbox.apache.org/repos/asf/cloudstack.git.
at 814af6a checkstyle trailing spaces
This branch includes the following new commits:
new afd060b Fixes snapshot deletion
new 8191b4d Remove legacy '@Component', it is not necessary in this bean/class.
new bc9387f Fix log message missing %d and remove snapshot on DB
new e109949 Remove "dummy" boolean return statement
new d31ecb8 Manage snapshot deletion for KVM + NFS (primary storage)
new 814af6a checkstyle trailing spaces
The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[cloudstack] 04/06: Remove "dummy" boolean return statement
Posted by ga...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gabriel pushed a commit to branch snapshot-deletion-issues
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit e109949e3a9e386b810c28132de6de420f5d6a03
Author: GabrielBrascher <ga...@apache.org>
AuthorDate: Wed Nov 27 12:24:29 2019 -0200
Remove "dummy" boolean return statement
---
.../apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
index a819384..b2ff46c 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
@@ -340,8 +340,7 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
if (snapshotOnPrimary != null) {
long volumeId = snapshotOnPrimary.getVolumeId();
VolumeVO volumeVO = volumeDao.findById(volumeId);
- boolean isVolumeOnPrimary = volumeVO != null && volumeVO.getRemoved() == null;
- return isVolumeOnPrimary;
+ return volumeVO != null && volumeVO.getRemoved() == null;
}
return false;
}
[cloudstack] 06/06: checkstyle trailing spaces
Posted by ga...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gabriel pushed a commit to branch snapshot-deletion-issues
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 814af6a1829b5835e240cc2b50aa9bde20ce2637
Author: GabrielBrascher <ga...@apache.org>
AuthorDate: Mon Mar 16 08:31:45 2020 -0300
checkstyle trailing spaces
---
.../java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
index 2bd55ad..f9f96d9 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
@@ -127,7 +127,7 @@ public class KVMStorageProcessor implements StorageProcessor {
private String _createTmplPath;
private String _manageSnapshotPath;
private int _cmdsTimeout;
-
+
private static final String MANAGE_SNAPSTHOT_CREATE = "-c";
private static final String MANAGE_SNAPSTHOT_DESTROY = "-d";
private static final String NAME = "-n";
@@ -1546,7 +1546,7 @@ public class KVMStorageProcessor implements StorageProcessor {
s_logger.debug("Succesfully connected to Ceph cluster at " + r.confGet(CEPH_MON_HOST));
return r;
}
-
+
@Override
public Answer deleteVolume(final DeleteCommand cmd) {
final VolumeObjectTO vol = (VolumeObjectTO)cmd.getData();
[cloudstack] 05/06: Manage snapshot deletion for KVM + NFS (primary
storage)
Posted by ga...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gabriel pushed a commit to branch snapshot-deletion-issues
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit d31ecb8cfccf13096f8d6013e804b598cbfe0b38
Author: GabrielBrascher <ga...@apache.org>
AuthorDate: Fri Mar 13 01:33:54 2020 -0300
Manage snapshot deletion for KVM + NFS (primary storage)
---
.../kvm/storage/KVMStorageProcessor.java | 62 +++++++++++++---------
1 file changed, 38 insertions(+), 24 deletions(-)
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
index 9a2fd27..2bd55ad 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
@@ -127,6 +127,14 @@ public class KVMStorageProcessor implements StorageProcessor {
private String _createTmplPath;
private String _manageSnapshotPath;
private int _cmdsTimeout;
+
+ private static final String MANAGE_SNAPSTHOT_CREATE = "-c";
+ private static final String MANAGE_SNAPSTHOT_DESTROY = "-d";
+ private static final String NAME = "-n";
+ private static final String CEPH_MON_HOST = "mon_host";
+ private static final String CEPH_AUTH_KEY = "key";
+ private static final String CEPH_CLIENT_MOUNT_TIMEOUT = "client_mount_timeout";
+ private static final String CEPH_DEFAULT_MOUNT_TIMEOUT = "30";
public KVMStorageProcessor(final KVMStoragePoolManager storagePoolMgr, final LibvirtComputingResource resource) {
this.storagePoolMgr = storagePoolMgr;
@@ -563,7 +571,7 @@ public class KVMStorageProcessor implements StorageProcessor {
final Script command = new Script(_createTmplPath, wait, s_logger);
command.add("-f", disk.getPath());
command.add("-t", tmpltPath);
- command.add("-n", templateName + ".qcow2");
+ command.add(NAME, templateName + ".qcow2");
final String result = command.execute();
@@ -949,7 +957,7 @@ public class KVMStorageProcessor implements StorageProcessor {
} else {
final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
+ command.add(NAME, snapshotName);
command.add("-p", snapshotDestPath);
if (isCreatedFromVmSnapshot) {
descName = UUID.randomUUID().toString();
@@ -1010,14 +1018,7 @@ public class KVMStorageProcessor implements StorageProcessor {
}
} else {
if (primaryPool.getType() != StoragePoolType.RBD) {
- final Script command = new Script(_manageSnapshotPath, _cmdsTimeout, s_logger);
- command.add("-d", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to delete snapshot on primary: " + result);
- // return new CopyCmdAnswer("Failed to backup snapshot: " + result);
- }
+ deleteSnapshotViaManageSnapshotScript(snapshotName, snapshotDisk);
}
}
} catch (final Exception ex) {
@@ -1035,6 +1036,16 @@ public class KVMStorageProcessor implements StorageProcessor {
}
}
+ private void deleteSnapshotViaManageSnapshotScript(final String snapshotName, KVMPhysicalDisk snapshotDisk) {
+ final Script command = new Script(_manageSnapshotPath, _cmdsTimeout, s_logger);
+ command.add(MANAGE_SNAPSTHOT_DESTROY, snapshotDisk.getPath());
+ command.add(NAME, snapshotName);
+ final String result = command.execute();
+ if (result != null) {
+ s_logger.debug("Failed to delete snapshot on primary: " + result);
+ }
+ }
+
protected synchronized String attachOrDetachISO(final Connect conn, final String vmName, String isoPath, final boolean isAttach) throws LibvirtException, URISyntaxException,
InternalErrorException {
String isoXml = null;
@@ -1489,12 +1500,7 @@ public class KVMStorageProcessor implements StorageProcessor {
*/
if (primaryPool.getType() == StoragePoolType.RBD) {
try {
- final Rados r = new Rados(primaryPool.getAuthUserName());
- r.confSet("mon_host", primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort());
- r.confSet("key", primaryPool.getAuthSecret());
- r.confSet("client_mount_timeout", "30");
- r.connect();
- s_logger.debug("Succesfully connected to Ceph cluster at " + r.confGet("mon_host"));
+ Rados r = radosConnect(primaryPool);
final IoCTX io = r.ioCtxCreate(primaryPool.getSourceDir());
final Rbd rbd = new Rbd(io);
@@ -1511,8 +1517,8 @@ public class KVMStorageProcessor implements StorageProcessor {
} else {
/* VM is not running, create a snapshot by ourself */
final Script command = new Script(_manageSnapshotPath, _cmdsTimeout, s_logger);
- command.add("-c", disk.getPath());
- command.add("-n", snapshotName);
+ command.add(MANAGE_SNAPSTHOT_CREATE, disk.getPath());
+ command.add(NAME, snapshotName);
final String result = command.execute();
if (result != null) {
s_logger.debug("Failed to manage snapshot: " + result);
@@ -1531,6 +1537,16 @@ public class KVMStorageProcessor implements StorageProcessor {
}
}
+ private Rados radosConnect(final KVMStoragePool primaryPool) throws RadosException {
+ Rados r = new Rados(primaryPool.getAuthUserName());
+ r.confSet(CEPH_MON_HOST, primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort());
+ r.confSet(CEPH_AUTH_KEY, primaryPool.getAuthSecret());
+ r.confSet(CEPH_CLIENT_MOUNT_TIMEOUT, CEPH_DEFAULT_MOUNT_TIMEOUT);
+ r.connect();
+ s_logger.debug("Succesfully connected to Ceph cluster at " + r.confGet(CEPH_MON_HOST));
+ return r;
+ }
+
@Override
public Answer deleteVolume(final DeleteCommand cmd) {
final VolumeObjectTO vol = (VolumeObjectTO)cmd.getData();
@@ -1619,12 +1635,7 @@ public class KVMStorageProcessor implements StorageProcessor {
String snapshotName = snapshotFullPath.substring(snapshotFullPath.lastIndexOf("/") + 1);
snap_full_name = disk.getName() + "@" + snapshotName;
if (primaryPool.getType() == StoragePoolType.RBD) {
- Rados r = new Rados(primaryPool.getAuthUserName());
- r.confSet("mon_host", primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort());
- r.confSet("key", primaryPool.getAuthSecret());
- r.confSet("client_mount_timeout", "30");
- r.connect();
- s_logger.debug("Succesfully connected to Ceph cluster at " + r.confGet("mon_host"));
+ Rados r = radosConnect(primaryPool);
IoCTX io = r.ioCtxCreate(primaryPool.getSourceDir());
Rbd rbd = new Rbd(io);
RbdImage image = rbd.open(disk.getName());
@@ -1644,6 +1655,9 @@ public class KVMStorageProcessor implements StorageProcessor {
rbd.close(image);
r.ioCtxDestroy(io);
}
+ } else if (primaryPool.getType() == StoragePoolType.NetworkFilesystem) {
+ s_logger.info(String.format("Attempting to remove snapshot on primary storage (id=%s, snapshot=%s, storage type=%s)", snapshotTO.getId(), snap_full_name, primaryPool.getType()));
+ deleteSnapshotViaManageSnapshotScript(snapshotName, disk);
} else {
s_logger.warn("Operation not implemented for storage pool type of " + primaryPool.getType().toString());
throw new InternalErrorException("Operation not implemented for storage pool type of " + primaryPool.getType().toString());
[cloudstack] 01/06: Fixes snapshot deletion
Posted by ga...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gabriel pushed a commit to branch snapshot-deletion-issues
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit afd060b54f48744caf4836e4a6334dc1711b2b17
Author: GabrielBrascher <ga...@apache.org>
AuthorDate: Thu Oct 17 17:16:48 2019 -0300
Fixes snapshot deletion
---
.../src/test/resources/fakeDriverTestContext.xml | 2 +-
.../src/test/resources/storageContext.xml | 2 +-
...tStrategy.java => DefaultSnapshotStrategy.java} | 114 +++++++++++----------
...ing-engine-storage-snapshot-storage-context.xml | 4 +-
4 files changed, 66 insertions(+), 56 deletions(-)
diff --git a/engine/storage/integration-test/src/test/resources/fakeDriverTestContext.xml b/engine/storage/integration-test/src/test/resources/fakeDriverTestContext.xml
index 944196d..b8a2274 100644
--- a/engine/storage/integration-test/src/test/resources/fakeDriverTestContext.xml
+++ b/engine/storage/integration-test/src/test/resources/fakeDriverTestContext.xml
@@ -49,7 +49,7 @@
<bean id="dataStoreProviderManagerImpl" class="org.apache.cloudstack.storage.datastore.provider.DataStoreProviderManagerImpl" />
<bean id="storageCacheManagerImpl" class="org.apache.cloudstack.storage.cache.manager.StorageCacheManagerImpl" />
<bean id="storageCacheRandomAllocator" class="org.apache.cloudstack.storage.cache.allocator.StorageCacheRandomAllocator" />
- <bean id="xenserverSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
+ <bean id="defaultSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
<bean id="bAREMETAL" class="org.apache.cloudstack.storage.image.format.BAREMETAL" />
<bean id="dataMotionServiceImpl" class="org.apache.cloudstack.storage.motion.DataMotionServiceImpl" />
<bean id="dataObjectManagerImpl" class="org.apache.cloudstack.storage.datastore.DataObjectManagerImpl" />
diff --git a/engine/storage/integration-test/src/test/resources/storageContext.xml b/engine/storage/integration-test/src/test/resources/storageContext.xml
index abf0876..f65e2ac 100644
--- a/engine/storage/integration-test/src/test/resources/storageContext.xml
+++ b/engine/storage/integration-test/src/test/resources/storageContext.xml
@@ -50,7 +50,7 @@
<bean id="ancientDataMotionStrategy" class="org.apache.cloudstack.storage.motion.AncientDataMotionStrategy" />
<bean id="storageCacheManagerImpl" class="org.apache.cloudstack.storage.cache.manager.StorageCacheManagerImpl" />
<bean id="storageCacheRandomAllocator" class="org.apache.cloudstack.storage.cache.allocator.StorageCacheRandomAllocator" />
- <bean id="xenserverSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
+ <bean id="defaultSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
<bean id="bAREMETAL" class="org.apache.cloudstack.storage.image.format.BAREMETAL" />
<bean id="dataMotionServiceImpl" class="org.apache.cloudstack.storage.motion.DataMotionServiceImpl" />
<bean id="dataObjectManagerImpl" class="org.apache.cloudstack.storage.datastore.DataObjectManagerImpl" />
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
similarity index 84%
rename from engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
rename to engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
index 3ab2129..2651162 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
@@ -34,7 +34,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.framework.jobs.AsyncJob;
-import org.apache.cloudstack.framework.jobs.dao.SyncQueueItemDao;
import org.apache.cloudstack.storage.command.CreateObjectAnswer;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
@@ -72,8 +71,8 @@ import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.fsm.NoTransitionException;
@Component
-public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
- private static final Logger s_logger = Logger.getLogger(XenserverSnapshotStrategy.class);
+public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
+ private static final Logger s_logger = Logger.getLogger(DefaultSnapshotStrategy.class);
@Inject
SnapshotService snapshotSvr;
@@ -90,12 +89,8 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
@Inject
SnapshotDataFactory snapshotDataFactory;
@Inject
- private SnapshotDao _snapshotDao;
- @Inject
private SnapshotDetailsDao _snapshotDetailsDao;
@Inject
- private SyncQueueItemDao _syncQueueItemDao;
- @Inject
VolumeDetailsDao _volumeDetailsDaoImpl;
@Override
@@ -269,63 +264,78 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
throw new InvalidParameterValueException("Can't delete snapshotshot " + snapshotId + " due to it is in " + snapshotVO.getState() + " Status");
}
- // first mark the snapshot as destroyed, so that ui can't see it, but we
- // may not destroy the snapshot on the storage, as other snapshots may
- // depend on it.
SnapshotInfo snapshotOnImage = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Image);
- if (snapshotOnImage == null) {
- s_logger.debug("Can't find snapshot on backup storage, delete it in db");
- snapshotDao.remove(snapshotId);
- return true;
- }
-
- SnapshotObject obj = (SnapshotObject)snapshotOnImage;
- try {
- obj.processEvent(Snapshot.Event.DestroyRequested);
- List<VolumeDetailVO> volumesFromSnapshot;
- volumesFromSnapshot = _volumeDetailsDaoImpl.findDetails("SNAPSHOT_ID", String.valueOf(snapshotId), null);
- if (volumesFromSnapshot.size() > 0) {
- try {
- obj.processEvent(Snapshot.Event.OperationFailed);
- } catch (NoTransitionException e1) {
- s_logger.debug("Failed to change snapshot state: " + e1.toString());
+ boolean deletedOnSecondary = false;
+ if (snapshotOnImage == null) {
+ s_logger.debug(String.format("Can't find snapshot [snapshot id: %d] on backup storage"));
+ } else {
+ SnapshotObject obj = (SnapshotObject)snapshotOnImage;
+ try {
+ deletedOnSecondary = deleteSnapshotOnSecundaryStorage(snapshotId, snapshotOnImage, obj);
+ if (!deletedOnSecondary) {
+ s_logger.debug(
+ String.format("Failed to find/delete snapshot (id: %d) on secondary storage. Still necessary to check and delete snapshot on primary storage.",
+ snapshotId));
+ } else {
+ s_logger.debug(String.format("Snapshot (id: %d) has been deleted on secondary storage.", snapshotId));
}
- throw new InvalidParameterValueException("Unable to perform delete operation, Snapshot with id: " + snapshotId + " is in use ");
+ } catch (NoTransitionException e) {
+ s_logger.debug("Failed to set the state to destroying: ", e);
+ return false;
}
- } catch (NoTransitionException e) {
- s_logger.debug("Failed to set the state to destroying: ", e);
- return false;
}
- try {
- boolean result = deleteSnapshotChain(snapshotOnImage);
- obj.processEvent(Snapshot.Event.OperationSucceeded);
- if (result) {
- //snapshot is deleted on backup storage, need to delete it on primary storage
- SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
- if (snapshotOnPrimary != null) {
- SnapshotInfo snapshotOnPrimaryInfo = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
- long volumeId = snapshotOnPrimary.getVolumeId();
- VolumeVO volumeVO = volumeDao.findById(volumeId);
- if (((PrimaryDataStoreImpl)snapshotOnPrimaryInfo.getDataStore()).getPoolType() == StoragePoolType.RBD && volumeVO != null) {
- snapshotSvr.deleteSnapshot(snapshotOnPrimaryInfo);
- }
- snapshotOnPrimary.setState(State.Destroyed);
- snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
- }
- }
- } catch (Exception e) {
- s_logger.debug("Failed to delete snapshot: ", e);
+ boolean deletedOnPrimary = deleteSnapshotOnPrimary(snapshotId);
+ if (deletedOnPrimary) {
+ s_logger.debug(String.format("Successfully deleted snapshot (id: %d) on primary storage.", snapshotId));
+ } else if (deletedOnSecondary) {
+ s_logger.debug(String.format("The snapshot was deleted on secondary storage. Could not find/delete snapshot (id: %d) on primary storage.", snapshotId));
+ }
+ return deletedOnSecondary || deletedOnPrimary;
+ }
+
+ /**
+ * Deletes the snapshot on secondary storage.
+ * It can return false when the snapshot was stored on primary storage and not backed up on secondary; therefore, the snapshot should also be deleted on primary storage even when this method returns false.
+ */
+ private boolean deleteSnapshotOnSecundaryStorage(Long snapshotId, SnapshotInfo snapshotOnImage, SnapshotObject obj) throws NoTransitionException {
+ obj.processEvent(Snapshot.Event.DestroyRequested);
+ List<VolumeDetailVO> volumesFromSnapshot;
+ volumesFromSnapshot = _volumeDetailsDaoImpl.findDetails("SNAPSHOT_ID", String.valueOf(snapshotId), null);
+
+ if (volumesFromSnapshot.size() > 0) {
try {
obj.processEvent(Snapshot.Event.OperationFailed);
} catch (NoTransitionException e1) {
- s_logger.debug("Failed to change snapshot state: " + e.toString());
+ s_logger.debug("Failed to change snapshot state: " + e1.toString());
}
- return false;
+ throw new InvalidParameterValueException("Unable to perform delete operation, Snapshot with id: " + snapshotId + " is in use ");
}
- return true;
+ boolean result = deleteSnapshotChain(snapshotOnImage);
+ obj.processEvent(Snapshot.Event.OperationSucceeded);
+ return result;
+ }
+
+ /**
+ * Deletes the snapshot on primary storage. It can return false when the snapshot was not stored on primary storage; however this does not means that it failed to delete the snapshot. </br>
+ * In case of failure, it will throw one of the following exceptions: CloudRuntimeException, InterruptedException, or ExecutionException. </br>
+ */
+ private boolean deleteSnapshotOnPrimary(Long snapshotId) {
+ boolean result = false;
+ SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
+ if (snapshotOnPrimary != null) {
+ SnapshotInfo snapshotOnPrimaryInfo = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
+ long volumeId = snapshotOnPrimary.getVolumeId();
+ VolumeVO volumeVO = volumeDao.findById(volumeId);
+ if (((PrimaryDataStoreImpl)snapshotOnPrimaryInfo.getDataStore()).getPoolType() == StoragePoolType.RBD && volumeVO != null) {
+ result = snapshotSvr.deleteSnapshot(snapshotOnPrimaryInfo);
+ }
+ snapshotOnPrimary.setState(State.Destroyed);
+ snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
+ }
+ return result;
}
@Override
diff --git a/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml b/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
index b295398..2bfb3c3 100644
--- a/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
+++ b/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
@@ -27,8 +27,8 @@
http://www.springframework.org/schema/context/spring-context.xsd"
>
- <bean id="xenserverSnapshotStrategy"
- class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
+ <bean id="defaultSnapshotStrategy"
+ class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
<bean id="storageSystemSnapshotStrategy"
class="org.apache.cloudstack.storage.snapshot.StorageSystemSnapshotStrategy" />
[cloudstack] 03/06: Fix log message missing %d and remove snapshot
on DB
Posted by ga...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gabriel pushed a commit to branch snapshot-deletion-issues
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit bc9387fb91918e02cc1296a3c82f563795bf89ea
Author: GabrielBrascher <ga...@apache.org>
AuthorDate: Mon Nov 25 17:21:09 2019 -0200
Fix log message missing %d and remove snapshot on DB
---
.../storage/snapshot/DefaultSnapshotStrategy.java | 28 +++++++++++++++-------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
index 3e8e473..a819384 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
@@ -266,7 +266,7 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
boolean deletedOnSecondary = false;
if (snapshotOnImage == null) {
- s_logger.debug(String.format("Can't find snapshot [snapshot id: %d] on backup storage"));
+ s_logger.debug(String.format("Can't find snapshot [snapshot id: %d] on backup storage", snapshotId));
} else {
SnapshotObject obj = (SnapshotObject)snapshotOnImage;
try {
@@ -321,19 +321,29 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
* In case of failure, it will throw one of the following exceptions: CloudRuntimeException, InterruptedException, or ExecutionException. </br>
*/
private boolean deleteSnapshotOnPrimary(Long snapshotId) {
- boolean result = false;
+ SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
+ SnapshotInfo snapshotOnPrimaryInfo = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
+ if (isSnapshotOnPrimaryStorage(snapshotId) && snapshotSvr.deleteSnapshot(snapshotOnPrimaryInfo)) {
+ snapshotOnPrimary.setState(State.Destroyed);
+ snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
+ snapshotDao.remove(snapshotId);
+ return true;
+ }
+ return false;
+ }
+
+ /**
+ * Returns true if the snapshot volume is on primary storage.
+ */
+ private boolean isSnapshotOnPrimaryStorage(long snapshotId) {
SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
if (snapshotOnPrimary != null) {
- SnapshotInfo snapshotOnPrimaryInfo = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
long volumeId = snapshotOnPrimary.getVolumeId();
VolumeVO volumeVO = volumeDao.findById(volumeId);
- if (((PrimaryDataStoreImpl)snapshotOnPrimaryInfo.getDataStore()).getPoolType() == StoragePoolType.RBD && volumeVO != null) {
- result = snapshotSvr.deleteSnapshot(snapshotOnPrimaryInfo);
- }
- snapshotOnPrimary.setState(State.Destroyed);
- snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
+ boolean isVolumeOnPrimary = volumeVO != null && volumeVO.getRemoved() == null;
+ return isVolumeOnPrimary;
}
- return result;
+ return false;
}
@Override
[cloudstack] 02/06: Remove legacy '@Component',
it is not necessary in this bean/class.
Posted by ga...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gabriel pushed a commit to branch snapshot-deletion-issues
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 8191b4da4815af8a3a13adcdd9f4692faf24d284
Author: GabrielBrascher <ga...@apache.org>
AuthorDate: Mon Oct 21 14:24:00 2019 -0300
Remove legacy '@Component', it is not necessary in this bean/class.
---
.../org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java | 2 --
1 file changed, 2 deletions(-)
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
index 2651162..3e8e473 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
@@ -21,7 +21,6 @@ import java.util.List;
import javax.inject.Inject;
import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
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.Event;
@@ -70,7 +69,6 @@ import com.cloud.utils.db.DB;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.fsm.NoTransitionException;
-@Component
public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
private static final Logger s_logger = Logger.getLogger(DefaultSnapshotStrategy.class);