This is an automated email from the ASF dual-hosted git repository.
weizhouapache pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 42322a59c7d feat(backup): incremental NAS backup support for KVM
(#13074)
42322a59c7d is described below
commit 42322a59c7d68247db03a538582e4c9ea1b259d5
Author: James Peru Mmbono <[email protected]>
AuthorDate: Wed Jul 8 15:06:41 2026 +0300
feat(backup): incremental NAS backup support for KVM (#13074)
Implements incremental backup support for the NAS backup provider on KVM,
using QEMU dirty bitmaps and libvirt's backup-begin API. RFC:
apache/cloudstack#12899.
For large VMs this reduces daily backup storage 80–95% and shortens backup
windows from hours to minutes (e.g. a 500 GB VM with moderate writes goes from
~500 GB/day to ~5–15 GB/day after the initial full backup).
Signed-off-by: James Peru <[email protected]>
Co-authored-by: jmsperu <[email protected]>
Co-authored-by: Abhisar Sinha <[email protected]>
---
.../java/org/apache/cloudstack/backup/Backup.java | 6 +-
.../apache/cloudstack/backup/BackupProvider.java | 12 +
.../org/apache/cloudstack/backup/BackupAnswer.java | 23 +
.../cloudstack/backup/TakeBackupCommand.java | 43 ++
.../org/apache/cloudstack/backup/BackupVO.java | 8 +-
.../cloudstack/backup/NASBackupChainKeys.java | 62 ++
.../cloudstack/backup/NASBackupProvider.java | 687 ++++++++++++++++++++-
.../cloudstack/backup/NASBackupProviderTest.java | 452 ++++++++++++++
.../LibvirtRestoreBackupCommandWrapper.java | 26 +
.../wrapper/LibvirtTakeBackupCommandWrapper.java | 152 ++++-
.../LibvirtRestoreBackupCommandWrapperTest.java | 2 +
.../java/com/cloud/hypervisor/guru/VMwareGuru.java | 2 +-
scripts/vm/hypervisor/kvm/nasbackup.sh | 252 +++++++-
.../main/java/com/cloud/hypervisor/KVMGuru.java | 2 +-
.../cloudstack/backup/BackupManagerImpl.java | 15 +-
test/integration/smoke/test_backup_recovery_nas.py | 320 ++++++++++
16 files changed, 2019 insertions(+), 45 deletions(-)
diff --git a/api/src/main/java/org/apache/cloudstack/backup/Backup.java
b/api/src/main/java/org/apache/cloudstack/backup/Backup.java
index 2d68f18b953..865d657a7a4 100644
--- a/api/src/main/java/org/apache/cloudstack/backup/Backup.java
+++ b/api/src/main/java/org/apache/cloudstack/backup/Backup.java
@@ -39,7 +39,11 @@ public interface Backup extends ControlledEntity,
InternalIdentity, Identity {
Long getHostId();
enum Status {
- Allocated, Queued, BackingUp, ReadyForImageTransfer,
FinalizingImageTransfer, BackedUp, Error, Failed, Restoring, Removed, Expunged
+ Allocated, Queued, BackingUp, ReadyForImageTransfer,
FinalizingImageTransfer, BackedUp, Error, Failed, Restoring, Removed, Expunged,
+ // Hidden: a chain backup kept as a tombstone after the user deleted
it while it still has
+ // live descendants (incremental chains). Excluded from listBackups
and from all backup
+ // operations (which require BackedUp); swept from the DB once its
last descendant is gone.
+ Hidden
}
class Metric {
diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java
b/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java
index 23b8092425d..4ae9148113a 100644
--- a/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java
+++ b/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java
@@ -87,6 +87,18 @@ public interface BackupProvider {
*/
boolean deleteBackup(Backup backup, boolean forced);
+ /**
+ * Whether {@link #deleteBackup(Backup, boolean)} owns DB-row removal and
resource-count /
+ * usage accounting for every backup it physically removes. Providers that
manage incremental
+ * chains (e.g. NAS) delete several backups per call — the leaf plus swept
delete-pending
+ * ancestors — and decrement once per removed backup themselves, so the
manager must NOT
+ * decrement or remove the row again. Defaults to {@code false}: the
manager does the
+ * single-backup accounting (the historical behaviour for non-chain
providers).
+ */
+ default boolean handlesChainDeleteResourceAccounting() {
+ return false;
+ }
+
Pair<Boolean, String> restoreBackupToVM(VirtualMachine vm, Backup backup,
String hostIp, String dataStoreUuid);
/**
diff --git a/core/src/main/java/org/apache/cloudstack/backup/BackupAnswer.java
b/core/src/main/java/org/apache/cloudstack/backup/BackupAnswer.java
index ffc67b628a7..abe78ee5553 100644
--- a/core/src/main/java/org/apache/cloudstack/backup/BackupAnswer.java
+++ b/core/src/main/java/org/apache/cloudstack/backup/BackupAnswer.java
@@ -29,6 +29,12 @@ public class BackupAnswer extends Answer {
private Long virtualSize;
private Map<String, String> volumes;
Boolean needsCleanup;
+ // Set by the NAS backup provider after a checkpoint/bitmap was created
during this backup.
+ // The provider persists it in backup_details under
NASBackupChainKeys.BITMAP_NAME.
+ private String bitmapCreated;
+ // Set when an incremental was requested but the agent had to fall back to
a full
+ // (e.g. VM was stopped). Provider should record this backup as type=full.
+ private Boolean incrementalFallback;
public BackupAnswer(final Command command, final boolean success, final
String details) {
super(command, success, details);
@@ -68,4 +74,21 @@ public class BackupAnswer extends Answer {
public void setNeedsCleanup(Boolean needsCleanup) {
this.needsCleanup = needsCleanup;
}
+
+ public String getBitmapCreated() {
+ return bitmapCreated;
+ }
+
+ public void setBitmapCreated(String bitmapCreated) {
+ this.bitmapCreated = bitmapCreated;
+ }
+
+ public Boolean getIncrementalFallback() {
+ return incrementalFallback != null && incrementalFallback;
+ }
+
+ public void setIncrementalFallback(Boolean incrementalFallback) {
+ this.incrementalFallback = incrementalFallback;
+ }
+
}
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 5402b6b2476..34f8d7b8bcd 100644
--- a/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java
+++ b/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java
@@ -36,6 +36,17 @@ public class TakeBackupCommand extends Command {
@LogLevel(LogLevel.Log4jLevel.Off)
private String mountOptions;
+ // 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 bitmapNew; // Checkpoint/bitmap name to create with
this backup (timestamp-based)
+ private String bitmapParent; // Incremental: parent bitmap to read
changes since
+
+ // Per-volume parent backup file paths (one per VM volume, ordered by
deviceId — same
+ // order as volumePaths). The script rebases each new qcow2 onto the
matching parent.
+ // Backup file UUIDs differ across volumes, so a single parentPath would
have rebased
+ // every data disk onto the root file. New callers MUST populate
parentPaths.
+ private List<String> parentPaths;
+
public TakeBackupCommand(String vmName, String backupPath) {
super();
this.vmName = vmName;
@@ -106,6 +117,38 @@ public class TakeBackupCommand extends Command {
this.quiesce = quiesce;
}
+ public String getMode() {
+ return mode;
+ }
+
+ public void setMode(String mode) {
+ this.mode = mode;
+ }
+
+ public String getBitmapNew() {
+ return bitmapNew;
+ }
+
+ public void setBitmapNew(String bitmapNew) {
+ this.bitmapNew = bitmapNew;
+ }
+
+ public String getBitmapParent() {
+ return bitmapParent;
+ }
+
+ public void setBitmapParent(String bitmapParent) {
+ this.bitmapParent = bitmapParent;
+ }
+
+ public List<String> getParentPaths() {
+ return parentPaths;
+ }
+
+ public void setParentPaths(List<String> parentPaths) {
+ this.parentPaths = parentPaths;
+ }
+
@Override
public boolean executeInSequence() {
return true;
diff --git
a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java
b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java
index d589f9e6bef..7754c2440c0 100644
--- a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java
+++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java
@@ -66,7 +66,7 @@ public class BackupVO implements Backup {
private String externalId;
@Column(name = "type")
- private String backupType;
+ private String type;
@Column(name = "date")
@Temporal(value = TemporalType.DATE)
@@ -125,7 +125,7 @@ public class BackupVO implements Backup {
@Override
public String toString() {
return String.format("Backup %s",
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(
- this, "id", "uuid", "vmId", "backupType", "externalId"));
+ this, "id", "uuid", "vmId", "type", "externalId"));
}
@Override
@@ -157,11 +157,11 @@ public class BackupVO implements Backup {
}
public String getType() {
- return backupType;
+ return type;
}
public void setType(String type) {
- this.backupType = type;
+ this.type = type;
}
@Override
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
new file mode 100644
index 00000000000..511f0ccb711
--- /dev/null
+++
b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java
@@ -0,0 +1,62 @@
+// 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.backup;
+
+/**
+ * Keys used by the NAS backup provider when storing incremental-chain metadata
+ * in the existing {@code backup_details} key/value table. Stored here (not on
+ * the {@code backups} table) so other providers do not need a schema change to
+ * support their own incremental implementations.
+ */
+public final class NASBackupChainKeys {
+
+ /** UUID of the parent backup (full or previous incremental). Empty for
full backups. */
+ public static final String PARENT_BACKUP_ID = "nas.parent_backup_id";
+
+ /** QEMU dirty-bitmap name created by this backup, used as the {@code
<incremental>} reference for the next one. */
+ public static final String BITMAP_NAME = "nas.bitmap_name";
+
+ /** Identifier shared by every backup in the same chain (the full anchors
a chain; its incrementals inherit the id). */
+ public static final String CHAIN_ID = "nas.chain_id";
+
+ /** Position within the chain: 0 for the full, 1 for the first
incremental, and so on. */
+ public static final String CHAIN_POSITION = "nas.chain_position";
+
+ /**
+ * In-memory chain-mode sentinels used by {@code ChainDecision.mode}. The
persisted
+ * full-vs-incremental backup type lives on the {@code backup.type} column
(set in
+ * {@code takeBackup}) — single source of truth. Not duplicated into
backup_details.
+ */
+ public static final String TYPE_FULL = "full";
+ public static final String TYPE_INCREMENTAL = "incremental";
+ // Feature disabled: behave exactly like the pre-incremental full-only
backup — no QEMU
+ // bitmap/checkpoint is created and no chain metadata is persisted.
Matches nasbackup.sh's
+ // "legacy-full" mode token (which sets make_checkpoint=0).
+ public static final String TYPE_LEGACY_FULL = "legacy-full";
+
+ /**
+ * 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
+ * for the next incremental backup. Written by {@link #BITMAP_NAME} on
each successful
+ * backup; cleared on restore (the restored disk image has no bitmap, so
the next backup
+ * must be a fresh full). When the VM has no detail, {@code decideChain}
forces full.
+ */
+ public static final String VM_ACTIVE_CHECKPOINT_ID =
"nas.active_checkpoint_id";
+
+ private NASBackupChainKeys() {
+ }
+}
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 a7121f149e3..fe56fbf7c1a 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
@@ -19,6 +19,7 @@ package org.apache.cloudstack.backup;
import com.cloud.agent.AgentManager;
import com.cloud.exception.AgentUnavailableException;
import com.cloud.exception.OperationTimedoutException;
+import com.cloud.configuration.Resource;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.host.Status;
@@ -38,8 +39,11 @@ import com.cloud.storage.dao.VolumeDao;
import com.cloud.utils.Pair;
import com.cloud.utils.component.AdapterBase;
import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.user.ResourceLimitService;
+import com.cloud.vm.VMInstanceDetailVO;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.vm.dao.VMInstanceDetailsDao;
import com.cloud.vm.snapshot.VMSnapshot;
import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
import com.cloud.vm.snapshot.VMSnapshotVO;
@@ -48,6 +52,7 @@ import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupDetailsDao;
import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -85,6 +90,29 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
true,
BackupFrameworkEnabled.key());
+ ConfigKey<Integer> NASBackupFullEvery = new ConfigKey<>("Advanced",
Integer.class,
+ "nas.backup.full.every",
+ "10",
+ "Take a full NAS backup every Nth backup; remaining backups in
between are incremental. " +
+ "Counts backups, not days, so it works for hourly, daily,
and ad-hoc schedules. " +
+ "Set to 1 to disable incrementals (every backup is full).",
+ true,
+ ConfigKey.Scope.Zone,
+ BackupFrameworkEnabled.key());
+
+ ConfigKey<Boolean> NASBackupIncrementalEnabled = new
ConfigKey<>("Advanced", Boolean.class,
+ "nas.backup.incremental.enabled",
+ "false",
+ "Master switch for NAS incremental backups. Defaults to false so
existing zones keep the " +
+ "legacy full-only behavior on upgrade; opt in per-zone
when ready to use chains. " +
+ "When false, every NAS backup is taken as a full
regardless of nas.backup.full.every. " +
+ "Toggling this is safe at any time: switching off forces
the next backup to be a fresh " +
+ "full anchor (existing chains stay restorable), switching
back on resumes incrementals " +
+ "on the next full + incremental cycle.",
+ true,
+ ConfigKey.Scope.Zone,
+ BackupFrameworkEnabled.key());
+
@Inject
private BackupDao backupDao;
@@ -106,6 +134,9 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
@Inject
private VMInstanceDao vmInstanceDao;
+ @Inject
+ private VMInstanceDetailsDao vmInstanceDetailsDao;
+
@Inject
private PrimaryDataStoreDao primaryDataStoreDao;
@@ -115,6 +146,9 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
@Inject
private AgentManager agentManager;
+ @Inject
+ private ResourceLimitService resourceLimitMgr;
+
@Inject
private VMSnapshotDao vmSnapshotDao;
@@ -130,6 +164,9 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
@Inject
private DiskOfferingDao diskOfferingDao;
+ @Inject
+ private BackupDetailsDao backupDetailsDao;
+
private Long getClusterIdFromRootVolume(VirtualMachine vm) {
VolumeVO rootVolume = volumeDao.getInstanceRootVolume(vm.getId());
StoragePoolVO rootDiskPool =
primaryDataStoreDao.findById(rootVolume.getPoolId());
@@ -168,6 +205,330 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
return
resourceManager.findOneRandomRunningHostByHypervisor(Hypervisor.HypervisorType.KVM,
vm.getDataCenterId());
}
+ /**
+ * 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.
+ */
+ static final class ChainDecision {
+ final String mode; // "full" or "incremental"
+ final String bitmapNew;
+ final String bitmapParent; // null for full
+ // Per-volume parent backup file paths, one per current VM volume in
deviceId order.
+ // null/empty for full. Each volume needs its own parent file because
backup files
+ // are named after each volume's own UUID (root.<uuid>.qcow2 /
datadisk.<uuid>.qcow2).
+ final List<String> parentPaths;
+ final String chainId; // chain identifier this backup belongs
to
+ final int chainPosition; // 0 for full, N for the Nth incremental
in the chain
+
+ private ChainDecision(String mode, String bitmapNew, String
bitmapParent, List<String> parentPaths,
+ String chainId, int chainPosition) {
+ this.mode = mode;
+ this.bitmapNew = bitmapNew;
+ this.bitmapParent = bitmapParent;
+ this.parentPaths = parentPaths;
+ this.chainId = chainId;
+ this.chainPosition = chainPosition;
+ }
+
+ static ChainDecision fullStart(String bitmapName) {
+ return new ChainDecision(NASBackupChainKeys.TYPE_FULL, bitmapName,
null, null,
+ UUID.randomUUID().toString(), 0);
+ }
+
+ /**
+ * Decision used when the incremental feature is disabled: a plain
full backup that
+ * creates no bitmap and carries no chain identity, so nothing
chain/checkpoint-related
+ * is sent to the agent or persisted. Keeps the feature-off path
byte-for-byte legacy.
+ */
+ static ChainDecision legacyFull() {
+ return new ChainDecision(NASBackupChainKeys.TYPE_LEGACY_FULL,
null, null, null, null, 0);
+ }
+
+ static ChainDecision incremental(String bitmapNew, String
bitmapParent, List<String> parentPaths,
+ String chainId, int chainPosition) {
+ return new ChainDecision(NASBackupChainKeys.TYPE_INCREMENTAL,
bitmapNew, bitmapParent,
+ parentPaths, chainId, chainPosition);
+ }
+
+ boolean isIncremental() {
+ return NASBackupChainKeys.TYPE_INCREMENTAL.equals(mode);
+ }
+
+ boolean isLegacyFull() {
+ return NASBackupChainKeys.TYPE_LEGACY_FULL.equals(mode);
+ }
+ }
+
+ /**
+ * Decides whether the next backup for {@code vm} should be a fresh full
or an incremental
+ * appended to the existing chain. Stopped VMs are always full (libvirt
{@code backup-begin}
+ * requires a running QEMU process). The {@code nas.backup.full.every}
ConfigKey controls
+ * how many backups (full + incrementals) form one chain before a new full
is forced.
+ *
+ * <p>The decision is anchored on the VM's {@code
nas.active_checkpoint_id} detail, which
+ * records the bitmap that currently exists on the running QEMU. After a
restore that
+ * detail is cleared, so the next backup is automatically full — even
though there may be
+ * a more recent "last backup taken" row in the database. The decision
deliberately avoids
+ * relying on "last backup taken", because that row is misleading after a
restore.</p>
+ */
+ 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).
+ Boolean incrementalEnabled =
NASBackupIncrementalEnabled.valueIn(vm.getDataCenterId());
+ if (incrementalEnabled == null || !incrementalEnabled) {
+ return ChainDecision.legacyFull();
+ }
+
+ // 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.
+ if (!allVolumesOnCheckpointCapableStorage(vm)) {
+ return ChainDecision.legacyFull();
+ }
+
+ final String newBitmap = "backup-" + System.currentTimeMillis() /
1000L;
+
+ // Stopped VMs cannot do incrementals — script will also fall back,
but we make the
+ // decision here so we register the right type up-front.
+ if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+ return ChainDecision.fullStart(newBitmap);
+ }
+
+ Integer fullEvery = NASBackupFullEvery.valueIn(vm.getDataCenterId());
+ if (fullEvery == null || fullEvery <= 1) {
+ // Disabled or every-backup-is-full mode.
+ return ChainDecision.fullStart(newBitmap);
+ }
+
+ // 1. If the VM has no active_checkpoint_id, there is no bitmap on the
host to use as
+ // a parent. This is the case after restore (we clear it), after VM
was just assigned
+ // to the offering, or on the very first backup.
+ String activeCheckpoint = readVmActiveCheckpoint(vm.getId());
+ if (activeCheckpoint == null) {
+ return ChainDecision.fullStart(newBitmap);
+ }
+
+ // 2. The most-recent BackedUp backup is the only safe parent — after
restore the
+ // next backup is always a fresh full, so anything older has a
rotated-out bitmap.
+ // If the latest backup's bitmap doesn't match the VM's
active_checkpoint_id, the
+ // chain is broken: force a full.
+ Backup parent = findLatestBackedUpBackup(vm.getId());
+ if (parent == null || !activeCheckpoint.equals(readDetail(parent,
NASBackupChainKeys.BITMAP_NAME))) {
+ LOG.debug("VM {} latest backup does not match
active_checkpoint_id={} — forcing full",
+ vm.getInstanceName(), activeCheckpoint);
+ return ChainDecision.fullStart(newBitmap);
+ }
+
+ String parentChainId = readDetail(parent, NASBackupChainKeys.CHAIN_ID);
+ int parentChainPosition = chainPosition(parent);
+ if (parentChainId == null || parentChainPosition == Integer.MAX_VALUE)
{
+ return ChainDecision.fullStart(newBitmap);
+ }
+
+ // Force a fresh full when the chain has reached the configured length.
+ if (parentChainPosition + 1 >= fullEvery) {
+ return ChainDecision.fullStart(newBitmap);
+ }
+
+ // The script needs the parent backup's on-NAS file path PER VOLUME so
it can rebase
+ // each new qcow2 onto the matching parent. The paths are stored
relative to the NAS
+ // mount root — the script resolves them inside its mount session.
When alignment
+ // fails (volume count changed, etc.) compose returns null and we fall
back to full
+ // so we don't risk corrupting the chain.
+ List<String> 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.fullStart(newBitmap);
+ }
+ return ChainDecision.incremental(newBitmap, activeCheckpoint,
parentPaths,
+ parentChainId, parentChainPosition + 1);
+ }
+
+ /**
+ * 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
+ * VM sits on HOST-scope local, {@code SharedMountPoint}, or {@code
NetworkFilesystem} (NFS)
+ * storage. Ceph-RBD, Linstor, and any other pool that cannot carry a
per-disk checkpoint
+ * make this return {@code false} so the caller falls back to the legacy
full-only path. A
+ * volume whose pool can no longer be resolved is treated as incapable
(safe default).
+ */
+ protected boolean allVolumesOnCheckpointCapableStorage(VirtualMachine vm) {
+ List<VolumeVO> volumes = volumeDao.findByInstance(vm.getId());
+ if (volumes == null) {
+ return true;
+ }
+ for (VolumeVO volume : volumes) {
+ StoragePoolVO pool =
primaryDataStoreDao.findById(volume.getPoolId());
+ if (pool == null) {
+ LOG.debug("VM {} volume {} has no resolvable storage pool —
forcing legacy full",
+ vm.getInstanceName(), volume.getUuid());
+ return false;
+ }
+ boolean checkpointCapable = ScopeType.HOST.equals(pool.getScope())
+ ||
Storage.StoragePoolType.SharedMountPoint.equals(pool.getPoolType())
+ ||
Storage.StoragePoolType.NetworkFilesystem.equals(pool.getPoolType());
+ if (!checkpointCapable) {
+ LOG.debug("VM {} volume {} is on {} (scope {}) which cannot
carry checkpoints — forcing legacy full",
+ vm.getInstanceName(), volume.getUuid(),
pool.getPoolType(), pool.getScope());
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Read the {@code nas.active_checkpoint_id} VM detail. Returns {@code
null} when no detail
+ * exists (post-restore, first backup, or after explicit reset).
+ */
+ private String readVmActiveCheckpoint(long vmId) {
+ VMInstanceDetailVO d = vmInstanceDetailsDao.findDetail(vmId,
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID);
+ if (d == null) {
+ return null;
+ }
+ String v = d.getValue();
+ return (v == null || v.isEmpty()) ? null : v;
+ }
+
+ /**
+ * Locate the most-recent {@code BackedUp} backup for {@code vmId}. The
chain invariant
+ * guarantees the latest backup is the only valid incremental parent —
after restore the
+ * next backup is always a fresh full, and {@link #decideChain} checks the
bitmap matches.
+ */
+ private Backup findLatestBackedUpBackup(long vmId) {
+ List<Backup> history = backupDao.listByVmId(null, vmId);
+ if (history == null || history.isEmpty()) {
+ return null;
+ }
+ return history.stream()
+ .filter(b -> Backup.Status.BackedUp.equals(b.getStatus()))
+ .max(Comparator.comparing(Backup::getDate))
+ .orElse(null);
+ }
+
+ private String readDetail(Backup backup, String key) {
+ BackupDetailVO d = backupDetailsDao.findDetail(backup.getId(), key);
+ return d == null ? null : d.getValue();
+ }
+
+ /**
+ * Compose the on-NAS path of EVERY parent backup file (one per VM volume)
in the same
+ * order the script will iterate the current VM's disks (deviceId asc).
Relative to the
+ * NAS mount, matches the layout written by {@code nasbackup.sh}:
+ * first disk -> {@code <backupPath>/root.<volUuid>.qcow2}
+ * others -> {@code <backupPath>/datadisk.<volUuid>.qcow2}
+ *
+ * Returns {@code null} if the parent's stored volume count doesn't match
the current VM's
+ * volume count. Volume attach/detach is blocked while a VM is assigned to
a backup offering;
+ * if the offering was removed and re-assigned the active checkpoint is
cleared in
+ * {@link #removeVMFromBackupOffering}, so this method doesn't need to
revalidate volume
+ * identities — a count mismatch is the only way to reach this branch with
a non-null
+ * active_checkpoint_id.
+ */
+ private List<String> composeParentBackupPaths(Backup parent, long vmId) {
+ // backupPath is stored as externalId by createBackupObject — e.g.
+ // "i-2-1234-VM/2026.04.27.13.45.00".
+ String dir = parent.getExternalId();
+ if (dir == null || dir.isEmpty()) {
+ return null;
+ }
+
+ List<Backup.VolumeInfo> parentVols = parent.getBackedUpVolumes();
+ if (parentVols == null || parentVols.isEmpty()) {
+ return null;
+ }
+
+ List<VolumeVO> currentVols = volumeDao.findByInstance(vmId);
+ if (currentVols == null || currentVols.size() != parentVols.size()) {
+ return null;
+ }
+
+ // parentVols is in deviceId order at the time the parent was taken.
The script names the
+ // per-disk files from the volume PATH basename (root.<path>.qcow2 /
datadisk.<path>.qcow2,
+ // see nasbackup.sh: volUuid="${fullpath##*/}"). Use getPath(), NOT
getUuid(): after a
+ // volume migration the uuid and the on-disk path diverge, and the
backup file is named by
+ // path — a uuid-based parent path then fails to resolve for the
incremental (test 13).
+ List<String> paths = new ArrayList<>(parentVols.size());
+ for (int i = 0; i < parentVols.size(); i++) {
+ String volPath = parentVols.get(i).getPath();
+ String prefix = (i == 0) ? "root" : "datadisk";
+ paths.add(dir + "/" + prefix + "." + volPath + ".qcow2");
+ }
+ return paths;
+ }
+
+ /**
+ * Persist chain metadata under backup_details. Stored here (not on the
backups table) so
+ * other providers can implement their own chain semantics without schema
changes.
+ */
+ private void persistChainMetadata(Backup backup, ChainDecision decision,
String bitmapFromAgent) {
+ // Only persist nas.bitmap_name when the agent confirmed the bitmap
exists on the host.
+ // The agent wrapper sets bitmapFromAgent=null when nasbackup.sh exits
+ // EXIT_BITMAP_NOT_SEEDED (=22) — currently only the stopped-VM path
where qemu-img
+ // bitmap --add failed on every source disk. Anchoring the next
incremental on a
+ // bitmap that doesn't exist would force a non-recoverable failure, so
we leave the
+ // detail empty and let the next backup start a fresh full chain.
+ if (bitmapFromAgent != null && !bitmapFromAgent.isEmpty()) {
+ backupDetailsDao.persist(new BackupDetailVO(backup.getId(),
NASBackupChainKeys.BITMAP_NAME, bitmapFromAgent, true));
+ }
+ backupDetailsDao.persist(new BackupDetailVO(backup.getId(),
NASBackupChainKeys.CHAIN_ID, decision.chainId, true));
+ backupDetailsDao.persist(new BackupDetailVO(backup.getId(),
NASBackupChainKeys.CHAIN_POSITION,
+ String.valueOf(decision.chainPosition), true));
+ // Backup full-vs-incremental type lives on backup.type (set by
takeBackup) — single
+ // 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);
+ if (parentUuid != null) {
+ backupDetailsDao.persist(new BackupDetailVO(backup.getId(),
NASBackupChainKeys.PARENT_BACKUP_ID, parentUuid, true));
+ }
+ }
+ }
+
+ /**
+ * Upsert the VM's {@code nas.active_checkpoint_id} detail to {@code
bitmapName}. Called
+ * after every successful backup so the next backup's parent-bitmap
decision is anchored
+ * on what actually exists on QEMU, not on "last backup taken".
+ */
+ private void upsertVmActiveCheckpoint(long vmId, String bitmapName) {
+ VMInstanceDetailVO existing = vmInstanceDetailsDao.findDetail(vmId,
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID);
+ if (existing == null) {
+ vmInstanceDetailsDao.addDetail(vmId,
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID, bitmapName, false);
+ return;
+ }
+ existing.setValue(bitmapName);
+ vmInstanceDetailsDao.update(existing.getId(), existing);
+ }
+
+ /**
+ * Remove the VM's {@code nas.active_checkpoint_id} detail. Called from
the restore paths:
+ * after restore the disk image has no QEMU bitmap attached, so any future
incremental
+ * would be based on stale state. Clearing forces the next backup to be a
fresh full.
+ */
+ private void clearVmActiveCheckpoint(long vmId) {
+ VMInstanceDetailVO existing = vmInstanceDetailsDao.findDetail(vmId,
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID);
+ if (existing != null) {
+ vmInstanceDetailsDao.removeDetail(vmId,
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID);
+ LOG.debug("Cleared nas.active_checkpoint_id for VM id={} (was
{})", vmId, existing.getValue());
+ }
+ }
+
+ private String lookupParentBackupUuid(long vmId, String parentBitmap) {
+ if (parentBitmap == null) {
+ return null;
+ }
+ for (Backup b : backupDao.listByVmId(null, vmId)) {
+ String bm = readDetail(b, NASBackupChainKeys.BITMAP_NAME);
+ if (parentBitmap.equals(bm)) {
+ return b.getUuid();
+ }
+ }
+ return null;
+ }
+
protected Host getVMHypervisorHostForBackup(VirtualMachine vm) {
Long hostId = vm.getHostId();
if (hostId == null &&
VirtualMachine.State.Running.equals(vm.getState())) {
@@ -205,12 +566,20 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
final String backupPath = String.format("%s/%s", vm.getInstanceName(),
new
SimpleDateFormat("yyyy.MM.dd.HH.mm.ss").format(creationDate));
- BackupVO backupVO = createBackupObject(vm, backupPath);
+ // Decide full vs incremental for this backup. Stopped VMs are always
full
+ // (libvirt backup-begin requires a running QEMU process).
+ ChainDecision decision = decideChain(vm);
+
+ BackupVO backupVO = createBackupObject(vm, backupPath,
decision.isIncremental() ? "INCREMENTAL" : "FULL");
TakeBackupCommand command = new
TakeBackupCommand(vm.getInstanceName(), backupPath);
command.setBackupRepoType(backupRepository.getType());
command.setBackupRepoAddress(backupRepository.getAddress());
command.setMountOptions(backupRepository.getMountOptions());
command.setQuiesce(quiesceVM);
+ command.setMode(decision.mode);
+ command.setBitmapNew(decision.bitmapNew);
+ command.setBitmapParent(decision.bitmapParent);
+ command.setParentPaths(decision.parentPaths);
if (VirtualMachine.State.Stopped.equals(vm.getState())) {
List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
@@ -239,9 +608,31 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
backupVO.setDate(new Date());
backupVO.setSize(answer.getSize());
backupVO.setStatus(Backup.Status.BackedUp);
+ // If the agent fell back to full (stopped VM mid-incremental
cycle), record this
+ // backup as a full and start a new chain.
+ ChainDecision effective = decision;
+ if (answer.getIncrementalFallback()) {
+ effective = ChainDecision.fullStart(decision.bitmapNew);
+ backupVO.setType("FULL");
+ }
List<Volume> volumes = new
ArrayList<>(volumeDao.findByInstance(vm.getId()));
backupVO.setBackedUpVolumes(backupManager.createVolumeInfoFromVolumes(volumes));
if (backupDao.update(backupVO.getId(), backupVO)) {
+ // Legacy-full (incremental feature disabled): persist no
chain/checkpoint metadata
+ // and do not touch the VM's active_checkpoint_id — keep the
feature-off path legacy.
+ if (!decision.isLegacyFull()) {
+ persistChainMetadata(backupVO, effective,
answer.getBitmapCreated());
+ // Pin the VM's active_checkpoint_id to whichever bitmap
the agent actually
+ // 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());
+ }
+ }
return new Pair<>(true, backupVO);
} else {
throw new CloudRuntimeException("Failed to update backup");
@@ -260,11 +651,11 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
}
}
- private BackupVO createBackupObject(VirtualMachine vm, String backupPath) {
+ private BackupVO createBackupObject(VirtualMachine vm, String backupPath,
String type) {
BackupVO backup = new BackupVO();
backup.setVmId(vm.getId());
backup.setExternalId(backupPath);
- backup.setType("FULL");
+ backup.setType(type);
backup.setDate(new Date());
long virtualSize = 0L;
for (final Volume volume: volumeDao.findByInstance(vm.getId())) {
@@ -333,6 +724,11 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
} catch (OperationTimedoutException e) {
throw new CloudRuntimeException("Operation to restore backup timed
out, please try again");
}
+ // After a restore the QEMU dirty-bitmap chain is gone — clear
active_checkpoint_id so
+ // the next backup is taken as a fresh full and starts a new chain.
See decideChain.
+ if (answer != null && answer.getResult()) {
+ clearVmActiveCheckpoint(vm.getId());
+ }
return new Pair<>(answer.getResult(), answer.getDetails());
}
@@ -457,6 +853,13 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
} catch (Exception e) {
throw new CloudRuntimeException("Unable to create restored
volume due to: " + e);
}
+ // The restored volume is attached to this VM with no QEMU bitmap
on its image, so the
+ // VM's tracked checkpoint is now stale; clear it to force the
next backup to be a full
+ // (mirrors the full restore paths
restoreVMFromBackup/restoreBackupToVM).
+ VirtualMachine restoreTargetVm =
vmInstanceDao.findVMByInstanceName(vmNameAndState.first());
+ if (restoreTargetVm != null) {
+ clearVmActiveCheckpoint(restoreTargetVm.getId());
+ }
}
return new Pair<>(answer.getResult(), answer.getDetails());
@@ -477,6 +880,14 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
.orElse(null);
}
+ @Override
+ public boolean handlesChainDeleteResourceAccounting() {
+ // This provider deletes whole incremental chains (leaf + swept
delete-pending ancestors)
+ // and decrements resource count/usage once per physically-removed
backup itself, so the
+ // manager must not also decrement or remove the DB row.
+ return true;
+ }
+
@Override
public boolean deleteBackup(Backup backup, boolean forced) {
final BackupRepository backupRepository =
backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
@@ -495,9 +906,42 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
throw new CloudRuntimeException(String.format("Unable to find a
running KVM host in zone %d to delete backup %s", backup.getZoneId(),
backup.getUuid()));
}
- DeleteBackupCommand command = new
DeleteBackupCommand(backup.getExternalId(), backupRepository.getType(),
- backupRepository.getAddress(),
backupRepository.getMountOptions());
+ // Backups outside any tracked chain (legacy or non-incremental
providers) are
+ // deleted straight away — no children semantics apply.
+ if (readDetail(backup, NASBackupChainKeys.CHAIN_ID) == null) {
+ return deleteBackupFileAndRow(backup, backupRepository, host);
+ }
+
+ // Snapshot-style cascade: defer the on-NAS rm + DB row while there
are live children,
+ // mark this backup as delete-pending, and let the leaf's deletion
sweep it up later.
+ // See DefaultSnapshotStrategy#deleteSnapshotChain for the same
pattern on incremental
+ // snapshots. forced=true means the caller wants the entire subtree
gone right now.
+ if (forced) {
+ return cascadeDeleteSubtree(backup, backupRepository, host);
+ }
+
+ List<Backup> liveChildren = findLiveChildren(backup);
+ if (!liveChildren.isEmpty()) {
+ markDeletePending(backup);
+ LOG.debug("Backup {} has {} live child backup(s); marking as
delete-pending. The on-NAS file " +
+ "and DB row will be removed once the last
descendant is gone, or pass forced=true.",
+ backup.getUuid(), liveChildren.size());
+ return true;
+ }
+
+ // No live children — physically delete this backup, then walk up the
chain and
+ // collect any ancestors that were left in delete-pending state.
+ return deleteLeafBackupAndSweepPendingAncestors(backup,
backupRepository, host);
+ }
+ /**
+ * The single physical-delete step: rm the on-NAS directory, then remove
the DB row.
+ * Returns {@code false} (and leaves both intact) if the agent reports
failure, so the
+ * caller's recursion stops cleanly.
+ */
+ private boolean deleteBackupFileAndRow(Backup backup, BackupRepository
repo, Host host) {
+ DeleteBackupCommand command = new
DeleteBackupCommand(backup.getExternalId(), repo.getType(),
+ repo.getAddress(), repo.getMountOptions());
BackupAnswer answer;
try {
answer = (BackupAnswer) agentManager.send(host.getId(), command);
@@ -506,13 +950,229 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
} catch (OperationTimedoutException e) {
throw new CloudRuntimeException("Operation to delete backup timed
out, please try again");
}
+ if (answer == null || !answer.getResult()) {
+ logger.warn("Failed to delete backup file for {} ({}); leaving DB
row intact",
+ backup.getUuid(), backup.getExternalId());
+ return false;
+ }
+ // Capture the deleted backup's bitmap before the row (and its
backup_details) are removed.
+ String deletedBitmap = readDetail(backup,
NASBackupChainKeys.BITMAP_NAME);
+ backupDao.remove(backup.getId());
+ // If this backup owned the bitmap the VM's active_checkpoint_id
points to, that on-host QEMU
+ // dirty-bitmap is gone with it — clear active_checkpoint_id so the
next backup starts a fresh
+ // full chain instead of anchoring an incremental on a deleted
checkpoint (test: delete last backup).
+ if (deletedBitmap != null &&
deletedBitmap.equals(readVmActiveCheckpoint(backup.getVmId()))) {
+ clearVmActiveCheckpoint(backup.getVmId());
+ }
+ // Exactly-once resource accounting: decrement at the single point a
backup row + file are
+ // physically removed. This runs for the leaf and for every swept
delete-pending ancestor,
+ // so a chain delete decrements once per actually-removed backup. The
manager skips its own
+ // accounting for this provider (see
handlesChainDeleteResourceAccounting()).
+ long size = backup.getSize() != null ? backup.getSize() : 0L;
+ resourceLimitMgr.decrementResourceCount(backup.getAccountId(),
Resource.ResourceType.backup);
+ resourceLimitMgr.decrementResourceCount(backup.getAccountId(),
Resource.ResourceType.backup_storage, size);
+ return true;
+ }
- if (answer != null && answer.getResult()) {
- return backupDao.remove(backup.getId());
+ /**
+ * Tombstone {@code backup} by moving it to {@link Backup.Status#Hidden}.
Idempotent.
+ * The row stays in the DB so the chain GC can sweep it once its last
descendant is deleted
+ * ({@code listByVmId} is status-agnostic), but it disappears from the
user-facing list
+ * ({@link BackupManagerImpl#listBackups} filters Hidden) and all backup
operations refuse it
+ * (they require {@code BackedUp}). Replaces the previous {@code
nas.delete_pending} detail.
+ */
+ private void markDeletePending(Backup backup) {
+ if (Backup.Status.Hidden.equals(backup.getStatus())) {
+ return;
+ }
+ BackupVO vo = backupDao.findById(backup.getId());
+ if (vo != null) {
+ vo.setStatus(Backup.Status.Hidden);
+ backupDao.update(vo.getId(), vo);
}
+ }
- logger.debug("There was an error removing the backup with id {}",
backup.getId());
- return false;
+ /**
+ * @return true if this backup is a tombstone (Hidden) awaiting chain
sweep.
+ */
+ private boolean isDeletePending(Backup backup) {
+ return Backup.Status.Hidden.equals(backup.getStatus());
+ }
+
+ /**
+ * Return the live (not delete-pending, not Removed) children of {@code
parent} within the
+ * same chain. Equivalent to "incrementals whose parent_backup_id points
at parent".
+ */
+ private List<Backup> findLiveChildren(Backup parent) {
+ String parentUuid = parent.getUuid();
+ String chainId = readDetail(parent, NASBackupChainKeys.CHAIN_ID);
+ if (parentUuid == null || chainId == null) {
+ return Collections.emptyList();
+ }
+ List<Backup> children = new ArrayList<>();
+ for (Backup b : backupDao.listByVmId(null, parent.getVmId())) {
+ if (b.getId() == parent.getId()) {
+ continue;
+ }
+ if (!chainId.equals(readDetail(b, NASBackupChainKeys.CHAIN_ID))) {
+ continue;
+ }
+ if (!parentUuid.equals(readDetail(b,
NASBackupChainKeys.PARENT_BACKUP_ID))) {
+ continue;
+ }
+ if (isDeletePending(b)) {
+ // Tombstoned children don't keep us alive — they're already
on the way out.
+ continue;
+ }
+ children.add(b);
+ }
+ return children;
+ }
+
+ /**
+ * Look up this backup's immediate parent in the chain (by {@code
PARENT_BACKUP_ID}).
+ * Returns {@code null} if this is the full (no parent) or the parent row
is gone.
+ *
+ * <p>Prefer {@link #getChainOrderedLeafToRoot(Backup)} when walking the
whole chain —
+ * this method hits the DB on each call and is O(N²) when used in a loop.
+ */
+ private Backup findChainParent(Backup backup) {
+ String parentUuid = readDetail(backup,
NASBackupChainKeys.PARENT_BACKUP_ID);
+ if (parentUuid == null || parentUuid.isEmpty()) {
+ return null;
+ }
+ for (Backup b : backupDao.listByVmId(null, backup.getVmId())) {
+ if (parentUuid.equals(b.getUuid())) {
+ return b;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Return the chain containing {@code member}, ordered leaf-first
+ * (highest {@code CHAIN_POSITION} → root).
+ *
+ * <p>Materialises the chain via a single {@link BackupDao#listByVmId}
call. Callers that
+ * previously walked the chain by repeatedly calling {@link
#findChainParent} were doing
+ * O(N) {@code listByVmId} calls (one per ancestor); this collapses that
to one.
+ *
+ * <p>If {@code member} has no {@code CHAIN_ID} metadata it is returned as
a one-element
+ * list (it is its own degenerate chain).
+ */
+ private List<Backup> getChainOrderedLeafToRoot(Backup member) {
+ String chainId = readDetail(member, NASBackupChainKeys.CHAIN_ID);
+ if (chainId == null) {
+ return Collections.singletonList(member);
+ }
+ List<Backup> chain = new ArrayList<>();
+ for (Backup b : backupDao.listByVmId(null, member.getVmId())) {
+ if (chainId.equals(readDetail(b, NASBackupChainKeys.CHAIN_ID))) {
+ chain.add(b);
+ }
+ }
+ // Descending CHAIN_POSITION = leaf-first (highest position = furthest
from root).
+ chain.sort((a, b) -> Integer.compare(chainPosition(b),
chainPosition(a)));
+ return chain;
+ }
+
+ /**
+ * Physically delete the leaf {@code backup}, then walk up the chain while
each ancestor
+ * is in delete-pending state. Mirrors the snapshot subsystem pattern:
once a leaf is
+ * gone, garbage-collect any tombstoned parents.
+ *
+ * <p>Caller must guarantee {@code backup} is a leaf (no live children).
Each tombstoned
+ * ancestor is by definition childless once its sole child is deleted
here, so no extra
+ * live-children check is needed inside the loop.
+ */
+ private boolean deleteLeafBackupAndSweepPendingAncestors(Backup backup,
BackupRepository repo, Host host) {
+ // Snapshot the chain BEFORE the leaf delete — deleteBackupFileAndRow
removes the row,
+ // after which the in-memory list still resolves but the DB no longer
would.
+ List<Backup> chain = getChainOrderedLeafToRoot(backup);
+ if (!deleteBackupFileAndRow(backup, repo, host)) {
+ return false;
+ }
+ // Walk the snapshot from leaf+1 upward, deleting tombstoned ancestors
until a live
+ // one is reached or the root is past.
+ int leafIdx = indexOfBackupById(chain, backup.getId());
+ if (leafIdx < 0) {
+ // Leaf wasn't in its own CHAIN_ID list — degenerate case, nothing
more to sweep.
+ return true;
+ }
+ for (int i = leafIdx + 1; i < chain.size(); i++) {
+ Backup ancestor = chain.get(i);
+ if (!isDeletePending(ancestor)) {
+ break;
+ }
+ if (!deleteBackupFileAndRow(ancestor, repo, host)) {
+ // Stop the sweep; the rest of the tombstoned chain will be
collected on a
+ // future delete that re-runs the sweep.
+ return true;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Forced delete of {@code root}'s entire chain, leaf-first. NAS backups
form a linear
+ * chain (full → inc → inc → …), not a tree, so we just walk the ordered
chain and
+ * delete each member without re-querying parents.
+ */
+ private boolean cascadeDeleteSubtree(Backup root, BackupRepository repo,
Host host) {
+ for (Backup b : getChainOrderedLeafToRoot(root)) {
+ if (!deleteBackupFileAndRow(b, repo, host)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private static int indexOfBackupById(List<Backup> chain, long id) {
+ for (int i = 0; i < chain.size(); i++) {
+ if (chain.get(i).getId() == id) {
+ return i;
+ }
+ }
+ return -1;
+ }
+
+ /**
+ * Return the backup with the highest {@code CHAIN_POSITION} sharing
{@code root}'s
+ * {@code CHAIN_ID}. Returns {@code root} if it has no chain metadata or
is itself the tail.
+ */
+ private Backup findChainTail(Backup root) {
+ String chainId = readDetail(root, NASBackupChainKeys.CHAIN_ID);
+ if (chainId == null) {
+ return root;
+ }
+ Backup tail = root;
+ int tailPos = chainPosition(root);
+ for (Backup b : backupDao.listByVmId(null, root.getVmId())) {
+ if (b.getId() == root.getId()) {
+ continue;
+ }
+ if (!chainId.equals(readDetail(b, NASBackupChainKeys.CHAIN_ID))) {
+ continue;
+ }
+ int pos = chainPosition(b);
+ if (pos > tailPos) {
+ tail = b;
+ tailPos = pos;
+ }
+ }
+ return tail;
+ }
+
+ private int chainPosition(Backup b) {
+ String s = readDetail(b, NASBackupChainKeys.CHAIN_POSITION);
+ if (s == null) {
+ return Integer.MAX_VALUE; // no metadata => sort to end
+ }
+ try {
+ return Integer.parseInt(s);
+ } catch (NumberFormatException e) {
+ return Integer.MAX_VALUE;
+ }
}
public void syncBackupMetrics(Long zoneId) {
@@ -543,6 +1203,11 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
@Override
public boolean removeVMFromBackupOffering(VirtualMachine vm) {
+ // Clear the VM's active checkpoint so any future re-assignment to a
backup offering
+ // starts a fresh chain. Without this, a detach-volume +
attach-different-volume cycle
+ // while the offering is unassigned would lead to the next backup
trying to rebase
+ // onto a stale parent (different volume identity, same VM id).
+ clearVmActiveCheckpoint(vm.getId());
return true;
}
@@ -629,7 +1294,9 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey[]{
- NASBackupRestoreMountTimeout
+ NASBackupRestoreMountTimeout,
+ NASBackupFullEvery,
+ NASBackupIncrementalEnabled
};
}
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 a512292cd28..3ba7dbad041 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
@@ -28,6 +28,7 @@ import com.cloud.vm.snapshot.dao.VMSnapshotDao;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
@@ -38,22 +39,35 @@ import org.springframework.test.util.ReflectionTestUtils;
import com.cloud.agent.AgentManager;
import com.cloud.exception.AgentUnavailableException;
import com.cloud.exception.OperationTimedoutException;
+import com.cloud.configuration.Resource;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.host.Status;
import com.cloud.host.dao.HostDao;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.resource.ResourceManager;
+import com.cloud.storage.DiskOfferingVO;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.Storage;
import com.cloud.storage.Volume;
import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.storage.dao.VolumeDao;
+import com.cloud.user.ResourceLimitService;
import com.cloud.utils.Pair;
+import com.cloud.vm.VMInstanceDetailVO;
import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.VirtualMachine;
import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.vm.dao.VMInstanceDetailsDao;
+
+import com.google.gson.Gson;
import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupDetailsDao;
import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
@@ -96,6 +110,21 @@ public class NASBackupProviderTest {
@Mock
private VMSnapshotDao vmSnapshotDaoMock;
+ @Mock
+ private BackupDetailsDao backupDetailsDao;
+
+ @Mock
+ private VMInstanceDetailsDao vmInstanceDetailsDao;
+
+ @Mock
+ private DiskOfferingDao diskOfferingDao;
+
+ @Mock
+ private DataStoreManager dataStoreMgr;
+
+ @Mock
+ private ResourceLimitService resourceLimitMgr;
+
@Test
public void testDeleteBackup() throws OperationTimedoutException,
AgentUnavailableException {
Long hostId = 1L;
@@ -353,4 +382,427 @@ public class NASBackupProviderTest {
Mockito.verify(hostDao).findHypervisorHostInCluster(clusterId);
Mockito.verify(resourceManager).findOneRandomRunningHostByHypervisor(Hypervisor.HypervisorType.KVM,
zoneId);
}
+
+ // -- nas.backup.incremental.enabled master switch
------------------------------------
+
+ /**
+ * When the operator sets nas.backup.incremental.enabled=false at the zone
level, every
+ * backup must be a fresh full anchor, regardless of VM state or
nas.backup.full.every.
+ * This is a single toggle the
+ * operator can flip without having to count remaining backups in a chain.
+ */
+ @Test
+ public void decideChainReturnsLegacyFullWhenIncrementalDisabled() {
+ Long zoneId = 1L;
+ VMInstanceVO vm = mock(VMInstanceVO.class);
+ Mockito.lenient().when(vm.getDataCenterId()).thenReturn(zoneId);
+
+ // Stub the master switch to false. ConfigKey.valueIn delegates to the
framework's
+ // ConfigDepot at runtime; for the unit test we override the in-memory
value via the
+ // ConfigKey's local override (set by ReflectionTestUtils on the spy
provider).
+ 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.assertNotNull(decision);
+ Assert.assertEquals(NASBackupChainKeys.TYPE_LEGACY_FULL,
decision.mode);
+ Assert.assertNull("legacy-full must not carry a bitmap",
decision.bitmapNew);
+ Assert.assertNull(decision.bitmapParent);
+ Assert.assertNull("legacy-full must not start a chain",
decision.chainId);
+ Assert.assertEquals(0, decision.chainPosition);
+ }
+
+ // -- decideChain anchored on VM's active_checkpoint_id
-------------------------------
+
+ /**
+ * No active_checkpoint_id on the VM (post-restore, first-ever backup, or
detail purged) =>
+ * decideChain must return a fresh full. Relying on the last backup taken
as the parent
+ * breaks after a restore, so the decision is anchored on the active
checkpoint instead.
+ */
+ @Test
+ public void decideChainReturnsFullWhenVmHasNoActiveCheckpoint() {
+ Long zoneId = 1L;
+ Long vmId = 42L;
+ VMInstanceVO vm = mock(VMInstanceVO.class);
+ Mockito.when(vm.getId()).thenReturn(vmId);
+ Mockito.when(vm.getDataCenterId()).thenReturn(zoneId);
+ Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Running);
+
+ // Master switch defaults to false (opt-in by zone) — explicitly
enable it for this
+ // test so we exercise the "no active_checkpoint_id" branch rather
than short-circuit
+ // at the master-switch gate.
+ 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));
+
+ Mockito.when(vmInstanceDetailsDao.findDetail(vmId,
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID)).thenReturn(null);
+
+ NASBackupProvider.ChainDecision decision =
nasBackupProvider.decideChain(vm);
+ Assert.assertNotNull(decision);
+ Assert.assertEquals(NASBackupChainKeys.TYPE_FULL, decision.mode);
+ Assert.assertNull(decision.bitmapParent);
+ Assert.assertEquals(0, decision.chainPosition);
+ }
+
+ // -- incremental storage-capability guard (Ceph-RBD / Linstor stay on
legacy full) ----
+
+ /**
+ * Incremental checkpoints are only possible on file-based qcow2 storage.
A VM whose every
+ * volume sits on NFS / HOST-scope local / SharedMountPoint is
checkpoint-capable.
+ */
+ @Test
+ public void
allVolumesOnCheckpointCapableStorageTrueForNfsHostAndSharedMount() {
+ Long vmId = 55L;
+ VMInstanceVO vm = mock(VMInstanceVO.class);
+ Mockito.when(vm.getId()).thenReturn(vmId);
+
+ VolumeVO nfsVol = mock(VolumeVO.class);
+ Mockito.when(nfsVol.getPoolId()).thenReturn(1L);
+ VolumeVO hostVol = mock(VolumeVO.class);
+ Mockito.when(hostVol.getPoolId()).thenReturn(2L);
+ VolumeVO smpVol = mock(VolumeVO.class);
+ Mockito.when(smpVol.getPoolId()).thenReturn(3L);
+
Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(nfsVol,
hostVol, smpVol));
+
+ StoragePoolVO nfs = mock(StoragePoolVO.class);
+
Mockito.when(nfs.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem);
+ StoragePoolVO host = mock(StoragePoolVO.class);
+ Mockito.when(host.getScope()).thenReturn(ScopeType.HOST);
+ StoragePoolVO smp = mock(StoragePoolVO.class);
+
Mockito.when(smp.getPoolType()).thenReturn(Storage.StoragePoolType.SharedMountPoint);
+ Mockito.when(storagePoolDao.findById(1L)).thenReturn(nfs);
+ Mockito.when(storagePoolDao.findById(2L)).thenReturn(host);
+ Mockito.when(storagePoolDao.findById(3L)).thenReturn(smp);
+
+
Assert.assertTrue(nasBackupProvider.allVolumesOnCheckpointCapableStorage(vm));
+ }
+
+ /**
+ * A single volume on Ceph-RBD (or any pool that cannot carry a per-disk
checkpoint) forces
+ * the whole VM onto the legacy full-only path — avoids regressing
RBD/Linstor storages.
+ */
+ @Test
+ public void allVolumesOnCheckpointCapableStorageFalseWhenAnyVolumeOnRbd() {
+ Long vmId = 56L;
+ VMInstanceVO vm = mock(VMInstanceVO.class);
+ Mockito.when(vm.getId()).thenReturn(vmId);
+
+ VolumeVO nfsVol = mock(VolumeVO.class);
+ Mockito.when(nfsVol.getPoolId()).thenReturn(1L);
+ VolumeVO rbdVol = mock(VolumeVO.class);
+ Mockito.when(rbdVol.getPoolId()).thenReturn(9L);
+
Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(nfsVol,
rbdVol));
+
+ StoragePoolVO nfs = mock(StoragePoolVO.class);
+
Mockito.when(nfs.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem);
+ StoragePoolVO rbd = mock(StoragePoolVO.class);
+
Mockito.when(rbd.getPoolType()).thenReturn(Storage.StoragePoolType.RBD);
+ Mockito.when(storagePoolDao.findById(1L)).thenReturn(nfs);
+ Mockito.when(storagePoolDao.findById(9L)).thenReturn(rbd);
+
+
Assert.assertFalse(nasBackupProvider.allVolumesOnCheckpointCapableStorage(vm));
+ }
+
+ /** A volume whose storage pool can no longer be resolved is treated as
incapable (safe). */
+ @Test
+ public void
allVolumesOnCheckpointCapableStorageFalseWhenPoolUnresolvable() {
+ Long vmId = 57L;
+ VMInstanceVO vm = mock(VMInstanceVO.class);
+ Mockito.when(vm.getId()).thenReturn(vmId);
+ VolumeVO vol = mock(VolumeVO.class);
+ Mockito.when(vol.getPoolId()).thenReturn(1L);
+ Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(vol));
+ Mockito.when(storagePoolDao.findById(1L)).thenReturn(null);
+
+
Assert.assertFalse(nasBackupProvider.allVolumesOnCheckpointCapableStorage(vm));
+ }
+
+ // -- restore clears active_checkpoint_id
---------------------------------------------
+
+ /**
+ * After a successful restoreVMFromBackup, decideChain on the next backup
must produce
+ * a full. We verify this end-to-end by checking that
vmInstanceDetailsDao.removeDetail
+ * is called with the active_checkpoint_id key.
+ */
+ @Test
+ public void restoreClearsActiveCheckpointDetail() throws
AgentUnavailableException, OperationTimedoutException {
+ Long vmId = 7L;
+ Long hostId = 8L;
+ Long backupOfferingId = 9L;
+
+ VMInstanceVO vm = mock(VMInstanceVO.class);
+ Mockito.when(vm.getId()).thenReturn(vmId);
+ Mockito.when(vm.getLastHostId()).thenReturn(hostId);
+ Mockito.when(vm.getRemoved()).thenReturn(null);
+ Mockito.when(vm.getName()).thenReturn("vm7");
+
+ HostVO host = mock(HostVO.class);
+ Mockito.when(host.getStatus()).thenReturn(Status.Up);
+ Mockito.when(host.getId()).thenReturn(hostId);
+ Mockito.when(hostDao.findById(hostId)).thenReturn(host);
+
+ BackupVO backup = new BackupVO();
+ backup.setVmId(vmId);
+ backup.setBackupOfferingId(backupOfferingId);
+ backup.setExternalId("i-2-7-VM/2026.05.16.10.00.00");
+ ReflectionTestUtils.setField(backup, "id", 100L);
+ // backedUpVolumes defaults to null => BackupVO.getBackedUpVolumes
returns emptyList().
+
+ BackupRepositoryVO repo = new BackupRepositoryVO(1L, "nas",
"test-repo",
+ "nfs", "address", "sync", 1024L, null);
+
Mockito.when(backupRepositoryDao.findByBackupOfferingId(backupOfferingId)).thenReturn(repo);
+
+
Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(Collections.emptyList());
+
+ BackupAnswer answer = mock(BackupAnswer.class);
+ Mockito.when(answer.getResult()).thenReturn(true);
+ Mockito.when(agentManager.send(Mockito.anyLong(),
Mockito.any(RestoreBackupCommand.class))).thenReturn(answer);
+
+ // Pre-existing checkpoint detail so removeDetail has something to
"clear".
+ VMInstanceDetailVO existing = mock(VMInstanceDetailVO.class);
+ Mockito.when(existing.getValue()).thenReturn("backup-1715000000");
+ Mockito.when(vmInstanceDetailsDao.findDetail(vmId,
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID)).thenReturn(existing);
+
+ boolean ok = nasBackupProvider.restoreVMFromBackup(vm, backup);
+ Assert.assertTrue(ok);
+ Mockito.verify(vmInstanceDetailsDao).removeDetail(vmId,
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID);
+ }
+
+ /**
+ * Single-volume restore (restoreBackedUpVolume) must also clear the
target VM's
+ * active_checkpoint_id, so the next backup of that VM is a fresh full —
the restored
+ * volume's image carries no QEMU bitmap.
+ */
+ @Test
+ public void restoreBackedUpVolumeClearsTargetVmActiveCheckpoint()
+ throws AgentUnavailableException, OperationTimedoutException {
+ Long targetVmId = 42L;
+ Long backupOfferingId = 9L;
+ String targetVmName = "i-2-42-VM";
+ String volUuid = "vol-uuid-1";
+ String hostIp = "10.0.0.5";
+ String dsUuid = "ds-uuid-1";
+
+ VolumeVO srcVolume = mock(VolumeVO.class);
+ Mockito.when(srcVolume.getUuid()).thenReturn(volUuid);
+ Mockito.when(srcVolume.getName()).thenReturn("data1");
+ Mockito.when(volumeDao.findByUuid(volUuid)).thenReturn(srcVolume);
+
+ DiskOfferingVO diskOffering = mock(DiskOfferingVO.class);
+ Mockito.when(diskOffering.getId()).thenReturn(5L);
+
Mockito.when(diskOffering.getProvisioningType()).thenReturn(Storage.ProvisioningType.THIN);
+
Mockito.when(diskOfferingDao.findByUuid(Mockito.anyString())).thenReturn(diskOffering);
+
+ StoragePoolVO pool = mock(StoragePoolVO.class);
+ Mockito.when(pool.getId()).thenReturn(11L);
+
Mockito.when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem);
+ Mockito.when(storagePoolDao.findByUuid(dsUuid)).thenReturn(pool);
+
+ HostVO host = mock(HostVO.class);
+ Mockito.when(host.getId()).thenReturn(8L);
+ Mockito.when(hostDao.findByIp(hostIp)).thenReturn(host);
+
+ Backup.VolumeInfo backedUp = new Backup.VolumeInfo(volUuid,
"i-2-99-VM/2026/data1.qcow2",
+ Volume.Type.DATADISK, 1024L, 1L, "disk-offering-uuid", null,
null);
+
+ BackupVO backup = new BackupVO();
+ backup.setVmId(99L);
+ backup.setBackupOfferingId(backupOfferingId);
+ backup.setExternalId("i-2-99-VM/2026.06.22.10.00.00");
+ backup.setSize(1024L);
+ backup.setBackedUpVolumes(new
Gson().toJson(Collections.singletonList(backedUp)));
+ ReflectionTestUtils.setField(backup, "id", 200L);
+
+ BackupRepositoryVO repo = new BackupRepositoryVO(1L, "nas",
"test-repo",
+ "nfs", "address", "sync", 1024L, null);
+
Mockito.when(backupRepositoryDao.findByBackupOfferingId(backupOfferingId)).thenReturn(repo);
+
+ BackupAnswer answer = mock(BackupAnswer.class);
+ Mockito.when(answer.getResult()).thenReturn(true);
+ Mockito.when(agentManager.send(Mockito.anyLong(),
Mockito.any(RestoreBackupCommand.class))).thenReturn(answer);
+
+ VMInstanceVO targetVm = mock(VMInstanceVO.class);
+ Mockito.when(targetVm.getId()).thenReturn(targetVmId);
+
Mockito.when(vmInstanceDao.findVMByInstanceName(targetVmName)).thenReturn(targetVm);
+
+ VMInstanceDetailVO existing = mock(VMInstanceDetailVO.class);
+ Mockito.when(existing.getValue()).thenReturn("backup-1718000000");
+ Mockito.when(vmInstanceDetailsDao.findDetail(targetVmId,
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID)).thenReturn(existing);
+
+ Pair<Boolean, String> result = nasBackupProvider.restoreBackedUpVolume(
+ backup, backedUp, hostIp, dsUuid, new Pair<>(targetVmName,
VirtualMachine.State.Stopped));
+
+ Assert.assertTrue(result.first());
+ Mockito.verify(vmInstanceDetailsDao).removeDetail(targetVmId,
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID);
+ }
+
+ // -- delete-pending cascade
----------------------------------------------------------
+
+ /**
+ * Deleting an incremental that has a live child must mark the incremental
as
+ * delete-pending in backup_details and NOT touch the on-NAS file or the
backups row.
+ * A parent with live children is soft-deleted (delete-pending) rather
than removed.
+ */
+ @Test
+ public void deleteWithLiveChildMarksDeletePendingAndPreservesFile()
+ throws AgentUnavailableException, OperationTimedoutException {
+ Long zoneId = 1L;
+ Long vmId = 2L;
+ Long hostId = 3L;
+ Long offeringId = 4L;
+
+ BackupVO parent = new BackupVO();
+ parent.setVmId(vmId);
+ parent.setBackupOfferingId(offeringId);
+ parent.setExternalId("i-2-2-VM/2026.05.10.10.00.00");
+ parent.setZoneId(zoneId);
+ ReflectionTestUtils.setField(parent, "id", 50L);
+ ReflectionTestUtils.setField(parent, "uuid", "parent-uuid");
+
+ VMInstanceVO vm = mock(VMInstanceVO.class);
+ Mockito.when(vm.getLastHostId()).thenReturn(hostId);
+ HostVO host = mock(HostVO.class);
+ Mockito.when(host.getStatus()).thenReturn(Status.Up);
+ // Note: host.getId() is intentionally not stubbed — the live-child
path never
+ // contacts the agent (verified below), so the stub would be
unnecessary.
+ Mockito.when(hostDao.findById(hostId)).thenReturn(host);
+
+ BackupRepositoryVO repo = new BackupRepositoryVO(1L, "nas",
"test-repo",
+ "nfs", "address", "sync", 1024L, null);
+
Mockito.when(backupRepositoryDao.findByBackupOfferingId(offeringId)).thenReturn(repo);
+
Mockito.when(vmInstanceDao.findByIdIncludingRemoved(vmId)).thenReturn(vm);
+
+ // CHAIN_ID on the parent => not the no-chain fast path.
+ BackupDetailVO chainIdDetail = new BackupDetailVO(50L,
NASBackupChainKeys.CHAIN_ID, "chain-1", true);
+ Mockito.when(backupDetailsDao.findDetail(50L,
NASBackupChainKeys.CHAIN_ID)).thenReturn(chainIdDetail);
+
+ // A live child references parent-uuid via PARENT_BACKUP_ID.
+ BackupVO child = new BackupVO();
+ child.setVmId(vmId);
+ child.setBackupOfferingId(offeringId);
+ child.setExternalId("i-2-2-VM/2026.05.10.10.30.00");
+ child.setZoneId(zoneId);
+ child.setStatus(Backup.Status.BackedUp);
+ ReflectionTestUtils.setField(child, "id", 51L);
+ ReflectionTestUtils.setField(child, "uuid", "child-uuid");
+
+ BackupDetailVO childChainId = new BackupDetailVO(51L,
NASBackupChainKeys.CHAIN_ID, "chain-1", true);
+ BackupDetailVO childParent = new BackupDetailVO(51L,
NASBackupChainKeys.PARENT_BACKUP_ID, "parent-uuid", true);
+ Mockito.when(backupDetailsDao.findDetail(51L,
NASBackupChainKeys.CHAIN_ID)).thenReturn(childChainId);
+ Mockito.when(backupDetailsDao.findDetail(51L,
NASBackupChainKeys.PARENT_BACKUP_ID)).thenReturn(childParent);
+
+ Mockito.when(backupDao.listByVmId(null,
vmId)).thenReturn(List.of(parent, child));
+ // markDeletePending loads the row to flip its status to Hidden.
+ Mockito.when(backupDao.findById(50L)).thenReturn(parent);
+
+ boolean result = nasBackupProvider.deleteBackup(parent, false);
+ Assert.assertTrue(result);
+
+ // No agent traffic — the on-NAS file must be preserved while children
are alive.
+ Mockito.verify(agentManager, Mockito.never()).send(Mockito.anyLong(),
Mockito.any(DeleteBackupCommand.class));
+ // No DB row removal — the row is the tombstone marker.
+ Mockito.verify(backupDao, Mockito.never()).remove(50L);
+ // A tombstoned backup is NOT decremented — its space is still
occupied until swept.
+ Mockito.verify(resourceLimitMgr,
Mockito.never()).decrementResourceCount(Mockito.anyLong(),
Mockito.eq(Resource.ResourceType.backup));
+ // The tombstoned backup is moved to Status.Hidden (replaces the old
DELETE_PENDING detail).
+ ArgumentCaptor<BackupVO> captor =
ArgumentCaptor.forClass(BackupVO.class);
+ Mockito.verify(backupDao).update(Mockito.eq(50L), captor.capture());
+ Assert.assertEquals(Backup.Status.Hidden,
captor.getValue().getStatus());
+ Mockito.verify(backupDetailsDao,
Mockito.never()).persist(Mockito.any(BackupDetailVO.class));
+ }
+
+ /**
+ * Deleting a leaf incremental whose parent is delete-pending must (a)
delete the leaf and
+ * then (b) sweep up the tombstoned parent. Mirrors
DefaultSnapshotStrategy's
+ * "delete leaf, then walk up while parent is destroying-and-childless".
+ */
+ @Test
+ public void deletingLeafSweepsUpDeletePendingParent()
+ throws AgentUnavailableException, OperationTimedoutException {
+ Long zoneId = 1L;
+ Long vmId = 2L;
+ Long hostId = 3L;
+ Long offeringId = 4L;
+
+ BackupVO leaf = new BackupVO();
+ leaf.setVmId(vmId);
+ leaf.setBackupOfferingId(offeringId);
+ leaf.setExternalId("i-2-2-VM/2026.05.10.11.00.00");
+ leaf.setZoneId(zoneId);
+ ReflectionTestUtils.setField(leaf, "id", 51L);
+ ReflectionTestUtils.setField(leaf, "uuid", "leaf-uuid");
+
+ BackupVO parent = new BackupVO();
+ parent.setVmId(vmId);
+ parent.setBackupOfferingId(offeringId);
+ parent.setExternalId("i-2-2-VM/2026.05.10.10.30.00");
+ parent.setZoneId(zoneId);
+ ReflectionTestUtils.setField(parent, "id", 50L);
+ ReflectionTestUtils.setField(parent, "uuid", "parent-uuid");
+
+ VMInstanceVO vm = mock(VMInstanceVO.class);
+ Mockito.when(vm.getLastHostId()).thenReturn(hostId);
+ HostVO host = mock(HostVO.class);
+ Mockito.when(host.getStatus()).thenReturn(Status.Up);
+ Mockito.when(host.getId()).thenReturn(hostId);
+ Mockito.when(hostDao.findById(hostId)).thenReturn(host);
+
+ BackupRepositoryVO repo = new BackupRepositoryVO(1L, "nas",
"test-repo",
+ "nfs", "address", "sync", 1024L, null);
+
Mockito.when(backupRepositoryDao.findByBackupOfferingId(offeringId)).thenReturn(repo);
+
Mockito.when(vmInstanceDao.findByIdIncludingRemoved(vmId)).thenReturn(vm);
+
+ // Leaf details. CHAIN_POSITION=1 puts the leaf after the full anchor
in the
+ // ordered chain — getChainOrderedLeafToRoot sorts by CHAIN_POSITION
descending.
+ BackupDetailVO leafChainId = new BackupDetailVO(51L,
NASBackupChainKeys.CHAIN_ID, "chain-1", true);
+ BackupDetailVO leafChainPos = new BackupDetailVO(51L,
NASBackupChainKeys.CHAIN_POSITION, "1", true);
+ Mockito.when(backupDetailsDao.findDetail(51L,
NASBackupChainKeys.CHAIN_ID)).thenReturn(leafChainId);
+ Mockito.when(backupDetailsDao.findDetail(51L,
NASBackupChainKeys.CHAIN_POSITION)).thenReturn(leafChainPos);
+
+ // Parent is the tombstoned full anchor (CHAIN_POSITION=0).
+ BackupDetailVO parentChainId = new BackupDetailVO(50L,
NASBackupChainKeys.CHAIN_ID, "chain-1", true);
+ BackupDetailVO parentChainPos = new BackupDetailVO(50L,
NASBackupChainKeys.CHAIN_POSITION, "0", true);
+ // The parent is the tombstone — now represented by Status.Hidden (was
the DELETE_PENDING detail).
+ parent.setStatus(Backup.Status.Hidden);
+ Mockito.when(backupDetailsDao.findDetail(50L,
NASBackupChainKeys.CHAIN_ID)).thenReturn(parentChainId);
+ Mockito.when(backupDetailsDao.findDetail(50L,
NASBackupChainKeys.CHAIN_POSITION)).thenReturn(parentChainPos);
+ // Parent has no parent of its own (it's the full anchor).
+ Mockito.when(backupDetailsDao.findDetail(50L,
NASBackupChainKeys.PARENT_BACKUP_ID)).thenReturn(null);
+
+ // listByVmId is called once now (chain snapshot taken before the leaf
delete).
+ // We still use a mutable list + remove() answer so the DAO contract
is realistic.
+ java.util.List<Backup> liveBackups = new
java.util.ArrayList<>(List.of(parent, leaf));
+ Mockito.when(backupDao.listByVmId(null, vmId)).thenAnswer(inv -> new
java.util.ArrayList<>(liveBackups));
+
+ // Agent acknowledges every delete.
+ Mockito.when(agentManager.send(Mockito.anyLong(),
Mockito.any(DeleteBackupCommand.class)))
+ .thenReturn(new BackupAnswer(new DeleteBackupCommand(null,
null, null, null), true, "ok"));
+ // backupDao.remove(id) drops the corresponding row from the live list
so the next
+ // listByVmId call reflects post-delete state — mirrors the real DAO
contract.
+ Mockito.when(backupDao.remove(Mockito.anyLong())).thenAnswer(inv -> {
+ Long id = inv.getArgument(0);
+ liveBackups.removeIf(b -> b.getId() == id);
+ return true;
+ });
+
+ boolean result = nasBackupProvider.deleteBackup(leaf, false);
+ Assert.assertTrue(result);
+
+ // Both backups must be physically deleted (leaf first, then
tombstoned parent).
+ Mockito.verify(agentManager, Mockito.times(2))
+ .send(Mockito.anyLong(),
Mockito.any(DeleteBackupCommand.class));
+ Mockito.verify(backupDao).remove(51L);
+ Mockito.verify(backupDao).remove(50L);
+ // Exactly-once resource accounting: decremented for BOTH
physically-removed backups
+ // (leaf + swept ancestor), not just one.
+ Mockito.verify(resourceLimitMgr, Mockito.times(2))
+ .decrementResourceCount(Mockito.anyLong(),
Mockito.eq(Resource.ResourceType.backup));
+ Mockito.verify(resourceLimitMgr, Mockito.times(2))
+ .decrementResourceCount(Mockito.anyLong(),
Mockito.eq(Resource.ResourceType.backup_storage), Mockito.any());
+ }
}
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 22dbfbdd67a..cc2a0868fe1 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
@@ -60,6 +60,15 @@ public class LibvirtRestoreBackupCommandWrapper extends
CommandWrapper<RestoreBa
private static final String ATTACH_RBD_DISK_XML_COMMAND = " virsh
attach-device %s /dev/stdin <<EOF%sEOF";
private static final String CURRRENT_DEVICE = "virsh domblklist --domain
%s | tail -n 3 | head -n 1 | awk '{print $1}'";
private static final String RSYNC_COMMAND = "rsync -az %s %s";
+ // Flattens the backing-file chain into a single self-contained qcow2
written to the
+ // destination volume path. Used when the source backup is an incremental
whose qcow2
+ // has a backing reference to its parent (chain set up by nasbackup.sh's
qemu-img rebase).
+ private static final String QEMU_IMG_FLATTEN_COMMAND = "qemu-img convert
-O qcow2 %s %s";
+ // Detects whether a qcow2 file references a parent in its backing-file
metadata.
+ // Returns 0 (true) when a backing file is present, 1 when not. Uses
--output=json
+ // so the test is robust to qemu-img version differences in human-readable
output.
+ private static final String QEMU_IMG_HAS_BACKING_COMMAND =
+ "qemu-img info --output=json %s 2>/dev/null | grep -q
'\"backing-filename\"'";
private String getVolumeUuidFromPath(String volumePath, PrimaryDataStoreTO
volumePool) {
if (Storage.StoragePoolType.Linstor.equals(volumePool.getPoolType())) {
@@ -270,10 +279,27 @@ public class LibvirtRestoreBackupCommandWrapper extends
CommandWrapper<RestoreBa
return replaceBlockDeviceWithBackup(storagePoolMgr, volumePool,
volumePath, backupPath, timeout, createTargetVolume, size);
}
+ // For NAS-backed incremental backups, the source qcow2 has a
backing-file
+ // reference to its parent (set by nasbackup.sh's qemu-img rebase). A
plain
+ // rsync would copy only the differential blocks, leaving a volume that
+ // depends on a backing file the primary storage doesn't have. Flatten
the
+ // chain via qemu-img convert, which follows the backing-file links and
+ // produces a single self-contained qcow2.
+ if (hasBackingChain(backupPath)) {
+ int flattenExit = Script.runSimpleBashScriptForExitValue(
+ String.format(QEMU_IMG_FLATTEN_COMMAND, backupPath,
volumePath), timeout, false);
+ return flattenExit == 0;
+ }
+
int exitValue =
Script.runSimpleBashScriptForExitValue(String.format(RSYNC_COMMAND, backupPath,
volumePath), timeout, false);
return exitValue == 0;
}
+ private boolean hasBackingChain(String qcow2Path) {
+ return Script.runSimpleBashScriptForExitValue(
+ String.format(QEMU_IMG_HAS_BACKING_COMMAND, qcow2Path)) == 0;
+ }
+
private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager
storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String
backupPath, int timeout, boolean createTargetVolume, Long size) {
KVMStoragePool volumeStoragePool =
storagePoolMgr.getStoragePool(volumePool.getPoolType(), volumePool.getUuid());
QemuImg qemu;
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 42953aa9f83..106fe31a0f1 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
@@ -42,6 +42,16 @@ import java.util.Objects;
@ResourceWrapper(handles = TakeBackupCommand.class)
public class LibvirtTakeBackupCommandWrapper extends
CommandWrapper<TakeBackupCommand, Answer, LibvirtComputingResource> {
private static final Integer EXIT_CLEANUP_FAILED = 20;
+ // nasbackup.sh prints this on stdout when it could not proceed as an
incremental and
+ // completed a full backup instead; the orchestrator then records the
backup as a full.
+ private static final String INCREMENTAL_FALLBACK_MARKER =
"INCREMENTAL_FALLBACK=true";
+
+ private static final String MODE_FULL = "full";
+ private static final String MODE_INCREMENTAL = "incremental";
+ // Incremental feature disabled: plain full backup with no QEMU
bitmap/checkpoint and no
+ // chain metadata. Matches nasbackup.sh's "legacy-full" mode
(make_checkpoint=0).
+ private static final String MODE_LEGACY_FULL = "legacy-full";
+
@Override
public Answer execute(TakeBackupCommand command, LibvirtComputingResource
libvirtComputingResource) {
final String vmName = command.getVmName();
@@ -54,6 +64,13 @@ public class LibvirtTakeBackupCommandWrapper extends
CommandWrapper<TakeBackupCo
KVMStoragePoolManager storagePoolMgr =
libvirtComputingResource.getStoragePoolMgr();
int timeout = command.getWait() > 0 ? command.getWait() * 1000 :
libvirtComputingResource.getCmdsTimeout();
+ // Pre-validate incremental args here rather than relying on the
script to error out.
+ // Keeps the script agnostic to caller policy (it just does what it's
told).
+ String validationError = validateBackupArgs(command);
+ if (validationError != null) {
+ return new BackupAnswer(command, false, validationError);
+ }
+
List<String> diskPaths = new ArrayList<>();
if (Objects.nonNull(volumePaths)) {
for (int idx = 0; idx < volumePaths.size(); idx++) {
@@ -69,8 +86,63 @@ public class LibvirtTakeBackupCommandWrapper extends
CommandWrapper<TakeBackupCo
}
}
- List<String[]> commands = new ArrayList<>();
- commands.add(new String[]{
+ Pair<Integer, String> result =
runBackupScript(libvirtComputingResource, command, vmName, backupRepoType,
backupRepoAddress,
+ mountOptions, backupPath, diskPaths, command.getMode(),
+ command.getBitmapNew(), command.getBitmapParent(),
command.getParentPaths(), timeout);
+
+ if (result.first() != 0) {
+ logger.debug("Failed to take VM backup: " + result.second());
+ BackupAnswer answer = new BackupAnswer(command, false,
result.second().trim());
+ if (EXIT_CLEANUP_FAILED.equals(result.first())) {
+ logger.debug("Backup cleanup failed");
+ answer.setNeedsCleanup(true);
+ }
+ return answer;
+ }
+
+ // The script self-heals to a full backup when an incremental can't
proceed (e.g. the
+ // parent checkpoint can't be re-registered) and signals it with
INCREMENTAL_FALLBACK
+ // on stdout. Detect it, then strip the marker line before parsing the
backup size.
+ String rawStdout = result.second();
+ boolean incrementalFallback =
rawStdout.contains(INCREMENTAL_FALLBACK_MARKER);
+ String stdout = stripMarkerLines(rawStdout).trim();
+ long backupSize = parseBackupSize(stdout, diskPaths);
+
+ BackupAnswer answer = new BackupAnswer(command, true, stdout);
+ answer.setSize(backupSize);
+ // A successful run always created command.getBitmapNew() (full and
incremental both do;
+ // it is null for legacy-full, which the orchestrator treats as "no
bitmap").
+ answer.setBitmapCreated(command.getBitmapNew());
+ answer.setIncrementalFallback(incrementalFallback);
+ return answer;
+ }
+
+ /** Remove nasbackup.sh's stdout signalling marker lines so they don't
pollute size parsing. */
+ private String stripMarkerLines(String stdout) {
+ if (stdout == null || stdout.isEmpty()) {
+ return "";
+ }
+ StringBuilder sb = new StringBuilder();
+ for (String line : stdout.split("\n", -1)) {
+ if (line.contains(INCREMENTAL_FALLBACK_MARKER)) {
+ continue;
+ }
+ if (sb.length() > 0) {
+ sb.append('\n');
+ }
+ sb.append(line);
+ }
+ return sb.toString();
+ }
+
+ /**
+ * Run nasbackup.sh once with the given args. Returns the exit code +
captured stdout.
+ */
+ private Pair<Integer, String> runBackupScript(LibvirtComputingResource
libvirtComputingResource,
+ TakeBackupCommand command, String vmName, String backupRepoType,
String backupRepoAddress,
+ String mountOptions, String backupPath, List<String> diskPaths,
String mode,
+ String bitmapNew, String bitmapParent, List<String> parentPaths,
int timeout) {
+ List<String> argv = new ArrayList<>(Arrays.asList(
libvirtComputingResource.getNasBackupPath(),
"-o", "backup",
"-v", vmName,
@@ -80,35 +152,79 @@ public class LibvirtTakeBackupCommandWrapper extends
CommandWrapper<TakeBackupCo
"-p", backupPath,
"-q", command.getQuiesce() != null && command.getQuiesce() ?
"true" : "false",
"-d", diskPaths.isEmpty() ? "" : String.join(",", diskPaths)
- });
+ ));
+ if (mode != null && !mode.isEmpty()) {
+ argv.add("-M");
+ argv.add(mode);
+ }
+ if (bitmapNew != null && !bitmapNew.isEmpty()) {
+ argv.add("--bitmap-new");
+ argv.add(bitmapNew);
+ }
+ if (bitmapParent != null && !bitmapParent.isEmpty()) {
+ argv.add("--bitmap-parent");
+ argv.add(bitmapParent);
+ }
+ if (parentPaths != null && !parentPaths.isEmpty()) {
+ argv.add("--parent-paths");
+ argv.add(String.join(",", parentPaths));
+ }
- Pair<Integer, String> result = Script.executePipedCommands(commands,
timeout);
+ List<String[]> commands = new ArrayList<>();
+ commands.add(argv.toArray(new String[0]));
+ return Script.executePipedCommands(commands, timeout);
+ }
- if (result.first() != 0) {
- logger.debug("Failed to take VM backup: " + result.second());
- BackupAnswer answer = new BackupAnswer(command, false,
result.second().trim());
- if (result.first() == EXIT_CLEANUP_FAILED) {
- logger.debug("Backup cleanup failed");
- answer.setNeedsCleanup(true);
+ /**
+ * Return a human-readable validation error string, or {@code null} if the
command's
+ * incremental-backup args are internally consistent.
+ */
+ private String validateBackupArgs(TakeBackupCommand command) {
+ String mode = command.getMode();
+ if (mode == null || mode.isEmpty()) {
+ return null; // legacy full-only — no extra args expected
+ }
+ if (MODE_INCREMENTAL.equals(mode)) {
+ if (command.getBitmapNew() == null ||
command.getBitmapNew().isEmpty()) {
+ return "incremental mode requires bitmapNew";
}
- return answer;
+ if (command.getBitmapParent() == null ||
command.getBitmapParent().isEmpty()) {
+ return "incremental mode requires bitmapParent";
+ }
+ if (command.getParentPaths() == null ||
command.getParentPaths().isEmpty()) {
+ return "incremental mode requires parentPaths";
+ }
+ return null;
+ }
+ if (MODE_FULL.equals(mode)) {
+ if (command.getBitmapNew() == null ||
command.getBitmapNew().isEmpty()) {
+ return "full mode requires bitmapNew (the bitmap to create for
the next incremental)";
+ }
+ return null;
+ }
+ if (MODE_LEGACY_FULL.equals(mode)) {
+ return null; // feature-off full backup — no bitmap or chain args
expected
}
+ return "Unknown backup mode: " + 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
+ * whose first whitespace-separated token is the byte count.
+ */
+ private long parseBackupSize(String stdout, List<String> diskPaths) {
long backupSize = 0L;
if (CollectionUtils.isNullOrEmpty(diskPaths)) {
- List<String> outputLines =
Arrays.asList(result.second().trim().split("\n"));
+ List<String> outputLines = Arrays.asList(stdout.split("\n"));
if (!outputLines.isEmpty()) {
backupSize = Long.parseLong(outputLines.get(outputLines.size()
- 1).trim());
}
} else {
- String[] outputLines = result.second().trim().split("\n");
- for(String line : outputLines) {
+ for (String line : stdout.split("\n")) {
backupSize = backupSize + Long.parseLong(line.split("
")[0].trim());
}
}
-
- BackupAnswer answer = new BackupAnswer(command, true,
result.second().trim());
- answer.setSize(backupSize);
- return answer;
+ return backupSize;
}
}
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 ef6b5c08189..fd8a3b02e0a 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
@@ -407,6 +407,8 @@ public class LibvirtRestoreBackupCommandWrapperTest {
return 0; // File exists
} else if (command.contains("qemu-img check")) {
return 0; // File is valid
+ } else if (command.contains("qemu-img info") &&
command.contains("backing-filename")) {
+ return 1; // No backing chain — exercise the
rsync path (full backups)
}
return 0; // Other commands success
});
diff --git
a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java
b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java
index 1a8d9f7b59e..287601d47d6 100644
---
a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java
+++
b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java
@@ -1164,7 +1164,7 @@ public class VMwareGuru extends HypervisorGuruBase
implements HypervisorGuru, Co
@Override
public VirtualMachine importVirtualMachineFromBackup(long zoneId, long
domainId, long accountId, long userId, String vmInternalName, Backup backup)
throws Exception {
logger.debug(String.format("Trying to import VM [vmInternalName: %s]
from Backup [%s].", vmInternalName,
-
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(backup, "id", "uuid",
"vmId", "externalId", "backupType")));
+
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(backup, "id", "uuid",
"vmId", "externalId", "type")));
DatacenterMO dcMo = getDatacenterMO(zoneId);
VirtualMachineMO vmToImport = dcMo.findVm(vmInternalName);
if (vmToImport == null) {
diff --git a/scripts/vm/hypervisor/kvm/nasbackup.sh
b/scripts/vm/hypervisor/kvm/nasbackup.sh
index b35908a433e..d252b1f4e97 100755
--- a/scripts/vm/hypervisor/kvm/nasbackup.sh
+++ b/scripts/vm/hypervisor/kvm/nasbackup.sh
@@ -33,6 +33,15 @@ MOUNT_OPTS=""
BACKUP_DIR=""
DISK_PATHS=""
QUIESCE=""
+# Incremental backup parameters (all optional; legacy callers omit them)
+MODE="" # "full" or "incremental"; empty => legacy full-only
behavior (no checkpoint created)
+BITMAP_NEW="" # Bitmap/checkpoint name to create with this backup
(e.g. "backup-1711586400")
+BITMAP_PARENT="" # For incremental: parent bitmap name to read changes
since
+PARENT_PATHS="" # For incremental: comma-separated list of parent backup
file paths,
+ # one per VM volume in the same order as DISK_PATHS.
Each new qcow2
+ # is rebased onto its corresponding parent file.
Required because
+ # data-disk backup files don't share the root volume's
UUID, so
+ # each disk must be rebased onto its own parent.
logFile="/var/log/cloudstack/agent/agent.log"
UNMOUNT_TIMEOUT=60
EXIT_CLEANUP_FAILED=20
@@ -113,20 +122,117 @@ backup_running_vm() {
mount_operation
mkdir -p "$dest" || { echo "Failed to create backup directory $dest"; exit
1; }
+ # Determine effective mode for this run.
+ # Legacy callers (no -M argument) get the original full-only behavior with
no checkpoint.
+ # The Java wrapper (LibvirtTakeBackupCommandWrapper) pre-validates required
args before
+ # invoking the script; the case below is a defensive fallback for direct
invocations.
+ local effective_mode="${MODE:-legacy-full}"
+ local make_checkpoint=0
+ case "$effective_mode" in
+ incremental|full)
+ make_checkpoint=1
+ ;;
+ legacy-full)
+ make_checkpoint=0
+ ;;
+ *)
+ echo "Unknown mode: $effective_mode"
+ cleanup
+ exit 1
+ ;;
+ esac
+
+ # Incremental needs the parent checkpoint registered with libvirt.
CloudStack rebuilds the
+ # domain XML on every VM start, wiping libvirt's checkpoint registry while
the dirty bitmap
+ # persists on the qcow2, so a fresh checkpoint-create fails with "Bitmap
already exists".
+ # Re-register the parent with --redefine (needs only a name + creationTime)
via a minimal
+ # synthesized XML. If the parent bitmap is missing from the qcow2 (e.g.
after a migration),
+ # fall back to a full backup instead of letting backup-begin fail below.
+ if [[ "$effective_mode" == "incremental" ]]; then
+ # The parent bitmap must be present on EVERY disk, not just one. A
snapshot restore or partial
+ # migration can wipe it on some disks; require it on all by comparing the
disk count to the
+ # number of disks that carry it.
+ disk_count=$(virsh -c qemu:///system domblklist "$VM" --details
2>/dev/null | awk '$2=="disk"{c++} END{print c+0}')
+ # Count per-device (one per inserted.file whose dirty-bitmaps holds the
parent), mirroring
+ # getVmDiskPathHasFromCheckpointMap(): query-block lists a bitmap under
multiple nodes, so raw
+ # name matches double-count. "|| echo 0" keeps a no-match from aborting
under "set -eo pipefail"
+ # before the fallback runs.
+ bitmap_count=$(virsh -c qemu:///system qemu-monitor-command "$VM"
'{"execute":"query-block"}' 2>/dev/null | python3 -c '
+import sys, json
+target = sys.argv[1]
+try:
+ data = json.load(sys.stdin)
+except Exception:
+ print(0); sys.exit(0)
+files = set()
+for dev in data.get("return", []) or []:
+ inserted = dev.get("inserted") or {}
+ f = inserted.get("file")
+ if not f:
+ continue
+ if any((b or {}).get("name") == target for b in
(inserted.get("dirty-bitmaps") or [])):
+ files.add(f)
+print(len(files))
+' "$BITMAP_PARENT" 2>/dev/null || echo 0)
+ if [[ "$disk_count" -eq 0 || "$bitmap_count" -lt "$disk_count" ]]; then
+ log -e "incremental: parent bitmap $BITMAP_PARENT present on
$bitmap_count/$disk_count disk(s) — falling back to full"
+ echo "INCREMENTAL_FALLBACK=true"
+ effective_mode="full"
+ fi
+ fi
+
+ if [[ "$effective_mode" == "incremental" ]]; then
+ if ! virsh -c qemu:///system checkpoint-list "$VM" --name 2>/dev/null |
grep -qx "$BITMAP_PARENT"; then
+ redefine_xml=$(mktemp)
+ printf
'<domaincheckpoint><name>%s</name><creationTime>%s</creationTime></domaincheckpoint>'
\
+ "$BITMAP_PARENT" "$(date +%s)" > "$redefine_xml"
+ if virsh -c qemu:///system checkpoint-create "$VM" --xmlfile
"$redefine_xml" --redefine > /dev/null 2>&1; then
+ rm -f "$redefine_xml" # parent checkpoint re-registered; the
incremental can proceed against it
+ else
+ rm -f "$redefine_xml"
+ # Parent checkpoint could not be re-registered — fall back to a full
backup in place so
+ # the chain restarts cleanly instead of failing. Emit a stdout marker
so the wrapper
+ # records this backup as a full (incrementalFallback=true).
+ log -e "incremental: parent checkpoint $BITMAP_PARENT could not be
re-registered — falling back to full"
+ echo "INCREMENTAL_FALLBACK=true"
+ effective_mode="full"
+ fi
+ fi
+ fi
+
+ # Build backup XML (and matching checkpoint XML when applicable).
name="root"
- echo "<domainbackup mode='push'><disks>" > $dest/backup.xml
+ echo "<domainbackup mode='push'>" > $dest/backup.xml
+ if [[ "$effective_mode" == "incremental" ]]; then
+ echo "<incremental>$BITMAP_PARENT</incremental>" >> $dest/backup.xml
+ fi
+ echo "<disks>" >> $dest/backup.xml
+ if [[ $make_checkpoint -eq 1 ]]; then
+ echo "<domaincheckpoint><name>$BITMAP_NEW</name><disks>" >
$dest/checkpoint.xml
+ fi
while read -r disk fullpath; do
if [[ "$fullpath" == /dev/drbd/by-res/* ]]; then
volUuid=$(get_linstor_uuid_from_path "$fullpath")
else
volUuid="${fullpath##*/}"
fi
- echo "<disk name='$disk' backup='yes' type='file'
backupmode='full'><driver type='qcow2'/><target
file='$dest/$name.$volUuid.qcow2' /></disk>" >> $dest/backup.xml
+ if [[ "$effective_mode" == "incremental" ]]; then
+ # Incremental disk entry — no backupmode attr, libvirt picks it up from
<incremental>.
+ echo "<disk name='$disk' backup='yes' type='file'><driver
type='qcow2'/><target file='$dest/$name.$volUuid.qcow2' /></disk>" >>
$dest/backup.xml
+ else
+ echo "<disk name='$disk' backup='yes' type='file'
backupmode='full'><driver type='qcow2'/><target
file='$dest/$name.$volUuid.qcow2' /></disk>" >> $dest/backup.xml
+ fi
+ if [[ $make_checkpoint -eq 1 ]]; then
+ echo "<disk name='$disk'/>" >> $dest/checkpoint.xml
+ fi
name="datadisk"
done < <(
virsh -c qemu:///system domblklist "$VM" --details 2>/dev/null | awk
'$2=="disk"{print $3, $4}'
)
echo "</disks></domainbackup>" >> $dest/backup.xml
+ if [[ $make_checkpoint -eq 1 ]]; then
+ echo "</disks></domaincheckpoint>" >> $dest/checkpoint.xml
+ fi
local thaw=0
if [[ ${QUIESCE} == "true" ]]; then
@@ -135,14 +241,22 @@ backup_running_vm() {
fi
fi
- # Start push backup
+ # Start push backup, atomically registering the new checkpoint when
applicable.
local backup_begin=0
- if virsh -c qemu:///system backup-begin --domain $VM --backupxml
$dest/backup.xml 2>&1 > /dev/null; then
- backup_begin=1;
+ if [[ $make_checkpoint -eq 1 ]]; then
+ # Order matters: redirect stdout to /dev/null first, then merge stderr
into stdout.
+ # The reversed `2>&1 > /dev/null` form leaves stderr pointing at the
original tty.
+ if virsh -c qemu:///system backup-begin --domain $VM --backupxml
$dest/backup.xml --checkpointxml $dest/checkpoint.xml > /dev/null 2>&1; then
+ backup_begin=1;
+ fi
+ else
+ if virsh -c qemu:///system backup-begin --domain $VM --backupxml
$dest/backup.xml > /dev/null 2>&1; then
+ backup_begin=1;
+ fi
fi
if [[ $thaw -eq 1 ]]; then
- if ! response=$(virsh -c qemu:///system qemu-agent-command "$VM"
'{"execute":"guest-fsfreeze-thaw"}' 2>&1 > /dev/null); then
+ if ! response=$(virsh -c qemu:///system qemu-agent-command "$VM"
'{"execute":"guest-fsfreeze-thaw"}' 2>&1); then
echo "Failed to thaw the filesystem for vm $VM: $response"
cleanup
exit 1
@@ -173,9 +287,47 @@ backup_running_vm() {
sleep 5
done
- # Use qemu-img convert to sparsify linstor backups which get bloated due to
virsh backup-begin.
+ # Sparsify behavior:
+ # - For LINSTOR backups (existing): qemu-img convert sparsifies the bloated
output.
+ # - For INCREMENTAL: rebase the resulting thin qcow2 onto its parent so the
chain is self-describing
+ # (so a future restore can flatten without external chain metadata).
name="root"
+ # PARENT_PATHS arrives as a comma-separated list, one entry per VM volume in
the same
+ # order as DISK_PATHS. Split into a bash array so we can index by disk
position.
+ local -a parent_paths_arr=()
+ if [[ "$effective_mode" == "incremental" && -n "$PARENT_PATHS" ]]; then
+ IFS=',' read -ra parent_paths_arr <<< "$PARENT_PATHS"
+ fi
+ local disk_idx=0
while read -r disk fullpath; do
+ if [[ "$effective_mode" == "incremental" ]]; then
+ volUuid="${fullpath##*/}"
+ # Pick this disk's specific parent file. Each volume's backup is named
after its
+ # own UUID, so a single PARENT_PATH would wrongly rebase data disks onto
the root
+ # parent.
+ if [[ $disk_idx -ge ${#parent_paths_arr[@]} ]]; then
+ echo "PARENT_PATHS list shorter than DISK_PATHS — missing parent for
disk index $disk_idx"
+ cleanup
+ exit 1
+ fi
+ local this_parent_rel="${parent_paths_arr[$disk_idx]}"
+ local parent_abs="$mount_point/$this_parent_rel"
+ if [[ ! -f "$parent_abs" ]]; then
+ echo "Parent backup file does not exist on NAS: $parent_abs"
+ cleanup
+ exit 1
+ fi
+ local parent_rel
+ parent_rel=$(realpath --relative-to="$dest" "$parent_abs")
+ if ! qemu-img rebase -u -b "$parent_rel" -F qcow2
"$dest/$name.$volUuid.qcow2" >> "$logFile" 2> >(cat >&2); then
+ echo "qemu-img rebase failed for $dest/$name.$volUuid.qcow2 onto
$parent_rel"
+ cleanup
+ exit 1
+ fi
+ name="datadisk"
+ disk_idx=$((disk_idx + 1))
+ continue
+ fi
if [[ "$fullpath" != /dev/drbd/by-res/* ]]; then
continue
fi
@@ -192,9 +344,43 @@ backup_running_vm() {
virsh -c qemu:///system domblklist "$VM" --details 2>/dev/null | awk
'$2=="disk"{print $3, $4}'
)
- rm -f $dest/backup.xml
+ rm -f $dest/backup.xml $dest/checkpoint.xml
sync
+ # Free the parent bitmap now that the incremental is written and rebased:
its delta is captured
+ # here and BITMAP_NEW tracks changes going forward, so it only accrues
metadata/IO cost over a
+ # long chain. Remove it per-disk with block-dirty-bitmap-remove (a clean
free) rather than
+ # checkpoint-delete, which would merge its bits into BITMAP_NEW and re-copy
backed-up regions.
+ # Best-effort: a failure here does not fail the backup, the bitmap is
reclaimed on a later run.
+ if [[ "$effective_mode" == "incremental" && -n "$BITMAP_PARENT" ]]; then
+ while read -r node; do
+ [[ -z "$node" ]] && continue
+ if ! virsh -c qemu:///system qemu-monitor-command "$VM" \
+
"{\"execute\":\"block-dirty-bitmap-remove\",\"arguments\":{\"node\":\"$node\",\"name\":\"$BITMAP_PARENT\"}}"
\
+ > /dev/null 2>>"$logFile"; then
+ log -e "cleanup: failed to remove parent bitmap $BITMAP_PARENT on node
$node (non-fatal)"
+ fi
+ done < <(
+ virsh -c qemu:///system qemu-monitor-command "$VM"
'{"execute":"query-block"}' 2>/dev/null | python3 -c '
+import sys, json
+target = sys.argv[1]
+try:
+ data = json.load(sys.stdin)
+except Exception:
+ sys.exit(0)
+seen = set()
+for dev in data.get("return", []) or []:
+ inserted = dev.get("inserted") or {}
+ node = inserted.get("node-name")
+ if not node or node in seen:
+ continue
+ if any((b or {}).get("name") == target for b in
(inserted.get("dirty-bitmaps") or [])):
+ seen.add(node)
+ print(node)
+' "$BITMAP_PARENT" 2>/dev/null || true
+ )
+ fi
+
# Print statistics
virsh -c qemu:///system domjobinfo $VM --completed
backup_size=$(du -sb "$dest" 2>>"$logFile" | cut -f1) || { log -ne "WARNING:
du failed for $dest, reporting size as 0"; backup_size=0; }
@@ -204,6 +390,8 @@ backup_running_vm() {
}
backup_stopped_vm() {
+ # Stopped VMs cannot use libvirt's backup-begin (no QEMU process); take a
full backup via
+ # qemu-img convert. The orchestrator never sends incremental mode for a
stopped VM.
mount_operation
mkdir -p "$dest" || { echo "Failed to create backup directory $dest"; exit
1; }
@@ -224,6 +412,23 @@ backup_stopped_vm() {
cleanup
exit 1
fi
+
+ # Pre-seed a persistent bitmap on the source disk so the NEXT backup (taken
+ # after this VM is started again) can be incremental against the qcow2 we
+ # just wrote. Without this, every backup after a stopped-VM backup would
+ # fall back to full because no parent bitmap exists on the host yet.
+ # Only applies to file-backed qcow2 sources — RBD/LINSTOR have their own
+ # snapshot mechanisms and qemu-img bitmap is not the right primitive there.
+ # bitmap --add should not fail on a file-backed qcow2; if it does, fail
the backup so the
+ # underlying problem is surfaced rather than silently degrading future
backups to full.
+ if [[ -n "$BITMAP_NEW" && "$disk" != rbd:* && "$disk" !=
/dev/drbd/by-res/* ]]; then
+ if ! qemu-img bitmap --add "$disk" "$BITMAP_NEW" 2>>"$logFile"; then
+ echo "Failed to pre-seed bitmap $BITMAP_NEW on $disk"
+ cleanup
+ exit 1
+ fi
+ fi
+
name="datadisk"
done
sync
@@ -293,6 +498,15 @@ cleanup() {
function usage {
echo ""
echo "Usage: $0 -o <operation> -v|--vm <domain name> -t <storage type> -s
<storage address> -m <mount options> -p <backup path> -d <disks path>
-q|--quiesce <true|false>"
+ echo " [-M|--mode <full|incremental>] [--bitmap-new <name>]
[--bitmap-parent <name>] [--parent-paths <p1,p2,...>]"
+ echo ""
+ echo "Incremental backup options (running VMs only; requires QEMU >= 4.2 and
libvirt >= 7.2):"
+ echo " -M|--mode full Take a full backup AND create a checkpoint
(--bitmap-new required) for future incrementals."
+ echo " -M|--mode incremental Take an incremental backup since
--bitmap-parent and create new checkpoint --bitmap-new."
+ echo " Requires --bitmap-parent, --bitmap-new, and
--parent-paths (comma-separated list, one"
+ echo " parent qcow2 path per disk:
root.<uuid>.qcow2, datadisk.<uuid>.qcow2, … same order"
+ echo " as -d|--disks)."
+ echo " Without -M, behaves as legacy full-only backup with no checkpoint
creation."
echo ""
exit 1
}
@@ -339,6 +553,26 @@ while [[ $# -gt 0 ]]; do
shift
shift
;;
+ -M|--mode)
+ MODE="$2"
+ shift
+ shift
+ ;;
+ --bitmap-new)
+ BITMAP_NEW="$2"
+ shift
+ shift
+ ;;
+ --bitmap-parent)
+ BITMAP_PARENT="$2"
+ shift
+ shift
+ ;;
+ --parent-paths)
+ PARENT_PATHS="$2"
+ shift
+ shift
+ ;;
-h|--help)
usage
shift
@@ -350,7 +584,7 @@ while [[ $# -gt 0 ]]; do
esac
done
-# Perform Initial sanity checks
+# Perform initial environment sanity checks (QEMU/libvirt version).
sanity_checks
if [ "$OP" = "backup" ]; then
diff --git a/server/src/main/java/com/cloud/hypervisor/KVMGuru.java
b/server/src/main/java/com/cloud/hypervisor/KVMGuru.java
index 6c1c3424b1b..6154522af4a 100644
--- a/server/src/main/java/com/cloud/hypervisor/KVMGuru.java
+++ b/server/src/main/java/com/cloud/hypervisor/KVMGuru.java
@@ -351,7 +351,7 @@ public class KVMGuru extends HypervisorGuruBase implements
HypervisorGuru {
@Override
public VirtualMachine importVirtualMachineFromBackup(long zoneId, long
domainId, long accountId, long userId, String vmInternalName, Backup backup) {
logger.debug(String.format("Trying to import VM [vmInternalName: %s]
from Backup [%s].", vmInternalName,
-
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(backup, "id", "uuid",
"vmId", "externalId", "backupType")));
+
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(backup, "id", "uuid",
"vmId", "externalId", "type")));
VMInstanceVO vm =
_instanceDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
if (vm == null) {
diff --git
a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
index f9c98cb141b..0636ffdc3c1 100644
--- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
@@ -1137,6 +1137,9 @@ public class BackupManagerImpl extends ManagerBase
implements BackupManager {
sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ);
sb.and("zoneId", sb.entity().getZoneId(), SearchCriteria.Op.EQ);
sb.and("backupOfferingId", sb.entity().getBackupOfferingId(),
SearchCriteria.Op.EQ);
+ // Tombstoned chain backups (Status.Hidden) are never shown to users;
they exist only so the
+ // incremental chain GC can sweep them once their last descendant is
deleted.
+ sb.and("statusNeq", sb.entity().getStatus(), SearchCriteria.Op.NEQ);
sb.and("backupStatus", sb.entity().getStatus(), SearchCriteria.Op.EQ);
if (keyword != null) {
@@ -1149,6 +1152,7 @@ public class BackupManagerImpl extends ManagerBase
implements BackupManager {
SearchCriteria<BackupVO> sc = sb.create();
accountManager.buildACLSearchCriteria(sc, domainId, isRecursive,
permittedAccounts, listProjectResourcesCriteria);
+ sc.setParameters("statusNeq", Backup.Status.Hidden);
if (id != null) {
sc.setParameters("id", id);
@@ -1190,7 +1194,7 @@ public class BackupManagerImpl extends ManagerBase
implements BackupManager {
vm = guru.importVirtualMachineFromBackup(zoneId, domainId,
accountId, userId, vmInternalName, backup);
} catch (final Exception e) {
logger.error(String.format("Failed to import VM [vmInternalName:
%s] from backup restoration [%s] with hypervisor [type: %s] due to: [%s].",
vmInternalName,
-
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(backup, "id", "uuid",
"vmId", "externalId", "backupType"), hypervisorType, e.getMessage()), e);
+
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(backup, "id", "uuid",
"vmId", "externalId", "type"), hypervisorType, e.getMessage()), e);
ActionEventUtils.onCompletedActionEvent(User.UID_SYSTEM,
vm.getAccountId(), EventVO.LEVEL_ERROR, EventTypes.EVENT_VM_BACKUP_RESTORE,
String.format("Failed to import Instance %s from Backup %s
with hypervisor [type: %s]", vmInternalName, backup.getUuid(), hypervisorType),
vm.getId(),
ApiCommandResourceType.VirtualMachine.toString(),0);
@@ -1722,6 +1726,15 @@ public class BackupManagerImpl extends ManagerBase
implements BackupManager {
reservationDao, resourceLimitMgr)) {
boolean result = backupProvider.deleteBackup(backup, forced);
if (result) {
+ // Chain-aware providers (e.g. NAS) physically remove several
backups per call
+ // (leaf + swept delete-pending ancestors) and decrement
resource count/usage and
+ // remove each DB row themselves, exactly once per removed
backup. Decrementing or
+ // removing again here would double-handle and destroy
delete-pending tombstones,
+ // so defer entirely to the provider for those.
+ if (backupProvider.handlesChainDeleteResourceAccounting()) {
+
checkAndGenerateUsageForLastBackupDeletedAfterOfferingRemove(vm, backup);
+ return true;
+ }
resourceLimitMgr.decrementResourceCount(backup.getAccountId(),
Resource.ResourceType.backup);
resourceLimitMgr.decrementResourceCount(backup.getAccountId(),
Resource.ResourceType.backup_storage, backupSize);
if (backupDao.remove(backup.getId())) {
diff --git a/test/integration/smoke/test_backup_recovery_nas.py
b/test/integration/smoke/test_backup_recovery_nas.py
index 409a08acc9f..e55c1b6f0f9 100644
--- a/test/integration/smoke/test_backup_recovery_nas.py
+++ b/test/integration/smoke/test_backup_recovery_nas.py
@@ -265,3 +265,323 @@ class TestNASBackupAndRecovery(cloudstackTestCase):
self.assertEqual(backup_repository.crosszoneinstancecreation, True,
"Cross-Zone Instance Creation could not be enabled on the backup repository")
self.vm_backup_create_vm_from_backup_int(template.id, [network.id])
+
+ # ------------------------------------------------------------------
+ # Incremental backup tests (RFC #12899 / PR #13074)
+ # ------------------------------------------------------------------
+ # These tests exercise the incremental NAS backup chain semantics:
+ # full -> incN cadence, restore-from-incremental, delete-middle chain
+ # repair, refuse-delete-full-with-children, and stopped-VM fallback.
+ #
+ # All tests set nas.backup.full.every to a small value (3) so a chain
+ # forms quickly without needing many backup iterations. The original
+ # zone value (whatever the test environment has configured) is captured
+ # before the test runs and restored verbatim in finally, so we don't
+ # leak config changes across tests on shared environments.
+
+ def _set_full_every(self, value):
+ Configurations.update(self.apiclient, name='nas.backup.full.every',
+ value=str(value), zoneid=self.zone.id)
+
+ def _get_full_every(self):
+ """Read the current zone-scoped (or global fallback) value of
nas.backup.full.every."""
+ configs = Configurations.list(self.apiclient,
name='nas.backup.full.every',
+ zoneid=self.zone.id)
+ if configs and len(configs) > 0 and configs[0].value is not None:
+ return configs[0].value
+ # Fall back to global default — Configurations.list returns the global
value
+ # when no zone override exists. Defensive fallback to '10' (the
framework default).
+ return '10'
+
+ def _backup_type(self, backup):
+ # Backup objects expose `type`; for chained backups it's
"INCREMENTAL", else "FULL".
+ return getattr(backup, 'type', 'FULL') or 'FULL'
+
+ @attr(tags=["advanced", "backup"], required_hardware="true")
+ def test_incremental_chain_cadence(self):
+ """
+ With nas.backup.full.every=3, the sequence of backups should be
+ FULL, INCREMENTAL, INCREMENTAL, FULL, INCREMENTAL, ...
+ """
+ self.backup_offering.assignOffering(self.apiclient, self.vm.id)
+ original_full_every = self._get_full_every()
+ self._set_full_every(3)
+ try:
+ ssh_client_vm = self.vm.get_ssh_client(reconnect=True)
+ ssh_client_vm.execute("touch /root/incremental_marker_1.txt")
+
+ created = []
+ for i in range(5):
+ Backup.create(self.apiclient, self.vm.id, "inc_chain_%d" % i)
+ # write a small change so each incremental has something to
capture
+ ssh_client_vm.execute("dd if=/dev/urandom of=/root/delta_%d
bs=64k count=4 2>/dev/null" % i)
+ time.sleep(2)
+ created = Backup.list(self.apiclient, self.vm.id)
+
+ self.assertEqual(len(created), 5, "Expected 5 backups after 5
Backup.create calls")
+ # Sort oldest-first by date
+ created.sort(key=lambda b: b.created)
+
+ expected = ['FULL', 'INCREMENTAL', 'INCREMENTAL', 'FULL',
'INCREMENTAL']
+ actual = [self._backup_type(b).upper() for b in created]
+ self.assertEqual(actual, expected,
+ "With nas.backup.full.every=3, chain pattern should be %s but
was %s" % (expected, actual))
+
+ # Cleanup all backups (newest first to satisfy chain rules without
forced=true)
+ for b in reversed(created):
+ Backup.delete(self.apiclient, b.id)
+ finally:
+ self._set_full_every(original_full_every)
+ self.backup_offering.removeOffering(self.apiclient, self.vm.id)
+
+ @attr(tags=["advanced", "backup"], required_hardware="true")
+ def test_incremental_after_vm_restart(self):
+ """
+ Regression for the parent-checkpoint recreation bug (PR #13074): an
incremental
+ backup taken AFTER the VM has been restarted must still succeed and
restore
+ correctly.
+
+ A VM (re)start rebuilds the libvirt domain XML and wipes libvirt's
checkpoint
+ registry, while the dirty bitmap persists on the qcow2. The agent must
then
+ re-register the parent checkpoint with `checkpoint-create --redefine`
(from the
+ saved checkpoint XML) rather than a fresh create — a fresh create
fails with
+ "Bitmap already exists", and qemu-img cannot drop the bitmap on a
running disk.
+
+ How this was reproduced manually on a libvirt 10.0.0 host, and what
this test
+ automates:
+ FULL + marker1 -> stop/start the VM (wipes the checkpoint
registry)
+ -> INCREMENTAL + marker2 -> restore the tip -> both markers
present.
+ """
+ self.backup_offering.assignOffering(self.apiclient, self.vm.id)
+ original_full_every = self._get_full_every()
+ # High cadence so the post-restart backup is INCREMENTAL, not a
periodic FULL.
+ self._set_full_every(100)
+ backups = []
+ try:
+ ssh_client_vm = self.vm.get_ssh_client(reconnect=True)
+ ssh_client_vm.execute("echo restart-test-1 >
/root/restart_marker_1.txt; sync")
+
+ # 1) FULL anchor
+ Backup.create(self.apiclient, self.vm.id, "restart_full")
+ time.sleep(2)
+
+ # 2) Restart the VM — wipes libvirt's checkpoint registry (the bug
trigger).
+ self.vm.stop(self.apiclient)
+ self.vm.start(self.apiclient)
+ ssh_client_vm = self.vm.get_ssh_client(reconnect=True)
+ ssh_client_vm.execute("echo restart-test-2 >
/root/restart_marker_2.txt; sync")
+
+ # 3) INCREMENTAL after the restart — the previously-broken path.
+ Backup.create(self.apiclient, self.vm.id, "restart_incr")
+ time.sleep(2)
+
+ backups = Backup.list(self.apiclient, self.vm.id)
+ self.assertEqual(len(backups), 2,
+ "Expected FULL + INCREMENTAL after restart, got %d" %
len(backups))
+ backups.sort(key=lambda b: b.created)
+ self.assertEqual(self._backup_type(backups[0]).upper(), 'FULL',
+ "First backup should be FULL")
+ self.assertEqual(self._backup_type(backups[1]).upper(),
'INCREMENTAL',
+ "Backup taken after the VM restart must be INCREMENTAL, not
silently a FULL")
+
+ # 4) Restore the tip (incremental) and verify BOTH markers
survived the chain
+ # across the restart — i.e. the post-restart incremental really
captured data.
+ new_vm_name = "vm-restart-restore-" + str(int(time.time()))
+ new_vm = Backup.createVMFromBackup(
+ self.apiclient,
+ self.services["small"],
+ mode=self.services["mode"],
+ backupid=backups[1].id,
+ vmname=new_vm_name,
+ accountname=self.account.name,
+ domainid=self.account.domainid,
+ zoneid=self.zone.id
+ )
+ self.cleanup.append(new_vm)
+ self.assertIsNotNone(new_vm, "Failed to create VM from the
post-restart incremental backup")
+ self.assertEqual(new_vm.state, "Running", "Restored VM should be
Running")
+
+ ssh_new = new_vm.get_ssh_client(reconnect=True)
+ r1 = "".join(ssh_new.execute("cat /root/restart_marker_1.txt"))
+ r2 = "".join(ssh_new.execute("cat /root/restart_marker_2.txt"))
+ self.assertIn("restart-test-1", r1,
+ "Marker written before the restart is missing from the
restore")
+ self.assertIn("restart-test-2", r2,
+ "Marker written after the restart (captured by the
post-restart incremental) "
+ "is missing from the restore")
+ finally:
+ for b in reversed(backups):
+ try:
+ Backup.delete(self.apiclient, b.id)
+ except Exception:
+ pass
+ self._set_full_every(original_full_every)
+ self.backup_offering.removeOffering(self.apiclient, self.vm.id)
+
+ @attr(tags=["advanced", "backup"], required_hardware="true")
+ def test_restore_from_incremental(self):
+ """
+ Take FULL + 2 INCREMENTAL backups, each with a marker file. Restore
from the
+ latest incremental and verify all three markers are present (chain
flatten).
+ """
+ self.backup_offering.assignOffering(self.apiclient, self.vm.id)
+ original_full_every = self._get_full_every()
+ self._set_full_every(5)
+ try:
+ ssh_client_vm = self.vm.get_ssh_client(reconnect=True)
+ ssh_client_vm.execute("touch /root/marker_full.txt")
+ Backup.create(self.apiclient, self.vm.id, "rfi_full")
+ time.sleep(3)
+
+ ssh_client_vm.execute("touch /root/marker_inc1.txt")
+ Backup.create(self.apiclient, self.vm.id, "rfi_inc1")
+ time.sleep(3)
+
+ ssh_client_vm.execute("touch /root/marker_inc2.txt")
+ Backup.create(self.apiclient, self.vm.id, "rfi_inc2")
+ time.sleep(3)
+
+ backups = Backup.list(self.apiclient, self.vm.id)
+ backups.sort(key=lambda b: b.created)
+ self.assertEqual(len(backups), 3)
+ self.assertEqual(self._backup_type(backups[0]).upper(), 'FULL')
+ self.assertEqual(self._backup_type(backups[2]).upper(),
'INCREMENTAL')
+
+ new_vm_name = "vm-from-inc-" + str(int(time.time()))
+ new_vm = Backup.createVMFromBackup(self.apiclient,
self.services["small"],
+ mode=self.services["mode"], backupid=backups[2].id,
vmname=new_vm_name,
+ accountname=self.account.name, domainid=self.account.domainid,
+ zoneid=self.zone.id)
+ self.cleanup.append(new_vm)
+
+ ssh_new = new_vm.get_ssh_client(reconnect=True)
+ for marker in ("marker_full.txt", "marker_inc1.txt",
"marker_inc2.txt"):
+ result = ssh_new.execute("ls /root/%s" % marker)
+ self.assertIn(marker, result[0],
+ "Restored VM should have %s (chain flattened correctly)" %
marker)
+
+ for b in reversed(backups):
+ Backup.delete(self.apiclient, b.id)
+ finally:
+ self._set_full_every(original_full_every)
+ self.backup_offering.removeOffering(self.apiclient, self.vm.id)
+
+ @attr(tags=["advanced", "backup"], required_hardware="true")
+ def test_delete_middle_incremental_repairs_chain(self):
+ """
+ Delete a MIDDLE incremental from a FULL -> INC1 -> INC2 chain.
+ The chain repair should rebase INC2 onto FULL, and the final restore
+ should still produce a working VM with all expected blocks.
+ """
+ self.backup_offering.assignOffering(self.apiclient, self.vm.id)
+ original_full_every = self._get_full_every()
+ self._set_full_every(5)
+ try:
+ ssh_client_vm = self.vm.get_ssh_client(reconnect=True)
+ ssh_client_vm.execute("touch /root/dmi_full.txt")
+ Backup.create(self.apiclient, self.vm.id, "dmi_full")
+ time.sleep(3)
+ ssh_client_vm.execute("touch /root/dmi_inc1.txt")
+ Backup.create(self.apiclient, self.vm.id, "dmi_inc1")
+ time.sleep(3)
+ ssh_client_vm.execute("touch /root/dmi_inc2.txt")
+ Backup.create(self.apiclient, self.vm.id, "dmi_inc2")
+ time.sleep(3)
+
+ backups = Backup.list(self.apiclient, self.vm.id)
+ backups.sort(key=lambda b: b.created)
+ full, inc1, inc2 = backups[0], backups[1], backups[2]
+
+ # Delete the middle incremental — should succeed via chain repair
(no force needed)
+ Backup.delete(self.apiclient, inc1.id)
+ remaining = Backup.list(self.apiclient, self.vm.id)
+ self.assertEqual(len(remaining), 2, "After deleting middle inc,
two backups should remain")
+
+ # Restore from the remaining tail (formerly inc2) — must still
produce a usable VM
+ new_vm_name = "vm-after-mid-del-" + str(int(time.time()))
+ new_vm = Backup.createVMFromBackup(self.apiclient,
self.services["small"],
+ mode=self.services["mode"], backupid=inc2.id,
vmname=new_vm_name,
+ accountname=self.account.name, domainid=self.account.domainid,
+ zoneid=self.zone.id)
+ self.cleanup.append(new_vm)
+ ssh_new = new_vm.get_ssh_client(reconnect=True)
+ # Both the FULL marker and (importantly) the deleted-INC1 marker
should still
+ # be present, because the rebase merged INC1's blocks into INC2.
+ for marker in ("dmi_full.txt", "dmi_inc1.txt", "dmi_inc2.txt"):
+ result = ssh_new.execute("ls /root/%s" % marker)
+ self.assertIn(marker, result[0],
+ "After mid-incremental delete and rebase, %s should still
be restorable" % marker)
+
+ Backup.delete(self.apiclient, inc2.id)
+ Backup.delete(self.apiclient, full.id)
+ finally:
+ self._set_full_every(original_full_every)
+ self.backup_offering.removeOffering(self.apiclient, self.vm.id)
+
+ @attr(tags=["advanced", "backup"], required_hardware="true")
+ def test_refuse_delete_full_with_children(self):
+ """
+ Deleting a FULL that has surviving incrementals must fail without
forced=true.
+ With forced=true it must succeed and remove the entire chain.
+ """
+ self.backup_offering.assignOffering(self.apiclient, self.vm.id)
+ original_full_every = self._get_full_every()
+ self._set_full_every(5)
+ try:
+ Backup.create(self.apiclient, self.vm.id, "rdc_full")
+ time.sleep(3)
+ Backup.create(self.apiclient, self.vm.id, "rdc_inc")
+ time.sleep(3)
+
+ backups = Backup.list(self.apiclient, self.vm.id)
+ backups.sort(key=lambda b: b.created)
+ full = backups[0]
+
+ failed = False
+ try:
+ Backup.delete(self.apiclient, full.id)
+ except Exception:
+ failed = True
+ self.assertTrue(failed, "Deleting a FULL with children should be
refused without forced=true")
+
+ # Forced delete should succeed and clear the whole chain
+ Backup.delete(self.apiclient, full.id, forced=True)
+ remaining = Backup.list(self.apiclient, self.vm.id)
+ self.assertIsNone(remaining, "Forced delete of FULL should remove
the entire chain")
+ finally:
+ self._set_full_every(original_full_every)
+ self.backup_offering.removeOffering(self.apiclient, self.vm.id)
+
+ @attr(tags=["advanced", "backup"], required_hardware="true")
+ def test_stopped_vm_falls_back_to_full(self):
+ """
+ When a backup is requested while the VM is stopped, even if the chain
cadence
+ would call for an incremental, the agent must fall back to a full and
start a
+ new chain. The incrementalFallback flag should be reflected in
backup.type=FULL.
+ """
+ self.backup_offering.assignOffering(self.apiclient, self.vm.id)
+ original_full_every = self._get_full_every()
+ self._set_full_every(2) # next backup after the first should be
incremental
+ try:
+ Backup.create(self.apiclient, self.vm.id, "svf_first")
+ time.sleep(3)
+
+ # Stop the VM and trigger another backup — should fall back to FULL
+ self.vm.stop(self.apiclient)
+ time.sleep(5)
+ Backup.create(self.apiclient, self.vm.id, "svf_second")
+ time.sleep(3)
+
+ backups = Backup.list(self.apiclient, self.vm.id)
+ backups.sort(key=lambda b: b.created)
+ self.assertEqual(len(backups), 2)
+ self.assertEqual(self._backup_type(backups[0]).upper(), 'FULL')
+ self.assertEqual(self._backup_type(backups[1]).upper(), 'FULL',
+ "Stopped-VM backup must be a FULL even when cadence would have
asked for an INCREMENTAL")
+
+ self.vm.start(self.apiclient)
+ for b in reversed(backups):
+ Backup.delete(self.apiclient, b.id)
+ finally:
+ self._set_full_every(original_full_every)
+ self.backup_offering.removeOffering(self.apiclient, self.vm.id)
