Skip to content

Commit 089eb36

Browse files
Kukuninclaude
andauthored
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) <noreply@anthropic.com>
1 parent e2c13da commit 089eb36

2 files changed

Lines changed: 137 additions & 2 deletions

File tree

plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ public Map<String, String> getCapabilities()
153153
// CAN_CREATE_VOLUME_FROM_SNAPSHOT see note from CAN_CREATE_VOLUME_FROM_VOLUME
154154
mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_VOLUME_FROM_SNAPSHOT.toString(), Boolean.TRUE.toString());
155155
mapCapabilities.put(DataStoreCapabilities.CAN_REVERT_VOLUME_TO_SNAPSHOT.toString(), Boolean.TRUE.toString());
156+
mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString(),
157+
Boolean.toString(system_snapshot));
156158

157159
return mapCapabilities;
158160
}
@@ -720,6 +722,13 @@ public void revertSnapshot(
720722
}
721723
}
722724

725+
private static boolean canCopySnapshotToVolumeCond(DataObject srcData, DataObject dstData) {
726+
return srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.VOLUME
727+
&& srcData.getDataStore().getRole() == DataStoreRole.Primary
728+
&& dstData.getDataStore().getRole() == DataStoreRole.Primary
729+
&& srcData.getDataStore().getId() == dstData.getDataStore().getId();
730+
}
731+
723732
private static boolean canCopySnapshotCond(DataObject srcData, DataObject dstData) {
724733
return srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.SNAPSHOT
725734
&& (dstData.getDataStore().getRole() == DataStoreRole.Image
@@ -747,7 +756,10 @@ public boolean canCopy(DataObject srcData, DataObject dstData)
747756
{
748757
logger.debug("LinstorPrimaryDataStoreDriverImpl.canCopy: " + srcData.getType() + " -> " + dstData.getType());
749758

750-
if (canCopySnapshotCond(srcData, dstData)) {
759+
if (canCopySnapshotToVolumeCond(srcData, dstData)) {
760+
StoragePoolVO storagePool = _storagePoolDao.findById(srcData.getDataStore().getId());
761+
return storagePool.getStorageProviderName().equals(LinstorUtil.PROVIDER_NAME);
762+
} else if (canCopySnapshotCond(srcData, dstData)) {
751763
SnapshotInfo sinfo = (SnapshotInfo) srcData;
752764
VolumeInfo volume = sinfo.getBaseVolume();
753765
StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId());
@@ -766,14 +778,28 @@ public boolean canCopy(DataObject srcData, DataObject dstData)
766778
return false;
767779
}
768780

781+
private CopyCommandResult copySnapshotToVolume(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo) {
782+
StoragePoolVO storagePoolVO = _storagePoolDao.findById(snapshotInfo.getDataStore().getId());
783+
String rscName = LinstorUtil.RSC_PREFIX + volumeInfo.getUuid();
784+
createResourceFromSnapshot(snapshotInfo.getId(), rscName, storagePoolVO);
785+
786+
VolumeObjectTO volumeTO = (VolumeObjectTO) volumeInfo.getTO();
787+
volumeTO.setPath(volumeInfo.getUuid());
788+
volumeTO.setSize(volumeInfo.getSize());
789+
790+
return new CopyCommandResult(null, new CopyCmdAnswer(volumeTO));
791+
}
792+
769793
@Override
770794
public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCallback<CopyCommandResult> callback)
771795
{
772796
logger.debug("LinstorPrimaryDataStoreDriverImpl.copyAsync: "
773797
+ srcData.getType() + " -> " + dstData.getType());
774798

775799
final CopyCommandResult res;
776-
if (canCopySnapshotCond(srcData, dstData)) {
800+
if (canCopySnapshotToVolumeCond(srcData, dstData)) {
801+
res = copySnapshotToVolume((SnapshotInfo) srcData, (VolumeInfo) dstData);
802+
} else if (canCopySnapshotCond(srcData, dstData)) {
777803
String errMsg = null;
778804
Answer answer = copySnapshot(srcData, dstData);
779805
if (answer != null && !answer.getResult()) {

plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImplTest.java

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,23 @@
2525
import java.util.Arrays;
2626
import java.util.Collections;
2727
import java.util.List;
28+
import java.util.Map;
2829

30+
import com.cloud.agent.api.to.DataObjectType;
31+
import com.cloud.storage.DataStoreRole;
32+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
33+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
34+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
35+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
36+
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
37+
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
2938
import org.apache.cloudstack.storage.datastore.util.LinstorUtil;
3039
import org.junit.Assert;
3140
import org.junit.Before;
3241
import org.junit.Test;
3342
import org.junit.runner.RunWith;
3443
import org.mockito.InjectMocks;
44+
import org.mockito.Mock;
3545
import org.mockito.junit.MockitoJUnitRunner;
3646

3747
import static org.mockito.Mockito.mock;
@@ -42,6 +52,9 @@ public class LinstorPrimaryDataStoreDriverImplTest {
4252

4353
private DevelopersApi api;
4454

55+
@Mock
56+
private PrimaryDataStoreDao _storagePoolDao;
57+
4558
@InjectMocks
4659
private LinstorPrimaryDataStoreDriverImpl linstorPrimaryDataStoreDriver;
4760

@@ -85,4 +98,100 @@ public void testGetEncryptedLayerList() throws ApiException {
8598
layers = LinstorUtil.getEncryptedLayerList(api, "EncryptedGrp");
8699
Assert.assertEquals(Arrays.asList(LayerType.DRBD, LayerType.LUKS, LayerType.STORAGE), layers);
87100
}
101+
102+
@Test
103+
public void testGetCapabilitiesIncludesCreateTemplateFromSnapshotMatchesSystemSnapshot() {
104+
Map<String, String> caps = linstorPrimaryDataStoreDriver.getCapabilities();
105+
106+
Assert.assertEquals(
107+
"CAN_CREATE_TEMPLATE_FROM_SNAPSHOT should match STORAGE_SYSTEM_SNAPSHOT",
108+
caps.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()),
109+
caps.get(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString()));
110+
}
111+
112+
@Test
113+
public void testCanCopySnapshotToVolumeOnSamePrimary() {
114+
DataStore primaryStore = mock(DataStore.class);
115+
when(primaryStore.getRole()).thenReturn(DataStoreRole.Primary);
116+
when(primaryStore.getId()).thenReturn(1L);
117+
118+
SnapshotInfo snapshot = mock(SnapshotInfo.class);
119+
when(snapshot.getType()).thenReturn(DataObjectType.SNAPSHOT);
120+
when(snapshot.getDataStore()).thenReturn(primaryStore);
121+
122+
VolumeInfo volume = mock(VolumeInfo.class);
123+
when(volume.getType()).thenReturn(DataObjectType.VOLUME);
124+
when(volume.getDataStore()).thenReturn(primaryStore);
125+
126+
StoragePoolVO pool = mock(StoragePoolVO.class);
127+
when(pool.getStorageProviderName()).thenReturn(LinstorUtil.PROVIDER_NAME);
128+
when(_storagePoolDao.findById(1L)).thenReturn(pool);
129+
130+
Assert.assertTrue("canCopy should return true for SNAPSHOT -> VOLUME on same Linstor primary",
131+
linstorPrimaryDataStoreDriver.canCopy(snapshot, volume));
132+
}
133+
134+
@Test
135+
public void testCanCopySnapshotToVolumeRejectsNonLinstor() {
136+
DataStore primaryStore = mock(DataStore.class);
137+
when(primaryStore.getRole()).thenReturn(DataStoreRole.Primary);
138+
when(primaryStore.getId()).thenReturn(1L);
139+
140+
SnapshotInfo snapshot = mock(SnapshotInfo.class);
141+
when(snapshot.getType()).thenReturn(DataObjectType.SNAPSHOT);
142+
when(snapshot.getDataStore()).thenReturn(primaryStore);
143+
144+
VolumeInfo volume = mock(VolumeInfo.class);
145+
when(volume.getType()).thenReturn(DataObjectType.VOLUME);
146+
when(volume.getDataStore()).thenReturn(primaryStore);
147+
148+
StoragePoolVO pool = mock(StoragePoolVO.class);
149+
when(pool.getStorageProviderName()).thenReturn("SomeOtherProvider");
150+
when(_storagePoolDao.findById(1L)).thenReturn(pool);
151+
152+
Assert.assertFalse("canCopy should return false for non-Linstor storage",
153+
linstorPrimaryDataStoreDriver.canCopy(snapshot, volume));
154+
}
155+
156+
@Test
157+
public void testCanCopySnapshotToVolumeRejectsCrossPrimary() {
158+
DataStore srcStore = mock(DataStore.class);
159+
when(srcStore.getRole()).thenReturn(DataStoreRole.Primary);
160+
when(srcStore.getId()).thenReturn(1L);
161+
162+
DataStore destStore = mock(DataStore.class);
163+
when(destStore.getRole()).thenReturn(DataStoreRole.Primary);
164+
when(destStore.getId()).thenReturn(2L);
165+
166+
SnapshotInfo snapshot = mock(SnapshotInfo.class);
167+
when(snapshot.getType()).thenReturn(DataObjectType.SNAPSHOT);
168+
when(snapshot.getDataStore()).thenReturn(srcStore);
169+
170+
VolumeInfo volume = mock(VolumeInfo.class);
171+
when(volume.getType()).thenReturn(DataObjectType.VOLUME);
172+
when(volume.getDataStore()).thenReturn(destStore);
173+
174+
Assert.assertFalse("canCopy should return false for SNAPSHOT -> VOLUME across different primary stores",
175+
linstorPrimaryDataStoreDriver.canCopy(snapshot, volume));
176+
}
177+
178+
@Test
179+
public void testCanCopySnapshotToVolumeRejectsImageDest() {
180+
DataStore primaryStore = mock(DataStore.class);
181+
when(primaryStore.getRole()).thenReturn(DataStoreRole.Primary);
182+
183+
DataStore imageStore = mock(DataStore.class);
184+
when(imageStore.getRole()).thenReturn(DataStoreRole.Image);
185+
186+
SnapshotInfo snapshot = mock(SnapshotInfo.class);
187+
when(snapshot.getType()).thenReturn(DataObjectType.SNAPSHOT);
188+
when(snapshot.getDataStore()).thenReturn(primaryStore);
189+
190+
VolumeInfo volume = mock(VolumeInfo.class);
191+
when(volume.getType()).thenReturn(DataObjectType.VOLUME);
192+
when(volume.getDataStore()).thenReturn(imageStore);
193+
194+
Assert.assertFalse("canCopy should return false when destination is Image store",
195+
linstorPrimaryDataStoreDriver.canCopy(snapshot, volume));
196+
}
88197
}

0 commit comments

Comments
 (0)