Skip to content

Commit 885d2a4

Browse files
committed
Fix multiple secondary storages for a single zone
1 parent e3443c8 commit 885d2a4

File tree

10 files changed

+91
-25
lines changed

10 files changed

+91
-25
lines changed

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public class SnapshotObjectTO extends DownloadableObjectTO implements DataTO {
4343
private long id;
4444
private boolean quiescevm;
4545
private String[] parents;
46+
private DataStoreTO parentStore;
4647
private String checkpointPath;
4748
private Long physicalSize = (long) 0;
4849
private long accountId;
@@ -211,6 +212,14 @@ public void setAccountId(long accountId) {
211212
this.accountId = accountId;
212213
}
213214

215+
public DataStoreTO getParentStore() {
216+
return parentStore;
217+
}
218+
219+
public void setParentStore(DataStoreTO parentStore) {
220+
this.parentStore = parentStore;
221+
}
222+
214223
@Override
215224
public String toString() {
216225
return new StringBuilder("SnapshotTO[datastore=").append(dataStore).append("|volume=").append(volume).append("|path").append(path).append("]").toString();

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3412,6 +3412,7 @@ protected List<VolumeObjectTO> getVmVolumesWithCheckpointsToRecreate(VMInstanceV
34123412
VolumeObjectTO volumeTo = new VolumeObjectTO();
34133413
volumeTo.setCheckpointPaths(volumeCheckpointPathsAndImageStoreUrls.first());
34143414
volumeTo.setCheckpointImageStoreUrls(volumeCheckpointPathsAndImageStoreUrls.second());
3415+
volumeTo.setPath(volume.getPath());
34153416
volumes.add(volumeTo);
34163417
}
34173418
return volumes;

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

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,19 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
7272
private SearchBuilder<SnapshotDataStoreVO> dataStoreAndInstallPathSearch;
7373
private SearchBuilder<SnapshotDataStoreVO> storeAndSnapshotIdsSearch;
7474
private SearchBuilder<SnapshotDataStoreVO> storeSnapshotDownloadStatusSearch;
75-
private SearchBuilder<SnapshotDataStoreVO> searchFilteringVolumeIdEqAndStateEqAndKVMCheckpointPathNotNull;
75+
private SearchBuilder<SnapshotDataStoreVO> searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull;
7676
private SearchBuilder<SnapshotDataStoreVO> searchFilterStateAndDownloadUrlNotNullAndDownloadUrlCreatedBefore;
77+
private SearchBuilder<SnapshotDataStoreVO> searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq;
7778

7879

7980
protected static final List<Hypervisor.HypervisorType> HYPERVISORS_SUPPORTING_SNAPSHOTS_CHAINING = List.of(Hypervisor.HypervisorType.XenServer);
8081

8182
@Inject
8283
protected SnapshotDao snapshotDao;
8384

85+
@Inject
86+
protected ImageStoreDao imageStoreDao;
87+
8488
private static final String FIND_OLDEST_OR_LATEST_SNAPSHOT = "select store_id, store_role, snapshot_id from cloud.snapshot_store_ref where " +
8589
" store_role = ? and volume_id = ? and state = 'Ready'" +
8690
" order by created %s " +
@@ -163,18 +167,26 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
163167
storeSnapshotDownloadStatusSearch.and("downloadState", storeSnapshotDownloadStatusSearch.entity().getDownloadState(), SearchCriteria.Op.IN);
164168
storeSnapshotDownloadStatusSearch.done();
165169

166-
searchFilteringVolumeIdEqAndStateEqAndKVMCheckpointPathNotNull = createSearchBuilder();
167-
searchFilteringVolumeIdEqAndStateEqAndKVMCheckpointPathNotNull.and(VOLUME_ID, searchFilteringVolumeIdEqAndStateEqAndKVMCheckpointPathNotNull.entity().getVolumeId(), SearchCriteria.Op.EQ);
168-
searchFilteringVolumeIdEqAndStateEqAndKVMCheckpointPathNotNull.and(STATE, searchFilteringVolumeIdEqAndStateEqAndKVMCheckpointPathNotNull.entity().getState(), SearchCriteria.Op.EQ);
169-
searchFilteringVolumeIdEqAndStateEqAndKVMCheckpointPathNotNull.and(KVM_CHECKPOINT_PATH, searchFilteringVolumeIdEqAndStateEqAndKVMCheckpointPathNotNull.entity().getKvmCheckpointPath(), SearchCriteria.Op.NNULL);
170-
searchFilteringVolumeIdEqAndStateEqAndKVMCheckpointPathNotNull.done();
170+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull = createSearchBuilder();
171+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.and(VOLUME_ID, searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.entity().getVolumeId(), SearchCriteria.Op.EQ);
172+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.and(STATE, searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.entity().getState(), SearchCriteria.Op.EQ);
173+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.and(STORE_ROLE, searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.entity().getRole(), SearchCriteria.Op.EQ);
174+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.and(KVM_CHECKPOINT_PATH, searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.entity().getKvmCheckpointPath(), SearchCriteria.Op.NNULL);
175+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.and(STORE_ID, searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.entity().getDataStoreId(), SearchCriteria.Op.IN);
176+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.done();
171177

172178
searchFilterStateAndDownloadUrlNotNullAndDownloadUrlCreatedBefore = createSearchBuilder();
173179
searchFilterStateAndDownloadUrlNotNullAndDownloadUrlCreatedBefore.and(STATE, searchFilterStateAndDownloadUrlNotNullAndDownloadUrlCreatedBefore.entity().getState(), SearchCriteria.Op.EQ);
174180
searchFilterStateAndDownloadUrlNotNullAndDownloadUrlCreatedBefore.and(DOWNLOAD_URL, searchFilterStateAndDownloadUrlNotNullAndDownloadUrlCreatedBefore.entity().getExtractUrl(), SearchCriteria.Op.NNULL);
175181
searchFilterStateAndDownloadUrlNotNullAndDownloadUrlCreatedBefore.and(URL_CREATED_BEFORE, searchFilterStateAndDownloadUrlNotNullAndDownloadUrlCreatedBefore.entity().getExtractUrlCreated(), SearchCriteria.Op.LT);
176182
searchFilterStateAndDownloadUrlNotNullAndDownloadUrlCreatedBefore.done();
177183

184+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq = createSearchBuilder();
185+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq.and(STATE, searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq.entity().getState(), SearchCriteria.Op.EQ);
186+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq.and(VOLUME_ID, searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq.entity().getVolumeId(), SearchCriteria.Op.EQ);
187+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq.and(STORE_ROLE, searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq.entity().getRole(), SearchCriteria.Op.EQ);
188+
searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq.and(STORE_ID, searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq.entity().getDataStoreId(), SearchCriteria.Op.IN);
189+
178190
return true;
179191
}
180192

@@ -318,12 +330,23 @@ public SnapshotDataStoreVO findParent(DataStoreRole role, Long storeId, Long zon
318330
return null;
319331
}
320332

321-
SearchCriteria<SnapshotDataStoreVO> sc = searchFilteringStoreIdEqStateEqStoreRoleEqIdEqUpdateCountEqSnapshotIdEqVolumeIdEq.create();
333+
SearchCriteria<SnapshotDataStoreVO> sc;
334+
if (kvmIncrementalSnapshot && Hypervisor.HypervisorType.KVM.equals(hypervisorType)) {
335+
sc = searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.create();
336+
} else {
337+
sc = searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq.create();
338+
}
339+
322340
sc.setParameters(VOLUME_ID, volumeId);
323341
sc.setParameters(STORE_ROLE, role.toString());
324342
sc.setParameters(STATE, ObjectInDataStoreStateMachine.State.Ready.name());
325-
sc.setParametersIfNotNull(STORE_ID, storeId);
326-
sc.setParametersIfNotNull();
343+
if (storeId != null) {
344+
sc.setParameters(STORE_ID, (Object) new Long[]{storeId});
345+
} else if (zoneId != null) {
346+
List<ImageStoreVO> imageStores = imageStoreDao.listStoresByZoneId(zoneId);
347+
Object[] imageStoreIds = imageStores.stream().map(ImageStoreVO::getId).toArray();
348+
sc.setParameters(STORE_ID, imageStoreIds);
349+
}
327350

328351
List<SnapshotDataStoreVO> snapshotList = listBy(sc, new Filter(SnapshotDataStoreVO.class, CREATED, false, null, null));
329352
if (CollectionUtils.isEmpty(snapshotList)) {
@@ -580,7 +603,7 @@ protected boolean isSnapshotChainingRequired(long volumeId, boolean kvmIncrement
580603

581604
@Override
582605
public List<SnapshotDataStoreVO> listReadyByVolumeIdAndCheckpointPathNotNull(long volumeId) {
583-
SearchCriteria<SnapshotDataStoreVO> sc = searchFilteringVolumeIdEqAndStateEqAndKVMCheckpointPathNotNull.create();
606+
SearchCriteria<SnapshotDataStoreVO> sc = searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.create();
584607
sc.setParameters(VOLUME_ID, volumeId);
585608
sc.setParameters(STATE, State.Ready);
586609
return listBy(sc);

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,13 +189,13 @@ public SnapshotInfo backupSnapshot(SnapshotInfo snapshot) {
189189

190190
protected boolean deleteSnapshotChain(SnapshotInfo snapshot, String storageToString) {
191191
long rootSnapshotId = getRootSnapshotId(snapshot);
192-
snapshotDao.acquireInLockTable(rootSnapshotId);
193192
DataTO snapshotTo = snapshot.getTO();
194193
logger.debug(String.format("Deleting %s chain of snapshots.", snapshotTo));
195194

196195
boolean result = false;
197196
boolean resultIsSet = false;
198197
try {
198+
snapshotDao.acquireInLockTable(rootSnapshotId);
199199
do {
200200
SnapshotInfo child = snapshot.getChild();
201201

@@ -243,15 +243,18 @@ protected boolean deleteSnapshotChain(SnapshotInfo snapshot, String storageToStr
243243
}
244244
} catch (Exception e) {
245245
logger.error(String.format("Failed to delete snapshot [%s] on storage [%s] due to [%s].", snapshotTo, storageToString, e.getMessage()), e);
246+
throw e;
246247
}
247248
}
248249

249250
snapshot = parent;
250251
} while (snapshot != null && snapshotStatesAbleToDeleteSnapshot.contains(snapshot.getState()));
251252
} catch (Exception e) {
252253
logger.error(String.format("Failed to delete snapshot [%s] on storage [%s] due to [%s].", snapshotTo, storageToString, e.getMessage()), e);
254+
throw new CloudRuntimeException("Failed to delete snapshot chain.");
255+
} finally {
256+
snapshotDao.releaseFromLockTable(rootSnapshotId);
253257
}
254-
snapshotDao.releaseFromLockTable(rootSnapshotId);
255258
return result;
256259
}
257260

engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020

2121
import com.cloud.host.dao.HostDao;
2222
import com.cloud.hypervisor.Hypervisor;
23+
import com.cloud.storage.ImageStore;
2324
import com.cloud.storage.snapshot.SnapshotManager;
25+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
2426
import org.apache.logging.log4j.Logger;
2527
import org.apache.logging.log4j.LogManager;
2628
import org.springframework.stereotype.Component;
@@ -193,7 +195,7 @@ public DataObject create(DataObject obj, DataStore dataStore) {
193195
ss.setSize(snapshot.getSize());
194196
ss.setVolumeId(snapshot.getVolumeId());
195197
Long clusterId = hostDao.findClusterIdByVolumeInfo(snapshot.getBaseVolume());
196-
SnapshotDataStoreVO snapshotDataStoreVO = snapshotDataStoreDao.findParent(dataStore.getRole(), dataStore.getId(), null, snapshot.getVolumeId(), SnapshotManager.kvmIncrementalSnapshot.valueIn(clusterId), snapshot.getHypervisorType());
198+
SnapshotDataStoreVO snapshotDataStoreVO = snapshotDataStoreDao.findParent(dataStore.getRole(), null, ((ImageStore)dataStore).getDataCenterId(), snapshot.getVolumeId(), SnapshotManager.kvmIncrementalSnapshot.valueIn(clusterId), snapshot.getHypervisorType());
197199
if (snapshotDataStoreVO != null && !snapshotDataStoreVO.isEndOfChain()) {
198200
ss.setParentSnapshotId(snapshotDataStoreVO.getSnapshotId());
199201
} else {
@@ -220,7 +222,7 @@ public DataObject create(DataObject obj, DataStore dataStore) {
220222
private SnapshotDataStoreVO tryToGetSnapshotOnSecondaryIfNotOnPrimaryAndIsKVM(DataStore dataStore, SnapshotInfo snapshotInfo, SnapshotDataStoreVO snapshotDataStoreVO,
221223
boolean kvmIncrementalSnapshot, Hypervisor.HypervisorType hypervisorType) {
222224
if (snapshotDataStoreVO == null && Hypervisor.HypervisorType.KVM.equals(snapshotInfo.getHypervisorType()) && DataStoreRole.Primary.equals(dataStore.getRole())) {
223-
snapshotDataStoreVO = snapshotDataStoreDao.findParent(DataStoreRole.Image, snapshotInfo.getImageStore().getId(), null, snapshotInfo.getVolumeId(), kvmIncrementalSnapshot, hypervisorType);
225+
snapshotDataStoreVO = snapshotDataStoreDao.findParent(DataStoreRole.Image, null, ((PrimaryDataStore)dataStore).getDataCenterId(), snapshotInfo.getVolumeId(), kvmIncrementalSnapshot, hypervisorType);
224226
}
225227
return snapshotDataStoreVO;
226228
}

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4691,6 +4691,7 @@ public boolean recreateCheckpointsOnVm(List<VolumeObjectTO> volumes, String vmNa
46914691
}
46924692
connectToAllVolumeSnapshotSecondaryStorages(volume);
46934693
recreateCheckpointsOfDisk(vmName, volume, mapDiskToDiskDef);
4694+
disconnectAllVolumeSnapshotSecondaryStorages(volume);
46944695
}
46954696
logger.debug("Successfully recreated all checkpoints on VM [{}].", vmName);
46964697
return true;
@@ -4700,6 +4701,13 @@ public void connectToAllVolumeSnapshotSecondaryStorages(VolumeObjectTO volumeObj
47004701
volumeObjectTO.getCheckpointImageStoreUrls().forEach(uri -> getStoragePoolMgr().getStoragePoolByURI(uri));
47014702
}
47024703

4704+
public void disconnectAllVolumeSnapshotSecondaryStorages(VolumeObjectTO volumeObjectTO) {
4705+
volumeObjectTO.getCheckpointImageStoreUrls().forEach(uri -> {
4706+
KVMStoragePool storage = getStoragePoolMgr().getStoragePoolByURI(uri);
4707+
getStoragePoolMgr().deleteStoragePool(storage.getType(), storage.getUuid());
4708+
});
4709+
}
4710+
47034711

47044712
protected void recreateCheckpointsOfDisk(String vmName, VolumeObjectTO volume, Map<VolumeObjectTO, DiskDef> mapDiskToDiskDef) {
47054713
for (String path : volume.getCheckpointPaths()) {

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertSnapshotCommandWrapper.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ public Answer execute(ConvertSnapshotCommand command, LibvirtComputingResource s
9191
} catch (LibvirtException | QemuImgException ex) {
9292
logger.error(String.format("Failed to convert snapshot [%s] due to %s.", snapshotObjectTO, ex.getMessage()), ex);
9393
return new Answer(command, ex);
94+
} finally {
95+
serverResource.disconnectAllVolumeSnapshotSecondaryStorages(snapshotObjectTO.getVolume());
9496
}
9597
}
9698
}

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ protected void revertVolumeToSnapshot(KVMStoragePool kvmStoragePoolSecondary, Sn
176176
logger.debug(String.format("Successfully reverted volume [%s] to snapshot [%s].", volumeObjectTo, snapshotToPrint));
177177
} catch (LibvirtException | QemuImgException ex) {
178178
throw new CloudRuntimeException(String.format("Unable to revert volume [%s] to snapshot [%s] due to [%s].", volumeObjectTo, snapshotToPrint, ex.getMessage()), ex);
179+
} finally {
180+
if (kvmStoragePoolSecondary != null) {
181+
resource.disconnectAllVolumeSnapshotSecondaryStorages(volumeObjectTo);
182+
}
179183
}
180184
}
181185

0 commit comments

Comments
 (0)