Skip to content

Commit 295198e

Browse files
committed
Avoid data loss
1 parent 8f2735a commit 295198e

File tree

7 files changed

+170
-40
lines changed

7 files changed

+170
-40
lines changed

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,6 @@ public interface SnapshotDao extends GenericDao<SnapshotVO, Long>, StateDao<Snap
5858
*/
5959
List<SnapshotVO> listByIds(Object... ids);
6060
List<SnapshotVO> searchByVolumes(List<Long> volumeIds);
61+
62+
List<SnapshotVO> listByVolumeIdAndTypeNotInAndStateNotRemoved(long volumeId, Type... types);
6163
}

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java

Lines changed: 69 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.sql.PreparedStatement;
2020
import java.sql.ResultSet;
2121
import java.util.ArrayList;
22+
import java.util.Arrays;
2223
import java.util.List;
2324

2425
import javax.annotation.PostConstruct;
@@ -56,6 +57,17 @@ public class SnapshotDaoImpl extends GenericDaoBase<SnapshotVO, Long> implements
5657
private static final String GET_LAST_SNAPSHOT =
5758
"SELECT snapshots.id FROM snapshot_store_ref, snapshots where snapshots.id = snapshot_store_ref.snapshot_id AND snapshosts.volume_id = ? AND snapshot_store_ref.role = ? ORDER BY created DESC";
5859

60+
private static final String VOLUME_ID = "volumeId";
61+
private static final String REMOVED = "removed";
62+
private static final String ID = "id";
63+
private static final String INSTANCE_ID = "instanceId";
64+
private static final String STATE = "state";
65+
private static final String INSTANCE_VOLUMES = "instanceVolumes";
66+
private static final String INSTANCE_SNAPSHOTS = "instanceSnapshots";
67+
private static final String VERSION = "version";
68+
private static final String ACCOUNT_ID = "accountId";
69+
private static final String NOT_TYPE = "notType";
70+
5971
private SearchBuilder<SnapshotVO> snapshotIdsSearch;
6072
private SearchBuilder<SnapshotVO> VolumeIdSearch;
6173
private SearchBuilder<SnapshotVO> VolumeIdTypeSearch;
@@ -66,6 +78,8 @@ public class SnapshotDaoImpl extends GenericDaoBase<SnapshotVO, Long> implements
6678
private SearchBuilder<SnapshotVO> StatusSearch;
6779
private SearchBuilder<SnapshotVO> notInStatusSearch;
6880
private GenericSearchBuilder<SnapshotVO, Long> CountSnapshotsByAccount;
81+
82+
private SearchBuilder<SnapshotVO> volumeIdAndTypeNotInSearch;
6983
@Inject
7084
ResourceTagDao _tagsDao;
7185
@Inject
@@ -76,9 +90,9 @@ public class SnapshotDaoImpl extends GenericDaoBase<SnapshotVO, Long> implements
7690
@Override
7791
public List<SnapshotVO> listByVolumeIdTypeNotDestroyed(long volumeId, Type type) {
7892
SearchCriteria<SnapshotVO> sc = VolumeIdTypeNotDestroyedSearch.create();
79-
sc.setParameters("volumeId", volumeId);
93+
sc.setParameters(VOLUME_ID, volumeId);
8094
sc.setParameters("type", type.ordinal());
81-
sc.setParameters("status", State.Destroyed);
95+
sc.setParameters(STATE, State.Destroyed);
8296
return listBy(sc, null);
8397
}
8498

