weizhouapache commented on a change in pull request #6057:
URL: https://github.com/apache/cloudstack/pull/6057#discussion_r821556212
##########
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##########
@@ -1583,12 +1583,27 @@ public Volume destroyVolume(long volumeId, Account
caller, boolean expunge, bool
s_logger.warn("Failed to expunge volume: " + volumeId);
return null;
}
+ markVolumeSnapshotsAsDestroyed(volume);
removeVolume(volume.getId());
}
return volume;
}
+ private void markVolumeSnapshotsAsDestroyed(VolumeVO volume) {
+ if (!Storage.ImageFormat.QCOW2.equals(volume.getFormat())) {
Review comment:
@slavkap
I was not quite sure if the snapshots still work and removing this check
will not lead to other unexpected issues.
If we can make sure it is not a problem to mark volume snapshot as destroyed
if snapshot does not exist in primary and secondary storage, we can remove this
check.
##########
File path:
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
##########
@@ -448,9 +448,9 @@ public Void
deleteVolumeCallback(AsyncCallbackDispatcher<VolumeServiceImpl, Comm
volDao.remove(vo.getId());
}
- SnapshotDataStoreVO snapStoreVo =
_snapshotStoreDao.findByVolume(vo.getId(), DataStoreRole.Primary);
+ List<SnapshotDataStoreVO> snapStoreVOs =
_snapshotStoreDao.listAllByVolumeAndDataStore(vo.getId(),
DataStoreRole.Primary);
- if (snapStoreVo != null) {
+ for (SnapshotDataStoreVO snapStoreVo : snapStoreVOs) {
Review comment:
@slavkap
this is a fix: all snapshots (not only the first snapshot) need to be
considered.
the process is same as before, see line 457 to 469
```
if (storagePoolVO.isManaged()) {
DataStore primaryDataStore =
dataStoreMgr.getPrimaryDataStore(storagePoolId);
Map<String, String> mapCapabilities =
primaryDataStore.getDriver().getCapabilities();
String value =
mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString());
Boolean supportsStorageSystemSnapshots = new
Boolean(value);
if (!supportsStorageSystemSnapshots) {
_snapshotStoreDao.remove(snapStoreVo.getId());
}
} else {
_snapshotStoreDao.remove(snapStoreVo.getId());
}
```
##########
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##########
@@ -1583,12 +1583,27 @@ public Volume destroyVolume(long volumeId, Account
caller, boolean expunge, bool
s_logger.warn("Failed to expunge volume: " + volumeId);
return null;
}
+ markVolumeSnapshotsAsDestroyed(volume);
Review comment:
@slavkap
I just tested it. you are right. The snapshots of ROOT volumes are still
BackedUp when vm/volumes are removed. I will fix it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
