diff --git a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java index 1cb9232eec14..e93db2b26e5a 100644 --- a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java +++ b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java @@ -636,6 +636,42 @@ public class AgentProperties{ */ public static final Property KVM_SCRIPTS_DIR = new Property<>("kvm.scripts.dir", "scripts/vm/hypervisor/kvm"); + /** + * Host-local parent directory for the NAS backup pull-mode scratch area: the NBD socket and + * the libvirt-managed fleecing (copy-before-write) images used while backing up a running VM + * on raw block-device storage such as LINSTOR/DRBD. + *

+ * Must be on a real on-disk filesystem with room for the guest's write churn during a backup, + * and must NOT be on the NAS share: copy-before-write runs inline with guest writes, so a slow + * or remote location stalls the VM. Keep the path short — the resulting socket path is subject + * to the kernel's 108-byte UNIX socket limit. + *

+ * Data type: String.
+ * Default value: /var/tmp + * + * @since 4.22.0 + */ + public static final Property NAS_BACKUP_PULL_SCRATCH_DIR = new Property<>("nas.backup.pull.scratch.dir", "/var/tmp"); + + /** + * Free space the NAS backup pull-mode scratch area ({@link #NAS_BACKUP_PULL_SCRATCH_DIR}) must + * have before a backup starts, as a percentage of the VM's total disk size. Otherwise the + * backup fails up front. + *

+ * Copy-before-write saves every guest block that is overwritten during the backup into the + * scratch area, at most once per block, so the total disk size is the worst case. If the scratch + * area fills up anyway, the guest's writes fail and the VM is paused. 100 rules that out; the + * default assumes the guest overwrites at most a quarter of its disks while a backup runs. + * 0 disables the check. + *

+ * Data type: Integer.
+ * Default value: 25 + * + * @since 4.22.0 + */ + public static final Property NAS_BACKUP_PULL_SCRATCH_MIN_FREE_PERCENT = + new Property<>("nas.backup.pull.scratch.min.free.percent", 25); + /** * Specifies start MAC address for private IP range.
* Data type: String.
diff --git a/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java b/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java index 7c2d3fc76fc1..3928b63dc772 100644 --- a/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java +++ b/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java @@ -39,6 +39,7 @@ public class TakeBackupCommand extends Command { // Incremental backup fields (NAS provider; null/empty for legacy full-only callers). private String mode; // "full" or "incremental"; null => legacy behaviour (script default) + private String method; // "qcow2" (bitmaps, push mode) or "content" (pull mode); null => "qcow2" private String bitmapNew; // Checkpoint/bitmap name to create with this backup (timestamp-based) private String bitmapParent; // Incremental: parent bitmap to read changes since @@ -134,6 +135,14 @@ public void setMode(String mode) { this.mode = mode; } + public String getMethod() { + return method; + } + + public void setMethod(String method) { + this.method = method; + } + public String getBitmapNew() { return bitmapNew; } diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java index 511f0ccb7114..287793114492 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java @@ -48,6 +48,24 @@ public final class NASBackupChainKeys { // "legacy-full" mode token (which sets make_checkpoint=0). public static final String TYPE_LEGACY_FULL = "legacy-full"; + /** + * In-memory backup-method sentinels used by {@code ChainDecision.method}: how a backup is + * taken, independent of whether it is a full or an incremental ({@code ChainDecision.mode}). + * Both methods write the same chain of qcow2 files, so restore and delete do not care. + * + *

{@link #METHOD_QCOW2}: libvirt push-mode backup; incrementals come from QEMU persistent + * dirty bitmaps (libvirt checkpoints), which only qcow2 volumes can carry.

+ */ + public static final String METHOD_QCOW2 = "qcow2"; + + /** + * Content-based libvirt pull-mode backup, used for raw block-device storage such as + * LINSTOR/DRBD which cannot carry persistent dirty bitmaps. A full is written as a sparse + * qcow2 in a single pass; an incremental is a qcow2 overlay on the disk's NBD export, + * safe-rebased onto the parent so it holds only the clusters that differ. + */ + public static final String METHOD_CONTENT = "content"; + /** * VM-scoped detail (stored in {@code vm_instance_details}) holding the QEMU dirty-bitmap * name that currently exists on the running VM and is therefore the only valid parent diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java index a72920fbae88..a068ee4a6049 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java @@ -224,11 +224,12 @@ protected Host getVMHypervisorHost(VirtualMachine vm) { /** * Returned by {@link #decideChain(VirtualMachine)} to describe the next backup's place in - * the chain: full vs incremental, the bitmap name to create, and (for incrementals) the - * parent bitmap and parent file path. + * the chain: full vs incremental, how it is taken, the bitmap name to create, and (for + * incrementals) the parent bitmap and parent file path. */ static final class ChainDecision { final String mode; // "full" or "incremental" + final String method; // NASBackupChainKeys.METHOD_QCOW2 or METHOD_CONTENT final String bitmapNew; final String bitmapParent; // null for full // Per-volume parent backup file paths, one per current VM volume in deviceId order. @@ -237,15 +238,26 @@ static final class ChainDecision { final List parentPaths; final String chainId; // chain identifier this backup belongs to final int chainPosition; // 0 for full, N for the Nth incremental in the chain + // Content-based chains have no bitmap to look the parent up by, so the parent backup's + // uuid is resolved when the decision is made. null for bitmap-based decisions. + final String parentBackupUuid; private ChainDecision(String mode, String bitmapNew, String bitmapParent, List parentPaths, String chainId, int chainPosition) { + this(mode, NASBackupChainKeys.METHOD_QCOW2, bitmapNew, bitmapParent, parentPaths, chainId, + chainPosition, null); + } + + private ChainDecision(String mode, String method, String bitmapNew, String bitmapParent, + List parentPaths, String chainId, int chainPosition, String parentBackupUuid) { this.mode = mode; + this.method = method; this.bitmapNew = bitmapNew; this.bitmapParent = bitmapParent; this.parentPaths = parentPaths; this.chainId = chainId; this.chainPosition = chainPosition; + this.parentBackupUuid = parentBackupUuid; } static ChainDecision fullStart(String bitmapName) { @@ -268,10 +280,30 @@ static ChainDecision incremental(String bitmapNew, String bitmapParent, List parentPaths, String chainId, + int chainPosition, String parentBackupUuid) { + return new ChainDecision(NASBackupChainKeys.TYPE_INCREMENTAL, NASBackupChainKeys.METHOD_CONTENT, null, + null, parentPaths, chainId, chainPosition, parentBackupUuid); + } + boolean isIncremental() { return NASBackupChainKeys.TYPE_INCREMENTAL.equals(mode); } + /** True for the pull-mode content-diff path (no dirty bitmaps involved). */ + boolean isContentBased() { + return NASBackupChainKeys.METHOD_CONTENT.equals(method); + } + boolean isLegacyFull() { return NASBackupChainKeys.TYPE_LEGACY_FULL.equals(mode); } @@ -293,15 +325,24 @@ protected ChainDecision decideChain(VirtualMachine vm) { // Master switch — when the operator disables incrementals at the zone level the backup // behaves exactly like the pre-incremental full-only path: no bitmap is generated and no // chain/checkpoint metadata is created, sent to the agent, or persisted (legacy-full). + // This holds for every storage, so enabling incrementals is also the opt-in to the + // content-based path below and its host-local scratch space. Boolean incrementalEnabled = NASBackupIncrementalEnabled.valueIn(vm.getDataCenterId()); if (incrementalEnabled == null || !incrementalEnabled) { return ChainDecision.legacyFull(); } + // Raw block-device storage (LINSTOR/DRBD) cannot carry QEMU persistent dirty bitmaps, so + // libvirt checkpoints — and with them the push-mode incremental path — are unavailable. + // Those VMs take the content-based pull-mode path instead. + if (allVolumesOnContentDiffCapableStorage(vm)) { + return decideContentChain(vm); + } + // Incremental backups rely on QEMU dirty bitmaps / libvirt checkpoints, which only exist - // on file-based qcow2 storage. Storage such as Ceph-RBD and Linstor cannot carry per-disk - // checkpoints, so a VM with any volume on such a pool must stay on the full-only (legacy) - // path — otherwise an incremental attempt would fail or regress those storages. + // on file-based qcow2 storage. Storage such as Ceph-RBD cannot carry per-disk checkpoints + // and has no content-diff path either, so such a VM stays on the full-only (legacy) path — + // otherwise an incremental attempt would fail or regress those storages. if (!allVolumesOnCheckpointCapableStorage(vm)) { return ChainDecision.legacyFull(); } @@ -365,6 +406,74 @@ protected ChainDecision decideChain(VirtualMachine vm) { parentChainId, parentChainPosition + 1); } + /** + * Chain decision for content-based (pull mode) storage such as LINSTOR/DRBD. + * + *

Unlike the bitmap path there is no host-side state to anchor on. The delta is derived by + * comparing the disk's point-in-time NBD export against the parent backup, so the chain cannot + * be invalidated by a VM restart, live migration or restore — the comparison is stateless. The + * parent is therefore simply the most recent BackedUp backup of the same chain, and the agent + * independently verifies the parent files still exist on the NAS, degrading to a full if they + * do not (INCREMENTAL_FALLBACK).

+ */ + protected ChainDecision decideContentChain(VirtualMachine vm) { + // Stopped VMs are backed up straight from the disk with qemu-img convert; that path has no + // point-in-time export to diff against a parent, so it is always a full. + if (VirtualMachine.State.Stopped.equals(vm.getState())) { + return ChainDecision.contentFull(); + } + + Integer fullEvery = NASBackupFullEvery.valueIn(vm.getDataCenterId()); + if (fullEvery == null || fullEvery <= 1) { + return ChainDecision.contentFull(); + } + + Backup parent = findLatestBackedUpBackup(vm.getId()); + if (parent == null) { + return ChainDecision.contentFull(); + } + + String parentChainId = readDetail(parent, NASBackupChainKeys.CHAIN_ID); + int parentChainPosition = chainPosition(parent); + if (parentChainId == null || parentChainPosition == Integer.MAX_VALUE) { + return ChainDecision.contentFull(); + } + + // Force a fresh full when the chain has reached the configured length. + if (parentChainPosition + 1 >= fullEvery) { + return ChainDecision.contentFull(); + } + + List parentPaths = composeParentBackupPaths(parent, vm.getId()); + if (parentPaths == null) { + LOG.debug("VM {} parent backup {} volume layout no longer matches current VM — forcing full", + vm.getInstanceName(), parent.getUuid()); + return ChainDecision.contentFull(); + } + return ChainDecision.contentIncremental(parentPaths, parentChainId, parentChainPosition + 1, + parent.getUuid()); + } + + /** + * True when EVERY volume of the VM sits on storage whose backups must be derived by content + * comparison instead of dirty bitmaps — currently LINSTOR, whose volumes are raw DRBD block + * devices and so cannot persist a bitmap (persistence is a qcow2-only feature). A VM with a + * mix of such volumes and others is not eligible, since one backup run uses a single mode. + */ + protected boolean allVolumesOnContentDiffCapableStorage(VirtualMachine vm) { + List volumes = volumeDao.findByInstance(vm.getId()); + if (CollectionUtils.isEmpty(volumes)) { + return false; + } + for (VolumeVO volume : volumes) { + StoragePoolVO pool = primaryDataStoreDao.findById(volume.getPoolId()); + if (pool == null || !Storage.StoragePoolType.Linstor.equals(pool.getPoolType())) { + return false; + } + } + return true; + } + /** * Incremental backups require QEMU dirty bitmaps / libvirt checkpoints, which are only * possible on file-based qcow2 storage. Returns {@code true} only when EVERY volume of the @@ -502,7 +611,10 @@ private void persistChainMetadata(Backup backup, ChainDecision decision, String // source of truth. Not duplicated into backup_details. if (decision.isIncremental()) { // Resolve the parent backup's UUID so restore can walk the chain by id, not by path. - String parentUuid = lookupParentBackupUuid(backup.getVmId(), decision.bitmapParent); + // Content-based decisions carry the parent uuid directly (no bitmap to look it up by). + String parentUuid = decision.parentBackupUuid != null + ? decision.parentBackupUuid + : lookupParentBackupUuid(backup.getVmId(), decision.bitmapParent); if (parentUuid != null) { backupDetailsDao.persist(new BackupDetailVO(backup.getId(), NASBackupChainKeys.PARENT_BACKUP_ID, parentUuid, true)); } @@ -593,6 +705,7 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce command.setQuiesce(quiesceVM); command.setQuiesceTimeout(NASBackupQuiesceAgentTimeout.valueIn(vm.getDataCenterId())); command.setMode(decision.mode); + command.setMethod(decision.method); command.setBitmapNew(decision.bitmapNew); command.setBitmapParent(decision.bitmapParent); command.setParentPaths(decision.parentPaths); @@ -628,7 +741,9 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce // backup as a full and start a new chain. ChainDecision effective = decision; if (answer.getIncrementalFallback()) { - effective = ChainDecision.fullStart(decision.bitmapNew); + effective = decision.isContentBased() + ? ChainDecision.contentFull() + : ChainDecision.fullStart(decision.bitmapNew); backupVO.setType("FULL"); } List volumes = new ArrayList<>(volumeDao.findByInstance(vm.getId())); @@ -642,11 +757,15 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce // created — the only valid parent for the next incremental (see decideChain). // If the agent reports no bitmap (bitmapCreated=null), clear any stale detail // so the next backup starts a fresh full. - String confirmedBitmap = answer.getBitmapCreated(); - if (confirmedBitmap != null) { - upsertVmActiveCheckpoint(vm.getId(), confirmedBitmap); - } else { - clearVmActiveCheckpoint(vm.getId()); + // The content-based path creates no bitmaps, so there is no active + // checkpoint to track — its chain is anchored purely on backup history. + if (!decision.isContentBased()) { + String confirmedBitmap = answer.getBitmapCreated(); + if (confirmedBitmap != null) { + upsertVmActiveCheckpoint(vm.getId(), confirmedBitmap); + } else { + clearVmActiveCheckpoint(vm.getId()); + } } } return new Pair<>(true, backupVO); diff --git a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java index 787d3ff94567..cbed09856e39 100644 --- a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java +++ b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java @@ -49,6 +49,7 @@ import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; +import java.util.Date; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; @@ -504,6 +505,7 @@ public void decideChainReturnsFullWhenVmHasNoActiveCheckpoint() { NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); Assert.assertNotNull(decision); Assert.assertEquals(NASBackupChainKeys.TYPE_FULL, decision.mode); + Assert.assertEquals(NASBackupChainKeys.METHOD_QCOW2, decision.method); Assert.assertNull(decision.bitmapParent); Assert.assertEquals(0, decision.chainPosition); } @@ -1099,4 +1101,196 @@ public void sweepContinuesPastFailedTombstoneDelete() Mockito.verify(backupDao, Mockito.never()).remove(51L); Mockito.verify(backupDao).remove(50L); } + + // -- content-based (pull mode) chain decisions for raw block storage ----------------- + + /** + * With the incremental master switch off, a LINSTOR-backed VM keeps the legacy push-mode full + * like every other VM: enabling incrementals is the opt-in to the content-based path and its + * host-local scratch space. The storage is not even looked at. + */ + @Test + public void decideChainReturnsLegacyFullForLinstorWhenIncrementalDisabled() { + Long vmId = 70L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.lenient().when(vm.getId()).thenReturn(vmId); + Mockito.lenient().when(vm.getDataCenterId()).thenReturn(1L); + + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupIncrementalEnabled", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Boolean.class, + "nas.backup.incremental.enabled", "false", + "test override — disabled", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertTrue(decision.isLegacyFull()); + Assert.assertEquals(NASBackupChainKeys.METHOD_QCOW2, decision.method); + Assert.assertFalse(decision.isContentBased()); + Assert.assertNull("legacy full anchors no chain", decision.chainId); + Mockito.verify(volumeDao, Mockito.never()).findByInstance(vmId); + } + + /** + * Stopped VMs are imaged straight from the disk by qemu-img, with no point-in-time export to + * diff against a parent, so a LINSTOR VM that is stopped must take a content full. + */ + @Test + public void decideChainReturnsContentFullForStoppedLinstorVm() { + Long vmId = 71L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Stopped); + stubAllVolumesOnLinstor(vmId, 1); + enableIncrementals(); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertEquals(NASBackupChainKeys.TYPE_FULL, decision.mode); + Assert.assertEquals(NASBackupChainKeys.METHOD_CONTENT, decision.method); + } + + /** + * Running LINSTOR VM with a healthy parent inside the cadence => content incremental, carrying + * the parent's chain id, the next position, the parent uuid (resolved directly since there is + * no bitmap to look it up by) and the per-volume parent paths the script rebases onto. + */ + @Test + public void decideChainReturnsContentIncrementalForLinstorWithValidParent() { + Long vmId = 72L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Running); + stubAllVolumesOnLinstor(vmId, 1); + enableIncrementals(); + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupFullEvery", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Integer.class, + "nas.backup.full.every", "3", "test override", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + + Backup parent = mock(Backup.class); + Mockito.when(parent.getId()).thenReturn(900L); + Mockito.when(parent.getStatus()).thenReturn(Backup.Status.BackedUp); + Mockito.lenient().when(parent.getDate()).thenReturn(new Date()); + Mockito.when(parent.getUuid()).thenReturn("parent-uuid"); + Mockito.when(parent.getExternalId()).thenReturn("i-2-3-VM/2026.01.01.00.00.00"); + Backup.VolumeInfo pv = mock(Backup.VolumeInfo.class); + Mockito.when(pv.getPath()).thenReturn("volpath1"); + Mockito.>when(parent.getBackedUpVolumes()).thenReturn(List.of(pv)); + Mockito.when(backupDao.listByVmId(null, vmId)).thenReturn(List.of(parent)); + + BackupDetailVO chainId = mock(BackupDetailVO.class); + Mockito.when(chainId.getValue()).thenReturn("chain-1"); + BackupDetailVO chainPos = mock(BackupDetailVO.class); + Mockito.when(chainPos.getValue()).thenReturn("0"); + Mockito.when(backupDetailsDao.findDetail(900L, NASBackupChainKeys.CHAIN_ID)).thenReturn(chainId); + Mockito.when(backupDetailsDao.findDetail(900L, NASBackupChainKeys.CHAIN_POSITION)).thenReturn(chainPos); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertEquals(NASBackupChainKeys.TYPE_INCREMENTAL, decision.mode); + Assert.assertEquals(NASBackupChainKeys.METHOD_CONTENT, decision.method); + Assert.assertTrue(decision.isIncremental()); + Assert.assertTrue(decision.isContentBased()); + Assert.assertNull("a content incremental carries no bitmap", decision.bitmapNew); + Assert.assertNull(decision.bitmapParent); + Assert.assertEquals("chain-1", decision.chainId); + Assert.assertEquals(1, decision.chainPosition); + Assert.assertEquals("parent-uuid", decision.parentBackupUuid); + Assert.assertEquals(List.of("i-2-3-VM/2026.01.01.00.00.00/root.volpath1.qcow2"), + decision.parentPaths); + } + + /** + * Once the chain reaches nas.backup.full.every, the next LINSTOR backup must anchor a new + * chain with a content full rather than extending the existing one. + */ + @Test + public void decideChainReturnsContentFullForLinstorAtChainEnd() { + Long vmId = 73L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Running); + stubAllVolumesOnLinstor(vmId, 1); + enableIncrementals(); + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupFullEvery", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Integer.class, + "nas.backup.full.every", "2", "test override", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + + Backup parent = mock(Backup.class); + Mockito.when(parent.getId()).thenReturn(901L); + Mockito.when(parent.getStatus()).thenReturn(Backup.Status.BackedUp); + Mockito.lenient().when(parent.getDate()).thenReturn(new Date()); + Mockito.when(backupDao.listByVmId(null, vmId)).thenReturn(List.of(parent)); + BackupDetailVO chainId = mock(BackupDetailVO.class); + Mockito.when(chainId.getValue()).thenReturn("chain-9"); + BackupDetailVO chainPos = mock(BackupDetailVO.class); + Mockito.when(chainPos.getValue()).thenReturn("1"); // 1 + 1 >= 2 => new full + Mockito.when(backupDetailsDao.findDetail(901L, NASBackupChainKeys.CHAIN_ID)).thenReturn(chainId); + Mockito.when(backupDetailsDao.findDetail(901L, NASBackupChainKeys.CHAIN_POSITION)).thenReturn(chainPos); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertEquals(NASBackupChainKeys.TYPE_FULL, decision.mode); + Assert.assertEquals(NASBackupChainKeys.METHOD_CONTENT, decision.method); + } + + /** + * A VM straddling LINSTOR and file-based storage is not content-diff capable: one backup run + * uses a single mode, so it must not be routed down the content path. + */ + @Test + public void allVolumesOnContentDiffCapableStorageFalseForMixedStorage() { + Long vmId = 74L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + + VolumeVO linVol = mock(VolumeVO.class); + Mockito.when(linVol.getPoolId()).thenReturn(1L); + VolumeVO nfsVol = mock(VolumeVO.class); + Mockito.when(nfsVol.getPoolId()).thenReturn(2L); + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(linVol, nfsVol)); + + StoragePoolVO lin = mock(StoragePoolVO.class); + Mockito.when(lin.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + StoragePoolVO nfs = mock(StoragePoolVO.class); + Mockito.when(nfs.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + Mockito.when(storagePoolDao.findById(1L)).thenReturn(lin); + Mockito.when(storagePoolDao.findById(2L)).thenReturn(nfs); + + Assert.assertFalse(nasBackupProvider.allVolumesOnContentDiffCapableStorage(vm)); + } + + /** A VM with no volumes is not content-diff capable (safe default). */ + @Test + public void allVolumesOnContentDiffCapableStorageFalseForNoVolumes() { + Long vmId = 75L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of()); + Assert.assertFalse(nasBackupProvider.allVolumesOnContentDiffCapableStorage(vm)); + } + + private void enableIncrementals() { + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupIncrementalEnabled", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Boolean.class, + "nas.backup.incremental.enabled", "true", + "test override — enabled", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + } + + /** Point every volume of {@code vmId} at a LINSTOR pool. */ + private void stubAllVolumesOnLinstor(Long vmId, int count) { + List vols = new java.util.ArrayList<>(); + StoragePoolVO lin = mock(StoragePoolVO.class); + Mockito.lenient().when(lin.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + for (int i = 0; i < count; i++) { + VolumeVO v = mock(VolumeVO.class); + long poolId = 100L + i; + Mockito.when(v.getPoolId()).thenReturn(poolId); + Mockito.when(storagePoolDao.findById(poolId)).thenReturn(lin); + vols.add(v); + } + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(vols); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java index 92de04b1b95a..f8d959e6587e 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java @@ -327,15 +327,22 @@ private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMg KVMStoragePool volumeStoragePool = storagePoolMgr.getStoragePool(volumePool.getPoolType(), volumePool.getUuid()); QemuImg qemu; try { - qemu = new QemuImg(timeout, true, false); String volumeUuid = getVolumeUuidFromPath(volumePath, volumePool); KVMPhysicalDisk disk = null; + boolean targetExists = true; + // Let qemu-img skip the backup's zero regions (--target-is-zero) only on a volume that + // provably reads as zeroes. An in-place restore overwrites a volume that still holds its + // old data, and a new LINSTOR volume reads as zeroes only on thin or ZFS backed pools. + boolean targetReadsAsZero = false; if (createTargetVolume) { if (Storage.StoragePoolType.Linstor.equals(volumePool.getPoolType())) { if (size == null) { throw new CloudRuntimeException("Restore volume size is required for Linstor pool when creating target volume"); } disk = volumeStoragePool.createPhysicalDisk(volumeUuid, QemuImg.PhysicalDiskFormat.RAW, Storage.ProvisioningType.THIN, size, null); + targetReadsAsZero = volumeStoragePool.newVolumeReadsAsZero(volumeUuid); + } else { + targetExists = false; } } else { if (Storage.StoragePoolType.Linstor.equals(volumePool.getPoolType())) { @@ -343,8 +350,9 @@ private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMg } else { disk = volumeStoragePool.getPhysicalDisk(volumePath); } - qemu.setSkipTargetVolumeCreation(true); } + qemu = new QemuImg(timeout, targetReadsAsZero, false); + qemu.setSkipTargetVolumeCreation(targetExists); if (disk != null) { logger.debug("Restoring volume: {}", disk.toString()); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java index b7e609587650..dc2ea651fc78 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java @@ -20,6 +20,8 @@ package com.cloud.hypervisor.kvm.resource.wrapper; import com.cloud.agent.api.Answer; +import com.cloud.agent.properties.AgentProperties; +import com.cloud.agent.properties.AgentPropertiesFileHandler; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; import com.cloud.hypervisor.kvm.storage.KVMStoragePool; @@ -53,6 +55,13 @@ public class LibvirtTakeBackupCommandWrapper extends CommandWrapper runBackupScript(LibvirtComputingResource libvirtCo argv.add("-M"); argv.add(mode); } + if (StringUtils.isNotBlank(command.getMethod())) { + argv.add("--method"); + argv.add(command.getMethod()); + } if (StringUtils.isNotBlank(bitmapNew)) { argv.add("--bitmap-new"); argv.add(bitmapNew); @@ -176,6 +207,20 @@ private Pair runBackupScript(LibvirtComputingResource libvirtCo argv.add("--parent-paths"); argv.add(String.join(",", parentPaths)); } + // Host-local scratch dir for the pull-mode NBD socket and fleecing images. Sent for every + // mode: harmless for the push-mode paths, which ignore it. + String scratchDir = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.NAS_BACKUP_PULL_SCRATCH_DIR); + if (StringUtils.isNotBlank(scratchDir)) { + argv.add("-S"); + argv.add(scratchDir); + } + if (METHOD_CONTENT.equals(command.getMethod())) { + Integer minFreePercent = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.NAS_BACKUP_PULL_SCRATCH_MIN_FREE_PERCENT); + if (minFreePercent != null) { + argv.add("--scratch-min-free-percent"); + argv.add(String.valueOf(Math.max(0, minFreePercent))); + } + } List commands = new ArrayList<>(); commands.add(argv.toArray(new String[0])); @@ -188,6 +233,13 @@ private Pair runBackupScript(LibvirtComputingResource libvirtCo */ private String validateBackupArgs(TakeBackupCommand command) { String mode = command.getMode(); + String method = StringUtils.defaultIfBlank(command.getMethod(), METHOD_QCOW2); + if (METHOD_CONTENT.equals(method)) { + return validateContentBackupArgs(command); + } + if (!METHOD_QCOW2.equals(method)) { + return "Unknown backup method: " + method; + } if (StringUtils.isBlank(mode)) { return null; // legacy full-only, no extra args expected } @@ -215,6 +267,21 @@ private String validateBackupArgs(TakeBackupCommand command) { return "Unknown backup mode: " + mode; } + private String validateContentBackupArgs(TakeBackupCommand command) { + String mode = command.getMode(); + if (MODE_FULL.equals(mode)) { + return null; // pull-mode full — no bitmap, no parent + } + if (MODE_INCREMENTAL.equals(mode)) { + // No bitmap is involved: the delta comes from comparing the disk against the parent. + if (CollectionUtils.isEmpty(command.getParentPaths())) { + return "content incremental mode requires parentPaths"; + } + return null; + } + return "Unsupported mode for the content backup method: " + mode; + } + /** * Sum the per-disk size lines emitted by nasbackup.sh. Single-volume mode emits one * line containing just the byte count; multi-volume mode emits one line per disk diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java index 3e35ed9476b1..d17e887176e5 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java @@ -125,4 +125,13 @@ default LibvirtVMDef.DiskDef.BlockIOSize getSupportedPhysicalBlockSize() { default void customizeLibvirtDiskDef(LibvirtVMDef.DiskDef disk) { } + + /** + * Whether the volume {@code volumeUuid}, freshly created in this pool, already reads as zeroes, + * so a copy onto it may skip zero regions (qemu-img --target-is-zero). Says nothing about a + * volume that has been written to: that still holds its old data. + */ + default boolean newVolumeReadsAsZero(String volumeUuid) { + return false; + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java index 3bdc23d27a15..5adf0eb04241 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java @@ -21,6 +21,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mockConstruction; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.when; @@ -34,10 +35,13 @@ import org.apache.cloudstack.backup.BackupAnswer; import org.apache.cloudstack.backup.RestoreBackupCommand; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.utils.qemu.QemuImg; +import org.apache.cloudstack.utils.qemu.QemuImgFile; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.MockedConstruction; import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; @@ -812,4 +816,67 @@ public void testUnmountIsBoundedByTheConfiguredTimeout() throws Exception { } } } + + /** + * Restores a backup onto a LINSTOR volume through replaceBlockDeviceWithBackup and returns + * the QemuImg instance used for the copy together with its constructor arguments. + */ + private Pair> restoreOntoLinstorVolume(boolean createTargetVolume, boolean newVolumeReadsAsZero) + throws Exception { + Method method = LibvirtRestoreBackupCommandWrapper.class.getDeclaredMethod("replaceBlockDeviceWithBackup", + KVMStoragePoolManager.class, PrimaryDataStoreTO.class, String.class, String.class, int.class, + boolean.class, Long.class); + method.setAccessible(true); + + PrimaryDataStoreTO volumePool = Mockito.mock(PrimaryDataStoreTO.class); + when(volumePool.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + when(volumePool.getUuid()).thenReturn("pool-uuid"); + KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class); + KVMStoragePool linstorPool = Mockito.mock(KVMStoragePool.class); + lenient().when(linstorPool.newVolumeReadsAsZero("volume-123")).thenReturn(newVolumeReadsAsZero); + when(storagePoolMgr.getStoragePool(Storage.StoragePoolType.Linstor, "pool-uuid")).thenReturn(linstorPool); + + final List[] ctorArgs = new List[1]; + try (MockedConstruction qemuMock = mockConstruction(QemuImg.class, + (mock, context) -> ctorArgs[0] = context.arguments())) { + boolean result = (boolean) method.invoke(wrapper, storagePoolMgr, volumePool, + "/dev/drbd/by-res/cs-volume-123/0", "/mnt/backup/root.volume-123.qcow2", 60, + createTargetVolume, 1024L); + + Assert.assertTrue(result); + Assert.assertEquals(1, qemuMock.constructed().size()); + QemuImg qemu = qemuMock.constructed().get(0); + Mockito.verify(qemu).convert(any(QemuImgFile.class), any(QemuImgFile.class)); + if (!createTargetVolume) { + Mockito.verify(linstorPool, Mockito.never()).newVolumeReadsAsZero(anyString()); + } + return new Pair<>(qemu, ctorArgs[0]); + } + } + + @Test + public void testInPlaceLinstorRestoreDoesNotSkipZeroes() throws Exception { + // Even on a thin pool: the volume still holds its old data, so the backup's zero regions + // must be written. + Pair> restore = restoreOntoLinstorVolume(false, true); + + Assert.assertEquals(Boolean.FALSE, restore.second().get(1)); + Mockito.verify(restore.first()).setSkipTargetVolumeCreation(true); + } + + @Test + public void testLinstorRestoreToNewThickVolumeDoesNotSkipZeroes() throws Exception { + Pair> restore = restoreOntoLinstorVolume(true, false); + + Assert.assertEquals(Boolean.FALSE, restore.second().get(1)); + Mockito.verify(restore.first()).setSkipTargetVolumeCreation(true); + } + + @Test + public void testLinstorRestoreToNewThinVolumeSkipsZeroes() throws Exception { + Pair> restore = restoreOntoLinstorVolume(true, true); + + Assert.assertEquals(Boolean.TRUE, restore.second().get(1)); + Mockito.verify(restore.first()).setSkipTargetVolumeCreation(true); + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java index 7ef64fd53b50..4e4437e84d9f 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java @@ -18,7 +18,9 @@ import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.when; import java.util.Arrays; @@ -99,4 +101,88 @@ public void executeOmitsQuiesceTimeoutWhenNotPositive() { Assert.assertFalse(runAndCaptureArgv().contains("--quiesce-timeout")); } + + private void runFailing(Pair result, boolean expectJobAbort) { + try (MockedStatic