From 961e85eb60a8e15d764d172d3516ea8237d583df Mon Sep 17 00:00:00 2001 From: slavkap <51903378+slavkap@users.noreply.github.com> Date: Tue, 31 Aug 2021 10:16:57 +0300 Subject: [PATCH] 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 --- .../orchestration/VolumeOrchestrator.java | 32 +++- .../datastore/db/SnapshotDataStoreDao.java | 2 + .../snapshot/SnapshotDataFactoryImpl.java | 2 +- .../image/db/SnapshotDataStoreDaoImpl.java | 10 ++ .../kvm/storage/KVMStorageProcessor.java | 154 +++++++++++++++--- .../kvm/storage/LibvirtStorageAdaptor.java | 8 +- .../CloudStackPrimaryDataStoreDriverImpl.java | 32 +++- 7 files changed, 199 insertions(+), 41 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 3e86cea32e2..6420cdedfba 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -114,6 +114,7 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.Snapshot; import com.cloud.storage.Storage; import com.cloud.storage.Storage.ImageFormat; +import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.VMTemplateStorageResourceAssoc; @@ -124,6 +125,7 @@ import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeDetailsDao; +import com.cloud.storage.snapshot.SnapshotManager; import com.cloud.template.TemplateManager; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; @@ -454,18 +456,11 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati if (snapInfo == null) { throw new CloudRuntimeException("Cannot find snapshot " + snapshot.getId()); } - // We need to copy the snapshot onto secondary. - SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.BACKUP); - snapshotStrategy.backupSnapshot(snapInfo); - // Attempt to grab it again. - snapInfo = snapshotFactory.getSnapshot(snapshot.getId(), dataStoreRole); - if (snapInfo == null) { - throw new CloudRuntimeException("Cannot find snapshot " + snapshot.getId() + " on secondary and could not create backup"); - } + snapInfo = backupSnapshotIfNeeded(snapshot, dataStoreRole, snapInfo); } // don't try to perform a sync if the DataStoreRole of the snapshot is equal to DataStoreRole.Primary - if (!DataStoreRole.Primary.equals(dataStoreRole)) { + if (!DataStoreRole.Primary.equals(snapInfo.getDataStore().getRole())) { try { // sync snapshot to region store if necessary DataStore snapStore = snapInfo.getDataStore(); @@ -497,6 +492,25 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati } + private SnapshotInfo backupSnapshotIfNeeded(Snapshot snapshot, DataStoreRole dataStoreRole, SnapshotInfo snapInfo) { + boolean backupSnapToSecondary = SnapshotManager.BackupSnapshotAfterTakingSnapshot.value() == null || SnapshotManager.BackupSnapshotAfterTakingSnapshot.value(); + + StoragePoolVO srcPool = _storagePoolDao.findById(snapInfo.getBaseVolume().getPoolId()); + // We need to copy the snapshot onto secondary. + //Skipping the backup to secondary storage with NFS/FS could be supported when CLOUDSTACK-5297 is accepted with small enhancement in: + //KVMStorageProcessor::createVolumeFromSnapshot and CloudStackPrimaryDataStoreDriverImpl::copyAsync/createAsync + if ((!backupSnapToSecondary && (StoragePoolType.NetworkFilesystem.equals(srcPool.getPoolType()) || StoragePoolType.Filesystem.equals(srcPool.getPoolType())))) { + SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.BACKUP); + snapshotStrategy.backupSnapshot(snapInfo); + // Attempt to grab it again. + snapInfo = snapshotFactory.getSnapshot(snapshot.getId(), dataStoreRole); + if (snapInfo == null) { + throw new CloudRuntimeException("Cannot find snapshot " + snapshot.getId() + " on secondary and could not create backup"); + } + } + return snapInfo; + } + public DataStoreRole getDataStoreRole(Snapshot snapshot) { SnapshotDataStoreVO snapshotStore = _snapshotDataStoreDao.findBySnapshot(snapshot.getId(), DataStoreRole.Primary); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java index 3263cbb5b1a..5c6222b8dc9 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java @@ -68,6 +68,8 @@ StateDao sc = volumeSearch.create(); + sc.setParameters("snapshot_id", snapshotId); + sc.setParameters("volume_id", volumeId); + sc.setParameters("store_role", role); + return findOneBy(sc); + } + @Override public List findBySnapshotId(long snapshotId) { SearchCriteria sc = snapshotIdSearch.create(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index a12bcd0d70d..213982cdcf1 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -85,6 +85,7 @@ import com.ceph.rados.exceptions.RadosException; import com.ceph.rbd.Rbd; import com.ceph.rbd.RbdException; import com.ceph.rbd.RbdImage; +import com.ceph.rbd.jna.RbdSnapInfo; import com.cloud.agent.api.Answer; import com.cloud.agent.api.storage.PrimaryStorageDownloadAnswer; import com.cloud.agent.api.to.DataObjectType; @@ -1594,39 +1595,24 @@ public class KVMStorageProcessor implements StorageProcessor { final DataStoreTO imageStore = srcData.getDataStore(); final VolumeObjectTO volume = snapshot.getVolume(); - if (!(imageStore instanceof NfsTO)) { + if (!(imageStore instanceof NfsTO || imageStore instanceof PrimaryDataStoreTO)) { return new CopyCmdAnswer("unsupported protocol"); } - final NfsTO nfsImageStore = (NfsTO)imageStore; - final String snapshotFullPath = snapshot.getPath(); final int index = snapshotFullPath.lastIndexOf("/"); final String snapshotPath = snapshotFullPath.substring(0, index); final String snapshotName = snapshotFullPath.substring(index + 1); - final KVMStoragePool secondaryPool = storagePoolMgr.getStoragePoolByURI(nfsImageStore.getUrl() + File.separator + snapshotPath); - final KVMPhysicalDisk snapshotDisk = secondaryPool.getPhysicalDisk(snapshotName); - - if (volume.getFormat() == ImageFormat.RAW) { - snapshotDisk.setFormat(PhysicalDiskFormat.RAW); - } else if (volume.getFormat() == ImageFormat.QCOW2) { - snapshotDisk.setFormat(PhysicalDiskFormat.QCOW2); + KVMPhysicalDisk disk = null; + if (imageStore instanceof NfsTO) { + disk = createVolumeFromSnapshotOnNFS(cmd, pool, imageStore, volume, snapshotPath, snapshotName); + } else { + disk = createVolumeFromRBDSnapshot(cmd, destData, pool, imageStore, volume, snapshotName, disk); } - final String primaryUuid = pool.getUuid(); - final KVMStoragePool primaryPool = storagePoolMgr.getStoragePool(pool.getPoolType(), primaryUuid); - final String volUuid = UUID.randomUUID().toString(); - - Map details = cmd.getOptions2(); - - String path = details != null ? details.get(DiskTO.IQN) : null; - - storagePoolMgr.connectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path, details); - - KVMPhysicalDisk disk = storagePoolMgr.copyPhysicalDisk(snapshotDisk, path != null ? path : volUuid, primaryPool, cmd.getWaitInMillSeconds()); - - storagePoolMgr.disconnectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path); - + if (disk == null) { + return new CopyCmdAnswer("Could not create volume from snapshot"); + } final VolumeObjectTO newVol = new VolumeObjectTO(); newVol.setPath(disk.getName()); newVol.setSize(disk.getVirtualSize()); @@ -1639,6 +1625,126 @@ public class KVMStorageProcessor implements StorageProcessor { } } + private KVMPhysicalDisk createVolumeFromRBDSnapshot(CopyCommand cmd, DataTO destData, + PrimaryDataStoreTO pool, DataStoreTO imageStore, VolumeObjectTO volume, String snapshotName, KVMPhysicalDisk disk) { + PrimaryDataStoreTO primaryStore = (PrimaryDataStoreTO) imageStore; + KVMStoragePool srcPool = storagePoolMgr.getStoragePool(primaryStore.getPoolType(), primaryStore.getUuid()); + KVMPhysicalDisk snapshotDisk = srcPool.getPhysicalDisk(volume.getPath()); + KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); + VolumeObjectTO newVol = (VolumeObjectTO) destData; + + if (StoragePoolType.RBD.equals(primaryStore.getPoolType())) { + s_logger.debug(String.format("Attempting to create volume from RBD snapshot %s", snapshotName)); + if (StoragePoolType.RBD.equals(pool.getPoolType())) { + disk = createRBDvolumeFromRBDSnapshot(snapshotDisk, snapshotName, newVol.getUuid(), + PhysicalDiskFormat.RAW, newVol.getSize(), destPool, cmd.getWaitInMillSeconds()); + s_logger.debug(String.format("Created RBD volume %s from snapshot %s", disk, snapshotDisk)); + } else { + Map details = cmd.getOptions2(); + + String path = details != null ? details.get(DiskTO.IQN) : null; + + storagePoolMgr.connectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path, details); + + snapshotDisk.setPath(snapshotDisk.getPath() + "@" + snapshotName); + disk = storagePoolMgr.copyPhysicalDisk(snapshotDisk, path != null ? path : newVol.getUuid(), + destPool, cmd.getWaitInMillSeconds()); + + storagePoolMgr.disconnectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path); + s_logger.debug(String.format("Created RBD volume %s from snapshot %s", disk, snapshotDisk)); + + } + } + return disk; + } + + private KVMPhysicalDisk createVolumeFromSnapshotOnNFS(CopyCommand cmd, PrimaryDataStoreTO pool, + DataStoreTO imageStore, VolumeObjectTO volume, String snapshotPath, String snapshotName) { + NfsTO nfsImageStore = (NfsTO)imageStore; + KVMStoragePool secondaryPool = storagePoolMgr.getStoragePoolByURI(nfsImageStore.getUrl() + File.separator + snapshotPath); + KVMPhysicalDisk snapshotDisk = secondaryPool.getPhysicalDisk(snapshotName); + if (volume.getFormat() == ImageFormat.RAW) { + snapshotDisk.setFormat(PhysicalDiskFormat.RAW); + } else if (volume.getFormat() == ImageFormat.QCOW2) { + snapshotDisk.setFormat(PhysicalDiskFormat.QCOW2); + } + + final String primaryUuid = pool.getUuid(); + final KVMStoragePool primaryPool = storagePoolMgr.getStoragePool(pool.getPoolType(), primaryUuid); + final String volUuid = UUID.randomUUID().toString(); + + Map details = cmd.getOptions2(); + + String path = details != null ? details.get(DiskTO.IQN) : null; + + storagePoolMgr.connectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path, details); + + KVMPhysicalDisk disk = storagePoolMgr.copyPhysicalDisk(snapshotDisk, path != null ? path : volUuid, primaryPool, cmd.getWaitInMillSeconds()); + + storagePoolMgr.disconnectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path); + return disk; + } + + private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, String snapshotName, String name, + PhysicalDiskFormat format, long size, KVMStoragePool destPool, int timeout) { + + KVMStoragePool srcPool = volume.getPool(); + KVMPhysicalDisk disk = null; + String newUuid = name; + + disk = new KVMPhysicalDisk(destPool.getSourceDir() + "/" + newUuid, newUuid, destPool); + disk.setFormat(format); + disk.setSize(size > volume.getVirtualSize() ? size : volume.getVirtualSize()); + disk.setVirtualSize(size > volume.getVirtualSize() ? size : disk.getSize()); + + try { + + Rados r = new Rados(srcPool.getAuthUserName()); + r.confSet("mon_host", srcPool.getSourceHost() + ":" + srcPool.getSourcePort()); + r.confSet("key", srcPool.getAuthSecret()); + r.confSet("client_mount_timeout", "30"); + r.connect(); + + IoCTX io = r.ioCtxCreate(srcPool.getSourceDir()); + Rbd rbd = new Rbd(io); + RbdImage srcImage = rbd.open(volume.getName()); + + List snaps = srcImage.snapList(); + boolean snapFound = false; + for (RbdSnapInfo snap : snaps) { + if (snapshotName.equals(snap.name)) { + snapFound = true; + break; + } + } + + if (!snapFound) { + s_logger.debug(String.format("Could not find snapshot %s on RBD", snapshotName)); + return null; + } + srcImage.snapProtect(snapshotName); + + s_logger.debug(String.format("Try to clone snapshot %s on RBD", snapshotName)); + rbd.clone(volume.getName(), snapshotName, io, disk.getName(), LibvirtStorageAdaptor.RBD_FEATURES, 0); + RbdImage diskImage = rbd.open(disk.getName()); + if (disk.getVirtualSize() > volume.getVirtualSize()) { + diskImage.resize(disk.getVirtualSize()); + } + + diskImage.flatten(); + rbd.close(diskImage); + + srcImage.snapUnprotect(snapshotName); + rbd.close(srcImage); + r.ioCtxDestroy(io); + } catch (RadosException | RbdException e) { + s_logger.error(String.format("Failed due to %s", e.getMessage()), e); + disk = null; + } + + return disk; + } + @Override public Answer deleteSnapshot(final DeleteCommand cmd) { String snap_full_name = ""; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index c9f806d0513..478f32c207b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -79,7 +79,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { private static final int RBD_FEATURE_OBJECT_MAP = 8; private static final int RBD_FEATURE_FAST_DIFF = 16; private static final int RBD_FEATURE_DEEP_FLATTEN = 32; - private int rbdFeatures = RBD_FEATURE_LAYERING + RBD_FEATURE_EXCLUSIVE_LOCK + RBD_FEATURE_OBJECT_MAP + RBD_FEATURE_FAST_DIFF + RBD_FEATURE_DEEP_FLATTEN; + 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; private int rbdOrder = 0; /* Order 0 means 4MB blocks (the default) */ public LibvirtStorageAdaptor(StorageLayer storage) { @@ -1103,7 +1103,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { s_logger.debug("The source image " + srcPool.getSourceDir() + "/" + template.getName() + " is RBD format 1. We have to perform a regular copy (" + toHumanReadableSize(disk.getVirtualSize()) + " bytes)"); - rbd.create(disk.getName(), disk.getVirtualSize(), rbdFeatures, rbdOrder); + rbd.create(disk.getName(), disk.getVirtualSize(), RBD_FEATURES, rbdOrder); RbdImage destImage = rbd.open(disk.getName()); s_logger.debug("Starting to copy " + srcImage.getName() + " to " + destImage.getName() + " in Ceph pool " + srcPool.getSourceDir()); @@ -1140,7 +1140,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { srcImage.snapProtect(rbdTemplateSnapName); } - rbd.clone(template.getName(), rbdTemplateSnapName, io, disk.getName(), rbdFeatures, rbdOrder); + rbd.clone(template.getName(), rbdTemplateSnapName, io, disk.getName(), RBD_FEATURES, rbdOrder); s_logger.debug("Succesfully cloned " + template.getName() + "@" + rbdTemplateSnapName + " to " + disk.getName()); /* We also need to resize the image if the VM was deployed with a larger root disk size */ if (disk.getVirtualSize() > template.getVirtualSize()) { @@ -1180,7 +1180,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { s_logger.debug("Creating " + disk.getName() + " on the destination cluster " + rDest.confGet("mon_host") + " in pool " + destPool.getSourceDir()); - dRbd.create(disk.getName(), disk.getVirtualSize(), rbdFeatures, rbdOrder); + dRbd.create(disk.getName(), disk.getVirtualSize(), RBD_FEATURES, rbdOrder); RbdImage srcImage = sRbd.open(template.getName()); RbdImage destImage = dRbd.open(disk.getName()); diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java index 65b55f72c39..cb977de5631 100644 --- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java @@ -69,6 +69,7 @@ import com.cloud.storage.CreateSnapshotPayload; import com.cloud.storage.DataStoreRole; import com.cloud.storage.ResizeVolumePayload; import com.cloud.storage.Storage; +import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.Volume; @@ -287,6 +288,20 @@ public class CloudStackPrimaryDataStoreDriverImpl implements PrimaryDataStoreDri } CopyCommandResult result = new CopyCommandResult("", answer); callback.complete(result); + } else if (srcdata.getType() == DataObjectType.SNAPSHOT && destData.getType() == DataObjectType.VOLUME) { + SnapshotObjectTO srcTO = (SnapshotObjectTO) srcdata.getTO(); + CopyCommand cmd = new CopyCommand(srcTO, destData.getTO(), StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(), true); + EndPoint ep = epSelector.select(srcdata, destData); + CopyCmdAnswer answer = null; + if (ep == null) { + String errMsg = "No remote endpoint to send command, check if host or ssvm is down?"; + s_logger.error(errMsg); + answer = new CopyCmdAnswer(errMsg); + } else { + answer = (CopyCmdAnswer) ep.sendMessage(cmd); + } + CopyCommandResult result = new CopyCommandResult("", answer); + callback.complete(result); } } } @@ -294,13 +309,24 @@ public class CloudStackPrimaryDataStoreDriverImpl implements PrimaryDataStoreDri @Override public boolean canCopy(DataObject srcData, DataObject destData) { //BUG fix for CLOUDSTACK-4618 - DataStore store = destData.getDataStore(); - if (store.getRole() == DataStoreRole.Primary && srcData.getType() == DataObjectType.TEMPLATE + DataStore destStore = destData.getDataStore(); + if (destStore.getRole() == DataStoreRole.Primary && srcData.getType() == DataObjectType.TEMPLATE && (destData.getType() == DataObjectType.TEMPLATE || destData.getType() == DataObjectType.VOLUME)) { - StoragePoolVO storagePoolVO = primaryStoreDao.findById(store.getId()); + StoragePoolVO storagePoolVO = primaryStoreDao.findById(destStore.getId()); if (storagePoolVO != null && storagePoolVO.getPoolType() == Storage.StoragePoolType.CLVM) { return true; } + } else if (DataObjectType.SNAPSHOT.equals(srcData.getType()) && DataObjectType.VOLUME.equals(destData.getType())) { + DataStore srcStore = srcData.getDataStore(); + if (DataStoreRole.Primary.equals(srcStore.getRole()) && DataStoreRole.Primary.equals(destStore.getRole())) { + StoragePoolVO srcStoragePoolVO = primaryStoreDao.findById(srcStore.getId()); + StoragePoolVO dstStoragePoolVO = primaryStoreDao.findById(destStore.getId()); + if (srcStoragePoolVO != null && StoragePoolType.RBD.equals(srcStoragePoolVO.getPoolType()) + && dstStoragePoolVO != null && (StoragePoolType.RBD.equals(dstStoragePoolVO.getPoolType()) + || StoragePoolType.NetworkFilesystem.equals(dstStoragePoolVO.getPoolType()))) { + return true; + } + } } return false; }