This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push:
new 8eca04e CLOUDSTACK-9572: Snapshot on primary storage not cleaned up
after Storage migration (#1740)
8eca04e is described below
commit 8eca04e1f6bbe5410dfd657588a67b2b2f71a411
Author: subhash yedugundla <[email protected]>
AuthorDate: Fri Jan 5 11:19:56 2018 +0530
CLOUDSTACK-9572: Snapshot on primary storage not cleaned up after Storage
migration (#1740)
Snapshot on primary storage not cleaned up after Storage migration. This
happens in the following scenario:
Steps To Reproduce
Create an instance on the local storage on any host
Create a scheduled snapshot of the volume:
Wait until ACS created the snapshot. ACS is creating a snapshot on local
storage and is transferring this snapshot to secondary storage. But the latest
snapshot on local storage will stay there. This is as expected.
Migrate the instance to another XenServer host with ACS UI and Storage Live
Migration
The Snapshot on the old host on local storage will not be cleaned up and is
staying on local storage. So local storage will fill up with unneeded snapshots.
---
.../subsystem/api/storage/SnapshotDataFactory.java | 2 +
.../storage/snapshot/SnapshotDataFactoryImpl.java | 18 ++++++
.../storage/volume/VolumeServiceImpl.java | 2 +
.../cloud/storage/snapshot/SnapshotManager.java | 2 +
.../storage/snapshot/SnapshotManagerImpl.java | 15 +++++
.../storage/snapshot/SnapshotManagerTest.java | 65 ++++++++++++----------
6 files changed, 74 insertions(+), 30 deletions(-)
diff --git
a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java
b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java
index 59e59a6..6bbeb85 100644
---
a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java
+++
b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java
@@ -29,6 +29,8 @@ public interface SnapshotDataFactory {
SnapshotInfo getSnapshot(long snapshotId, DataStoreRole role);
+ List<SnapshotInfo> getSnapshots(long volumeId, DataStoreRole store);
+
List<SnapshotInfo> listSnapshotOnCache(long snapshotId);
SnapshotInfo getReadySnapshotOnCache(long snapshotId);
diff --git
a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java
b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java
index 5dd63e3..ad58f42 100644
---
a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java
+++
b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java
@@ -65,6 +65,24 @@ public class SnapshotDataFactoryImpl implements
SnapshotDataFactory {
}
@Override
+ public List<SnapshotInfo> getSnapshots(long volumeId, DataStoreRole role) {
+
+ SnapshotDataStoreVO snapshotStore =
snapshotStoreDao.findByVolume(volumeId, role);
+ if (snapshotStore == null) {
+ return new ArrayList<>();
+ }
+ DataStore store =
storeMgr.getDataStore(snapshotStore.getDataStoreId(), role);
+ List<SnapshotVO> volSnapShots = snapshotDao.listByVolumeId(volumeId);
+ List<SnapshotInfo> infos = new ArrayList<>();
+ for(SnapshotVO snapshot: volSnapShots) {
+ SnapshotObject info = SnapshotObject.getSnapshotObject(snapshot,
store);
+ infos.add(info);
+ }
+ return infos;
+ }
+
+
+ @Override
public SnapshotInfo getSnapshot(long snapshotId, DataStoreRole role) {
SnapshotVO snapshot = snapshotDao.findById(snapshotId);
SnapshotDataStoreVO snapshotStore =
snapshotStoreDao.findBySnapshot(snapshotId, role);
diff --git
a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
index 8818724..ad0418f 100644
---
a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
+++
b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
@@ -1543,6 +1543,7 @@ public class VolumeServiceImpl implements VolumeService {
future.complete(res);
} else {
srcVolume.processEvent(Event.OperationSuccessed);
+ snapshotMgr.cleanupSnapshotsByVolume(srcVolume.getId());
future.complete(res);
}
} catch (Exception e) {
@@ -1624,6 +1625,7 @@ public class VolumeServiceImpl implements VolumeService {
} else {
for (Map.Entry<VolumeInfo, DataStore> entry :
volumeToPool.entrySet()) {
VolumeInfo volume = entry.getKey();
+ snapshotMgr.cleanupSnapshotsByVolume(volume.getId());
volume.processEvent(Event.OperationSuccessed);
}
future.complete(res);
diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManager.java
b/server/src/com/cloud/storage/snapshot/SnapshotManager.java
index c27c699..f3557d0 100644
--- a/server/src/com/cloud/storage/snapshot/SnapshotManager.java
+++ b/server/src/com/cloud/storage/snapshot/SnapshotManager.java
@@ -74,6 +74,8 @@ public interface SnapshotManager extends Configurable {
boolean canOperateOnVolume(Volume volume);
+ void cleanupSnapshotsByVolume(Long volumeId);
+
Answer sendToPool(Volume vol, Command cmd);
SnapshotVO getParentSnapshot(VolumeInfo volume);
diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
index 2c4de7e..1f2ca64 100755
--- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
+++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
@@ -1340,6 +1340,21 @@ public class SnapshotManagerImpl extends
MutualExclusiveIdsManagerBase implement
}
@Override
+ public void cleanupSnapshotsByVolume(Long volumeId) {
+ List<SnapshotInfo> infos = snapshotFactory.getSnapshots(volumeId,
DataStoreRole.Primary);
+ for(SnapshotInfo info: infos) {
+ try {
+ if(info != null) {
+ snapshotSrv.deleteSnapshot(info);
+ }
+ } catch(CloudRuntimeException e) {
+ String msg = "Cleanup of Snapshot with uuid " + info.getUuid()
+ " in primary storage is failed. Ignoring";
+ s_logger.warn(msg);
+ }
+ }
+ }
+
+ @Override
public Snapshot allocSnapshot(Long volumeId, Long policyId, String
snapshotName, Snapshot.LocationType locationType) throws
ResourceAllocationException {
Account caller = CallContext.current().getCallingAccount();
VolumeInfo volume = volFactory.getVolume(volumeId);
diff --git a/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java
b/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java
index 685f495..05eb8b9 100755
--- a/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java
+++ b/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java
@@ -16,6 +16,37 @@
// under the License.
package com.cloud.storage.snapshot;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.List;
+import java.util.UUID;
+
+import org.apache.cloudstack.acl.ControlledEntity;
+import org.apache.cloudstack.acl.SecurityChecker.AccessType;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+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.SnapshotStrategy;
+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.SnapshotStrategy.SnapshotOperation;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
+import org.mockito.Spy;
+
import com.cloud.configuration.Resource.ResourceType;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.ResourceAllocationException;
@@ -44,38 +75,10 @@ import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.snapshot.VMSnapshot;
import com.cloud.vm.snapshot.VMSnapshotVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
-import org.apache.cloudstack.acl.ControlledEntity;
-import org.apache.cloudstack.acl.SecurityChecker.AccessType;
-import org.apache.cloudstack.context.CallContext;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
-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.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.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.junit.After;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
-import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.MockitoAnnotations;
-import org.mockito.Spy;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
-import java.util.List;
-import java.util.UUID;
-
-import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyLong;
-import static org.mockito.Mockito.doNothing;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
public class SnapshotManagerTest {
@Spy
@@ -126,6 +129,9 @@ public class SnapshotManagerTest {
SnapshotDataStoreDao _snapshotStoreDao;
@Mock
SnapshotDataStoreVO snapshotStoreMock;
+ @Mock
+ SnapshotService snapshotSrv;
+
private static final long TEST_SNAPSHOT_ID = 3L;
private static final long TEST_VOLUME_ID = 4L;
@@ -330,5 +336,4 @@ public class SnapshotManagerTest {
Snapshot snapshot =
_snapshotMgr.backupSnapshotFromVmSnapshot(TEST_SNAPSHOT_ID, TEST_VM_ID,
TEST_VOLUME_ID, TEST_VM_SNAPSHOT_ID);
Assert.assertNull(snapshot);
}
-
}
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
