From f89df718e598dfe345bc7683d56ab183abd888b3 Mon Sep 17 00:00:00 2001 From: Sergiy Kukunin Date: Fri, 17 Apr 2026 18:53:06 -0700 Subject: [PATCH] Linstor: fix create volume from snapshot on primary storage When creating a volume from a snapshot on Linstor primary storage (with lin.backup.snapshots=false), the operation fails with: "Only the following image types are currently supported: VHD, OVA, QCOW2, RAW (for PowerFlex and FiberChannel)" Root cause: the Linstor driver does not handle SNAPSHOT -> VOLUME in its canCopy()/copyAsync() methods. This causes DataMotionServiceImpl to fall through to StorageSystemDataMotionStrategy (selected because Linstor advertises STORAGE_SYSTEM_SNAPSHOT=true). That strategy's verifyFormatWithPoolType() rejects RAW format for Linstor pools, since RAW is only allowed for PowerFlex and FiberChannel. Additionally, VolumeOrchestrator.createVolumeFromSnapshot() attempts to back up the snapshot to secondary storage when the storage plugin does not advertise CAN_CREATE_TEMPLATE_FROM_SNAPSHOT. This backup fails because the snapshot only exists on Linstor primary storage. Fix: - Add CAN_CREATE_TEMPLATE_FROM_SNAPSHOT capability so the orchestrator skips the backup-to-secondary path - Add canCopySnapshotToVolumeCond() to match SNAPSHOT -> VOLUME when both are on the same Linstor primary store - Wire it into canCopy() to intercept at DataMotionServiceImpl before strategy selection, bypassing StorageSystemDataMotionStrategy - Implement copySnapshotToVolume() which delegates to the existing createResourceFromSnapshot() for native Linstor snapshot restore This follows the same pattern used by the StorPool plugin, which handles SNAPSHOT -> VOLUME directly in its driver rather than going through StorageSystemDataMotionStrategy. Tested on CloudStack 4.22 with Linstor LVM_THIN storage, creating a volume from a 1TB CNPG Postgres database snapshot. Volume creates successfully with correct path and deletes cleanly. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../LinstorPrimaryDataStoreDriverImpl.java | 38 +++++- ...LinstorPrimaryDataStoreDriverImplTest.java | 108 ++++++++++++++++++ 2 files changed, 144 insertions(+), 2 deletions(-) diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java index 3f06bee8ac83..95326e18431a 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java @@ -153,6 +153,7 @@ public Map getCapabilities() // CAN_CREATE_VOLUME_FROM_SNAPSHOT see note from CAN_CREATE_VOLUME_FROM_VOLUME mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_VOLUME_FROM_SNAPSHOT.toString(), Boolean.TRUE.toString()); mapCapabilities.put(DataStoreCapabilities.CAN_REVERT_VOLUME_TO_SNAPSHOT.toString(), Boolean.TRUE.toString()); + mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString(), Boolean.TRUE.toString()); return mapCapabilities; } @@ -720,6 +721,13 @@ public void revertSnapshot( } } + private static boolean canCopySnapshotToVolumeCond(DataObject srcData, DataObject dstData) { + return srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.VOLUME + && srcData.getDataStore().getRole() == DataStoreRole.Primary + && dstData.getDataStore().getRole() == DataStoreRole.Primary + && srcData.getDataStore().getId() == dstData.getDataStore().getId(); + } + private static boolean canCopySnapshotCond(DataObject srcData, DataObject dstData) { return srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.SNAPSHOT && (dstData.getDataStore().getRole() == DataStoreRole.Image @@ -747,7 +755,10 @@ public boolean canCopy(DataObject srcData, DataObject dstData) { logger.debug("LinstorPrimaryDataStoreDriverImpl.canCopy: " + srcData.getType() + " -> " + dstData.getType()); - if (canCopySnapshotCond(srcData, dstData)) { + if (canCopySnapshotToVolumeCond(srcData, dstData)) { + StoragePoolVO storagePool = _storagePoolDao.findById(srcData.getDataStore().getId()); + return storagePool.getStorageProviderName().equals(LinstorUtil.PROVIDER_NAME); + } else if (canCopySnapshotCond(srcData, dstData)) { SnapshotInfo sinfo = (SnapshotInfo) srcData; VolumeInfo volume = sinfo.getBaseVolume(); StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId()); @@ -766,6 +777,27 @@ public boolean canCopy(DataObject srcData, DataObject dstData) return false; } + private CopyCommandResult copySnapshotToVolume(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo) { + String errMsg = null; + CopyCmdAnswer answer = null; + try { + StoragePoolVO storagePoolVO = _storagePoolDao.findById(snapshotInfo.getDataStore().getId()); + String rscName = LinstorUtil.RSC_PREFIX + volumeInfo.getUuid(); + createResourceFromSnapshot(snapshotInfo.getId(), rscName, storagePoolVO); + + VolumeObjectTO volumeTO = (VolumeObjectTO) volumeInfo.getTO(); + volumeTO.setPath(volumeInfo.getUuid()); + volumeTO.setSize(volumeInfo.getSize()); + answer = new CopyCmdAnswer(volumeTO); + } catch (Exception e) { + errMsg = "Failed to create volume from snapshot: " + e.getMessage(); + logger.error(errMsg, e); + } + CopyCommandResult result = new CopyCommandResult(null, answer); + result.setResult(errMsg); + return result; + } + @Override public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCallback callback) { @@ -773,7 +805,9 @@ public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCal + srcData.getType() + " -> " + dstData.getType()); final CopyCommandResult res; - if (canCopySnapshotCond(srcData, dstData)) { + if (canCopySnapshotToVolumeCond(srcData, dstData)) { + res = copySnapshotToVolume((SnapshotInfo) srcData, (VolumeInfo) dstData); + } else if (canCopySnapshotCond(srcData, dstData)) { String errMsg = null; Answer answer = copySnapshot(srcData, dstData); if (answer != null && !answer.getResult()) { diff --git a/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImplTest.java b/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImplTest.java index 4653cfa358b0..1aa49b29d244 100644 --- a/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImplTest.java +++ b/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImplTest.java @@ -25,13 +25,24 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; +import com.cloud.agent.api.to.DataObjectType; +import com.cloud.storage.DataStoreRole; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.util.LinstorUtil; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; +import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import static org.mockito.Mockito.mock; @@ -42,6 +53,9 @@ public class LinstorPrimaryDataStoreDriverImplTest { private DevelopersApi api; + @Mock + private PrimaryDataStoreDao _storagePoolDao; + @InjectMocks private LinstorPrimaryDataStoreDriverImpl linstorPrimaryDataStoreDriver; @@ -85,4 +99,98 @@ public void testGetEncryptedLayerList() throws ApiException { layers = LinstorUtil.getEncryptedLayerList(api, "EncryptedGrp"); Assert.assertEquals(Arrays.asList(LayerType.DRBD, LayerType.LUKS, LayerType.STORAGE), layers); } + + @Test + public void testGetCapabilitiesIncludesCreateTemplateFromSnapshot() { + Map caps = linstorPrimaryDataStoreDriver.getCapabilities(); + + Assert.assertTrue("Linstor should advertise CAN_CREATE_TEMPLATE_FROM_SNAPSHOT", + Boolean.parseBoolean(caps.get(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString()))); + } + + @Test + public void testCanCopySnapshotToVolumeOnSamePrimary() { + DataStore primaryStore = mock(DataStore.class); + when(primaryStore.getRole()).thenReturn(DataStoreRole.Primary); + when(primaryStore.getId()).thenReturn(1L); + + SnapshotInfo snapshot = mock(SnapshotInfo.class); + when(snapshot.getType()).thenReturn(DataObjectType.SNAPSHOT); + when(snapshot.getDataStore()).thenReturn(primaryStore); + + VolumeInfo volume = mock(VolumeInfo.class); + when(volume.getType()).thenReturn(DataObjectType.VOLUME); + when(volume.getDataStore()).thenReturn(primaryStore); + + StoragePoolVO pool = mock(StoragePoolVO.class); + when(pool.getStorageProviderName()).thenReturn(LinstorUtil.PROVIDER_NAME); + when(_storagePoolDao.findById(1L)).thenReturn(pool); + + Assert.assertTrue("canCopy should return true for SNAPSHOT -> VOLUME on same Linstor primary", + linstorPrimaryDataStoreDriver.canCopy(snapshot, volume)); + } + + @Test + public void testCanCopySnapshotToVolumeRejectsNonLinstor() { + DataStore primaryStore = mock(DataStore.class); + when(primaryStore.getRole()).thenReturn(DataStoreRole.Primary); + when(primaryStore.getId()).thenReturn(1L); + + SnapshotInfo snapshot = mock(SnapshotInfo.class); + when(snapshot.getType()).thenReturn(DataObjectType.SNAPSHOT); + when(snapshot.getDataStore()).thenReturn(primaryStore); + + VolumeInfo volume = mock(VolumeInfo.class); + when(volume.getType()).thenReturn(DataObjectType.VOLUME); + when(volume.getDataStore()).thenReturn(primaryStore); + + StoragePoolVO pool = mock(StoragePoolVO.class); + when(pool.getStorageProviderName()).thenReturn("SomeOtherProvider"); + when(_storagePoolDao.findById(1L)).thenReturn(pool); + + Assert.assertFalse("canCopy should return false for non-Linstor storage", + linstorPrimaryDataStoreDriver.canCopy(snapshot, volume)); + } + + @Test + public void testCanCopySnapshotToVolumeRejectsCrossPrimary() { + DataStore srcStore = mock(DataStore.class); + when(srcStore.getRole()).thenReturn(DataStoreRole.Primary); + when(srcStore.getId()).thenReturn(1L); + + DataStore destStore = mock(DataStore.class); + when(destStore.getRole()).thenReturn(DataStoreRole.Primary); + when(destStore.getId()).thenReturn(2L); + + SnapshotInfo snapshot = mock(SnapshotInfo.class); + when(snapshot.getType()).thenReturn(DataObjectType.SNAPSHOT); + when(snapshot.getDataStore()).thenReturn(srcStore); + + VolumeInfo volume = mock(VolumeInfo.class); + when(volume.getType()).thenReturn(DataObjectType.VOLUME); + when(volume.getDataStore()).thenReturn(destStore); + + Assert.assertFalse("canCopy should return false for SNAPSHOT -> VOLUME across different primary stores", + linstorPrimaryDataStoreDriver.canCopy(snapshot, volume)); + } + + @Test + public void testCanCopySnapshotToVolumeRejectsImageDest() { + DataStore primaryStore = mock(DataStore.class); + when(primaryStore.getRole()).thenReturn(DataStoreRole.Primary); + + DataStore imageStore = mock(DataStore.class); + when(imageStore.getRole()).thenReturn(DataStoreRole.Image); + + SnapshotInfo snapshot = mock(SnapshotInfo.class); + when(snapshot.getType()).thenReturn(DataObjectType.SNAPSHOT); + when(snapshot.getDataStore()).thenReturn(primaryStore); + + VolumeInfo volume = mock(VolumeInfo.class); + when(volume.getType()).thenReturn(DataObjectType.VOLUME); + when(volume.getDataStore()).thenReturn(imageStore); + + Assert.assertFalse("canCopy should return false when destination is Image store", + linstorPrimaryDataStoreDriver.canCopy(snapshot, volume)); + } }