@@ -95,28 +109,28 @@ public List<SnapshotVO> listByVolumeId(long volumeId) {
95109
@Override
96110
public List<SnapshotVO> listByVolumeId(Filter filter, long volumeId) {
97111
SearchCriteria<SnapshotVO> sc = VolumeIdSearch.create();
98-
sc.setParameters("volumeId", volumeId);
112+
sc.setParameters(VOLUME_ID, volumeId);
99113
return listBy(sc, filter);
100114
}
101115

102116
@Override
103117
public List<SnapshotVO> listByVolumeIdIncludingRemoved(long volumeId) {
104118
SearchCriteria<SnapshotVO> sc = VolumeIdSearch.create();
105-
sc.setParameters("volumeId", volumeId);
119+
sc.setParameters(VOLUME_ID, volumeId);
106120
return listIncludingRemovedBy(sc, null);
107121
}
108122

109123
public List<SnapshotVO> listByVolumeIdType(Filter filter, long volumeId, Type type) {
110124
SearchCriteria<SnapshotVO> sc = VolumeIdTypeSearch.create();
111-
sc.setParameters("volumeId", volumeId);
125+
sc.setParameters(VOLUME_ID, volumeId);
112126
sc.setParameters("type", type.ordinal());
113127
return listBy(sc, filter);
114128
}
115129

116130
public List<SnapshotVO> listByVolumeIdVersion(Filter filter, long volumeId, String version) {
117131
SearchCriteria<SnapshotVO> sc = VolumeIdVersionSearch.create();
118-
sc.setParameters("volumeId", volumeId);
119-
sc.setParameters("version", version);
132+
sc.setParameters(VOLUME_ID, volumeId);
133+
sc.setParameters(VERSION, version);
120134
return listBy(sc, filter);
121135
}
122136

@@ -126,61 +140,67 @@ public SnapshotDaoImpl() {
126140
@PostConstruct
127141
protected void init() {
128142
VolumeIdSearch = createSearchBuilder();
129-
VolumeIdSearch.and("volumeId", VolumeIdSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
143+
VolumeIdSearch.and(VOLUME_ID, VolumeIdSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
130144
VolumeIdSearch.done();
131145

132146
VolumeIdTypeSearch = createSearchBuilder();
133-
VolumeIdTypeSearch.and("volumeId", VolumeIdTypeSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
147+
VolumeIdTypeSearch.and(VOLUME_ID, VolumeIdTypeSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
134148
VolumeIdTypeSearch.and("type", VolumeIdTypeSearch.entity().getSnapshotType(), SearchCriteria.Op.EQ);
135149
VolumeIdTypeSearch.done();
136150

137151
VolumeIdTypeNotDestroyedSearch = createSearchBuilder();
138-
VolumeIdTypeNotDestroyedSearch.and("volumeId", VolumeIdTypeNotDestroyedSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
152+
VolumeIdTypeNotDestroyedSearch.and(VOLUME_ID, VolumeIdTypeNotDestroyedSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
139153
VolumeIdTypeNotDestroyedSearch.and("type", VolumeIdTypeNotDestroyedSearch.entity().getSnapshotType(), SearchCriteria.Op.EQ);
140-
VolumeIdTypeNotDestroyedSearch.and("status", VolumeIdTypeNotDestroyedSearch.entity().getState(), SearchCriteria.Op.NEQ);
154+
VolumeIdTypeNotDestroyedSearch.and(STATE, VolumeIdTypeNotDestroyedSearch.entity().getState(), SearchCriteria.Op.NEQ);
141155
VolumeIdTypeNotDestroyedSearch.done();
142156

143157
VolumeIdVersionSearch = createSearchBuilder();
144-
VolumeIdVersionSearch.and("volumeId", VolumeIdVersionSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
145-
VolumeIdVersionSearch.and("version", VolumeIdVersionSearch.entity().getVersion(), SearchCriteria.Op.EQ);
158+
VolumeIdVersionSearch.and(VOLUME_ID, VolumeIdVersionSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
159+
VolumeIdVersionSearch.and(VERSION, VolumeIdVersionSearch.entity().getVersion(), SearchCriteria.Op.EQ);
146160
VolumeIdVersionSearch.done();
147161

148162
AccountIdSearch = createSearchBuilder();
149-
AccountIdSearch.and("accountId", AccountIdSearch.entity().getAccountId(), SearchCriteria.Op.EQ);
163+
AccountIdSearch.and(ACCOUNT_ID, AccountIdSearch.entity().getAccountId(), SearchCriteria.Op.EQ);
150164
AccountIdSearch.done();
151165

152166
StatusSearch = createSearchBuilder();
153-
StatusSearch.and("volumeId", StatusSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
154-
StatusSearch.and("status", StatusSearch.entity().getState(), SearchCriteria.Op.IN);
167+
StatusSearch.and(VOLUME_ID, StatusSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
168+
StatusSearch.and(STATE, StatusSearch.entity().getState(), SearchCriteria.Op.IN);
155169
StatusSearch.done();
156170

157171
notInStatusSearch = createSearchBuilder();
158-
notInStatusSearch.and("volumeId", notInStatusSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
159-
notInStatusSearch.and("status", notInStatusSearch.entity().getState(), SearchCriteria.Op.NOTIN);
172+
notInStatusSearch.and(VOLUME_ID, notInStatusSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
173+
notInStatusSearch.and(STATE, notInStatusSearch.entity().getState(), SearchCriteria.Op.NOTIN);
160174
notInStatusSearch.done();
161175

162176
CountSnapshotsByAccount = createSearchBuilder(Long.class);
163177
CountSnapshotsByAccount.select(null, Func.COUNT, null);
164-
CountSnapshotsByAccount.and("account", CountSnapshotsByAccount.entity().getAccountId(), SearchCriteria.Op.EQ);
165-
CountSnapshotsByAccount.and("status", CountSnapshotsByAccount.entity().getState(), SearchCriteria.Op.NIN);
166-
CountSnapshotsByAccount.and("removed", CountSnapshotsByAccount.entity().getRemoved(), SearchCriteria.Op.NULL);
178+
CountSnapshotsByAccount.and(ACCOUNT_ID, CountSnapshotsByAccount.entity().getAccountId(), SearchCriteria.Op.EQ);
179+
CountSnapshotsByAccount.and(STATE, CountSnapshotsByAccount.entity().getState(), SearchCriteria.Op.NIN);
180+
CountSnapshotsByAccount.and(REMOVED, CountSnapshotsByAccount.entity().getRemoved(), SearchCriteria.Op.NULL);
167181
CountSnapshotsByAccount.done();
168182

169183
InstanceIdSearch = createSearchBuilder();
170-
InstanceIdSearch.and("status", InstanceIdSearch.entity().getState(), SearchCriteria.Op.IN);
184+
InstanceIdSearch.and(STATE, InstanceIdSearch.entity().getState(), SearchCriteria.Op.IN);
171185

172186
snapshotIdsSearch = createSearchBuilder();
173-
snapshotIdsSearch.and("id", snapshotIdsSearch.entity().getId(), SearchCriteria.Op.IN);
187+
snapshotIdsSearch.and(ID, snapshotIdsSearch.entity().getId(), SearchCriteria.Op.IN);
174188

175189
SearchBuilder<VMInstanceVO> instanceSearch = _instanceDao.createSearchBuilder();
176-
instanceSearch.and("instanceId", instanceSearch.entity().getId(), SearchCriteria.Op.EQ);
190+
instanceSearch.and(INSTANCE_ID, instanceSearch.entity().getId(), SearchCriteria.Op.EQ);
177191

178192
SearchBuilder<VolumeVO> volumeSearch = _volumeDao.createSearchBuilder();
179-
volumeSearch.and("state", volumeSearch.entity().getState(), SearchCriteria.Op.EQ);
180-
volumeSearch.join("instanceVolumes", instanceSearch, instanceSearch.entity().getId(), volumeSearch.entity().getInstanceId(), JoinType.INNER);
193+
volumeSearch.and(STATE, volumeSearch.entity().getState(), SearchCriteria.Op.EQ);
194+
volumeSearch.join(INSTANCE_VOLUMES, instanceSearch, instanceSearch.entity().getId(), volumeSearch.entity().getInstanceId(), JoinType.INNER);
181195

182-
InstanceIdSearch.join("instanceSnapshots", volumeSearch, volumeSearch.entity().getId(), InstanceIdSearch.entity().getVolumeId(), JoinType.INNER);
196+
InstanceIdSearch.join(INSTANCE_SNAPSHOTS, volumeSearch, volumeSearch.entity().getId(), InstanceIdSearch.entity().getVolumeId(), JoinType.INNER);
183197
InstanceIdSearch.done();
198+
199+
volumeIdAndTypeNotInSearch = createSearchBuilder();
200+
volumeIdAndTypeNotInSearch.and(VOLUME_ID, volumeIdAndTypeNotInSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
201+
volumeIdAndTypeNotInSearch.and(STATE, volumeIdAndTypeNotInSearch.entity().getState(), SearchCriteria.Op.NEQ);
202+
volumeIdAndTypeNotInSearch.and(NOT_TYPE, volumeIdAndTypeNotInSearch.entity().getTypeDescription(), SearchCriteria.Op.NOTIN);
203+
volumeIdAndTypeNotInSearch.done();
184204
}
185205

186206
@Override
@@ -205,8 +225,8 @@ public long getLastSnapshot(long volumeId, DataStoreRole role) {
205225
@Override
206226
public Long countSnapshotsForAccount(long accountId) {
207227
SearchCriteria<Long> sc = CountSnapshotsByAccount.create();
208-
sc.setParameters("account", accountId);
209-
sc.setParameters("status", State.Error, State.Destroyed);
228+
sc.setParameters(ACCOUNT_ID, accountId);
229+
sc.setParameters(STATE, State.Error, State.Destroyed);
210230
return customSearch(sc, null).get(0);
211231
}
212232

@@ -215,19 +235,19 @@ public List<SnapshotVO> listByInstanceId(long instanceId, Snapshot.State... stat
215235
SearchCriteria<SnapshotVO> sc = InstanceIdSearch.create();
216236

217237
if (status != null && status.length != 0) {
218-
sc.setParameters("status", (Object[])status);
238+
sc.setParameters(STATE, (Object[])status);
219239
}
220240

221-
sc.setJoinParameters("instanceSnapshots", "state", Volume.State.Ready);
222-
sc.setJoinParameters("instanceVolumes", "instanceId", instanceId);
241+
sc.setJoinParameters(INSTANCE_SNAPSHOTS, STATE, Volume.State.Ready);
242+
sc.setJoinParameters(INSTANCE_VOLUMES, INSTANCE_ID, instanceId);
223243
return listBy(sc, null);
224244
}
225245

226246
@Override
227247
public List<SnapshotVO> listByStatus(long volumeId, Snapshot.State... status) {
228248
SearchCriteria<SnapshotVO> sc = StatusSearch.create();
229-
sc.setParameters("volumeId", volumeId);
230-
sc.setParameters("status", (Object[])status);
249+
sc.setParameters(VOLUME_ID, volumeId);
250+
sc.setParameters(STATE, (Object[])status);
231251
return listBy(sc, null);
232252
}
233253

@@ -248,14 +268,14 @@ public boolean remove(Long id) {
248268
@Override
249269
public List<SnapshotVO> listAllByStatus(Snapshot.State... status) {
250270
SearchCriteria<SnapshotVO> sc = StatusSearch.create();
251-
sc.setParameters("status", (Object[])status);
271+
sc.setParameters(STATE, (Object[])status);
252272
return listBy(sc, null);
253273
}
254274

255275
@Override
256276
public List<SnapshotVO> listByIds(Object... ids) {
257277
SearchCriteria<SnapshotVO> sc = snapshotIdsSearch.create();
258-
sc.setParameters("id", ids);
278+
sc.setParameters(ID, ids);
259279
return listBy(sc, null);
260280
}
261281

@@ -273,7 +293,7 @@ public boolean updateState(State currentState, Event event, State nextState, Sna
273293
@Override
274294
public void updateVolumeIds(long oldVolId, long newVolId) {
275295
SearchCriteria<SnapshotVO> sc = VolumeIdSearch.create();
276-
sc.setParameters("volumeId", oldVolId);
296+
sc.setParameters(VOLUME_ID, oldVolId);
277297
SnapshotVO snapshot = createForUpdate();
278298
snapshot.setVolumeId(newVolId);
279299
UpdateBuilder ub = getUpdateBuilder(snapshot);
@@ -283,8 +303,8 @@ public void updateVolumeIds(long oldVolId, long newVolId) {
283303
@Override
284304
public List<SnapshotVO> listByStatusNotIn(long volumeId, Snapshot.State... status) {
285305
SearchCriteria<SnapshotVO> sc = this.notInStatusSearch.create();
286-
sc.setParameters("volumeId", volumeId);
287-
sc.setParameters("status", (Object[]) status);
306+
sc.setParameters(VOLUME_ID, volumeId);
307+
sc.setParameters(STATE, (Object[]) status);
288308
return listBy(sc, null);
289309
}
290310

@@ -299,4 +319,14 @@ public List<SnapshotVO> searchByVolumes(List<Long> volumeIds) {
299319
sc.setParameters("volumeIds", volumeIds.toArray());
300320
return search(sc, null);
301321
}
322+
323+
@Override
324+
public List<SnapshotVO> listByVolumeIdAndTypeNotInAndStateNotRemoved(long volumeId, Type... types) {
325+
SearchCriteria<SnapshotVO> sc = volumeIdAndTypeNotInSearch.create();
326+
sc.setParameters(VOLUME_ID, volumeId);
327+
sc.setParameters(NOT_TYPE, Arrays.stream(types).map(Type::toString).toArray());
328+
sc.setParameters(STATE, State.Destroyed);
329+
330+
return listBy(sc);
331+
}
302332
}

engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ public interface VMSnapshotDao extends GenericDao<VMSnapshotVO, Long>, StateDao<
2727

2828
List<VMSnapshotVO> findByVm(Long vmId);
2929

30+
List<VMSnapshotVO> findByVmAndByType(Long vmId, VMSnapshot.Type type);
31+
3032
List<VMSnapshotVO> listExpungingSnapshot();
3133

3234
List<VMSnapshotVO> listByInstanceId(Long vmId, VMSnapshot.State... status);

engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,14 @@ public List<VMSnapshotVO> findByVm(Long vmId) {
8080
return listBy(sc, null);
8181
}
8282

83+
@Override
84+
public List<VMSnapshotVO> findByVmAndByType(Long vmId, VMSnapshot.Type type) {
85+
SearchCriteria<VMSnapshotVO> sc = AllFieldsSearch.create();
86+
sc.setParameters("vm_id", vmId);
87+
sc.setParameters("vm_snapshot_type", type);
88+
return listBy(sc, null);
89+
}
90+
8391
@Override
8492
public List<VMSnapshotVO> listExpungingSnapshot() {
8593
SearchCriteria<VMSnapshotVO> sc = ExpungingSnapshotSearch.create();

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323

2424
import javax.inject.Inject;
2525

26+
import com.cloud.vm.snapshot.VMSnapshot;
27+
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
2628
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
2729
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
2830
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
@@ -100,6 +102,9 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
100102
@Inject
101103
SnapshotZoneDao snapshotZoneDao;
102104

105+
@Inject
106+
private VMSnapshotDao vmSnapshotDao;
107+
103108
private final List<Snapshot.State> snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error);
104109

105110
public SnapshotDataStoreVO getSnapshotImageStoreRef(long snapshotId, long zoneId) {
@@ -571,6 +576,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
571576

572577
@Override
573578
public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperation op) {
579+
if (SnapshotOperation.TAKE.equals(op)) {
580+
return canHandleTake(snapshot);
581+
}
574582
if (SnapshotOperation.REVERT.equals(op)) {
575583
long volumeId = snapshot.getVolumeId();
576584
VolumeVO volumeVO = volumeDao.findById(volumeId);
@@ -597,4 +605,16 @@ protected boolean isSnapshotStoredOnSameZoneStoreForQCOW2Volume(Snapshot snapsho
597605
dataStoreMgr.getStoreZoneId(s.getDataStoreId(), s.getRole()), volumeVO.getDataCenterId()));
598606
}
599607

608+
private StrategyPriority canHandleTake(Snapshot snapshot) {
609+
VolumeVO volumeVO = volumeDao.findById(snapshot.getVolumeId());
610+
if (volumeVO.getInstanceId() == null) {
611+
return StrategyPriority.DEFAULT;
612+
}
613+
if (CollectionUtils.isNotEmpty(vmSnapshotDao.findByVmAndByType(volumeVO.getInstanceId(), VMSnapshot.Type.DiskAndMemory))) {
614+
logger.debug("DefaultSnapshotStrategy cannot handle snapshot [{}] for volume [{}] as the volume is attached to a VM with disk-and-memory VM snapshots." +
615+
"Restoring the volume snapshot will corrupt any newer disk-and-memory VM snapshots.", snapshot);
616+
return StrategyPriority.CANT_HANDLE;
617+
}
618+
return StrategyPriority.DEFAULT;
619+
}
600620
}

0 commit comments

Comments
 (0)