Skip to content

Commit 961e85e

Browse files
slavkapslavkap
authored
Fix of creating volumes from snapshots without backup to secondary storage (#5349)
* Fix of creating volumes from snapshots without backup When few snaphots are created onyl on primary storage, and try to create a volume or a template from the snapshot only the first operation is successful. Its because the snapshot is backup on secondary storage with wrong SQL query. The problem appears on Ceph/NFS but may affects other storage plugins. Bypassing secondary storage is implemented only for Ceph primary storage and it didn't cover the functionality to create volume from snapshot which is kept only on Ceph * Address review
1 parent 14323c9 commit 961e85e

File tree

7 files changed

+199
-41
lines changed

7 files changed

+199
-41
lines changed

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@
114114
import com.cloud.storage.Snapshot;
115115
import com.cloud.storage.Storage;
116116
import com.cloud.storage.Storage.ImageFormat;
117+
import com.cloud.storage.Storage.StoragePoolType;
117118
import com.cloud.storage.StorageManager;
118119
import com.cloud.storage.StoragePool;
119120
import com.cloud.storage.VMTemplateStorageResourceAssoc;
@@ -124,6 +125,7 @@
124125
import com.cloud.storage.dao.SnapshotDao;
125126
import com.cloud.storage.dao.VolumeDao;
126127
import com.cloud.storage.dao.VolumeDetailsDao;
128+
import com.cloud.storage.snapshot.SnapshotManager;
127129
import com.cloud.template.TemplateManager;
128130
import com.cloud.template.VirtualMachineTemplate;
129131
import com.cloud.user.Account;
@@ -454,18 +456,11 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use
454456
if (snapInfo == null) {
455457
throw new CloudRuntimeException("Cannot find snapshot " + snapshot.getId());
456458
}
457-
// We need to copy the snapshot onto secondary.
458-
SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.BACKUP);
459-
snapshotStrategy.backupSnapshot(snapInfo);
460459

