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

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 = backupSnapshotIfNeeded(snapshot, dataStoreRole, snapInfo);
-            snapInfo = snapshotFactory.getSnapshot(snapshot.getId(), dataStoreRole);
-            if (snapInfo == null) {
-                throw new CloudRuntimeException("Cannot find snapshot " + snapshot.getId() + " on secondary and could not create backup");
-            }
         }
         // 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);
 

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

@@ -68,6 +68,8 @@ StateDao<ObjectInDataStoreStateMachine.State, ObjectInDataStoreStateMachine.Even
 
     SnapshotDataStoreVO findByVolume(long volumeId, DataStoreRole role);
 
+    SnapshotDataStoreVO findByVolume(long snapshotId, long volumeId, DataStoreRole role);
+
     /**
      * 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.
      */

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

@@ -92,7 +92,7 @@ public class SnapshotDataFactoryImpl implements SnapshotDataFactory {
         }
         SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findBySnapshot(snapshotId, role);
         if (snapshotStore == null) {
-            snapshotStore = snapshotStoreDao.findByVolume(snapshot.getVolumeId(), role);
+            snapshotStore = snapshotStoreDao.findByVolume(snapshotId, snapshot.getVolumeId(), role);
             if (snapshotStore == null) {
                 return null;
             }

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

@@ -142,6 +142,7 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
         volumeSearch = createSearchBuilder();
         volumeSearch.and("volume_id", volumeSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
         volumeSearch.and("store_role", volumeSearch.entity().getRole(), SearchCriteria.Op.EQ);
+        volumeSearch.and("snapshot_id", volumeSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ);
         volumeSearch.done();
 
         stateSearch = createSearchBuilder();
@@ -365,6 +366,15 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
         return findOneBy(sc);
     }
 
+    @Override
+    public SnapshotDataStoreVO findByVolume(long snapshotId, long volumeId, DataStoreRole role) {
+        SearchCriteria<SnapshotDataStoreVO> sc = volumeSearch.create();
+        sc.setParameters("snapshot_id", snapshotId);
+        sc.setParameters("volume_id", volumeId);
+        sc.setParameters("store_role", role);
+        return findOneBy(sc);
+    }
+
     @Override
     public List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId) {
         SearchCriteria<SnapshotDataStoreVO> sc = snapshotIdSearch.create();

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,19 +1595,74 @@ 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);
+            KVMPhysicalDisk disk = null;
-            final KVMPhysicalDisk snapshotDisk = secondaryPool.getPhysicalDisk(snapshotName);
+            if (imageStore instanceof NfsTO) {
+                disk = createVolumeFromSnapshotOnNFS(cmd, pool, imageStore, volume, snapshotPath, snapshotName);
+            } else {
+                disk = createVolumeFromRBDSnapshot(cmd, destData, pool, imageStore, volume, snapshotName, disk);
+            }
 
+            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());
+            newVol.setFormat(ImageFormat.valueOf(disk.getFormat().toString().toUpperCase()));
+
+            return new CopyCmdAnswer(newVol);
+        } catch (final CloudRuntimeException e) {
+            s_logger.debug("Failed to createVolumeFromSnapshot: ", e);
+            return new CopyCmdAnswer(e.toString());
+        }
+    }
+
+    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<String, String> 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) {
@@ -1626,17 +1682,67 @@ public class KVMStorageProcessor implements StorageProcessor {
         KVMPhysicalDisk disk = storagePoolMgr.copyPhysicalDisk(snapshotDisk, path != null ? path : volUuid, primaryPool, cmd.getWaitInMillSeconds());
 
         storagePoolMgr.disconnectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path);
-
+        return disk;
-            final VolumeObjectTO newVol = new VolumeObjectTO();
-            newVol.setPath(disk.getName());
-            newVol.setSize(disk.getVirtualSize());
-            newVol.setFormat(ImageFormat.valueOf(disk.getFormat().toString().toUpperCase()));
-
-            return new CopyCmdAnswer(newVol);
-        } catch (final CloudRuntimeException e) {
-            s_logger.debug("Failed to createVolumeFromSnapshot: ", e);
-            return new CopyCmdAnswer(e.toString());
     }
+
+    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<RbdSnapInfo> 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

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());

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();
+        DataStore destStore = destData.getDataStore();
-        if (store.getRole() == DataStoreRole.Primary && srcData.getType() == DataObjectType.TEMPLATE
+        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;
     }