slavkap commented on code in PR #9270:
URL: https://github.com/apache/cloudstack/pull/9270#discussion_r1742079329
##########
server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java:
##########
@@ -687,9 +731,33 @@ private void postCreateSnapshot(Long volumeId, Long
snapshotId, Long policyId) {
}
}
- if (snapshot != null && snapshot.isRecursive()) {
+ if (snapshot == null) {
+ return;
+ }
+
+ if (snapshot.isRecursive()) {
postCreateRecurringSnapshotForPolicy(userId, volumeId, snapshotId,
policyId);
}
+
+ if (HypervisorType.KVM.equals(snapshot.getHypervisorType()) &&
kvmIncrementalSnapshot.valueIn(clusterId)) {
+ endChainIfNeeded(snapshotId);
+ }
+ }
+
+ private void endChainIfNeeded(Long snapshotId) {
+ SnapshotDataStoreVO snapshotDataStoreVo =
_snapshotStoreDao.findOneBySnapshotId(snapshotId);
Review Comment:
```suggestion
SnapshotDataStoreVO snapshotDataStoreVo =
_snapshotStoreDao.findOneBySnapshotId(snapshotId, zoneId);
```
if there are copies to different zones you could get as a result one of them
##########
server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java:
##########
@@ -1502,6 +1579,29 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume)
throws ResourceAllocationExc
return snapshot;
}
+ private void postSnapshotDirectlyToSecondary(SnapshotInfo snapshot,
SnapshotInfo snapshotOnPrimary, Long snapshotId) {
+ logger.debug("{} was directly copied to secondary storage because the
hypervisor is KVM, the primary storage is file-based and the [{}]
configuration" +
+ " is set to true.", snapshot.getSnapshotVO().toString(),
SnapshotInfo.BackupSnapshotAfterTakingSnapshot);
+
+ SnapshotDataStoreVO snapshotStore =
_snapshotStoreDao.findBySnapshotIdAndDataStoreRoleAndState(snapshotId,
DataStoreRole.Image, ObjectInDataStoreStateMachine.State.Allocated);
+
+ snapshotStore.setInstallPath(snapshotOnPrimary.getPath());
Review Comment:
Still, I cannot reproduce how this is happening
> 2024-09-03T16:15:15,094 DEBUG [c.c.s.s.SnapshotManagerImpl]
(Work-Job-Executor-116:[ctx-e3734d9c, job-25314/job-25315, ctx-466dbaa3])
(logid:bfa643b8) Failed to create snapshot java.lang.NullPointerException at
com.cloud.storage.snapshot.SnapshotManagerImpl.postSnapshotDirectlyToSecondary(SnapshotManagerImpl.java:1588)
at
com.cloud.storage.snapshot.SnapshotManagerImpl.takeSnapshot(SnapshotManagerImpl.java:1534)
##########
engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java:
##########
@@ -201,6 +216,14 @@ public DataObject create(DataObject obj, DataStore
dataStore) {
return this.get(obj, dataStore, null);
}
+ private SnapshotDataStoreVO
tryToGetSnapshotOnSecondaryIfNotOnPrimaryAndIsKVM(DataStore dataStore,
SnapshotInfo snapshotInfo, SnapshotDataStoreVO snapshotDataStoreVO,
+
boolean kvmIncrementalSnapshot, Hypervisor.HypervisorType hypervisorType) {
+ if (snapshotDataStoreVO == null &&
Hypervisor.HypervisorType.KVM.equals(snapshotInfo.getHypervisorType()) &&
DataStoreRole.Primary.equals(dataStore.getRole())) {
+ snapshotDataStoreVO =
snapshotDataStoreDao.findParent(DataStoreRole.Image, null,
snapshotInfo.getVolumeId(), kvmIncrementalSnapshot, hypervisorType);
Review Comment:
```suggestion
snapshotDataStoreVO =
snapshotDataStoreDao.findParent(DataStoreRole.Image, dataStore.getId(),
snapshotInfo.getVolumeId(), kvmIncrementalSnapshot, hypervisorType);
```
Sorry to mention again the copies to different zones, but most of the time
you don't get the required result.
here the parent will be the copy and will return null
##########
server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java:
##########
@@ -687,9 +731,33 @@ private void postCreateSnapshot(Long volumeId, Long
snapshotId, Long policyId) {
}
}
- if (snapshot != null && snapshot.isRecursive()) {
+ if (snapshot == null) {
+ return;
+ }
+
+ if (snapshot.isRecursive()) {
postCreateRecurringSnapshotForPolicy(userId, volumeId, snapshotId,
policyId);
}
+
+ if (HypervisorType.KVM.equals(snapshot.getHypervisorType()) &&
kvmIncrementalSnapshot.valueIn(clusterId)) {
+ endChainIfNeeded(snapshotId);
+ }
+ }
+
+ private void endChainIfNeeded(Long snapshotId) {
Review Comment:
```suggestion
private void endChainIfNeeded(Long snapshotId, Long zoneId) {
```
--
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]
