Skip to content

Commit b8b6b8c

Browse files
JoaoJandreJoaoJandre
committed
continue fix
1 parent 8e26579 commit b8b6b8c

File tree

14 files changed

+69
-38
lines changed

14 files changed

+69
-38
lines changed

‎api/src/main/java/com/cloud/agent/api/to/NfsTO.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.cloud.agent.api.to;
1818

1919
import com.cloud.storage.DataStoreRole;
20+
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
2021

2122
public class NfsTO implements DataStoreTO {
2223

@@ -80,4 +81,9 @@ public String getNfsVersion() {
8081
public void setNfsVersion(String nfsVersion) {
8182
this.nfsVersion = nfsVersion;
8283
}
84+
85+
@Override
86+
public String toString() {
87+
return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "_role", "uuid", "url");
88+
}
8389
}

‎api/src/main/java/com/cloud/storage/Snapshot.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
4848
}
4949

5050
public enum State {
51-
Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
51+
Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed, Hidden,
5252
//it's a state, user can't see the snapshot from ui, while the snapshot may still exist on the storage
5353
Error;
5454

‎core/src/main/java/org/apache/cloudstack/storage/to/SnapshotObjectTO.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ public SnapshotObjectTO() {
5252

5353
}
5454

55+
@Override
56+
public DataObjectType getObjectType() {
57+
return DataObjectType.SNAPSHOT;
58+
}
59+
5560
public SnapshotObjectTO(SnapshotInfo snapshot) {
5661
this.path = snapshot.getPath();
5762
this.setId(snapshot.getId());
@@ -86,11 +91,6 @@ public SnapshotObjectTO(SnapshotInfo snapshot) {
8691
ArrayUtils.reverse(parents);
8792
}
8893

89-
@Override
90-
public DataObjectType getObjectType() {
91-
return DataObjectType.SNAPSHOT;
92-
}
93-
9494
@Override
9595
public DataStoreTO getDataStore() {
9696
return this.dataStore;

‎engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ObjectInDataStoreStateMachine.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ enum State {
3232
Migrated("The object has been migrated"),
3333
Destroying("Template is destroying"),
3434
Destroyed("Template is destroyed"),
35-
Failed("Failed to download template");
35+
Failed("Failed to download template"),
36+
Hidden("The object is hidden from the user");
3637
String _description;
3738

3839
private State(String description) {

‎engine/schema/src/main/java/com/cloud/storage/SnapshotVO.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,6 @@ public Class<?> getEntityType() {
284284
@Override
285285
public String toString() {
286286
return String.format("Snapshot %s", new ToStringBuilder(this, ToStringStyle.JSON_STYLE).append("uuid", getUuid()).append("name", getName())
287-
.append("volumeId", getVolumeId()).toString());
287+
.append("volumeId", getVolumeId()).append("state", getState()).toString());
288288
}
289289
}

‎engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
6565

6666
SnapshotDataStoreVO findBySourceSnapshot(long snapshotId, DataStoreRole role);
6767

68+
List<SnapshotDataStoreVO> findBySnapshotIdAndNotInDestroyedHiddenState(long snapshotId);
69+
6870
List<SnapshotDataStoreVO> listDestroyed(long storeId);
6971

7072
List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId);

‎engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
6666
private SearchBuilder<SnapshotDataStoreVO> searchFilteringStoreIdEqStoreRoleEqStateNeqRefCntNeq;
6767
protected SearchBuilder<SnapshotDataStoreVO> searchFilteringStoreIdEqStateEqStoreRoleEqIdEqUpdateCountEqSnapshotIdEqVolumeIdEq;
6868
private SearchBuilder<SnapshotDataStoreVO> stateSearch;
69-
private SearchBuilder<SnapshotDataStoreVO> idStateNeqSearch;
69+
private SearchBuilder<SnapshotDataStoreVO> idStateNinSearch;
7070
protected SearchBuilder<SnapshotVO> snapshotVOSearch;
7171
private SearchBuilder<SnapshotDataStoreVO> snapshotCreatedSearch;
7272
private SearchBuilder<SnapshotDataStoreVO> dataStoreAndInstallPathSearch;
@@ -131,10 +131,10 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
131131
stateSearch.done();
132132

133133

134-
idStateNeqSearch = createSearchBuilder();
135-
idStateNeqSearch.and(SNAPSHOT_ID, idStateNeqSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ);
136-
idStateNeqSearch.and(STATE, idStateNeqSearch.entity().getState(), SearchCriteria.Op.NEQ);
137-
idStateNeqSearch.done();
134+
idStateNinSearch = createSearchBuilder();
135+
idStateNinSearch.and(SNAPSHOT_ID, idStateNinSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ);
136+
idStateNinSearch.and(STATE, idStateNinSearch.entity().getState(), SearchCriteria.Op.NOTIN);
137+
idStateNinSearch.done();
138138

139139
snapshotVOSearch = snapshotDao.createSearchBuilder();
140140
snapshotVOSearch.and(VOLUME_ID, snapshotVOSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
@@ -411,9 +411,17 @@ public List<SnapshotDataStoreVO> findByVolume(long snapshotId, long volumeId, Da
411411

412412
@Override
413413
public List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId) {
414-
SearchCriteria<SnapshotDataStoreVO> sc = idStateNeqSearch.create();
414+
SearchCriteria<SnapshotDataStoreVO> sc = idStateNinSearch.create();
415415
sc.setParameters(SNAPSHOT_ID, snapshotId);
416-
sc.setParameters(STATE, State.Destroyed);
416+
sc.setParameters(STATE, State.Destroyed.name());
417+
return listBy(sc);
418+
}
419+
420+
@Override
421+
public List<SnapshotDataStoreVO> findBySnapshotIdAndNotInDestroyedHiddenState(long snapshotId) {
422+
SearchCriteria<SnapshotDataStoreVO> sc = idStateNinSearch.create();
423+
sc.setParameters(SNAPSHOT_ID, snapshotId);
424+
sc.setParameters(STATE, State.Destroyed.name(), State.Hidden.name());
417425
return listBy(sc);
418426
}
419427

‎engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,12 @@ CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.vm_instance', 'delete_protection', '
427427
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.volumes', 'delete_protection', 'boolean DEFAULT FALSE COMMENT "delete protection for volumes" ');
428428

429429
--- KVM Incremental Snapshots
430+
--- This
431+
--- Should
432+
--- Be
433+
--- Updated
434+
--- When
435+
--- 420to421 is created
430436
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.snapshot_store_ref', 'kvm_checkpoint_path', 'varchar(255)');
431437

432438
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.snapshot_store_ref', 'end_of_chain', 'int(1) unsigned');

‎engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
9999
@Inject
100100
SnapshotZoneDao snapshotZoneDao;
101101

102-
private final List<Snapshot.State> snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error);
102+
private final List<Snapshot.State> snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error, Snapshot.State.Hidden);
103103

104104
public SnapshotDataStoreVO getSnapshotImageStoreRef(long snapshotId, long zoneId) {
105105
List<SnapshotDataStoreVO> snaps = snapshotStoreDao.listReadyBySnapshot(snapshotId, DataStoreRole.Image);
@@ -197,7 +197,10 @@ protected boolean deleteSnapshotChain(SnapshotInfo snapshot, String storageToStr
197197
SnapshotInfo child = snapshot.getChild();
198198

199199
if (child != null) {
200-
logger.debug(String.format("Snapshot [%s] has child [%s], not deleting it on the storage [%s]", snapshotTo, child.getTO(), storageToString));
200+
logger.debug(String.format("Snapshot [%s] has child [%s], not deleting it on the storage [%s], will only set it as hidden.", snapshotTo, child.getTO(), storageToString));
201+
SnapshotDataStoreVO snapshotDataStoreVo = snapshotStoreDao.findByStoreSnapshot(snapshot.getDataStore().getRole(), snapshot.getDataStore().getId(), snapshot.getSnapshotId());
202+
snapshotDataStoreVo.setState(State.Hidden);
203+
snapshotStoreDao.update(snapshotDataStoreVo.getId(), snapshotDataStoreVo);
201204
break;
202205
}
203206

@@ -206,6 +209,7 @@ protected boolean deleteSnapshotChain(SnapshotInfo snapshot, String storageToStr
206209
SnapshotInfo parent = snapshot.getParent();
207210
boolean deleted = false;
208211
if (parent != null) {
212+
logger.debug("Snapshot [{}] has parent [{}].", snapshot, parent);
209213
if (parent.getPath() != null && parent.getPath().equalsIgnoreCase(snapshot.getPath())) {
210214
//NOTE: if both snapshots share the same path, it's for xenserver's empty delta snapshot. We can't delete the snapshot on the backend, as parent snapshot still reference to it
211215
//Instead, mark it as destroyed in the db.
@@ -219,6 +223,7 @@ protected boolean deleteSnapshotChain(SnapshotInfo snapshot, String storageToStr
219223
}
220224

221225
if (!deleted) {
226+
logger.debug("Deleting snapshot [{}].", snapshot);
222227
try {
223228
boolean r = snapshotSvr.deleteSnapshot(snapshot);
224229
if (r) {
@@ -360,6 +365,7 @@ protected List<SnapshotDataStoreVO> findLastAliveAncestors(long snapshotId) {
360365

361366
protected boolean deleteSnapshotInfos(SnapshotVO snapshotVo, Long zoneId) {
362367
List<SnapshotInfo> snapshotInfos = retrieveSnapshotEntries(snapshotVo.getId(), zoneId);
368+
logger.debug("Found {} snapshot references to delete.", snapshotInfos);
363369

364370
boolean result = false;
365371
for (var snapshotInfo : snapshotInfos) {
@@ -378,7 +384,7 @@ protected boolean deleteSnapshotInfos(SnapshotVO snapshotVo, Long zoneId) {
378384
protected Boolean deleteSnapshotInfo(SnapshotInfo snapshotInfo, SnapshotVO snapshotVo) {
379385
DataStore dataStore = snapshotInfo.getDataStore();
380386
String storageToString = String.format("%s {uuid: \"%s\", name: \"%s\"}", dataStore.getRole().name(), dataStore.getUuid(), dataStore.getName());
381-
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotId(snapshotVo.getId());
387+
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotIdAndNotInDestroyedHiddenState(snapshotVo.getId());
382388
boolean isLastSnapshotRef = CollectionUtils.isEmpty(snapshotStoreRefs) || snapshotStoreRefs.size() == 1;
383389
try {
384390
SnapshotObject snapshotObject = castSnapshotInfoToSnapshotObject(snapshotInfo);
@@ -389,7 +395,7 @@ protected Boolean deleteSnapshotInfo(SnapshotInfo snapshotInfo, SnapshotVO snaps
389395
if (deleteSnapshotChain(snapshotInfo, storageToString)) {
390396
logger.debug(String.format("%s was deleted on %s. We will mark the snapshot as destroyed.", snapshotVo, storageToString));
391397
} else {
392-
logger.debug(String.format("%s was not deleted on %s; however, we will mark the snapshot as destroyed for future garbage collecting.", snapshotVo,
398+
logger.debug(String.format("%s was not deleted on %s; however, we will mark the snapshot as hidden for future garbage collecting.", snapshotVo,
393399
storageToString));
394400
}
395401
snapshotStoreDao.updateDisplayForSnapshotStoreRole(snapshotVo.getId(), dataStore.getId(), dataStore.getRole(), false);

‎engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,13 @@ public DataStore getStore() {
113113

114114
@Override
115115
public SnapshotInfo getParent() {
116-
116+
logger.debug("Searching for parents of snapshot [{}], in store [{}] with role [{}].", snapshot.getSnapshotId(), store.getId(), store.getRole());
117117
SnapshotDataStoreVO snapStoreVO = snapshotStoreDao.findByStoreSnapshot(store.getRole(), store.getId(), snapshot.getId());
118118
if (snapStoreVO != null) {
119119
long parentId = snapStoreVO.getParentSnapshotId();
120120
if (parentId != 0) {
121121
if (HypervisorType.KVM.equals(snapshot.getHypervisorType())) {
122-
return getParentInfoFromImageIfPossible(parentId);
122+
return getCorrectIncrementalParent(parentId);
123123
}
124124
return snapshotFactory.getSnapshot(parentId, store);
125125
}
@@ -129,28 +129,26 @@ public SnapshotInfo getParent() {
129129
}
130130

131131
/**
132-
* Returns the snapshotInfo of the passed snapshot parentId. If the parent is on image store, will return the info from image store;
133-
* else, will return the info from primary store.
132+
* Returns the snapshotInfo of the passed snapshot parentId. Will search for the snapshot reference which has a checkpoint path. If none is found, throws an exception.
134133
* */
135-
protected SnapshotInfo getParentInfoFromImageIfPossible(long parentId) {
134+
protected SnapshotInfo getCorrectIncrementalParent(long parentId) {
136135
List<SnapshotDataStoreVO> parentSnapshotDatastoreVos = snapshotStoreDao.findBySnapshotId(parentId);
137136

138137
if (parentSnapshotDatastoreVos.isEmpty()) {
139138
return null;
140139
}
141140

142-
SnapshotDataStoreVO parent = null;
141+
logger.debug("Found parent snapshot references {}, will filter to just one.", parentSnapshotDatastoreVos);
143142

144-
for (SnapshotDataStoreVO parentSnapshotDatastoreVo : parentSnapshotDatastoreVos) {
145-
parent = parentSnapshotDatastoreVo;
146-
if (parentSnapshotDatastoreVo.getRole().equals(DataStoreRole.Image)) {
147-
break;
148-
}
149-
}
143+
SnapshotDataStoreVO parent = parentSnapshotDatastoreVos.stream().filter(snapshotDataStoreVO -> snapshotDataStoreVO.getKvmCheckpointPath() != null)
144+
.findFirst().
145+
orElseThrow(() -> new CloudRuntimeException(String.format("Could not find snapshot parent with id [%s]. None of the records have a checkpoint path.", parentId)));
150146

151147
SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(parentId, parent.getDataStoreId(), parent.getRole());
152148
snapshotInfo.setKvmIncrementalSnapshot(parent.getKvmCheckpointPath() != null);
153149

150+
logger.debug("Filtered snapshot references {} to just {}.", parentSnapshotDatastoreVos, parent);
151+
154152
return snapshotInfo;
155153
}
156154

@@ -527,4 +525,10 @@ public boolean delete() {
527525
public Class<?> getEntityType() {
528526
return Snapshot.class;
529527
}
528+
529+
@Override
530+
public String toString() {
531+
return String.format("%s, dataStoreId %s, imageStore id %s, checkpointPath %s.", snapshot, store != null? store.getId() : 0,
532+
imageStore != null ? imageStore.getId() : 0, checkpointPath);
533+
}
530534
}

0 commit comments

Comments
 (0)