461-
// Attempt to grab it again.
462-
snapInfo = snapshotFactory.getSnapshot(snapshot.getId(), dataStoreRole);
463-
if (snapInfo == null) {
464-
throw new CloudRuntimeException("Cannot find snapshot " + snapshot.getId() + " on secondary and could not create backup");
465-
}
460+
snapInfo = backupSnapshotIfNeeded(snapshot, dataStoreRole, snapInfo);
466461
}
467462
// don't try to perform a sync if the DataStoreRole of the snapshot is equal to DataStoreRole.Primary
468-
if (!DataStoreRole.Primary.equals(dataStoreRole)) {
463+
if (!DataStoreRole.Primary.equals(snapInfo.getDataStore().getRole())) {
469464
try {
470465
// sync snapshot to region store if necessary
471466
DataStore snapStore = snapInfo.getDataStore();
@@ -497,6 +492,25 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use
497492

498493
}
499494

495+
private SnapshotInfo backupSnapshotIfNeeded(Snapshot snapshot, DataStoreRole dataStoreRole, SnapshotInfo snapInfo) {
496+
boolean backupSnapToSecondary = SnapshotManager.BackupSnapshotAfterTakingSnapshot.value() == null || SnapshotManager.BackupSnapshotAfterTakingSnapshot.value();
497+
498+
StoragePoolVO srcPool = _storagePoolDao.findById(snapInfo.getBaseVolume().getPoolId());
499+
// We need to copy the snapshot onto secondary.
500+
//Skipping the backup to secondary storage with NFS/FS could be supported when CLOUDSTACK-5297 is accepted with small enhancement in:
501+
//KVMStorageProcessor::createVolumeFromSnapshot and CloudStackPrimaryDataStoreDriverImpl::copyAsync/createAsync
502+
if ((!backupSnapToSecondary && (StoragePoolType.NetworkFilesystem.equals(srcPool.getPoolType()) || StoragePoolType.Filesystem.equals(srcPool.getPoolType())))) {
503+
SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.BACKUP);
504+
snapshotStrategy.backupSnapshot(snapInfo);
505+
// Attempt to grab it again.
506+
snapInfo = snapshotFactory.getSnapshot(snapshot.getId(), dataStoreRole);
507+
if (snapInfo == null) {
508+
throw new CloudRuntimeException("Cannot find snapshot " + snapshot.getId() + " on secondary and could not create backup");
509+
}
510+
}
511+
return snapInfo;
512+
}
513+
500514
public DataStoreRole getDataStoreRole(Snapshot snapshot) {
501515
SnapshotDataStoreVO snapshotStore = _snapshotDataStoreDao.findBySnapshot(snapshot.getId(), DataStoreRole.Primary);
502516

‎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
@@ -68,6 +68,8 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
6868

6969
SnapshotDataStoreVO findByVolume(long volumeId, DataStoreRole role);
7070

71+
SnapshotDataStoreVO findByVolume(long snapshotId, long volumeId, DataStoreRole role);
72+
7173
/**
7274
* List all snapshots in 'snapshot_store_ref' by volume and data store role. Therefore, it is possible to list all snapshots that are in the primary storage or in the secondary storage.
7375
*/

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public SnapshotInfo getSnapshot(long snapshotId, DataStoreRole role) {
9292
}
9393
SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findBySnapshot(snapshotId, role);
9494
if (snapshotStore == null) {
95-
snapshotStore = snapshotStoreDao.findByVolume(snapshot.getVolumeId(), role);
95+
snapshotStore = snapshotStoreDao.findByVolume(snapshotId, snapshot.getVolumeId(), role);
9696
if (snapshotStore == null) {
9797
return null;
9898
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
142142
volumeSearch = createSearchBuilder();
143143
volumeSearch.and("volume_id", volumeSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
144144
volumeSearch.and("store_role", volumeSearch.entity().getRole(), SearchCriteria.Op.EQ);
145+
volumeSearch.and("snapshot_id", volumeSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ);
145146
volumeSearch.done();
146147

147148
stateSearch = createSearchBuilder();
@@ -365,6 +366,15 @@ public SnapshotDataStoreVO findByVolume(long volumeId, DataStoreRole role) {
365366
return findOneBy(sc);
366367
}
367368

369+
@Override
370+
public SnapshotDataStoreVO findByVolume(long snapshotId, long volumeId, DataStoreRole role) {
371+
SearchCriteria<SnapshotDataStoreVO> sc = volumeSearch.create();
372+
sc.setParameters("snapshot_id", snapshotId);
373+
sc.setParameters("volume_id", volumeId);
374+
sc.setParameters("store_role", role);
375+
return findOneBy(sc);
376+
}
377+
368378
@Override
369379
public List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId) {
370380
SearchCriteria<SnapshotDataStoreVO> sc = snapshotIdSearch.create();

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java‎

Lines changed: 130 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
import com.ceph.rbd.Rbd;
8686
import com.ceph.rbd.RbdException;
8787
import com.ceph.rbd.RbdImage;
88+
import com.ceph.rbd.jna.RbdSnapInfo;
8889
import com.cloud.agent.api.Answer;
8990
import com.cloud.agent.api.storage.PrimaryStorageDownloadAnswer;
9091
import com.cloud.agent.api.to.DataObjectType;
@@ -1594,39 +1595,24 @@ public Answer createVolumeFromSnapshot(final CopyCommand cmd) {
15941595
final DataStoreTO imageStore = srcData.getDataStore();
15951596
final VolumeObjectTO volume = snapshot.getVolume();
15961597

1597-
if (!(imageStore instanceof NfsTO)) {
1598+
if (!(imageStore instanceof NfsTO || imageStore instanceof PrimaryDataStoreTO)) {
15981599
return new CopyCmdAnswer("unsupported protocol");
15991600
}
16001601

1601-
final NfsTO nfsImageStore = (NfsTO)imageStore;
1602-
16031602
final String snapshotFullPath = snapshot.getPath();
16041603
final int index = snapshotFullPath.lastIndexOf("/");
16051604
final String snapshotPath = snapshotFullPath.substring(0, index);
16061605
final String snapshotName = snapshotFullPath.substring(index + 1);
1607-
final KVMStoragePool secondaryPool = storagePoolMgr.getStoragePoolByURI(nfsImageStore.getUrl() + File.separator + snapshotPath);
1608-
final KVMPhysicalDisk snapshotDisk = secondaryPool.getPhysicalDisk(snapshotName);
1609-
1610-
if (volume.getFormat() == ImageFormat.RAW) {
1611-
snapshotDisk.setFormat(PhysicalDiskFormat.RAW);
1612-
} else if (volume.getFormat() == ImageFormat.QCOW2) {
1613-
snapshotDisk.setFormat(PhysicalDiskFormat.QCOW2);
1606+
KVMPhysicalDisk disk = null;
1607+
if (imageStore instanceof NfsTO) {
1608+
disk = createVolumeFromSnapshotOnNFS(cmd, pool, imageStore, volume, snapshotPath, snapshotName);
1609+
} else {
1610+
disk = createVolumeFromRBDSnapshot(cmd, destData, pool, imageStore, volume, snapshotName, disk);
16141611
}
16151612

1616-
final String primaryUuid = pool.getUuid();
1617-
final KVMStoragePool primaryPool = storagePoolMgr.getStoragePool(pool.getPoolType(), primaryUuid);
1618-
final String volUuid = UUID.randomUUID().toString();
1619-
1620-
Map<String, String> details = cmd.getOptions2();
1621-
1622-
String path = details != null ? details.get(DiskTO.IQN) : null;
1623-
1624-
storagePoolMgr.connectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path, details);
1625-
1626-
KVMPhysicalDisk disk = storagePoolMgr.copyPhysicalDisk(snapshotDisk, path != null ? path : volUuid, primaryPool, cmd.getWaitInMillSeconds());
1627-
1628-
storagePoolMgr.disconnectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path);
1629-
1613+
if (disk == null) {
1614+
return new CopyCmdAnswer("Could not create volume from snapshot");
1615+
}
16301616
final VolumeObjectTO newVol = new VolumeObjectTO();
16311617
newVol.setPath(disk.getName());
16321618
newVol.setSize(disk.getVirtualSize());
@@ -1639,6 +1625,126 @@ public Answer createVolumeFromSnapshot(final CopyCommand cmd) {
16391625
}
16401626
}
16411627

1628+
private KVMPhysicalDisk createVolumeFromRBDSnapshot(CopyCommand cmd, DataTO destData,
1629+
PrimaryDataStoreTO pool, DataStoreTO imageStore, VolumeObjectTO volume, String snapshotName, KVMPhysicalDisk disk) {
1630+
PrimaryDataStoreTO primaryStore = (PrimaryDataStoreTO) imageStore;
1631+
KVMStoragePool srcPool = storagePoolMgr.getStoragePool(primaryStore.getPoolType(), primaryStore.getUuid());
1632+
KVMPhysicalDisk snapshotDisk = srcPool.getPhysicalDisk(volume.getPath());
1633+
KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid());
1634+
VolumeObjectTO newVol = (VolumeObjectTO) destData;
1635+
1636+
if (StoragePoolType.RBD.equals(primaryStore.getPoolType())) {
1637+
s_logger.debug(String.format("Attempting to create volume from RBD snapshot %s", snapshotName));
1638+
if (StoragePoolType.RBD.equals(pool.getPoolType())) {
1639+
disk = createRBDvolumeFromRBDSnapshot(snapshotDisk, snapshotName, newVol.getUuid(),
1640+
PhysicalDiskFormat.RAW, newVol.getSize(), destPool, cmd.getWaitInMillSeconds());
1641+
s_logger.debug(String.format("Created RBD volume %s from snapshot %s", disk, snapshotDisk));
1642+
} else {
1643+
Map<String, String> details = cmd.getOptions2();
1644+
1645+
String path = details != null ? details.get(DiskTO.IQN) : null;
1646+
1647+
storagePoolMgr.connectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path, details);
1648+
1649+
snapshotDisk.setPath(snapshotDisk.getPath() + "@" + snapshotName);
1650+
disk = storagePoolMgr.copyPhysicalDisk(snapshotDisk, path != null ? path : newVol.getUuid(),
1651+
destPool, cmd.getWaitInMillSeconds());
1652+
1653+
storagePoolMgr.disconnectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path);
1654+
s_logger.debug(String.format("Created RBD volume %s from snapshot %s", disk, snapshotDisk));
1655+
1656+
}
1657+
}
1658+
return disk;
1659+
}
1660+
1661+
private KVMPhysicalDisk createVolumeFromSnapshotOnNFS(CopyCommand cmd, PrimaryDataStoreTO pool,
1662+
DataStoreTO imageStore, VolumeObjectTO volume, String snapshotPath, String snapshotName) {
1663+
NfsTO nfsImageStore = (NfsTO)imageStore;
1664+
KVMStoragePool secondaryPool = storagePoolMgr.getStoragePoolByURI(nfsImageStore.getUrl() + File.separator + snapshotPath);
1665+
KVMPhysicalDisk snapshotDisk = secondaryPool.getPhysicalDisk(snapshotName);
1666+
if (volume.getFormat() == ImageFormat.RAW) {
1667+
snapshotDisk.setFormat(PhysicalDiskFormat.RAW);
1668+
} else if (volume.getFormat() == ImageFormat.QCOW2) {
1669+
snapshotDisk.setFormat(PhysicalDiskFormat.QCOW2);
1670+
}
1671+
1672+
final String primaryUuid = pool.getUuid();
1673+
final KVMStoragePool primaryPool = storagePoolMgr.getStoragePool(pool.getPoolType(), primaryUuid);
1674+
final String volUuid = UUID.randomUUID().toString();
1675+
1676+
Map<String, String> details = cmd.getOptions2();
1677+
1678+
String path = details != null ? details.get(DiskTO.IQN) : null;
1679+
1680+
storagePoolMgr.connectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path, details);
1681+
1682+
KVMPhysicalDisk disk = storagePoolMgr.copyPhysicalDisk(snapshotDisk, path != null ? path : volUuid, primaryPool, cmd.getWaitInMillSeconds());
1683+
1684+
storagePoolMgr.disconnectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path);
1685+
return disk;
1686+
}
1687+
1688+
private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, String snapshotName, String name,
1689+
PhysicalDiskFormat format, long size, KVMStoragePool destPool, int timeout) {
1690+
1691+
KVMStoragePool srcPool = volume.getPool();
1692+
KVMPhysicalDisk disk = null;
1693+
String newUuid = name;
1694+
1695+
disk = new KVMPhysicalDisk(destPool.getSourceDir() + "/" + newUuid, newUuid, destPool);
1696+
disk.setFormat(format);
1697+
disk.setSize(size > volume.getVirtualSize() ? size : volume.getVirtualSize());
1698+
disk.setVirtualSize(size > volume.getVirtualSize() ? size : disk.getSize());
1699+
1700+
try {
1701+
1702+
Rados r = new Rados(srcPool.getAuthUserName());
1703+
r.confSet("mon_host", srcPool.getSourceHost() + ":" + srcPool.getSourcePort());
1704+
r.confSet("key", srcPool.getAuthSecret());
1705+
r.confSet("client_mount_timeout", "30");
1706+
r.connect();
1707+
1708+
IoCTX io = r.ioCtxCreate(srcPool.getSourceDir());
1709+
Rbd rbd = new Rbd(io);
1710+
RbdImage srcImage = rbd.open(volume.getName());
1711+
1712+
List<RbdSnapInfo> snaps = srcImage.snapList();
1713+
boolean snapFound = false;
1714+
for (RbdSnapInfo snap : snaps) {
1715+
if (snapshotName.equals(snap.name)) {
1716+
snapFound = true;
1717+
break;
1718+
}
1719+
}
1720+
1721+
if (!snapFound) {
1722+
s_logger.debug(String.format("Could not find snapshot %s on RBD", snapshotName));
1723+
return null;
1724+
}
1725+
srcImage.snapProtect(snapshotName);
1726+
1727+
s_logger.debug(String.format("Try to clone snapshot %s on RBD", snapshotName));
1728+
rbd.clone(volume.getName(), snapshotName, io, disk.getName(), LibvirtStorageAdaptor.RBD_FEATURES, 0);
1729+
RbdImage diskImage = rbd.open(disk.getName());
1730+
if (disk.getVirtualSize() > volume.getVirtualSize()) {
1731+
diskImage.resize(disk.getVirtualSize());
1732+
}
1733+
1734+
diskImage.flatten();
1735+
rbd.close(diskImage);
1736+
1737+
srcImage.snapUnprotect(snapshotName);
1738+
rbd.close(srcImage);
1739+
r.ioCtxDestroy(io);
1740+
} catch (RadosException | RbdException e) {
1741+
s_logger.error(String.format("Failed due to %s", e.getMessage()), e);
1742+
disk = null;
1743+
}
1744+
1745+
return disk;
1746+
}
1747+
16421748
@Override
16431749
public Answer deleteSnapshot(final DeleteCommand cmd) {
16441750
String snap_full_name = "";

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
7979
private static final int RBD_FEATURE_OBJECT_MAP = 8;
8080
private static final int RBD_FEATURE_FAST_DIFF = 16;
8181
private static final int RBD_FEATURE_DEEP_FLATTEN = 32;
82-
private int rbdFeatures = RBD_FEATURE_LAYERING + RBD_FEATURE_EXCLUSIVE_LOCK + RBD_FEATURE_OBJECT_MAP + RBD_FEATURE_FAST_DIFF + RBD_FEATURE_DEEP_FLATTEN;
82+
public static final int RBD_FEATURES = RBD_FEATURE_LAYERING + RBD_FEATURE_EXCLUSIVE_LOCK + RBD_FEATURE_OBJECT_MAP + RBD_FEATURE_FAST_DIFF + RBD_FEATURE_DEEP_FLATTEN;
8383
private int rbdOrder = 0; /* Order 0 means 4MB blocks (the default) */
8484

8585
public LibvirtStorageAdaptor(StorageLayer storage) {
@@ -1103,7 +1103,7 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template,
11031103
s_logger.debug("The source image " + srcPool.getSourceDir() + "/" + template.getName() +
11041104
" is RBD format 1. We have to perform a regular copy (" + toHumanReadableSize(disk.getVirtualSize()) + " bytes)");
11051105

1106-
rbd.create(disk.getName(), disk.getVirtualSize(), rbdFeatures, rbdOrder);
1106+
rbd.create(disk.getName(), disk.getVirtualSize(), RBD_FEATURES, rbdOrder);
11071107
RbdImage destImage = rbd.open(disk.getName());
11081108

11091109
s_logger.debug("Starting to copy " + srcImage.getName() + " to " + destImage.getName() + " in Ceph pool " + srcPool.getSourceDir());
@@ -1140,7 +1140,7 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template,
11401140
srcImage.snapProtect(rbdTemplateSnapName);
11411141
}
11421142

1143-
rbd.clone(template.getName(), rbdTemplateSnapName, io, disk.getName(), rbdFeatures, rbdOrder);
1143+
rbd.clone(template.getName(), rbdTemplateSnapName, io, disk.getName(), RBD_FEATURES, rbdOrder);
11441144
s_logger.debug("Succesfully cloned " + template.getName() + "@" + rbdTemplateSnapName + " to " + disk.getName());
11451145
/* We also need to resize the image if the VM was deployed with a larger root disk size */
11461146
if (disk.getVirtualSize() > template.getVirtualSize()) {
@@ -1180,7 +1180,7 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template,
11801180

11811181
s_logger.debug("Creating " + disk.getName() + " on the destination cluster " + rDest.confGet("mon_host") + " in pool " +
11821182
destPool.getSourceDir());
1183-
dRbd.create(disk.getName(), disk.getVirtualSize(), rbdFeatures, rbdOrder);
1183+
dRbd.create(disk.getName(), disk.getVirtualSize(), RBD_FEATURES, rbdOrder);
11841184

11851185
RbdImage srcImage = sRbd.open(template.getName());
11861186
RbdImage destImage = dRbd.open(disk.getName());

0 commit comments

Comments
 (0)