This is an automated email from the ASF dual-hosted git repository.
sureshanaparti pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 089eb36e471 Linstor: fix create volume from snapshot on primary
storage (#13043)
089eb36e471 is described below
commit 089eb36e471c263ff1a90376cadaba7a595fc9e8
Author: Sergiy Kukunin <[email protected]>
AuthorDate: Tue Apr 28 22:53:08 2026 -0700
Linstor: fix create volume from snapshot on primary storage (#13043)
* 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.
* Let CloudRuntimeException propagate from copySnapshotToVolume
Remove try/catch in copySnapshotToVolume so that CloudRuntimeException
from createResourceFromSnapshot propagates to the caller, ensuring
CloudStack properly notices and reports the failure.
* Fix CAN_CREATE_TEMPLATE_FROM_SNAPSHOT breaking template creation
Setting CAN_CREATE_TEMPLATE_FROM_SNAPSHOT unconditionally to true
caused createTemplate from snapshot to take the StorPool-specific
code path in TemplateManagerImpl, which sends a CopyCommand to a
system VM that Linstor cannot handle.
Fix: make CAN_CREATE_TEMPLATE_FROM_SNAPSHOT conditional on the same
flag as STORAGE_SYSTEM_SNAPSHOT (!BackupSnapshots). When snapshots
are backed up to secondary (the default), the old template creation
flow works. When snapshots stay on primary, the direct path is used.
Also fix checkstyle: remove unused DataObject import in test.
Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
---
.../driver/LinstorPrimaryDataStoreDriverImpl.java | 30 +++++-
.../LinstorPrimaryDataStoreDriverImplTest.java | 109 +++++++++++++++++++++
2 files changed, 137 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 3f06bee8ac8..83dacf74e8d 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,8 @@ public class LinstorPrimaryDataStoreDriverImpl implements
PrimaryDataStoreDriver
// 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.toString(system_snapshot));
return mapCapabilities;
}
@@ -720,6 +722,13 @@ public class LinstorPrimaryDataStoreDriverImpl implements
PrimaryDataStoreDriver
}
}
+ 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 +756,10 @@ public class LinstorPrimaryDataStoreDriverImpl implements
PrimaryDataStoreDriver
{
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 +778,18 @@ public class LinstorPrimaryDataStoreDriverImpl implements
PrimaryDataStoreDriver
return false;
}
+ private CopyCommandResult copySnapshotToVolume(SnapshotInfo snapshotInfo,
VolumeInfo volumeInfo) {
+ 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());
+
+ return new CopyCommandResult(null, new CopyCmdAnswer(volumeTO));
+ }
+
@Override
public void copyAsync(DataObject srcData, DataObject dstData,
AsyncCompletionCallback<CopyCommandResult> callback)
{
@@ -773,7 +797,9 @@ public class LinstorPrimaryDataStoreDriverImpl implements
PrimaryDataStoreDriver
+ 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 4653cfa358b..23266e17f4f 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,23 @@ import com.linbit.linstor.api.model.ResourceGroup;
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.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 +52,9 @@ public class LinstorPrimaryDataStoreDriverImplTest {
private DevelopersApi api;
+ @Mock
+ private PrimaryDataStoreDao _storagePoolDao;
+
@InjectMocks
private LinstorPrimaryDataStoreDriverImpl linstorPrimaryDataStoreDriver;
@@ -85,4 +98,100 @@ public class LinstorPrimaryDataStoreDriverImplTest {
layers = LinstorUtil.getEncryptedLayerList(api, "EncryptedGrp");
Assert.assertEquals(Arrays.asList(LayerType.DRBD, LayerType.LUKS,
LayerType.STORAGE), layers);
}
+
+ @Test
+ public void
testGetCapabilitiesIncludesCreateTemplateFromSnapshotMatchesSystemSnapshot() {
+ Map<String, String> caps =
linstorPrimaryDataStoreDriver.getCapabilities();
+
+ Assert.assertEquals(
+ "CAN_CREATE_TEMPLATE_FROM_SNAPSHOT should match
STORAGE_SYSTEM_SNAPSHOT",
+
caps.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()),
+
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));
+ }
}
