Skip to content

Linstor: fix create volume from snapshot on primary storage #13043

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Kukunin wants to merge 1 commit into
base: main
Choose a base branch
from
+144 −2
Open

Linstor: fix create volume from snapshot on primary storage #13043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions ...ava/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ public Map<String, String> 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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand All @@ -766,14 +777,37 @@ 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<CopyCommandResult> callback)
{
logger.debug("LinstorPrimaryDataStoreDriverImpl.copyAsync: "
+ 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()) {
Expand Down
108 changes: 108 additions & 0 deletions ...org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,6 +53,9 @@ public class LinstorPrimaryDataStoreDriverImplTest {

private DevelopersApi api;

@Mock
private PrimaryDataStoreDao _storagePoolDao;

@InjectMocks
private LinstorPrimaryDataStoreDriverImpl linstorPrimaryDataStoreDriver;

Expand Down Expand Up @@ -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<String, String> 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));
}
}