diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java index 3329983d711e..e81bf7305fd6 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java @@ -49,6 +49,13 @@ public interface SnapshotDataStoreDao extends GenericDao listBySnapshotIdAndDataStoreRoleAndStateIn(long snapshotId, DataStoreRole role, ObjectInDataStoreStateMachine.State... state); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java index 8b7a2b78de7e..8a044cd8211c 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java @@ -19,8 +19,11 @@ import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.DataStoreRole; import com.cloud.storage.SnapshotVO; +import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateStorageResourceAssoc; +import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.VolumeDao; import com.cloud.utils.db.DB; import com.cloud.utils.db.Filter; import com.cloud.utils.db.GenericDaoBase; @@ -88,14 +91,24 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase 0) DESC, (ssr.kvm_checkpoint_path IS NOT NULL) DESC LIMIT 1;"; private static final String GET_PHYSICAL_SIZE_OF_SNAPSHOTS_ON_PRIMARY_BY_ACCOUNT = "SELECT SUM(s.physical_size) " + "FROM cloud.snapshot_store_ref s " + @@ -352,8 +365,15 @@ public SnapshotDataStoreVO findParent(DataStoreRole role, Long storeId, Long zon return null; } + boolean contentBasedChain = kvmIncrementalSnapshot && Hypervisor.HypervisorType.KVM.equals(hypervisorType) && usesContentBasedChain(volumeId); + if (contentBasedChain && (role == null || !role.isImageStore())) { + logger.trace("Content-based snapshot chains only exist on the image store. Returning null as parent for volume [{}] and role [{}].", volumeId, role); + return null; + } + boolean checkpointBasedChain = kvmIncrementalSnapshot && Hypervisor.HypervisorType.KVM.equals(hypervisorType) && !contentBasedChain; + SearchCriteria sc; - if (kvmIncrementalSnapshot && Hypervisor.HypervisorType.KVM.equals(hypervisorType)) { + if (checkpointBasedChain) { sc = searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEqKVMCheckpointNotNull.create(); } else { sc = searchFilteringStoreIdInVolumeIdEqStoreRoleEqStateEq.create(); @@ -379,13 +399,29 @@ public SnapshotDataStoreVO findParent(DataStoreRole role, Long storeId, Long zon SnapshotDataStoreVO parent = snapshotList.get(0); - if (kvmIncrementalSnapshot && parent.getKvmCheckpointPath() == null && Hypervisor.HypervisorType.KVM.equals(hypervisorType)) { + if (checkpointBasedChain && parent.getKvmCheckpointPath() == null) { return null; } return parent; } + /** + * Volumes on Linstor primary storage chain incremental snapshots on secondary storage through a + * content diff (qemu-img rebase) against the parent snapshot file instead of qemu checkpoints, so + * parent selection must not require a checkpoint path. Encrypted volumes are excluded as they are + * always backed up as full copies (a rebase would need the LUKS secret for delta and backing file). + */ + @Override + public boolean usesContentBasedChain(long volumeId) { + VolumeVO volume = volumeDao.findByIdIncludingRemoved(volumeId); + if (volume == null || volume.getPoolId() == null || volume.getPassphraseId() != null) { + return false; + } + StoragePoolVO pool = storagePoolDao.findById(volume.getPoolId()); + return pool != null && Storage.StoragePoolType.Linstor.equals(pool.getPoolType()); + } + @Override public SnapshotDataStoreVO findBySnapshotIdAndDataStoreRoleAndState(long snapshotId, DataStoreRole role, State state) { SearchCriteria sc = createSearchCriteriaBySnapshotIdAndStoreRole(snapshotId, role); diff --git a/engine/schema/src/test/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImplTest.java b/engine/schema/src/test/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImplTest.java index 85240ab4a058..2146ac6cc770 100644 --- a/engine/schema/src/test/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImplTest.java +++ b/engine/schema/src/test/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImplTest.java @@ -20,13 +20,18 @@ import java.util.List; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; import org.mockito.stubbing.Answer; +import com.cloud.storage.Storage; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; @@ -36,6 +41,18 @@ public class SnapshotDataStoreDaoImplTest { @Spy SnapshotDataStoreDaoImpl snapshotDataStoreDaoImplSpy; + @Mock + VolumeDao volumeDaoMock; + + @Mock + PrimaryDataStoreDao storagePoolDaoMock; + + @Before + public void setUp() { + snapshotDataStoreDaoImplSpy.volumeDao = volumeDaoMock; + snapshotDataStoreDaoImplSpy.storagePoolDao = storagePoolDaoMock; + } + @Test public void testExpungeByVmListNoVms() { Assert.assertEquals(0, snapshotDataStoreDaoImplSpy.expungeBySnapshotList( @@ -64,4 +81,48 @@ public void testExpungeByVmList() { Mockito.verify(snapshotDataStoreDaoImplSpy, Mockito.times(1)) .batchExpunge(sc, batchSize); } + + private VolumeVO mockVolume(Long poolId, Long passphraseId) { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getPoolId()).thenReturn(poolId); + Mockito.lenient().when(volume.getPassphraseId()).thenReturn(passphraseId); + Mockito.when(volumeDaoMock.findByIdIncludingRemoved(1L)).thenReturn(volume); + return volume; + } + + @Test + public void testUsesContentBasedChainVolumeNotFound() { + Mockito.when(volumeDaoMock.findByIdIncludingRemoved(1L)).thenReturn(null); + Assert.assertFalse(snapshotDataStoreDaoImplSpy.usesContentBasedChain(1L)); + } + + @Test + public void testUsesContentBasedChainNoPool() { + mockVolume(null, null); + Assert.assertFalse(snapshotDataStoreDaoImplSpy.usesContentBasedChain(1L)); + } + + @Test + public void testUsesContentBasedChainEncryptedVolume() { + mockVolume(3L, 7L); + Assert.assertFalse(snapshotDataStoreDaoImplSpy.usesContentBasedChain(1L)); + } + + @Test + public void testUsesContentBasedChainLinstorPool() { + mockVolume(3L, null); + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Mockito.when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + Mockito.when(storagePoolDaoMock.findById(3L)).thenReturn(pool); + Assert.assertTrue(snapshotDataStoreDaoImplSpy.usesContentBasedChain(1L)); + } + + @Test + public void testUsesContentBasedChainNonLinstorPool() { + mockVolume(3L, null); + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Mockito.when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + Mockito.when(storagePoolDaoMock.findById(3L)).thenReturn(pool); + Assert.assertFalse(snapshotDataStoreDaoImplSpy.usesContentBasedChain(1L)); + } } diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java index 6a8bbd93ca4e..213014db88f5 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java @@ -129,7 +129,9 @@ public SnapshotInfo getParent() { } /** - * Returns the snapshotInfo of the passed snapshot parentId. Will search for the snapshot reference which has a checkpoint path. If none is found, throws an exception. + * Returns the snapshotInfo of the passed snapshot parentId. Will search for the snapshot reference which has a checkpoint path. + * If none is found, returns the plain parent on this snapshot's store: KVM snapshots may also be chained without checkpoints, + * e.g. Linstor chains deltas through a content diff against the parent snapshot file on secondary storage. * */ protected SnapshotInfo getCorrectIncrementalParent(long parentId) { List parentSnapshotDatastoreVos = snapshotStoreDao.findBySnapshotId(parentId); @@ -141,8 +143,11 @@ protected SnapshotInfo getCorrectIncrementalParent(long parentId) { logger.debug("Found parent snapshot references {}, will filter to just one.", parentSnapshotDatastoreVos); SnapshotDataStoreVO parent = parentSnapshotDatastoreVos.stream().filter(snapshotDataStoreVO -> snapshotDataStoreVO.getKvmCheckpointPath() != null) - .findFirst(). - orElseThrow(() -> new CloudRuntimeException(String.format("Could not find snapshot parent with id [%s]. None of the records have a checkpoint path.", parentId))); + .findFirst().orElse(null); + + if (parent == null) { + return snapshotFactory.getSnapshot(parentId, store); + } SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(parentId, parent.getDataStoreId(), parent.getRole()); snapshotInfo.setKvmIncrementalSnapshot(parent.getKvmCheckpointPath() != null); diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/SnapshotObjectTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/SnapshotObjectTest.java new file mode 100644 index 000000000000..8b4e6b07dc16 --- /dev/null +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/SnapshotObjectTest.java @@ -0,0 +1,101 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.storage.snapshot; + +import java.util.Collections; +import java.util.List; + +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.SnapshotVO; + +@RunWith(MockitoJUnitRunner.class) +public class SnapshotObjectTest { + + private static final long PARENT_SNAPSHOT_ID = 2L; + + @Mock + SnapshotDataStoreDao snapshotStoreDao; + + @Mock + SnapshotDataFactory snapshotFactory; + + @Mock + DataStore store; + + @Mock + SnapshotVO snapshotVO; + + SnapshotObject snapshotObject; + + @Before + public void setUp() { + snapshotObject = new SnapshotObject(); + snapshotObject.configure(snapshotVO, store); + snapshotObject.snapshotStoreDao = snapshotStoreDao; + snapshotObject.snapshotFactory = snapshotFactory; + } + + @Test + public void testGetCorrectIncrementalParentNoRefsReturnsNull() { + Mockito.when(snapshotStoreDao.findBySnapshotId(PARENT_SNAPSHOT_ID)).thenReturn(Collections.emptyList()); + + Assert.assertNull(snapshotObject.getCorrectIncrementalParent(PARENT_SNAPSHOT_ID)); + } + + @Test + public void testGetCorrectIncrementalParentPrefersCheckpointBearingRef() { + SnapshotDataStoreVO refWithoutCheckpoint = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(refWithoutCheckpoint.getKvmCheckpointPath()).thenReturn(null); + SnapshotDataStoreVO refWithCheckpoint = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(refWithCheckpoint.getKvmCheckpointPath()).thenReturn("checkpoints/2/5/uuid"); + Mockito.when(refWithCheckpoint.getDataStoreId()).thenReturn(5L); + Mockito.when(refWithCheckpoint.getRole()).thenReturn(DataStoreRole.Image); + Mockito.when(snapshotStoreDao.findBySnapshotId(PARENT_SNAPSHOT_ID)).thenReturn(List.of(refWithoutCheckpoint, refWithCheckpoint)); + + SnapshotInfo parentInfo = Mockito.mock(SnapshotInfo.class); + Mockito.when(snapshotFactory.getSnapshot(PARENT_SNAPSHOT_ID, 5L, DataStoreRole.Image)).thenReturn(parentInfo); + + Assert.assertEquals(parentInfo, snapshotObject.getCorrectIncrementalParent(PARENT_SNAPSHOT_ID)); + Mockito.verify(parentInfo).setKvmIncrementalSnapshot(true); + } + + @Test + public void testGetCorrectIncrementalParentFallsBackToPlainParentWithoutCheckpointRefs() { + SnapshotDataStoreVO refWithoutCheckpoint = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(refWithoutCheckpoint.getKvmCheckpointPath()).thenReturn(null); + Mockito.when(snapshotStoreDao.findBySnapshotId(PARENT_SNAPSHOT_ID)).thenReturn(List.of(refWithoutCheckpoint)); + + SnapshotInfo parentInfo = Mockito.mock(SnapshotInfo.class); + Mockito.when(snapshotFactory.getSnapshot(PARENT_SNAPSHOT_ID, store)).thenReturn(parentInfo); + + Assert.assertEquals(parentInfo, snapshotObject.getCorrectIncrementalParent(PARENT_SNAPSHOT_ID)); + Mockito.verify(parentInfo, Mockito.never()).setKvmIncrementalSnapshot(Mockito.anyBoolean()); + } +} diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java index d03be9c4d294..dbf1f9de0bd3 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java @@ -222,7 +222,10 @@ public DataObject create(DataObject obj, DataStore dataStore) { private SnapshotDataStoreVO findParent(DataStore dataStore, Long clusterId, SnapshotInfo snapshotInfo) { boolean kvmIncrementalSnapshot = SnapshotManager.kvmIncrementalSnapshot.valueIn(clusterId); SnapshotDataStoreVO snapshotDataStoreVO; - if (Hypervisor.HypervisorType.KVM.equals(snapshotInfo.getHypervisorType()) && kvmIncrementalSnapshot) { + // content-based chains (Linstor) live purely on the image store; the cross-role checkpoint + // handling below (with its end-of-chain marking) must not be applied to their parents + if (Hypervisor.HypervisorType.KVM.equals(snapshotInfo.getHypervisorType()) && kvmIncrementalSnapshot + && !snapshotDataStoreDao.usesContentBasedChain(snapshotInfo.getVolumeId())) { snapshotDataStoreVO = snapshotDataStoreDao.findParent(null, null, null, snapshotInfo.getVolumeId(), kvmIncrementalSnapshot, snapshotInfo.getHypervisorType()); snapshotDataStoreVO = returnNullIfNotOnSameTypeOfStoreRole(snapshotInfo, snapshotDataStoreVO); diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java index e51c80e521c7..94978e53e12c 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java @@ -252,14 +252,12 @@ public void create(final QemuImgFile file, final QemuImgFile backingFile, final Shouldn't this be -o backing_file=filename instead? */ s.add("-f"); + s.add(file.getFormat().toString()); if (backingFile != null) { - s.add(backingFile.getFormat().toString()); s.add("-F"); s.add(backingFile.getFormat().toString()); s.add("-b"); s.add(backingFile.getFileName()); - } else { - s.add(file.getFormat().toString()); } s.add(file.getFileName()); diff --git a/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java index 15f6785c1fd4..017ca7391d15 100644 --- a/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java +++ b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java @@ -316,6 +316,25 @@ public void testCreateWithBackingFile() throws QemuImgException, LibvirtExceptio } } + @Test + public void testCreateWithBackingFileOfOtherFormat() throws QemuImgException, LibvirtException { + String backingFileName = "/tmp/" + UUID.randomUUID() + ".raw"; + String overlayFileName = "/tmp/" + UUID.randomUUID() + ".qcow2"; + + QemuImgFile backingFile = new QemuImgFile(backingFileName, 20480, PhysicalDiskFormat.RAW); + QemuImgFile overlayFile = new QemuImgFile(overlayFileName, 20480, PhysicalDiskFormat.QCOW2); + + QemuImg qemu = new QemuImg(0); + qemu.create(backingFile); + qemu.create(overlayFile, backingFile); + + // the created file keeps its own format, the backing file's format is only passed as -F + Map info = qemu.info(overlayFile); + assertEquals(PhysicalDiskFormat.QCOW2.toString(), info.get(QemuImg.FILE_FORMAT)); + assertEquals(backingFileName, info.get(QemuImg.BACKING_FILE)); + assertEquals(PhysicalDiskFormat.RAW.toString(), info.get(QemuImg.BACKING_FILE_FORMAT)); + } + @Test public void testConvertBasic() throws QemuImgException, LibvirtException { long srcSize = 20480; diff --git a/plugins/storage/volume/linstor/CHANGELOG.md b/plugins/storage/volume/linstor/CHANGELOG.md index a6ab050b090e..3f0ef3fc417d 100644 --- a/plugins/storage/volume/linstor/CHANGELOG.md +++ b/plugins/storage/volume/linstor/CHANGELOG.md @@ -24,6 +24,12 @@ All notable changes to Linstor CloudStack plugin will be documented in this file The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2026-07-30] + +### Added + +- Support for incremental snapshot backups on secondary storage + ## [2026-06-24] ### Fixed diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/api/storage/LinstorBackupSnapshotCommand.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/api/storage/LinstorBackupSnapshotCommand.java index 8d887dbba21a..069368bd7640 100644 --- a/plugins/storage/volume/linstor/src/main/java/com/cloud/api/storage/LinstorBackupSnapshotCommand.java +++ b/plugins/storage/volume/linstor/src/main/java/com/cloud/api/storage/LinstorBackupSnapshotCommand.java @@ -21,6 +21,13 @@ public class LinstorBackupSnapshotCommand extends CopyCommand { + /** + * Option holding the secondary storage install path of the parent snapshot qcow2. When set (and + * fullSnapshot=false), the agent writes an incremental backup: a qcow2 containing only the blocks + * that differ from the parent, with the parent as its backing file. + */ + public static final String OPTION_PARENT_PATH = "parentPath"; + public LinstorBackupSnapshotCommand(DataTO srcData, DataTO destData, int timeout, boolean executeInSequence) { super(srcData, destData, timeout, executeInSequence); diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java index c111d320cb4e..1c48eb2b22f4 100644 --- a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java +++ b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java @@ -121,6 +121,36 @@ private String convertImageToQCow2( return dstPath; } + /** + * Writes an incremental backup: a qcow2 on secondary storage containing only the blocks of the + * snapshot device that differ from the parent snapshot qcow2, with the parent as backing file. + * The overlay starts out backed by the raw snapshot device itself; the safe-mode rebase onto the + * parent then copies every cluster in which the two backing files differ into the overlay. The + * explicit virtual size clips the DRBD metadata trailing the storage snapshot device. + */ + private String createIncrementalQCow2(final String srcPath, final SnapshotObjectTO dst, final KVMStoragePool secondaryPool, + final File parentFile, final long netSize, int waitMilliSeconds) throws LibvirtException, QemuImgException, IOException { + final String dstDir = secondaryPool.getLocalPathFor(dst.getPath()); + FileUtils.forceMkdir(new File(dstDir)); + final String dstPath = dstDir + File.separator + dst.getName(); + + final QemuImg qemu = new QemuImg(waitMilliSeconds); + final QemuImgFile dstFile = new QemuImgFile(dstPath, netSize, QemuImg.PhysicalDiskFormat.QCOW2); + try { + qemu.create(dstFile, new QemuImgFile(srcPath, QemuImg.PhysicalDiskFormat.RAW)); + qemu.rebase(dstFile, new QemuImgFile(parentFile.getAbsolutePath(), QemuImg.PhysicalDiskFormat.QCOW2), + QemuImg.PhysicalDiskFormat.QCOW2.toString(), true); + // metadata-only rewrite to a relative backing name, so the chain stays valid on any mount point + qemu.rebase(dstFile, new QemuImgFile(parentFile.getName(), QemuImg.PhysicalDiskFormat.QCOW2), + QemuImg.PhysicalDiskFormat.QCOW2.toString(), false); + } catch (final QemuImgException e) { + FileUtils.deleteQuietly(new File(dstPath)); + throw e; + } + LOGGER.info("Incremental backup snapshot '{}' to '{}' (parent '{}')", srcPath, dstPath, parentFile.getName()); + return dstPath; + } + private SnapshotObjectTO setCorrectSnapshotSize(final SnapshotObjectTO dst, final String dstPath) { final File snapFile = new File(dstPath); long size; @@ -176,12 +206,33 @@ public CopyCmdAnswer execute(LinstorBackupSnapshotCommand cmd, LibvirtComputingR final byte[] passphrase = src.getVolume() != null ? src.getVolume().getPassphrase() : null; final boolean encrypted = passphrase != null && passphrase.length > 0; - String dstPath = convertImageToQCow2(srcPath, dst, secondaryPool, passphrase, cmd.getWaitInMillSeconds()); + final Map options = cmd.getOptions(); + final String parentInstallPath = options != null ? + options.get(LinstorBackupSnapshotCommand.OPTION_PARENT_PATH) : null; + + boolean incremental = false; + String dstPath = null; + if (!encrypted && parentInstallPath != null && src.getVolume() != null) { + final File parentFile = new File(secondaryPool.getLocalPathFor(parentInstallPath)); + if (parentFile.isFile()) { + dstPath = createIncrementalQCow2( + srcPath, dst, secondaryPool, parentFile, src.getVolume().getSize(), cmd.getWaitInMillSeconds()); + incremental = true; + } else { + LOGGER.warn("Parent snapshot file '{}' missing on secondary storage, taking a full backup instead", + parentFile.getAbsolutePath()); + } + } + + if (dstPath == null) { + dstPath = convertImageToQCow2(srcPath, dst, secondaryPool, passphrase, cmd.getWaitInMillSeconds()); + } - if (!encrypted) { + if (!encrypted && !incremental) { // resize to real volume size, cutting of drbd metadata // For encrypted volumes the source is the decrypted DRBD device (already net-sized, // no drbd metadata to cut); shrinking an encrypted qcow2 would also need the secret. + // Incremental backups are created with the net size already, nothing to cut there. String result = qemuShrink(dstPath, src.getVolume().getSize(), cmd.getWaitInMillSeconds()); if (result != null) { return new CopyCmdAnswer("qemu-img shrink failed: " + result); @@ -190,6 +241,12 @@ public CopyCmdAnswer execute(LinstorBackupSnapshotCommand cmd, LibvirtComputingR } SnapshotObjectTO snapshot = setCorrectSnapshotSize(dst, dstPath); + // tells the management server whether the file is a delta qcow2 backed by the parent + // snapshot or a standalone full copy, so it can fix up the chain bookkeeping: + // SnapshotObject.processEvent drops the store ref's parent link when + // parentSnapshotPath is null, and the driver clears it when the answer is not incremental + snapshot.setKvmIncrementalSnapshot(incremental); + snapshot.setParentSnapshotPath(incremental ? parentInstallPath : null); LOGGER.info("Actual file size for '{}' is {}", dstPath, snapshot.getPhysicalSize()); return new CopyCmdAnswer(snapshot); } catch (final Exception e) { 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 c3b4e73ead03..00383c249f88 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 @@ -103,6 +103,8 @@ import org.apache.cloudstack.storage.command.CopyCommand; import org.apache.cloudstack.storage.command.CreateObjectAnswer; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.util.LinstorConfigurationManager; import org.apache.cloudstack.storage.datastore.util.LinstorUtil; @@ -124,6 +126,7 @@ public class LinstorPrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver @Inject private VMTemplatePoolDao _vmTemplatePoolDao; @Inject private SnapshotDao _snapshotDao; @Inject private SnapshotDetailsDao _snapshotDetailsDao; + @Inject private SnapshotDataStoreDao _snapshotStoreDao; @Inject private StorageManager _storageMgr; @Inject ConfigurationDao _configDao; @@ -857,15 +860,17 @@ private Optional getLinstorEP(DevelopersApi api, StoragePool private Optional getDiskfullEP(DevelopersApi api, StoragePool storagePool, String rscName) throws ApiException { - List linSPs = LinstorUtil.getDiskfulStoragePools(api, rscName); - if (linSPs != null) { - List linstorNodeNames = linSPs.stream() - .map(com.linbit.linstor.api.model.StoragePool::getNodeName) - .collect(Collectors.toList()); - Host host = getEnabledClusterHost(storagePool, linstorNodeNames); + // Take the first diskful copy whose host can be used: the copies are ordered by how + // suited they are (in use, then active, then inactive), and for shared storage pools + // only the active node may be read, so the order must not be broken here. + for (com.linbit.linstor.api.model.StoragePool linSP + : LinstorUtil.getDiskfulStoragePoolsByPreference(api, rscName)) { + Host host = getEnabledClusterHost(storagePool, Collections.singletonList(linSP.getNodeName())); if (host != null) { return Optional.of(RemoteHostEndPoint.getHypervisorHostEndPoint(host)); } + logger.debug("Linstor: no usable cloudstack host for diskful node {}, trying next copy", + linSP.getNodeName()); } logger.error("Linstor: No diskfull host found."); return Optional.empty(); @@ -1071,15 +1076,30 @@ protected Answer copySnapshot(DataObject srcData, DataObject destData) { value, Integer.parseInt(Config.BackupSnapshotWait.getDefaultValue())); SnapshotObject snapshotObject = (SnapshotObject)srcData; - Boolean snapshotFullBackup = snapshotObject.getFullBackup(); final StoragePoolVO pool = _storagePoolDao.findById(srcData.getDataStore().getId()); final DevelopersApi api = getLinstorAPI(pool); - boolean fullSnapshot = true; - if (snapshotFullBackup != null) { - fullSnapshot = snapshotFullBackup; - } + + // For encrypted volumes Linstor adds a LUKS layer (DRBD -> LUKS -> STORAGE). The storage + // layer snapshot device (getSnapshotPath) therefore only exposes the raw LUKS ciphertext, + // while restore writes onto the decrypted DRBD device (/dev/drbd/by-res/.../0). Backing up + // the ciphertext and writing it back to the decrypted layer corrupts the volume (and the + // shrink to the net volume size would even truncate the ciphertext). So for encrypted + // volumes we never read the storage snapshot directly: restore the snapshot into a temporary + // resource and back up its decrypted DRBD device instead, symmetric to the restore path. + final boolean encrypted = snapshotObject.getBaseVolume().getPassphraseId() != null; + + SnapshotDataStoreVO destRef = _snapshotStoreDao.findByStoreSnapshot( + destData.getDataStore().getRole(), destData.getDataStore().getId(), destData.getId()); + // encrypted volumes are always backed up as full copies: an incremental rebase would need + // the LUKS secret for both the delta and the backing file + String parentPath = encrypted ? null : getIncrementalParentPath(destRef); + boolean fullSnapshot = parentPath == null; + Map options = new HashMap<>(); options.put("fullSnapshot", fullSnapshot + ""); + if (parentPath != null) { + options.put(LinstorBackupSnapshotCommand.OPTION_PARENT_PATH, parentPath); + } options.put(SnapshotInfo.BackupSnapshotAfterTakingSnapshot.key(), String.valueOf(SnapshotInfo.BackupSnapshotAfterTakingSnapshot.value())); options.put("volumeSize", snapshotObject.getBaseVolume().getSize() + ""); @@ -1095,14 +1115,6 @@ protected Answer copySnapshot(DataObject srcData, DataObject destData) { VirtualMachineManager.ExecuteInSequence.value()); cmd.setOptions(options); - // For encrypted volumes Linstor adds a LUKS layer (DRBD -> LUKS -> STORAGE). The storage - // layer snapshot device (getSnapshotPath) therefore only exposes the raw LUKS ciphertext, - // while restore writes onto the decrypted DRBD device (/dev/drbd/by-res/.../0). Backing up - // the ciphertext and writing it back to the decrypted layer corrupts the volume (and the - // shrink to the net volume size would even truncate the ciphertext). So for encrypted - // volumes we never read the storage snapshot directly: restore the snapshot into a temporary - // resource and back up its decrypted DRBD device instead, symmetric to the restore path. - final boolean encrypted = snapshotObject.getBaseVolume().getPassphraseId() != null; Optional optEP = encrypted ? Optional.empty() : getDiskfullEP(api, pool, rscName); Answer answer; @@ -1113,6 +1125,9 @@ protected Answer copySnapshot(DataObject srcData, DataObject destData) { encrypted); answer = copyFromTemporaryResource(api, pool, rscName, snapshotName, snapshotObject, cmd); } + if (answer != null && answer.getResult()) { + clearChainParentIfFullCopy(destRef, answer); + } return answer; } catch (Exception e) { logger.debug("copy snapshot failed, please cleanup snapshot manually: ", e); @@ -1121,6 +1136,64 @@ protected Answer copySnapshot(DataObject srcData, DataObject destData) { } + /** + * Returns the secondary storage install path of the parent snapshot to build an incremental + * (content-diff) backup against, or null if a full backup has to be taken. The parent link on the + * destination store ref was set at allocation time (honoring end_of_chain and kvm.incremental.snapshot). + */ + protected String getIncrementalParentPath(SnapshotDataStoreVO destRef) { + if (destRef == null || destRef.getParentSnapshotId() <= 0) { + return null; + } + if (!LinstorConfigurationManager.BackupSnapshots.value()) { + // snapshots-kept-on-primary mode: copies to secondary are only temporary (template from + // snapshot, extract, cross-zone copy) and their refs get expunged after use. A delta + // against such a parent would be left with a dangling backing file, so always copy full. + logger.debug("{} is disabled, taking full backup of snapshot {}", + LinstorConfigurationManager.BackupSnapshots.key(), destRef.getSnapshotId()); + return null; + } + SnapshotDataStoreVO parentRef = _snapshotStoreDao.findByStoreSnapshot( + destRef.getRole(), destRef.getDataStoreId(), destRef.getParentSnapshotId()); + if (parentRef == null || parentRef.getInstallPath() == null + || parentRef.getState() != ObjectInDataStoreStateMachine.State.Ready) { + logger.debug("Parent snapshot {} of snapshot {} not ready on store, taking full backup instead", + destRef.getParentSnapshotId(), destRef.getSnapshotId()); + return null; + } + if (parentRef.getSize() != destRef.getSize()) { + // volume was resized between the two snapshots; the delta would need to cover the size + // change through beyond-EOF backing semantics, take a full backup instead + logger.debug("Parent snapshot {} has a different volume size than snapshot {} ({} != {}), taking full backup instead", + destRef.getParentSnapshotId(), destRef.getSnapshotId(), parentRef.getSize(), destRef.getSize()); + return null; + } + return parentRef.getInstallPath(); + } + + /** + * The parent link is only valid if the backup really was written as a delta of the parent file. + * If a full copy was made instead (encrypted volume, missing/unready parent or agent-side + * fallback), drop the link so chain bookkeeping (delete ordering, chain length, flattening) + * matches what is on disk. + */ + protected void clearChainParentIfFullCopy(SnapshotDataStoreVO destRef, Answer answer) { + if (destRef == null || destRef.getParentSnapshotId() <= 0) { + return; + } + boolean deltaWritten = false; + if (answer instanceof CopyCmdAnswer) { + DataTO newData = ((CopyCmdAnswer) answer).getNewData(); + deltaWritten = newData instanceof SnapshotObjectTO && ((SnapshotObjectTO) newData).isKvmIncrementalSnapshot(); + } + if (!deltaWritten) { + logger.debug("Snapshot {} was backed up as a full copy, clearing its chain parent {}", + destRef.getSnapshotId(), destRef.getParentSnapshotId()); + destRef.setParentSnapshotId(0); + _snapshotStoreDao.update(destRef.getId(), destRef); + } + } + @Override public void copyAsync(DataObject srcData, DataObject destData, Host destHost, AsyncCompletionCallback callback) { diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java index 67c070f84eb0..434f6ca0162a 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java @@ -41,9 +41,11 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -148,8 +150,26 @@ public static List getLinstorNodeNames(@Nonnull DevelopersApi api) throw return nodes.stream().map(Node::getName).collect(Collectors.toList()); } + /** + * How suitable a diskful resource is for reading its data (snapshots, volume copies): + * the node it is in use on is best, then nodes with an active resource, then inactive ones. + * Thick LVM snapshots on shared storage pools (dm-snapshot) are not cluster aware, so they + * must be read on the node the origin volume is active on - callers must honor this order. + */ + private static int diskfulPreference(ResourceWithVolumes rwv) { + if (rwv.getState() != null && Boolean.TRUE.equals(rwv.getState().isInUse())) { + return 2; + } + return rwv.getFlags() == null || !rwv.getFlags().contains(ApiConsts.FLAG_RSC_INACTIVE) ? 1 : 0; + } + + /** + * All storage pools holding a diskful copy of the resource, best suited first + * (see {@link #diskfulPreference}). Callers that need a single pool should take the first + * one, callers picking a host should walk the list in order and use the first usable one. + */ public static List - getDiskfulStoragePools(@Nonnull DevelopersApi api, @Nonnull String rscName) throws ApiException + getDiskfulStoragePoolsByPreference(@Nonnull DevelopersApi api, @Nonnull String rscName) throws ApiException { List resources = api.viewResources( Collections.emptyList(), @@ -159,43 +179,50 @@ public static List getLinstorNodeNames(@Nonnull DevelopersApi api) throw null, null); - String nodeName = null; - String storagePoolName = null; - for (ResourceWithVolumes rwv : resources) { - if (rwv.getVolumes().isEmpty()) { - continue; - } - Volume vol = rwv.getVolumes().get(0); - if (vol.getProviderKind() != ProviderKind.DISKLESS) { - nodeName = rwv.getNodeName(); - storagePoolName = vol.getStoragePoolName(); - break; - } - } + // node name -> storage pool name, ordered by how suited the copy is + List> candidates = resources.stream() + .filter(rwv -> !rwv.getVolumes().isEmpty() + && rwv.getVolumes().get(0).getProviderKind() != ProviderKind.DISKLESS) + .sorted(Comparator.comparingInt(LinstorUtil::diskfulPreference).reversed()) + .map(rwv -> new Pair<>(rwv.getNodeName(), rwv.getVolumes().get(0).getStoragePoolName())) + .collect(Collectors.toList()); - if (nodeName == null) { - return null; + if (candidates.isEmpty()) { + return Collections.emptyList(); } List sps = api.viewStoragePools( - Collections.singletonList(nodeName), - Collections.singletonList(storagePoolName), + candidates.stream().map(Pair::first).distinct().collect(Collectors.toList()), + candidates.stream().map(Pair::second).distinct().collect(Collectors.toList()), Collections.emptyList(), null, null, true ); - return sps != null ? sps : Collections.emptyList(); + if (sps == null) { + return Collections.emptyList(); + } + + // viewStoragePools does not keep our ordering, so re-apply it + List ordered = new ArrayList<>(); + for (Pair candidate : candidates) { + sps.stream() + .filter(sp -> candidate.first().equals(sp.getNodeName()) + && candidate.second().equals(sp.getStoragePoolName())) + .findFirst() + .ifPresent(ordered::add); + } + return ordered; } + /** + * The storage pool of the best suited diskful copy, null if the resource has none. + */ public static com.linbit.linstor.api.model.StoragePool getDiskfulStoragePool(@Nonnull DevelopersApi api, @Nonnull String rscName) throws ApiException { - List sps = getDiskfulStoragePools(api, rscName); - if (sps != null) { - return !sps.isEmpty() ? sps.get(0) : null; - } - return null; + List sps = getDiskfulStoragePoolsByPreference(api, rscName); + return !sps.isEmpty() ? sps.get(0) : null; } public static String getSnapshotPath(com.linbit.linstor.api.model.StoragePool sp, String rscName, String snapshotName) { 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..69a6e90128f7 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 @@ -26,14 +26,23 @@ import java.util.Collections; import java.util.List; +import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; +import org.apache.cloudstack.storage.command.CopyCmdAnswer; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; import org.apache.cloudstack.storage.datastore.util.LinstorUtil; +import org.apache.cloudstack.storage.to.SnapshotObjectTO; 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.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import com.cloud.storage.DataStoreRole; + import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -42,6 +51,9 @@ public class LinstorPrimaryDataStoreDriverImplTest { private DevelopersApi api; + @Mock + private SnapshotDataStoreDao snapshotStoreDao; + @InjectMocks private LinstorPrimaryDataStoreDriverImpl linstorPrimaryDataStoreDriver; @@ -50,6 +62,87 @@ public void setUp() { api = mock(DevelopersApi.class); } + private SnapshotDataStoreVO mockDestRef(long parentSnapshotId, long size) { + SnapshotDataStoreVO destRef = mock(SnapshotDataStoreVO.class); + when(destRef.getParentSnapshotId()).thenReturn(parentSnapshotId); + if (parentSnapshotId > 0) { + Mockito.lenient().when(destRef.getRole()).thenReturn(DataStoreRole.Image); + Mockito.lenient().when(destRef.getDataStoreId()).thenReturn(10L); + Mockito.lenient().when(destRef.getSize()).thenReturn(size); + } + return destRef; + } + + private SnapshotDataStoreVO mockParentRef(String installPath, ObjectInDataStoreStateMachine.State state, long size) { + SnapshotDataStoreVO parentRef = mock(SnapshotDataStoreVO.class); + Mockito.lenient().when(parentRef.getInstallPath()).thenReturn(installPath); + Mockito.lenient().when(parentRef.getState()).thenReturn(state); + Mockito.lenient().when(parentRef.getSize()).thenReturn(size); + when(snapshotStoreDao.findByStoreSnapshot(DataStoreRole.Image, 10L, 2L)).thenReturn(parentRef); + return parentRef; + } + + @Test + public void testGetIncrementalParentPathNoDestRef() { + Assert.assertNull(linstorPrimaryDataStoreDriver.getIncrementalParentPath(null)); + } + + @Test + public void testGetIncrementalParentPathNoParentLink() { + Assert.assertNull(linstorPrimaryDataStoreDriver.getIncrementalParentPath(mockDestRef(0, 100L))); + } + + @Test + public void testGetIncrementalParentPathParentRefMissing() { + when(snapshotStoreDao.findByStoreSnapshot(DataStoreRole.Image, 10L, 2L)).thenReturn(null); + Assert.assertNull(linstorPrimaryDataStoreDriver.getIncrementalParentPath(mockDestRef(2L, 100L))); + } + + @Test + public void testGetIncrementalParentPathParentNotReady() { + mockParentRef("snapshots/2/5/parent", ObjectInDataStoreStateMachine.State.Destroyed, 100L); + Assert.assertNull(linstorPrimaryDataStoreDriver.getIncrementalParentPath(mockDestRef(2L, 100L))); + } + + @Test + public void testGetIncrementalParentPathVolumeResized() { + mockParentRef("snapshots/2/5/parent", ObjectInDataStoreStateMachine.State.Ready, 50L); + Assert.assertNull(linstorPrimaryDataStoreDriver.getIncrementalParentPath(mockDestRef(2L, 100L))); + } + + @Test + public void testGetIncrementalParentPathReadyParent() { + mockParentRef("snapshots/2/5/parent", ObjectInDataStoreStateMachine.State.Ready, 100L); + Assert.assertEquals("snapshots/2/5/parent", + linstorPrimaryDataStoreDriver.getIncrementalParentPath(mockDestRef(2L, 100L))); + } + + @Test + public void testClearChainParentIfFullCopyClearsOnFullBackup() { + SnapshotDataStoreVO destRef = mock(SnapshotDataStoreVO.class); + when(destRef.getParentSnapshotId()).thenReturn(2L); + + SnapshotObjectTO to = new SnapshotObjectTO(); + to.setKvmIncrementalSnapshot(false); + linstorPrimaryDataStoreDriver.clearChainParentIfFullCopy(destRef, new CopyCmdAnswer(to)); + + Mockito.verify(destRef).setParentSnapshotId(0); + Mockito.verify(snapshotStoreDao).update(Mockito.anyLong(), Mockito.eq(destRef)); + } + + @Test + public void testClearChainParentIfFullCopyKeepsLinkOnIncremental() { + SnapshotDataStoreVO destRef = mock(SnapshotDataStoreVO.class); + when(destRef.getParentSnapshotId()).thenReturn(2L); + + SnapshotObjectTO to = new SnapshotObjectTO(); + to.setKvmIncrementalSnapshot(true); + linstorPrimaryDataStoreDriver.clearChainParentIfFullCopy(destRef, new CopyCmdAnswer(to)); + + Mockito.verify(destRef, Mockito.never()).setParentSnapshotId(Mockito.anyLong()); + Mockito.verify(snapshotStoreDao, Mockito.never()).update(Mockito.anyLong(), Mockito.any()); + } + @Test public void testGetEncryptedLayerList() throws ApiException { ResourceGroup dfltRscGrp = new ResourceGroup(); diff --git a/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorUtilTest.java b/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorUtilTest.java index 55f0c6ebe6dc..b3a6f127f5a2 100644 --- a/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorUtilTest.java +++ b/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorUtilTest.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.storage.datastore.util; +import com.linbit.linstor.api.ApiConsts; import com.linbit.linstor.api.ApiException; import com.linbit.linstor.api.DevelopersApi; import com.linbit.linstor.api.model.AutoSelectFilter; @@ -23,7 +24,10 @@ import com.linbit.linstor.api.model.Properties; import com.linbit.linstor.api.model.ProviderKind; import com.linbit.linstor.api.model.ResourceGroup; +import com.linbit.linstor.api.model.ResourceState; +import com.linbit.linstor.api.model.ResourceWithVolumes; import com.linbit.linstor.api.model.StoragePool; +import com.linbit.linstor.api.model.Volume; import java.util.Arrays; import java.util.Collections; @@ -115,6 +119,63 @@ public void testGetSnapshotPath() { } } + private ResourceWithVolumes mockResource(String node, String pool, boolean inUse, boolean inactive) { + ResourceWithVolumes rwv = new ResourceWithVolumes(); + rwv.setName("cs-test"); + rwv.setNodeName(node); + Volume vol = new Volume(); + vol.setProviderKind(ProviderKind.LVM); + vol.setStoragePoolName(pool); + rwv.setVolumes(Collections.singletonList(vol)); + ResourceState state = new ResourceState(); + state.setInUse(inUse); + rwv.setState(state); + if (inactive) { + rwv.setFlags(Collections.singletonList(ApiConsts.FLAG_RSC_INACTIVE)); + } + return rwv; + } + + @Test + public void testDiskfulStoragePoolsOrderedByPreference() throws ApiException { + // inactive copy first, in-use copy last: the result must be in-use, active, inactive + when(api.viewResources(Collections.emptyList(), Collections.singletonList("cs-test"), + Collections.emptyList(), Collections.emptyList(), null, null)) + .thenReturn(Arrays.asList( + mockResource("nodeC", "poolC", false, true), + mockResource("nodeB", "poolB", false, false), + mockResource("nodeA", "poolA", true, false))); + // the pools are queried in preference order (in use, active, inactive) + when(api.viewStoragePools(Arrays.asList("nodeA", "nodeB", "nodeC"), + Arrays.asList("poolA", "poolB", "poolC"), Collections.emptyList(), null, null, true)) + .thenReturn(Arrays.asList( + mockStoragePool("poolB", "nodeB", ProviderKind.LVM), + mockStoragePool("poolA", "nodeA", ProviderKind.LVM), + mockStoragePool("poolC", "nodeC", ProviderKind.LVM))); + + List pools = LinstorUtil.getDiskfulStoragePoolsByPreference(api, "cs-test"); + Assert.assertEquals(Arrays.asList("nodeA", "nodeB", "nodeC"), + pools.stream().map(StoragePool::getNodeName).collect(Collectors.toList())); + // the single-pool accessor keeps returning the best suited copy + Assert.assertEquals("nodeA", LinstorUtil.getDiskfulStoragePool(api, "cs-test").getNodeName()); + } + + @Test + public void testDiskfulStoragePoolsIgnoresDiskless() throws ApiException { + ResourceWithVolumes diskless = new ResourceWithVolumes(); + diskless.setName("cs-test"); + diskless.setNodeName("nodeD"); + Volume dlVol = new Volume(); + dlVol.setProviderKind(ProviderKind.DISKLESS); + diskless.setVolumes(Collections.singletonList(dlVol)); + when(api.viewResources(Collections.emptyList(), Collections.singletonList("cs-test"), + Collections.emptyList(), Collections.emptyList(), null, null)) + .thenReturn(Collections.singletonList(diskless)); + + Assert.assertTrue(LinstorUtil.getDiskfulStoragePoolsByPreference(api, "cs-test").isEmpty()); + Assert.assertNull(LinstorUtil.getDiskfulStoragePool(api, "cs-test")); + } + @Test public void testGetRscGroupStoragePools() throws ApiException { List storagePools = LinstorUtil.getRscGroupStoragePools(api, "cloudstack"); diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index d60bff095406..3fbe33aa6d89 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -597,7 +597,7 @@ public String extractSnapshot(ExtractSnapshotCmd cmd) { SnapshotInfo snapshotObject = snapshotFactory.getSnapshot(snapshotId, chosenStore); - if (snapshotDataStoreReference.getKvmCheckpointPath() != null) { + if (isIncrementalChainRef(snapshotDataStoreReference, snapshot.getHypervisorType())) { snapshotSrv.convertSnapshot(snapshotObject); } @@ -779,10 +779,10 @@ private void postCreateSnapshot(Long volumeId, Long snapshotId, Long policyId, L /** * Will mark this snapshot as the end of the chain if it has reached the value of snapshot.delta.max. * */ - private void endCurrentChainIfNeeded(Long snapshotId, Long zoneId) { + protected void endCurrentChainIfNeeded(Long snapshotId, Long zoneId) { SnapshotDataStoreVO snapshotDataStoreVo = _snapshotStoreDao.findOneBySnapshotId(snapshotId, zoneId); int chainSize = 1; - while (snapshotDataStoreVo.getParentSnapshotId() > 0) { + while (snapshotDataStoreVo != null && snapshotDataStoreVo.getParentSnapshotId() > 0) { snapshotDataStoreVo = _snapshotStoreDao.findOneBySnapshotId(snapshotDataStoreVo.getParentSnapshotId(), zoneId); chainSize++; } @@ -814,7 +814,7 @@ protected void endLastChainIfNeeded(long clusterId, long volumeId) { SnapshotDataStoreVO snapshotDataStoreVO; for (int i = 1; i < volumeSnapshots.size(); i++) { snapshotDataStoreVO = volumeSnapshots.get(i); - if (snapshotDataStoreVO.getKvmCheckpointPath() != null) { + if (isIncrementalChainRef(snapshotDataStoreVO, HypervisorType.KVM)) { if (!snapshotDataStoreVO.isEndOfChain()) { logger.debug("Found snapshot reference [{}] that used to belong to a now dead snapshot chain. Will mark it as end of chain.", snapshotDataStoreVO); snapshotDataStoreVO.setEndOfChain(true); @@ -825,6 +825,16 @@ protected void endLastChainIfNeeded(long clusterId, long volumeId) { } } + /** + * Whether this snapshot store reference is part of a KVM incremental snapshot chain on secondary + * storage. File-based storage chains carry a qemu checkpoint path; Linstor chains only link deltas + * through the parent snapshot id (content-diff qcow2 chain). Members of either chain kind must be + * flattened (converted) before being used standalone. + */ + protected boolean isIncrementalChainRef(SnapshotDataStoreVO ref, HypervisorType hypervisorType) { + return HypervisorType.KVM.equals(hypervisorType) && (ref.getKvmCheckpointPath() != null || ref.getParentSnapshotId() > 0); + } + private void postCreateRecurringSnapshotForPolicy(long userId, long volumeId, long snapshotId, long policyId) { // Use count query SnapshotVO spstVO = _snapshotDao.findById(snapshotId); @@ -2182,7 +2192,7 @@ private boolean copySnapshotChainToZone(SnapshotVO snapshotVO, DataStore srcSecS List snapshotChain = new ArrayList<>(); long size = 0L; DataStore dstSecStore = null; - boolean kvmIncrementalSnapshot = currentSnap.getKvmCheckpointPath() != null; + boolean kvmIncrementalSnapshot = isIncrementalChainRef(currentSnap, snapshotVO.getHypervisorType()); do { dstSecStore = getSnapshotZoneImageStore(currentSnap.getSnapshotId(), destZone.getId()); if (dstSecStore != null) { diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java index ff0888c184c0..e283fd88ed8c 100644 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java @@ -20,6 +20,7 @@ import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; import com.cloud.event.ActionEventUtils; +import com.cloud.hypervisor.Hypervisor; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceUnavailableException; @@ -610,4 +611,83 @@ public void testDeleteSnapshotPoliciesManualPolicyId() { snapshotManager.deleteSnapshotPolicies(cmd); } + + @Test + public void testIsIncrementalChainRefNonKvmHypervisor() { + SnapshotDataStoreVO ref = Mockito.mock(SnapshotDataStoreVO.class); + Assert.assertFalse(snapshotManager.isIncrementalChainRef(ref, Hypervisor.HypervisorType.XenServer)); + } + + @Test + public void testIsIncrementalChainRefKvmWithCheckpointPath() { + SnapshotDataStoreVO ref = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(ref.getKvmCheckpointPath()).thenReturn("checkpoints/2/5/uuid"); + Assert.assertTrue(snapshotManager.isIncrementalChainRef(ref, Hypervisor.HypervisorType.KVM)); + } + + @Test + public void testIsIncrementalChainRefKvmWithParentLinkOnly() { + SnapshotDataStoreVO ref = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(ref.getKvmCheckpointPath()).thenReturn(null); + Mockito.when(ref.getParentSnapshotId()).thenReturn(2L); + Assert.assertTrue(snapshotManager.isIncrementalChainRef(ref, Hypervisor.HypervisorType.KVM)); + } + + @Test + public void testIsIncrementalChainRefKvmStandalone() { + SnapshotDataStoreVO ref = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(ref.getKvmCheckpointPath()).thenReturn(null); + Mockito.when(ref.getParentSnapshotId()).thenReturn(0L); + Assert.assertFalse(snapshotManager.isIncrementalChainRef(ref, Hypervisor.HypervisorType.KVM)); + } + + private void mockParentLinkedChain(long chainLength, long zoneId) { + // snapshot i is a delta on snapshot i - 1, snapshot 1 is the full chain start (parent 0) + for (long i = 1; i <= chainLength; i++) { + SnapshotDataStoreVO ref = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(ref.getParentSnapshotId()).thenReturn(i - 1); + Mockito.when(snapshotStoreDao.findOneBySnapshotId(i, zoneId)).thenReturn(ref); + } + } + + @Test + public void testEndCurrentChainIfNeededMarksEndOfChainAtDeltaMax() { + final long zoneId = 1L; + final long deltaMax = SnapshotManager.snapshotDeltaMax.value(); + mockParentLinkedChain(deltaMax, zoneId); + SnapshotDataStoreVO leafPrimaryRef = Mockito.mock(SnapshotDataStoreVO.class); + SnapshotDataStoreVO leafImageRef = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(snapshotStoreDao.listBySnapshotId(deltaMax)).thenReturn(List.of(leafPrimaryRef, leafImageRef)); + + snapshotManager.endCurrentChainIfNeeded(deltaMax, zoneId); + + Mockito.verify(leafPrimaryRef).setEndOfChain(true); + Mockito.verify(leafImageRef).setEndOfChain(true); + Mockito.verify(snapshotStoreDao, Mockito.times(2)).update(Mockito.anyLong(), Mockito.any(SnapshotDataStoreVO.class)); + } + + @Test + public void testEndCurrentChainIfNeededKeepsChainBelowDeltaMax() { + final long zoneId = 1L; + final long chainLength = SnapshotManager.snapshotDeltaMax.value() - 1; + mockParentLinkedChain(chainLength, zoneId); + + snapshotManager.endCurrentChainIfNeeded(chainLength, zoneId); + + Mockito.verify(snapshotStoreDao, Mockito.never()).listBySnapshotId(Mockito.anyLong()); + Mockito.verify(snapshotStoreDao, Mockito.never()).update(Mockito.anyLong(), Mockito.any(SnapshotDataStoreVO.class)); + } + + @Test + public void testEndCurrentChainIfNeededStopsOnMissingParentRef() { + final long zoneId = 1L; + SnapshotDataStoreVO leafRef = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(leafRef.getParentSnapshotId()).thenReturn(5L); + Mockito.when(snapshotStoreDao.findOneBySnapshotId(6L, zoneId)).thenReturn(leafRef); + Mockito.when(snapshotStoreDao.findOneBySnapshotId(5L, zoneId)).thenReturn(null); + + snapshotManager.endCurrentChainIfNeeded(6L, zoneId); + + Mockito.verify(snapshotStoreDao, Mockito.never()).update(Mockito.anyLong(), Mockito.any(SnapshotDataStoreVO.class)); + } } diff --git a/test/integration/plugins/linstor/README.md b/test/integration/plugins/linstor/README.md index 4971c9506b5b..f3c486de03c2 100644 --- a/test/integration/plugins/linstor/README.md +++ b/test/integration/plugins/linstor/README.md @@ -66,3 +66,33 @@ Extra prerequisites: ``` nosetests --with-marvin --marvin-config= /test/integration/plugins/linstor/test_linstor_encrypted_snapshots.py --zone= --hypervisor=kvm ``` + +## Incremental snapshot tests + +`test_linstor_incremental_snapshots.py` covers incremental (content-diff) snapshot backups on +NFS secondary storage: full -> delta chaining, `snapshot.delta.max` chain rotation, revert to a +mid-chain delta, template creation from a delta (chain flattening), the fallback to a full backup +when the parent file is missing on secondary storage, mid-chain deletion, and that encrypted +volumes are always backed up as full copies. + +Extra prerequisites: + +* NFS secondary storage (incremental snapshots are only supported there). +* `kvm.incremental.snapshot`, `kvm.snapshot.enabled` and `lin.backup.snapshots` are set by the + tests themselves (and restored afterwards). Snapshots are taken while the VMs are running + (crash-consistent storage snapshots; markers are synced before each snapshot); VMs are only + stopped where CloudStack requires it (revert). +* The fallback and qcow2-inspection checks need host SSH credentials, either in the marvin config + (zones->pods->clusters->hosts) or via the `HOST_SSH_USER` / `HOST_SSH_PASSWORD` env vars; those + tests self-skip without them. +* The parent-link assertions read `snapshot_store_ref` directly and therefore need the DB + connection of the marvin config to work; without it the tests fall back to comparing + physical sizes. + +``` +nosetests --with-marvin --marvin-config= /test/integration/plugins/linstor/test_linstor_incremental_snapshots.py --zone= --hypervisor=kvm +``` + +Note: select single tests with `... test_linstor_incremental_snapshots.py:TestLinstorIncrementalSnapshots -m ` +(class selection plus a method filter); the `file.py:Class.test_method` form bypasses the marvin +plugin's test client injection and fails with `'NoneType' object has no attribute 'getApiClient'`. diff --git a/test/integration/plugins/linstor/linstor_test_utils.py b/test/integration/plugins/linstor/linstor_test_utils.py new file mode 100644 index 000000000000..c048f93951f1 --- /dev/null +++ b/test/integration/plugins/linstor/linstor_test_utils.py @@ -0,0 +1,54 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Helpers shared by the Linstor plugin integration tests.""" + +import socket +import time + + +class ServiceReady: + @classmethod + def ready(cls, hostname: str, port: int) -> bool: + try: + s = socket.create_connection((hostname, port), timeout=1) + s.close() + return True + except (ConnectionRefusedError, socket.timeout, OSError): + return False + + @classmethod + def wait(cls, hostname: str, port: int, wait_interval: float = 5, timeout: int = 120, + service_name: str = 'ssh') -> bool: + """ + Wait until the given service can be reached, raise RuntimeError on timeout. + :param hostname: host to connect to + :param port: port of the application + :param wait_interval: seconds between connection attempts + :param timeout: seconds to wait before raising + :param service_name: name of the service waited for (used in the error message) + """ + starttime = int(round(time.time() * 1000)) + while not cls.ready(hostname, port): + if starttime + timeout * 1000 < int(round(time.time() * 1000)): + raise RuntimeError("{s} {h} cannot be reached.".format(s=service_name, h=hostname)) + time.sleep(wait_interval) + return True + + @classmethod + def wait_ssh_ready(cls, hostname: str, wait_interval: float = 2, timeout: int = 120) -> bool: + return cls.wait(hostname, 22, wait_interval, timeout, "ssh") diff --git a/test/integration/plugins/linstor/test_linstor_encrypted_snapshots.py b/test/integration/plugins/linstor/test_linstor_encrypted_snapshots.py index 5f440309bb34..97902942f412 100644 --- a/test/integration/plugins/linstor/test_linstor_encrypted_snapshots.py +++ b/test/integration/plugins/linstor/test_linstor_encrypted_snapshots.py @@ -19,8 +19,6 @@ import logging import os import random -import socket -import time # All tests inherit from cloudstackTestCase from marvin.cloudstackTestCase import cloudstackTestCase @@ -35,6 +33,8 @@ from marvin.sshClient import SshClient from nose.plugins.attrib import attr +from linstor_test_utils import ServiceReady + # Prerequisites: # Only one zone / pod / cluster # Only KVM hypervisor (Linstor only supports KVM) @@ -119,30 +119,6 @@ def __init__(self, linstor_controller_url): } -class ServiceReady: - @classmethod - def ready(cls, hostname, port): - try: - s = socket.create_connection((hostname, port), timeout=1) - s.close() - return True - except (ConnectionRefusedError, socket.timeout, OSError): - return False - - @classmethod - def wait(cls, hostname, port, wait_interval=5, timeout=120, service_name='ssh'): - starttime = int(round(time.time() * 1000)) - while not cls.ready(hostname, port): - if starttime + timeout * 1000 < int(round(time.time() * 1000)): - raise RuntimeError("{s} {h} cannot be reached.".format(s=service_name, h=hostname)) - time.sleep(wait_interval) - return True - - @classmethod - def wait_ssh_ready(cls, hostname, wait_interval=2, timeout=120): - return cls.wait(hostname, 22, wait_interval, timeout, "ssh") - - class TestLinstorEncryptedSnapshots(cloudstackTestCase): @classmethod diff --git a/test/integration/plugins/linstor/test_linstor_incremental_snapshots.py b/test/integration/plugins/linstor/test_linstor_incremental_snapshots.py new file mode 100644 index 000000000000..340d90d4d163 --- /dev/null +++ b/test/integration/plugins/linstor/test_linstor_incremental_snapshots.py @@ -0,0 +1,639 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import json +import logging +import os +import random + +# All tests inherit from cloudstackTestCase +from marvin.cloudstackTestCase import cloudstackTestCase + +# Import Integration Libraries +from marvin.lib.base import Account, Configurations, ServiceOffering, \ + Snapshot, StoragePool, Template, User, VirtualMachine, Volume +from marvin.lib.common import get_domain, get_template, get_zone, list_hosts, list_virtual_machines, list_volumes +from marvin.lib.utils import cleanup_resources +from marvin.sshClient import SshClient +from nose.plugins.attrib import attr + +from linstor_test_utils import ServiceReady + +# Prerequisites: +# Only one zone / pod / cluster +# Only KVM hypervisor (Linstor only supports KVM) +# One Linstor storage pool (resource group without a LUKS layer; plain volumes) +# NFS secondary storage (incremental snapshots are only supported on NFS secondary) +# 'lin.backup.snapshots' enabled (default true) so snapshots are backed up to secondary storage. +# 'kvm.snapshot.enabled' enabled (default true): snapshots are taken on running VMs (the tests +# set it). Markers are synced before each snapshot, so the crash-consistent images contain them. +# +# What this exercises (the Linstor incremental snapshot feature): +# Volumes on Linstor primary storage back their snapshots up to secondary storage as qcow2 files. +# With kvm.incremental.snapshot enabled, every snapshot after a full one is stored as a delta +# qcow2 (a content diff produced by qemu-img rebase) whose backing file is the previous snapshot, +# so a chain member only consumes the space of the blocks that changed since its parent. +# +# * chaining: full -> delta -> delta, parent links recorded on the image store refs +# * rotation: snapshot.delta.max caps the chain length, then a new full is taken +# * restore: reverting to a chain member follows the backing chain +# * flatten: templates/volumes created from a mid-chain delta get standalone content +# * fallback: a missing parent file on secondary storage degrades to a full backup +# * exclusion: encrypted volumes are always backed up as full copies +# +# Note on verification: the parent/child relationship of the backups is bookkeeping internal to the +# management server (snapshot_store_ref.parent_snapshot_id), not exposed through the API, so these +# tests read it from the DB where a DB connection is available and otherwise fall back to comparing +# physical sizes (a delta of a small change is orders of magnitude smaller than a full). + +MARKER_PATH = "/root/cs_incr_marker.txt" + + +class TestData: + account = "account" + computeOffering = "computeoffering" + domainId = "domainId" + hypervisor = "hypervisor" + provider = "provider" + scope = "scope" + storageTag = "linstor" + tags = "tags" + user = "user" + virtualMachine = "virtualmachine" + zoneId = "zoneId" + + def __init__(self, linstor_controller_url): + self.testdata = { + TestData.account: { + "email": "test-incr@test.com", + "firstname": "John", + "lastname": "Doe", + "username": "test-incr", + "password": "test" + }, + TestData.user: { + "email": "user-incr@test.com", + "firstname": "Jane", + "lastname": "Doe", + "username": "test-incr-user", + "password": "password" + }, + "primarystorage": { + "name": "LinstorIncrPool-%d" % random.randint(0, 100000), + TestData.scope: "ZONE", + "url": linstor_controller_url, + TestData.provider: "Linstor", + TestData.tags: TestData.storageTag, + TestData.hypervisor: "KVM", + "details": { + "resourceGroup": "acs-basic" + } + }, + TestData.virtualMachine: { + "name": "TestIncrVM", + "displayname": "Test Incremental VM" + }, + TestData.computeOffering: { + "name": "Linstor_Compute_Incr", + "displaytext": "Linstor_Compute_Incr", + "cpunumber": 1, + "cpuspeed": 500, + "memory": 512, + "storagetype": "shared", + TestData.tags: TestData.storageTag + }, + TestData.zoneId: 1, + TestData.domainId: 1, + } + + +class TestLinstorIncrementalSnapshots(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + testclient = super(TestLinstorIncrementalSnapshots, cls).getClsTestClient() + + cls.apiClient = testclient.getApiClient() + cls.dbConnection = testclient.getDbConnection() + + cls._cleanup = [] + cls.skip_reason = None + cls.original_config = {} + + # The first host runs the Linstor controller (per the Linstor test prerequisites). + first_host = list_hosts(cls.apiClient)[0] + cls.testdata = TestData(first_host.ipaddress).testdata + + cls.zone = get_zone(cls.apiClient, zone_id=cls.testdata[TestData.zoneId]) + cls.domain = get_domain(cls.apiClient, cls.testdata[TestData.domainId]) + cls.template = get_template(cls.apiClient, cls.zone.id, hypervisor="KVM") + + # Host SSH credentials, needed by the tests that inspect or manipulate the backed-up qcow2 + # files on secondary storage. A full marvin config carries these under + # zones->pods->clusters->hosts; a lightweight config may omit them - fall back to + # HOST_SSH_USER / HOST_SSH_PASSWORD env vars, and skip those tests if neither is present. + cls.hostConfig = None + try: + cls.hostConfig = cls.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0] \ + .__dict__["hosts"][0].__dict__ + except (KeyError, IndexError, AttributeError, TypeError): + host_user = os.environ.get("HOST_SSH_USER") + host_pass = os.environ.get("HOST_SSH_PASSWORD") + if host_user and host_pass: + cls.hostConfig = {"username": host_user, "password": host_pass} + + # The feature under test needs snapshot backups on secondary storage and the (cluster-scoped, + # here globally set) incremental snapshot switch. Save the original values and restore them in + # tearDownClass so the run leaves no configuration behind. + cls._set_config("lin.backup.snapshots", "true") + cls._set_config("kvm.incremental.snapshot", "true") + # all snapshots in these tests are taken while the VM is running (crash-consistent + # storage snapshots; also exercises the running-VM path of the feature) + cls._set_config("kvm.snapshot.enabled", "true") + + primarystorage = cls.testdata["primarystorage"] + api_token = os.environ.get("LINSTOR_API_TOKEN") + if api_token: + primarystorage["details"]["lin.auth.apitoken"] = api_token + + try: + cls.primary_storage = StoragePool.create( + cls.apiClient, + primarystorage, + scope=primarystorage[TestData.scope], + zoneid=cls.zone.id, + provider=primarystorage[TestData.provider], + tags=primarystorage[TestData.tags], + hypervisor=primarystorage[TestData.hypervisor] + ) + except Exception as e: + cls.skip_reason = ( + "Could not register the Linstor primary storage pool (%s). If the Linstor controller " + "requires authentication, set the LINSTOR_API_TOKEN env var to a valid controller API " + "token before running these tests." % e) + return + + cls.compute_offering = ServiceOffering.create( + cls.apiClient, + cls.testdata[TestData.computeOffering] + ) + + cls.account = Account.create(cls.apiClient, cls.testdata[TestData.account], admin=1) + cls.user = User.create( + cls.apiClient, cls.testdata[TestData.user], + account=cls.account.name, domainid=cls.domain.id) + + cls._cleanup = [ + cls.compute_offering, + cls.user, + cls.account, + ] + + @classmethod + def tearDownClass(cls): + try: + cleanup_resources(cls.apiClient, cls._cleanup) + if getattr(cls, "primary_storage", None) is not None: + cls.primary_storage.delete(cls.apiClient) + except Exception as e: + logging.debug("Exception in tearDownClass: %s" % e) + finally: + cls._restore_configs() + + def setUp(self): + if self.skip_reason: + self.skipTest(self.skip_reason) + self.cleanup = [] + + def tearDown(self): + cleanup_resources(self.apiClient, self.cleanup) + + # --------------------------------------------------------------------- # + # Tests + # --------------------------------------------------------------------- # + + @attr(tags=['basic'], required_hardware=True) + def test_01_second_snapshot_is_incremental(self): + """The first snapshot is a full backup, later ones are deltas chained to their parent.""" + vm = self._deploy_vm("TestIncrVM-chain") + + self._write_marker(vm, "incr-chain-v1") + snap_full = self._snapshot_root_volume(vm) + self._assert_full(snap_full) + + self._write_marker(vm, "incr-chain-v2") + snap_delta1 = self._snapshot_root_volume(vm) + self._assert_delta(snap_delta1, parent=snap_full) + + self._write_marker(vm, "incr-chain-v3") + snap_delta2 = self._snapshot_root_volume(vm) + self._assert_delta(snap_delta2, parent=snap_delta1, full=snap_full) + + # a delta of a one-line change must be far smaller than the full backup + self.assertLess( + int(snap_delta1.physicalsize) * 4, int(snap_full.physicalsize), + "Delta snapshot (%s bytes) is not substantially smaller than the full one (%s bytes)" + % (snap_delta1.physicalsize, snap_full.physicalsize)) + + # if we can reach the files on secondary storage, the delta must be a qcow2 whose + # backing file is its parent, referenced by a relative name (portable across mounts) + info = self._qemu_img_info_of_backed_up_snapshot(snap_delta1) + if info is not None: + backing = info.get("backing-filename") + self.assertIsNotNone(backing, "Delta snapshot qcow2 has no backing file: %s" % json.dumps(info)) + self.assertNotIn("/", backing, "Backing file %r is not a relative name" % backing) + + @attr(tags=['basic'], required_hardware=True) + def test_02_revert_to_delta_snapshot(self): + """Reverting to a mid-chain delta restores exactly that snapshot's content.""" + vm = self._deploy_vm("TestIncrVM-revert") + + self._write_marker(vm, "incr-revert-v1") + snap1 = self._snapshot_root_volume(vm) + + self._write_marker(vm, "incr-revert-v2") + snap2 = self._snapshot_root_volume(vm) + self._assert_delta(snap2, parent=snap1) + + # change the data once more so a successful revert is detectable + self._write_marker(vm, "incr-revert-v3-CHANGED") + vm.stop(self.apiClient) + + # revert to the delta: the restore has to follow the backing chain (full + delta) + Volume.revertToSnapshot(self.apiClient, snap2.id) + self._start_vm(vm) + self.assertEqual("incr-revert-v2", self._read_marker(vm), + "Revert to the delta snapshot did not restore its content") + + # revert to the chain-starting full as well + vm.stop(self.apiClient) + Volume.revertToSnapshot(self.apiClient, snap1.id) + self._start_vm(vm) + self.assertEqual("incr-revert-v1", self._read_marker(vm), + "Revert to the full snapshot did not restore its content") + + @attr(tags=['basic'], required_hardware=True) + def test_03_delta_max_rotates_chain(self): + """After snapshot.delta.max chain members the chain is ended and a new full is taken.""" + original = Configurations.list(self.apiClient, name="snapshot.delta.max")[0].value + Configurations.update(self.apiClient, name="snapshot.delta.max", value="2") + try: + vm = self._deploy_vm("TestIncrVM-rotate") + + snap1 = self._snapshot_root_volume(vm) # chain member 1: full + snap2 = self._snapshot_root_volume(vm) # chain member 2: delta, reaches the cap + snap3 = self._snapshot_root_volume(vm) # must start a new chain: full + + self._assert_full(snap1) + self._assert_delta(snap2, parent=snap1) + self._assert_full(snap3, full=snap1) + + ref2 = self._image_store_ref(snap2) + if ref2 is not None: + self.assertTrue(ref2["end_of_chain"], + "Snapshot that reached snapshot.delta.max was not marked end of chain") + finally: + Configurations.update(self.apiClient, name="snapshot.delta.max", value=original) + + @attr(tags=['basic'], required_hardware=True) + def test_04_template_from_delta_snapshot(self): + """A template created from a mid-chain delta must contain the full (flattened) content.""" + vm = self._deploy_vm("TestIncrVM-tmpl") + + self._write_marker(vm, "incr-tmpl-v1") + snap1 = self._snapshot_root_volume(vm) + + self._write_marker(vm, "incr-tmpl-v2") + snap2 = self._snapshot_root_volume(vm) + self._assert_delta(snap2, parent=snap1) + + template = Template.create_from_snapshot( + self.apiClient, snap2, + { + "name": "incr-tmpl", + "displaytext": "template from delta snapshot", + "ostypeid": self.template.ostypeid, + "ispublic": False, + }) + self.cleanup.insert(0, template) + + # a VM deployed from that template must carry the delta's content: the template was + # created from a delta qcow2 and only works if the chain got flattened along the way + vm_from_template = VirtualMachine.create( + self.apiClient, + {"name": "TestIncrVM-fromtmpl", "displayname": "TestIncrVM-fromtmpl"}, + accountid=self.account.name, + zoneid=self.zone.id, + serviceofferingid=self.compute_offering.id, + templateid=template.id, + domainid=self.domain.id, + startvm=True, + mode='basic', + ) + self.cleanup.insert(0, vm_from_template) + # the template holds a crash-consistent image (snapshot of a running VM), so the first + # boot replays the filesystem journal / fscks and can take considerably longer + self._start_vm(vm_from_template, ssh_timeout=600) + self.assertEqual("incr-tmpl-v2", self._read_marker(vm_from_template), + "VM from the delta-snapshot template does not contain the snapshot's content") + + @attr(tags=['basic'], required_hardware=True) + def test_05_full_backup_when_parent_file_missing(self): + """If the parent file vanished from secondary storage, the next snapshot degrades to a full.""" + if not self.hostConfig: + self.skipTest("No host SSH credentials available (set HOST_SSH_USER/HOST_SSH_PASSWORD or " + "provide them in the marvin config) - cannot manipulate secondary storage") + + vm = self._deploy_vm("TestIncrVM-fallback") + + snap1 = self._snapshot_root_volume(vm, add_to_cleanup=False) + self._assert_full(snap1) + + # remove the would-be parent behind CloudStack's back + if not self._delete_backed_up_snapshot_file(snap1): + self.skipTest("Could not remove the backed-up snapshot from secondary storage") + + # the driver still offers snap1 as parent (its ref is Ready), but the agent must + # notice the missing file, fall back to a full copy and report it as such, upon + # which the management server drops the parent link again + snap2 = self._snapshot_root_volume(vm) + self._assert_full(snap2, full=snap1) + + # snap1's backing file is gone; delete it via the API and tolerate the missing file + try: + Snapshot.delete(snap1, self.apiClient) + except Exception as e: + logging.debug("Deleting the sabotaged snapshot failed (tolerated): %s" % e) + + @attr(tags=['basic'], required_hardware=True) + def test_06_delete_mid_chain_member(self): + """Deleting a mid-chain delta keeps its children restorable (leaf-first physical deletion).""" + vm = self._deploy_vm("TestIncrVM-del") + + self._write_marker(vm, "incr-del-v1") + snap1 = self._snapshot_root_volume(vm) + + self._write_marker(vm, "incr-del-v2") + snap2 = self._snapshot_root_volume(vm, add_to_cleanup=False) + self._assert_delta(snap2, parent=snap1) + + self._write_marker(vm, "incr-del-v3") + snap3 = self._snapshot_root_volume(vm) + self._assert_delta(snap3, parent=snap2, full=snap1) + + # snap3's delta is backed by snap2's file: deleting snap2 must not break snap3 + Snapshot.delete(snap2, self.apiClient) + + vm.stop(self.apiClient) + + Volume.revertToSnapshot(self.apiClient, snap3.id) + self._start_vm(vm) + self.assertEqual("incr-del-v3", self._read_marker(vm), + "Revert to a delta broke after its parent snapshot was deleted") + + @attr(tags=['basic'], required_hardware=True) + def test_07_encrypted_volumes_stay_full(self): + """Snapshots of encrypted volumes are never incremental (a delta would need the LUKS secret).""" + if not self._encryption_capable_host_exists(): + self.skipTest("No KVM host with volume-encryption support found") + + offering_data = dict(self.testdata[TestData.computeOffering]) + offering_data["name"] = offering_data["displaytext"] = "Linstor_Compute_Incr_Enc" + offering = ServiceOffering.create(self.apiClient, offering_data, encryptroot=True) + self.cleanup.append(offering) + + vm = VirtualMachine.create( + self.apiClient, + {"name": "TestIncrVM-enc", "displayname": "TestIncrVM-enc"}, + accountid=self.account.name, + zoneid=self.zone.id, + serviceofferingid=offering.id, + templateid=self.template.id, + domainid=self.domain.id, + startvm=False, + mode='basic', + ) + self.cleanup.insert(0, vm) + # the root volume is only provisioned on first start; a never-started + # VM's volume stays Allocated and cannot be snapshotted + self._start_vm(vm) + + snap1 = self._snapshot_root_volume(vm) + snap2 = self._snapshot_root_volume(vm) + + self._assert_full(snap1) + self._assert_full(snap2, full=snap1) + + # --------------------------------------------------------------------- # + # Helpers + # --------------------------------------------------------------------- # + + @classmethod + def _set_config(cls, name, value): + """Set a global configuration, remembering the original value for tearDownClass.""" + if name not in cls.original_config: + cls.original_config[name] = Configurations.list(cls.apiClient, name=name)[0].value + Configurations.update(cls.apiClient, name=name, value=value) + + @classmethod + def _restore_configs(cls): + for name, value in cls.original_config.items(): + try: + Configurations.update(cls.apiClient, name=name, value=value) + except Exception as e: + logging.debug("Could not restore configuration %s=%s: %s" % (name, value, e)) + + def _deploy_vm(self, name): + vm = VirtualMachine.create( + self.apiClient, + {"name": name, "displayname": name}, + accountid=self.account.name, + zoneid=self.zone.id, + serviceofferingid=self.compute_offering.id, + templateid=self.template.id, + domainid=self.domain.id, + startvm=False, + mode='basic', + ) + self.cleanup.insert(0, vm) + self._start_vm(vm) + return vm + + def _snapshot_root_volume(self, vm, add_to_cleanup=True): + root = list_volumes(self.apiClient, virtualmachineid=vm.id, type="ROOT", listall=True)[0] + snapshot = Snapshot.create( + self.apiClient, + volume_id=root.id, + account=self.account.name, + domainid=self.domain.id, + ) + self.assertIsNotNone(snapshot, "Could not create snapshot of root volume") + if add_to_cleanup: + self.cleanup.insert(0, snapshot) + return snapshot + + def _image_store_ref(self, snapshot): + """The snapshot's image-store reference bookkeeping, or None if the DB is not reachable. + + The parent link and end-of-chain flag are internal to the management server and not part of + any API response, so they can only be checked directly in the database. + """ + try: + rows = self.dbConnection.execute( + "SELECT ss.parent_snapshot_id, ss.end_of_chain, ss.physical_size, ss.install_path " + "FROM snapshot_store_ref ss JOIN snapshots s ON s.id = ss.snapshot_id " + "WHERE s.uuid = '%s' AND ss.store_role = 'Image' AND ss.state != 'Destroyed'" + % snapshot.id) + except Exception as e: + logging.debug("DB lookup of the image store ref failed: %s" % e) + return None + if not rows: + return None + return { + "parent_snapshot_id": rows[0][0], + "end_of_chain": bool(rows[0][1]), + "physical_size": rows[0][2], + "install_path": rows[0][3], + } + + def _db_snapshot_id(self, snapshot): + rows = self.dbConnection.execute("SELECT id FROM snapshots WHERE uuid = '%s'" % snapshot.id) + return rows[0][0] if rows else None + + def _assert_full(self, snapshot, full=None): + """Assert the snapshot was backed up as a standalone full copy. + + Without DB access the parent link cannot be checked; if a reference full backup of the + same volume is given, fall back to comparing physical sizes (a full is in the order of + the volume's used space, a delta of a small change is orders of magnitude smaller). + """ + ref = self._image_store_ref(snapshot) + if ref is not None: + self.assertEqual( + 0, ref["parent_snapshot_id"], + "Snapshot %s should be a full backup but has parent %s" % (snapshot.name, ref["parent_snapshot_id"])) + elif full is not None: + self.assertGreater( + int(snapshot.physicalsize) * 2, int(full.physicalsize), + "Snapshot %s (%s bytes) does not look like a full backup (reference full %s is %s bytes)" + % (snapshot.name, snapshot.physicalsize, full.name, full.physicalsize)) + else: + logging.debug("No DB access - cannot verify that snapshot %s is a full backup" % snapshot.name) + + def _assert_delta(self, snapshot, parent, full=None): + """Assert the snapshot was backed up as a delta of the given parent. + + Without DB access the parent link cannot be checked; the fallback compares the size + against the chain's full backup (never against the parent - that may itself be a + similarly sized delta). + """ + ref = self._image_store_ref(snapshot) + if ref is not None: + self.assertEqual( + self._db_snapshot_id(parent), ref["parent_snapshot_id"], + "Snapshot %s should be an incremental backup of %s" % (snapshot.name, parent.name)) + else: + if full is None: + full = parent + self.assertLess( + int(snapshot.physicalsize) * 4, int(full.physicalsize), + "Snapshot %s (%s bytes) does not look like a delta (full backup %s is %s bytes)" + % (snapshot.name, snapshot.physicalsize, full.name, full.physicalsize)) + + def _vm_ssh(self, vm): + # The VM is deployed stopped, so its instance has no ssh_ip yet; the IP may also change across + # stop/start cycles. Always pass the current address from a fresh lookup. + ipaddress = self._get_vm(vm.id).ipaddress + return vm.get_ssh_client(ipaddress=ipaddress, reconnect=True, retries=5) + + def _write_marker(self, vm, content): + ssh = self._vm_ssh(vm) + ssh.execute("echo '%s' > %s" % (content, MARKER_PATH)) + ssh.execute("sync") + + def _read_marker(self, vm): + ssh = self._vm_ssh(vm) + result = ssh.execute("cat %s" % MARKER_PATH) + return result[0].strip() if result else None + + @classmethod + def _encryption_capable_host_exists(cls): + hosts = list_hosts(cls.apiClient, zoneid=cls.zone.id, type='Routing', hypervisor='KVM', state='Up') + return any(getattr(h, "encryptionsupported", False) for h in (hosts or [])) + + @classmethod + def _get_vm(cls, vm_id): + return list_virtual_machines(cls.apiClient, id=vm_id)[0] + + @classmethod + def _start_vm(cls, vm, ssh_timeout=120): + vm_for_check = cls._get_vm(vm.id) + if vm_for_check.state == VirtualMachine.STOPPED: + vm.start(cls.apiClient) + vm_for_check = cls._get_vm(vm.id) + ServiceReady.wait_ssh_ready(vm_for_check.ipaddress, timeout=ssh_timeout) + return vm_for_check + + def _host_ssh(self): + host = list_hosts(self.apiClient, type='Routing', hypervisor='KVM', state='Up')[0] + return SshClient( + host=host.ipaddress, port=22, + user=self.hostConfig['username'], passwd=self.hostConfig['password']) + + def _on_mounted_secondary(self, snapshot, action): + """Self-mount the secondary NFS export on a host and run action(ssh, snapshot_path) on it.""" + ref = self._image_store_ref(snapshot) + if ref is None or not ref["install_path"]: + return None + try: + store = self.dbConnection.execute( + "SELECT url FROM image_store WHERE role = 'Image' AND removed IS NULL LIMIT 1") + except Exception as e: + logging.debug("DB lookup of the image store url failed: %s" % e) + return None + if not store or not store[0][0] or not store[0][0].startswith("nfs://"): + return None + server, export = store[0][0][len("nfs://"):].split("/", 1) + + ssh = self._host_ssh() + mount_point = "/tmp/cs_sectest_%d" % random.randint(0, 100000) + try: + ssh.execute("mkdir -p %s" % mount_point) + ssh.execute("mount -t nfs %s:/%s %s" % (server, export, mount_point)) + return action(ssh, "%s/%s" % (mount_point, ref["install_path"])) + except Exception as e: + logging.debug("Action on mounted secondary storage failed: %s" % e) + return None + finally: + ssh.execute("umount %s 2>/dev/null; rmdir %s 2>/dev/null" % (mount_point, mount_point)) + + def _qemu_img_info_of_backed_up_snapshot(self, snapshot): + if not self.hostConfig: + return None + + def qemu_img_info(ssh, path): + out = ssh.execute("qemu-img info --output=json %s" % path) + return json.loads("".join(out)) if out else None + + return self._on_mounted_secondary(snapshot, qemu_img_info) + + def _delete_backed_up_snapshot_file(self, snapshot): + def delete_file(ssh, path): + ssh.execute("rm -f %s" % path) + return True + + return bool(self._on_mounted_secondary(snapshot, delete_file)) diff --git a/test/integration/plugins/linstor/test_linstor_volumes.py b/test/integration/plugins/linstor/test_linstor_volumes.py index c2c220bfe9e9..a6883afe0333 100644 --- a/test/integration/plugins/linstor/test_linstor_volumes.py +++ b/test/integration/plugins/linstor/test_linstor_volumes.py @@ -19,7 +19,6 @@ import os import random import time -import socket # All tests inherit from cloudstackTestCase from marvin.cloudstackTestCase import cloudstackTestCase @@ -38,6 +37,8 @@ from marvin.codes import PASS from nose.plugins.attrib import attr +from linstor_test_utils import ServiceReady + # Prerequisites: # Only one zone # Only one pod @@ -226,45 +227,6 @@ def __init__(self, linstor_controller_url): }, } -class ServiceReady: - @classmethod - def ready(cls, hostname: str, port: int) -> bool: - try: - s = socket.create_connection((hostname, port), timeout=1) - s.close() - return True - except (ConnectionRefusedError, socket.timeout, OSError): - return False - - @classmethod - def wait( - cls, - hostname, - port, - wait_interval = 5, - timeout = 90, - service_name = 'ssh') -> bool: - """ - Wait until the controller can be reached. - :param hostname: - :param port: port of the application - :param wait_interval: - :param timeout: time to wait until exit with False - :param service_name: name of the service to wait - :return: - """ - starttime = int(round(time.time() * 1000)) - while not cls.ready(hostname, port): - if starttime + timeout * 1000 < int(round(time.time() * 1000)): - raise RuntimeError("{s} {h} cannot be reached.".format(s=service_name, h=hostname)) - time.sleep(wait_interval) - return True - - @classmethod - def wait_ssh_ready(cls, hostname, wait_interval = 1, timeout = 90): - return cls.wait(hostname, 22, wait_interval, timeout, "ssh") - - class TestLinstorVolumes(cloudstackTestCase): _volume_vm_id_and_vm_id_do_not_match_err_msg = "The volume's VM ID and the VM's ID do not match." _vm_not_in_running_state_err_msg = "The VM is not in the 'Running' state."