Skip to content

Commit f89df71

Browse files
Kukuninclaude
Kukunin
and
claude
committed
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) <noreply@anthropic.com>
1 parent 3166e64 commit f89df71

File tree

2 files changed

+144
-2
lines changed

2 files changed

+144
-2
lines changed

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

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ 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(), Boolean.TRUE.toString());
156157

157158
return mapCapabilities;
158159
}
@@ -720,6 +721,13 @@ public void revertSnapshot(
720721
}
721722
}
722723

724+
private static boolean canCopySnapshotToVolumeCond(DataObject srcData, DataObject dstData) {
725+
return srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.VOLUME
726+
&& srcData.getDataStore().getRole() == DataStoreRole.Primary
727+
&& dstData.getDataStore().getRole() == DataStoreRole.Primary
728+
&& srcData.getDataStore().getId() == dstData.getDataStore().getId();
729+
}
730+
723731
private static boolean canCopySnapshotCond(DataObject srcData, DataObject dstData) {
724732
return srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.SNAPSHOT
725733
&& (dstData.getDataStore().getRole() == DataStoreRole.Image
@@ -747,7 +755,10 @@ public boolean canCopy(DataObject srcData, DataObject dstData)
747755
{
748756
logger.debug("LinstorPrimaryDataStoreDriverImpl.canCopy: " + srcData.getType() + " -> " + dstData.getType());
749757

750-
if (canCopySnapshotCond(srcData, dstData)) {
758+
if (canCopySnapshotToVolumeCond(srcData, dstData)) {
759+
StoragePoolVO storagePool = _storagePoolDao.findById(srcData.getDataStore().getId());
760+
return storagePool.getStorageProviderName().equals(LinstorUtil.PROVIDER_NAME);
761+
} else if (canCopySnapshotCond(srcData, dstData)) {
751762
SnapshotInfo sinfo = (SnapshotInfo) srcData;
752763
VolumeInfo volume = sinfo.getBaseVolume();
753764
StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId());
@@ -766,14 +777,37 @@ public boolean canCopy(DataObject srcData, DataObject dstData)
766777
return false;
767778
}
768779

780+
private CopyCommandResult copySnapshotToVolume(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo) {
781+
String errMsg = null;
782+
CopyCmdAnswer answer = null;
783+
try {
784+
StoragePoolVO storagePoolVO = _storagePoolDao.findById(snapshotInfo.getDataStore().getId());
785+
String rscName = LinstorUtil.RSC_PREFIX + volumeInfo.getUuid();
786+
createResourceFromSnapshot(snapshotInfo.getId(), rscName, storagePoolVO);
787+
788+
VolumeObjectTO volumeTO = (VolumeObjectTO) volumeInfo.getTO();
789+
volumeTO.setPath(volumeInfo.getUuid());
790+
volumeTO.setSize(volumeInfo.getSize());
791+
answer = new CopyCmdAnswer(volumeTO);
792+
} catch (Exception e) {
793+
errMsg = "Failed to create volume from snapshot: " + e.getMessage();
794+
logger.error(errMsg, e);
795+
}
796+
CopyCommandResult result = new CopyCommandResult(null, answer);
797+
result.setResult(errMsg);
798+
return result;
799+
}
800+
769801
@Override
770802
public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCallback<CopyCommandResult> callback)
771803
{
772804
logger.debug("LinstorPrimaryDataStoreDriverImpl.copyAsync: "
773805
+ srcData.getType() + " -> " + dstData.getType());
774806

775807
final CopyCommandResult res;
776-
if (canCopySnapshotCond(srcData, dstData)) {
808+
if (canCopySnapshotToVolumeCond(srcData, dstData)) {
809+
res = copySnapshotToVolume((SnapshotInfo) srcData, (VolumeInfo) dstData);
810+
} else if (canCopySnapshotCond(srcData, dstData)) {
777811
String errMsg = null;
778812
Answer answer = copySnapshot(srcData, dstData);
779813
if (answer != null && !answer.getResult()) {

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,24 @@
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.DataObject;
33+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
34+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
35+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
36+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
37+
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
38+
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
2939
import org.apache.cloudstack.storage.datastore.util.LinstorUtil;
3040
import org.junit.Assert;
3141
import org.junit.Before;
3242
import org.junit.Test;
3343
import org.junit.runner.RunWith;
3444
import org.mockito.InjectMocks;
45+
import org.mockito.Mock;
3546
import org.mockito.junit.MockitoJUnitRunner;
3647

3748
import static org.mockito.Mockito.mock;
@@ -42,6 +53,9 @@ public class LinstorPrimaryDataStoreDriverImplTest {
4253

4354
private DevelopersApi api;
4455

56+
@Mock
57+
private PrimaryDataStoreDao _storagePoolDao;
58+
4559
@InjectMocks
4660
private LinstorPrimaryDataStoreDriverImpl linstorPrimaryDataStoreDriver;
4761

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

0 commit comments

Comments
 (0)