From 68a403c650b882a4325b0b67c8d50e1ba9697b89 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Tue, 8 Sep 2026 17:32:35 +0200 Subject: [PATCH 01/14] test: support Ceph/RBD primary storage in NAS backup smoke test test_backup_recovery_nas.py only allowed NFS primary storage, since it reused the primary storage pool's own path as the NAS backup repository address, and always required incremental-backup semantics that only qcow2/NFS storage can provide. Neither holds on Ceph/RBD. - setUpClass now accepts RBD alongside NFS as the primary storage pool type, picking a pool that's actually Up rather than list()[0] -- environments that added Ceph/RBD after the zone's original NFS primary storage keep that old pool around in Disabled state, and it still sorts first, silently exercising its path as if it were the storage VMs actually deploy on. - The NAS backup repository's NFS export address is resolved independently of the primary storage: when the primary pool isn't NFS, reuse the nfs test data entry (services[nfs][url]) -- the same temporary NFS mount point test_primary_storage.py uses for its temporary NFS primary storage pool, and something every marvin environment already has configured. An explicit nas_backup_repository_address test data entry or NAS_BACKUP_REPO_ADDRESS environment variable, if set, takes precedence. - The external offering imported in setUpClass is matched to the repository just created by externalid (== the repository's own id for the nas provider) rather than blindly taking index 0 -- a stray repository left over from an earlier interrupted run, whose backups didn't get cleaned up so its own teardown couldn't remove it either, sorts alongside the new one with no guarantee of which comes first. - Incremental NAS backups require QEMU dirty bitmaps / libvirt checkpoints, which only exist on file-based qcow2 storage (NASBackupProvider.allVolumesOnCheckpointCapableStorage). The six incremental-chain tests now skip on RBD/Ceph, where the provider always falls back to full-only backups server-side, rather than failing on assertions that storage type can never satisfy. - Added test_restore_volume_and_attach_to_vm, which exercises restoreVolumeFromBackupAndAttachToVM end-to-end (restoring a backed-up ROOT and DATADISK volume onto a second, stopped Instance) -- the API that drives the restore-and-attach code fixed by the previous commit (apache/cloudstack#14007). The target Instance is stopped with forced=True: a graceful ACPI stop was observed to time out (~2 minutes) before falling back to a hard destroy anyway, and once forced to a hard destroy the domain drops out of libvirt entirely, so the periodic ping-based PowerState sync the restore call depends on falls back to a much slower heuristic well past any reasonable wait. A forced stop destroys the domain immediately and deterministically. --- .../smoke/test_backup_recovery_nas.py | 166 ++++++++++++++++-- 1 file changed, 155 insertions(+), 11 deletions(-) diff --git a/test/integration/smoke/test_backup_recovery_nas.py b/test/integration/smoke/test_backup_recovery_nas.py index 96021b9edff8..4046c13a638c 100644 --- a/test/integration/smoke/test_backup_recovery_nas.py +++ b/test/integration/smoke/test_backup_recovery_nas.py @@ -24,7 +24,11 @@ from marvin.lib.common import (get_domain, get_zone, get_template) from nose.plugins.attrib import attr from marvin.codes import FAILED +import os import time +from urllib.parse import urlsplit + +SUPPORTED_PRIMARY_STORAGE_POOL_TYPES = ['networkfilesystem', 'rbd'] class TestNASBackupAndRecovery(cloudstackTestCase): @@ -39,19 +43,56 @@ def setUpClass(cls): cls.services["mode"] = cls.zone.networktype cls.hypervisor = cls.testClient.getHypervisorInfo() cls.domain = get_domain(cls.api_client) + cls._cleanup = [] + + if cls.hypervisor.lower() != 'kvm': + cls.skipTest(cls, reason="Test can be run only on KVM hypervisor") + cls.template = get_template(cls.api_client, cls.zone.id, cls.services["ostype"]) if cls.template == FAILED: assert False, "get_template() failed to return template with description %s" % cls.services["ostype"] cls.services["small"]["zoneid"] = cls.zone.id cls.services["small"]["template"] = cls.template.id - cls._cleanup = [] - if cls.hypervisor.lower() != 'kvm': - cls.skipTest(cls, reason="Test can be run only on KVM hypervisor") - - cls.storage_pool = StoragePool.list(cls.api_client)[0] - if cls.storage_pool.type.lower() != 'networkfilesystem': - cls.skipTest(cls, reason="Test can be run only if the primary storage is of type NFS") + # Pick a pool that's actually usable, not just list()[0] -- environments that + # added Ceph/RBD storage after the zone's original NFS primary storage keep the + # old NFS pool around in Disabled state, and it still sorts first. Falling back + # to index 0 there silently exercises the disabled NFS pool's path as if it were + # the primary storage in use, rather than the RBD pool VMs actually deploy on. + storage_pools = StoragePool.list(cls.api_client) + usable_pools = [p for p in storage_pools if getattr(p, 'state', 'Up') == 'Up'] + cls.storage_pool = usable_pools[0] if usable_pools else storage_pools[0] + if cls.storage_pool.type.lower() not in SUPPORTED_PRIMARY_STORAGE_POOL_TYPES: + cls.skipTest(cls, reason="Test can be run only if the primary storage is of type NFS or RBD (Ceph)") + + # The NAS backup repository needs an NFS export to mount. When the primary + # storage is itself NFS, its own path can double as that export (the + # historical behaviour). When the primary storage is Ceph/RBD, the primary + # storage location can't be reused as a NAS export, so fall back to the + # "nfs" test data entry -- the same NFS mount point test_primary_storage.py + # uses to create its temporary NFS primary storage pool, and something every + # marvin environment already has configured (services["nfs"]["url"], e.g. + # "nfs://nfs/export/automation/1/testprimary"). An explicit + # "nas_backup_repository_address" test data entry or NAS_BACKUP_REPO_ADDRESS + # environment variable, if set, takes precedence over both. + if cls.storage_pool.type.lower() == 'networkfilesystem': + default_nas_repository_address = cls.storage_pool.ipaddress + ":" + cls.storage_pool.path + else: + nfs_test_data = cls.services.get("nfs") + if nfs_test_data and nfs_test_data.get("url"): + nfs_url = urlsplit(nfs_test_data["url"]) + default_nas_repository_address = "%s:%s" % (nfs_url.hostname, nfs_url.path) + else: + default_nas_repository_address = None + cls.nas_repository_address = cls.services.get("nas_backup_repository_address") \ + or os.environ.get("NAS_BACKUP_REPO_ADDRESS") \ + or default_nas_repository_address + if not cls.nas_repository_address: + cls.skipTest(cls, reason="No NAS backup repository export configured. Set " + "'nas_backup_repository_address' in the test data, the " + "NAS_BACKUP_REPO_ADDRESS environment variable, or the standard " + "'nfs' test data entry, when the primary storage is not NFS " + "(e.g. Ceph/RBD)") # Check backup configuration values, set them to enable the nas provider backup_enabled_cfg = Configurations.list(cls.api_client, name='backup.framework.enabled') @@ -72,14 +113,22 @@ def setUpClass(cls): cls._cleanup = [cls.account] - # Create NAS backup repository and offering. Use the same directory as the storage pool + # Create NAS backup repository and offering. cls.backup_repository = BackupRepository.add(cls.api_client, zoneid=cls.zone.id, name="Nas", - address=cls.storage_pool.ipaddress + ":" + cls.storage_pool.path, + address=cls.nas_repository_address, provider="nas", type="nfs",) cls._cleanup.append(cls.backup_repository) + # Match the external offering to the repository just created above by externalid + # (== the repository's own id for the nas provider) rather than blindly taking + # index 0 -- a stray repository left over from an earlier interrupted run (e.g. + # one whose backups didn't get cleaned up, so its own teardown couldn't remove + # it either) sorts alongside the new one, and index 0 has no guarantee of being + # the one this run owns. cls.provider_offerings = BackupOffering.listExternal(cls.api_client, cls.zone.id) - cls.backup_offering = BackupOffering.importExisting(cls.api_client, cls.zone.id, cls.provider_offerings[0].externalid, - cls.provider_offerings[0].name, cls.provider_offerings[0].description) + matching_offerings = [o for o in cls.provider_offerings if o.externalid == cls.backup_repository.id] + provider_offering = matching_offerings[0] if matching_offerings else cls.provider_offerings[0] + cls.backup_offering = BackupOffering.importExisting(cls.api_client, cls.zone.id, provider_offering.externalid, + provider_offering.name, provider_offering.description) cls._cleanup.append(cls.backup_offering) cls.offering = ServiceOffering.create(cls.api_client,cls.services["service_offerings"]["small"]) @@ -303,12 +352,26 @@ def _backup_type(self, backup): # Backup objects expose `type`; for chained backups it's "INCREMENTAL", else "FULL". return getattr(backup, 'type', 'FULL') or 'FULL' + def _require_incremental_capable_storage(self): + """ + Incremental NAS backups rely on QEMU dirty bitmaps / libvirt checkpoints, which + only exist on file-based qcow2 storage -- see + NASBackupProvider.allVolumesOnCheckpointCapableStorage(), which forces every VM + on RBD/Ceph (or Linstor) onto the legacy full-only path server-side. On such + storage every backup comes back FULL regardless of cadence, so these chain/type + assertions can't pass (and some would pass vacuously without exercising the + chain logic at all). Skip rather than fail when running against RBD. + """ + if self.storage_pool.type.lower() == 'rbd': + self.skipTest("Incremental backups are not supported on RBD/Ceph primary Storage") + @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._require_incremental_capable_storage() self.backup_offering.assignOffering(self.apiclient, self.vm.id) original_full_every = self._get_full_every() self._set_full_every(3) @@ -358,6 +421,7 @@ def test_incremental_after_vm_restart(self): FULL + marker1 -> stop/start the VM (wipes the checkpoint registry) -> INCREMENTAL + marker2 -> restore the tip -> both markers present. """ + self._require_incremental_capable_storage() 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. @@ -430,6 +494,7 @@ 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._require_incremental_capable_storage() self.backup_offering.assignOffering(self.apiclient, self.vm.id) original_full_every = self._get_full_every() self._set_full_every(5) @@ -479,6 +544,7 @@ def test_delete_middle_incremental_repairs_chain(self): The chain repair should rebase INC2 onto FULL, and the final restore should still produce a working VM with all expected blocks. """ + self._require_incremental_capable_storage() self.backup_offering.assignOffering(self.apiclient, self.vm.id) original_full_every = self._get_full_every() self._set_full_every(5) @@ -531,6 +597,7 @@ def test_delete_full_with_children_is_deferred(self): FULL is hidden from the backup list while its child survives, and it is physically swept once the last descendant is deleted. """ + self._require_incremental_capable_storage() self.backup_offering.assignOffering(self.apiclient, self.vm.id) original_full_every = self._get_full_every() self._set_full_every(5) @@ -568,6 +635,7 @@ def test_stopped_vm_falls_back_to_full(self): 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._require_incremental_capable_storage() 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 @@ -594,3 +662,79 @@ def test_stopped_vm_falls_back_to_full(self): finally: self._set_full_every(original_full_every) self.backup_offering.removeOffering(self.apiclient, self.vm.id) + + # ------------------------------------------------------------------ + # Restore-volume-and-attach regression (PR apache/cloudstack#14007) + # ------------------------------------------------------------------ + # This test exercises the fixed path end to end via restoreVolumeFromBackupAndAttachToVM, + # on whichever primary storage this environment is running with (NFS or Ceph/RBD). + + @attr(tags=["advanced", "backup"], required_hardware="true") + def test_restore_volume_and_attach_to_vm(self): + """ + Test restoring the ROOT and DATADISK volumes of a backup and attaching them + to a different Instance (restoreVolumeFromBackupAndAttachToVM). + """ + target_vm = None + self.backup_offering.assignOffering(self.apiclient, self.vm.id) + try: + ssh_client_vm = self.vm.get_ssh_client(reconnect=True) + ssh_client_vm.execute("echo restore-attach-marker > /root/restore_attach_marker.txt; sync") + + Backup.create(self.apiclient, self.vm.id, "restore_attach_backup") + + backups = Backup.list(self.apiclient, self.vm.id) + self.assertEqual(len(backups), 1, "There should exist only one backup for the VM") + backup = backups[0] + + volumes = Volume.list(self.apiclient, virtualmachineid=self.vm.id, listall=True) + self.assertTrue(isinstance(volumes, list), "List volumes should return a valid list") + root_disk_id = None + data_disk_id = None + for volume in volumes: + if volume.type == 'ROOT': + root_disk_id = volume.id + elif volume.type == 'DATADISK': + data_disk_id = volume.id + self.assertIsNotNone(root_disk_id, "The backed up VM should have a ROOT volume") + + # Target Instance that will receive the restored volumes. The nas provider + # (unlike KBOSS) requires the target Instance to be stopped before a volume + # can be restored and attached to it. + target_vm = VirtualMachine.create( + self.apiclient, self.services["small"], accountid=self.account.name, + domainid=self.account.domainid, serviceofferingid=self.offering.id, + mode=self.services["mode"] + ) + target_vm.stop(self.apiclient, forced=True) + + # Restore and attach the ROOT volume backup as an extra disk. + Backup.restoreVolumeFromBackupAndAttachToVM( + self.apiclient, backupid=backup.id, volumeid=root_disk_id, virtualmachineid=target_vm.id + ) + target_volumes = Volume.list(self.apiclient, virtualmachineid=target_vm.id, listall=True) + self.assertTrue(isinstance(target_volumes, list), "List volumes should return a valid list") + self.assertEqual(2, len(target_volumes), + "Target Instance should have its own ROOT volume plus the restored volume") + + if data_disk_id: + # Restore and attach the DATADISK volume backup as well. + Backup.restoreVolumeFromBackupAndAttachToVM( + self.apiclient, backupid=backup.id, volumeid=data_disk_id, virtualmachineid=target_vm.id + ) + target_volumes = Volume.list(self.apiclient, virtualmachineid=target_vm.id, listall=True) + self.assertEqual(3, len(target_volumes), + "Target Instance should have 3 volumes after restoring both the ROOT and DATADISK backups") + + # Start the target Instance to verify the restored disk(s) are actually + # usable and libvirt accepted the attach-device/attach-disk calls. + target_vm.start(self.apiclient) + + Backup.delete(self.apiclient, backup.id) + finally: + if target_vm is not None: + try: + target_vm.delete(self.apiclient) + except Exception: + pass + self.backup_offering.removeOffering(self.apiclient, self.vm.id) From 5fa0054bc2e92fe7937d0f712ecb8df797bd6e15 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 12 Oct 2026 10:28:16 +0200 Subject: [PATCH 02/14] KVM: support incremental NAS backups of RBD volumes Incremental NAS backups so far relied on QEMU dirty bitmaps / libvirt checkpoints, which only exist on file-based qcow2 storage. RBD images are raw and have nowhere to persist a bitmap, so any VM with a volume on Ceph was forced onto the full-only path and re-uploaded the whole image on every cycle. Ceph provides the equivalent natively, and this adds a second chain format that uses it: an `rbd snap` per cycle as the anchor plus `rbd export-diff --from-snap`, which computes the changed extents inside Ceph from the image object-map. NASBackupProvider now selects the mechanism per VM via getIncrementalFlavor() - QCOW2 for checkpoint-capable pools, RBD for Ceph, UNSUPPORTED (legacy full-only) for mixed or other storage, since the two formats cannot be mixed inside one backup. Everything else - chain identity, the nas.backup.full.every cadence, delete/tombstone semantics and resource accounting - is shared unchanged. Restore differs because an export-diff stream carries no pointer to the diff before it, unlike a rebased qcow2. The provider therefore sends the whole chain from its full anchor up to the backup being restored, and the agent recreates the image and replays each diff with `rbd import-diff` in order. The image has to be recreated: a full diff only carries the extents that were allocated when it was taken, so any block still allocated in the target would otherwise show through as stale data. That also drops RBD snapshots the volume carried, which is inherent to rewinding a volume wholesale. Two behaviours follow from the anchor living in Ceph rather than in the QEMU process: a stopped VM keeps its incremental cadence (no live QEMU needed), and a chain survives a VM restart or host migration. Only the newest anchor is kept per image; a full additionally sweeps every older snapshot carrying the csbackup- prefix, so a deleted chain, an unassigned offering or a restore cannot leak snapshots. The feature stays behind the existing nas.backup.incremental.enabled zone setting, which still defaults to false. Note: the agent drives the `rbd` CLI, so KVM hosts backing up Ceph volumes need ceph-common installed. Verified on a Ceph Reef 18.2.8 cluster with KVM hosts on Ceph Quincy 17.2.7 clients: FULL 1.86 GB followed by incrementals of 71 KB - 2.6 MB, restores replaying 2- and 3-diff chains across two RBD pools including through a tombstoned ancestor. test_backup_recovery_nas.py: the blanket RBD skip is replaced by an _is_rbd_storage() predicate, so the five chain tests now run on Ceph. test_stopped_vm_falls_back_to_full keeps a targeted skip because RBD has the opposite behaviour by design, and a new test_stopped_rbd_vm_keeps_incremental_cadence asserts that other side. --- .../backup/RestoreBackupCommand.java | 32 ++ .../cloudstack/backup/NASBackupChainKeys.java | 25 +- .../cloudstack/backup/NASBackupProvider.java | 278 ++++++++++++---- .../backup/NASBackupProviderTest.java | 143 ++++++++- .../LibvirtRestoreBackupCommandWrapper.java | 301 ++++++++++++++++-- .../LibvirtTakeBackupCommandWrapper.java | 29 +- ...ibvirtRestoreBackupCommandWrapperTest.java | 92 ++++++ scripts/vm/hypervisor/kvm/nasbackup.sh | 296 ++++++++++++++++- .../smoke/test_backup_recovery_nas.py | 84 ++++- 9 files changed, 1157 insertions(+), 123 deletions(-) diff --git a/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java b/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java index 972c2eaf7bb4..a11f0d011398 100644 --- a/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java +++ b/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java @@ -41,6 +41,22 @@ public class RestoreBackupCommand extends Command { private VirtualMachine.State vmState; private Integer mountTimeout; + /** + * On-NAS format of the backup files: {@code qcow2} (default, also when null) or {@code rbddiff} + * for Ceph/RBD chains written with {@code rbd export-diff}. Decides both the file extension the + * agent looks for and how the volume is reconstructed. + */ + private String backupFormat; + + /** + * For {@code rbddiff} backups: every backup directory of the chain from its full anchor up to + * and including the backup being restored, ordered root-first. {@code rbd import-diff} streams + * have no backing-file metadata, so the chain cannot be self-describing the way rebased qcow2 + * files are — the agent has to be told which diffs to replay and in which order. Null/empty for + * qcow2 backups. + */ + private List backupChainPaths; + protected RestoreBackupCommand() { super(); } @@ -164,4 +180,20 @@ public Integer getMountTimeout() { public void setMountTimeout(Integer mountTimeout) { this.mountTimeout = mountTimeout; } + + public String getBackupFormat() { + return backupFormat; + } + + public void setBackupFormat(String backupFormat) { + this.backupFormat = backupFormat; + } + + public List getBackupChainPaths() { + return backupChainPaths; + } + + public void setBackupChainPaths(List backupChainPaths) { + this.backupChainPaths = backupChainPaths; + } } diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java index 511f0ccb7114..aaeafee5d7e2 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java @@ -27,9 +27,21 @@ 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 } reference for the next one. */ + /** + * Name of the point-in-time anchor this backup created, used as the parent reference for the + * next backup in the chain. Its meaning depends on the chain format: for {@link #FORMAT_QCOW2} + * chains it is the QEMU dirty-bitmap name used as the {@code } reference; for + * {@link #FORMAT_RBD_DIFF} chains it is the RBD snapshot name used as {@code --from-snap}. + */ public static final String BITMAP_NAME = "nas.bitmap_name"; + /** + * On-NAS format of the chain this backup belongs to: {@link #FORMAT_QCOW2} (rebased qcow2 + * files, the file-based storage path) or {@link #FORMAT_RBD_DIFF} ({@code rbd export-diff} + * streams, the Ceph/RBD path). Absent on legacy-full backups, which are always qcow2. + */ + public static final String CHAIN_FORMAT = "nas.chain_format"; + /** 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"; @@ -43,6 +55,13 @@ public final class NASBackupChainKeys { */ public static final String TYPE_FULL = "full"; public static final String TYPE_INCREMENTAL = "incremental"; + /** + * Ceph/RBD equivalents of {@link #TYPE_FULL} / {@link #TYPE_INCREMENTAL}. RBD images are raw + * and cannot carry a QEMU dirty bitmap, so these anchor each cycle on an RBD snapshot and + * export the delta with {@code rbd export-diff} instead of a libvirt checkpoint + push backup. + */ + public static final String TYPE_RBD_FULL = "rbd-full"; + public static final String TYPE_RBD_INCREMENTAL = "rbd-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). @@ -57,6 +76,10 @@ public final class NASBackupChainKeys { */ public static final String VM_ACTIVE_CHECKPOINT_ID = "nas.active_checkpoint_id"; + /** Chain format values persisted under {@link #CHAIN_FORMAT}; also the on-NAS file extension. */ + public static final String FORMAT_QCOW2 = "qcow2"; + public static final String FORMAT_RBD_DIFF = "rbddiff"; + 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 d08b4775c8bd..ad11d3ced9aa 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 @@ -86,6 +86,13 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co private static final int CHAIN_LOCK_TIMEOUT_SECONDS = 300; + /** + * Prefix of the RBD snapshots this provider creates as chain anchors. Deliberately distinct + * from the names CloudStack volume snapshots use, so an operator (and the agent's cleanup) can + * tell a backup anchor apart from a user snapshot on the same image. + */ + private static final String RBD_SNAPSHOT_PREFIX = "csbackup-"; + ConfigKey NASBackupRestoreMountTimeout = new ConfigKey<>("Advanced", Integer.class, "nas.backup.restore.mount.timeout", "30", @@ -214,9 +221,12 @@ protected Host getVMHypervisorHost(VirtualMachine vm) { * parent bitmap and parent file path. */ static final class ChainDecision { - final String mode; // "full" or "incremental" + final String mode; // "full"/"incremental", or their "rbd-" prefixed equivalents + // On-NAS format of the chain this backup belongs to: NASBackupChainKeys.FORMAT_QCOW2 for + // the libvirt-checkpoint path, FORMAT_RBD_DIFF for the Ceph-native rbd export-diff path. + final String format; final String bitmapNew; - final String bitmapParent; // null for full + final String bitmapParent; // the anchor to read changes since; null for a 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..qcow2 / datadisk..qcow2). @@ -224,9 +234,10 @@ static final class ChainDecision { 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 parentPaths, + private ChainDecision(String mode, String format, String bitmapNew, String bitmapParent, List parentPaths, String chainId, int chainPosition) { this.mode = mode; + this.format = format; this.bitmapNew = bitmapNew; this.bitmapParent = bitmapParent; this.parentPaths = parentPaths; @@ -235,8 +246,29 @@ private ChainDecision(String mode, String bitmapNew, String bitmapParent, List parentPaths, String chainId, int chainPosition) { - return new ChainDecision(NASBackupChainKeys.TYPE_INCREMENTAL, bitmapNew, bitmapParent, - parentPaths, chainId, chainPosition); + return new ChainDecision(NASBackupChainKeys.TYPE_INCREMENTAL, NASBackupChainKeys.FORMAT_QCOW2, + bitmapNew, bitmapParent, parentPaths, chainId, chainPosition); } boolean isIncremental() { - return NASBackupChainKeys.TYPE_INCREMENTAL.equals(mode); + return NASBackupChainKeys.TYPE_INCREMENTAL.equals(mode) + || NASBackupChainKeys.TYPE_RBD_INCREMENTAL.equals(mode); } boolean isLegacyFull() { return NASBackupChainKeys.TYPE_LEGACY_FULL.equals(mode); } + + boolean isRbd() { + return NASBackupChainKeys.FORMAT_RBD_DIFF.equals(format); + } + + /** + * The same decision re-cast as the start of a brand new chain, keeping the anchor the agent + * actually created and the storage flavour. Used when the agent reports it could not + * proceed as an incremental and completed a full instead. + */ + ChainDecision asFullRestart() { + return isRbd() ? rbdFullStart(bitmapNew) : fullStart(bitmapNew); + } + } + + /** + * Which incremental mechanism, if any, a VM's volumes can support. + */ + protected enum IncrementalFlavor { + /** Every volume is on file-based qcow2 storage: libvirt checkpoints / QEMU dirty bitmaps. */ + QCOW2, + /** Every volume is on Ceph/RBD: RBD snapshots + {@code rbd export-diff}. */ + RBD, + /** Mixed or otherwise incapable storage — the VM stays on the full-only (legacy) path. */ + UNSUPPORTED } /** @@ -284,103 +343,155 @@ protected ChainDecision decideChain(VirtualMachine vm) { 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)) { + // Pick the incremental mechanism that matches the VM's storage. Mixed or incapable + // storage stays on the full-only (legacy) path. + IncrementalFlavor flavor = getIncrementalFlavor(vm); + if (IncrementalFlavor.UNSUPPORTED.equals(flavor)) { return ChainDecision.legacyFull(); } + final boolean rbd = IncrementalFlavor.RBD.equals(flavor); - final String newBitmap = "backup-" + System.currentTimeMillis() / 1000L; + final String newAnchor = (rbd ? RBD_SNAPSHOT_PREFIX : "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); + // A QEMU dirty bitmap needs a live QEMU process, so a stopped VM always takes a full. + // RBD snapshots are taken inside Ceph and work exactly the same whether the VM runs or + // not, so RBD chains keep their cadence across a stop/start. + if (!rbd && VirtualMachine.State.Stopped.equals(vm.getState())) { + return ChainDecision.fullStart(newAnchor); } + // The VM's active_checkpoint_id records the anchor that currently exists on the storage: + // the QEMU bitmap for qcow2 chains, the RBD snapshot for RBD chains. Null means there is + // nothing to base an incremental on (first backup, post-restore, freshly assigned offering). + String activeCheckpoint = readVmActiveCheckpoint(vm.getId()); + Integer fullEvery = NASBackupFullEvery.valueIn(vm.getDataCenterId()); if (fullEvery == null || fullEvery <= 1) { // Disabled or every-backup-is-full mode. - return ChainDecision.fullStart(newBitmap); + return fullDecision(rbd, newAnchor); } - // 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()); + // 1. No anchor on the storage => nothing to take an incremental against. if (activeCheckpoint == null) { - return ChainDecision.fullStart(newBitmap); + return fullDecision(rbd, newAnchor); } // 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 + // next backup is always a fresh full, so anything older has a rotated-out anchor. + // If the latest backup's anchor 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); + return fullDecision(rbd, newAnchor); + } + + // A chain cannot change storage flavour halfway through: the on-NAS files of a qcow2 chain + // and of an RBD chain are not interchangeable. A volume migration between pool types + // therefore starts a new chain in the new format. + String parentFormat = chainFormatOf(parent); + String requiredFormat = rbd ? NASBackupChainKeys.FORMAT_RBD_DIFF : NASBackupChainKeys.FORMAT_QCOW2; + if (!requiredFormat.equals(parentFormat)) { + LOG.debug("VM {} parent backup {} is a {} chain but the VM's storage now needs {} — forcing full", + vm.getInstanceName(), parent.getUuid(), parentFormat, requiredFormat); + return fullDecision(rbd, newAnchor); } String parentChainId = readDetail(parent, NASBackupChainKeys.CHAIN_ID); int parentChainPosition = chainPosition(parent); if (parentChainId == null || parentChainPosition == Integer.MAX_VALUE) { - return ChainDecision.fullStart(newBitmap); + return fullDecision(rbd, newAnchor); } // Force a fresh full when the chain has reached the configured length. if (parentChainPosition + 1 >= fullEvery) { - return ChainDecision.fullStart(newBitmap); + return fullDecision(rbd, newAnchor); } // 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 parentPaths = composeParentBackupPaths(parent, vm.getId()); + // so we don't risk corrupting the chain. RBD chains don't rebase, but the same call + // still validates that the parent's volume layout still matches the VM's. + List parentPaths = composeParentBackupPaths(parent, vm.getId(), requiredFormat); 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 fullDecision(rbd, newAnchor); } - return ChainDecision.incremental(newBitmap, activeCheckpoint, parentPaths, - parentChainId, parentChainPosition + 1); + return rbd + ? ChainDecision.rbdIncremental(newAnchor, activeCheckpoint, parentChainId, parentChainPosition + 1) + : ChainDecision.incremental(newAnchor, activeCheckpoint, parentPaths, parentChainId, parentChainPosition + 1); + } + + /** Start of a fresh chain in the format the VM's storage calls for. */ + private ChainDecision fullDecision(boolean rbd, String newAnchor) { + return rbd ? ChainDecision.rbdFullStart(newAnchor) : ChainDecision.fullStart(newAnchor); } /** - * 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). + * The on-NAS format of {@code backup}'s chain. Backups taken before chain formats existed (and + * legacy fulls) carry no detail and are qcow2. */ - protected boolean allVolumesOnCheckpointCapableStorage(VirtualMachine vm) { + private String chainFormatOf(Backup backup) { + String format = readDetail(backup, NASBackupChainKeys.CHAIN_FORMAT); + return format == null ? NASBackupChainKeys.FORMAT_QCOW2 : format; + } + + /** + * Decide which incremental mechanism a VM's volumes can support. + * + *

{@link IncrementalFlavor#QCOW2} needs QEMU dirty bitmaps / libvirt checkpoints, which only + * exist on file-based qcow2 storage: HOST-scope local, {@code SharedMountPoint} or + * {@code NetworkFilesystem} (NFS) pools.

+ * + *

{@link IncrementalFlavor#RBD} uses Ceph's own primitives instead — an {@code rbd snap} per + * cycle plus {@code rbd export-diff --from-snap}, computed inside Ceph from the image's + * object-map/fast-diff. RBD images are raw and have nowhere to persist a QEMU bitmap, so they + * could never join a qcow2 chain, but they need no host-side checkpoint at all.

+ * + *

The two mechanisms produce different on-NAS files and cannot be mixed inside one backup, + * so a VM whose volumes span both (or sits on any other pool type, e.g. Linstor) is reported + * {@link IncrementalFlavor#UNSUPPORTED} and stays on the legacy full-only path. A volume whose + * pool can no longer be resolved is treated as incapable (safe default).

+ */ + protected IncrementalFlavor getIncrementalFlavor(VirtualMachine vm) { List volumes = volumeDao.findByInstance(vm.getId()); - if (volumes == null) { - return true; + if (CollectionUtils.isEmpty(volumes)) { + return IncrementalFlavor.QCOW2; } + IncrementalFlavor flavor = null; 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; + return IncrementalFlavor.UNSUPPORTED; } - boolean checkpointCapable = ScopeType.HOST.equals(pool.getScope()) + IncrementalFlavor volumeFlavor; + if (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", + || Storage.StoragePoolType.NetworkFilesystem.equals(pool.getPoolType())) { + volumeFlavor = IncrementalFlavor.QCOW2; + } else if (Storage.StoragePoolType.RBD.equals(pool.getPoolType())) { + volumeFlavor = IncrementalFlavor.RBD; + } else { + LOG.debug("VM {} volume {} is on {} (scope {}) which supports neither checkpoints nor " + + "RBD snapshots — forcing legacy full", vm.getInstanceName(), volume.getUuid(), pool.getPoolType(), pool.getScope()); - return false; + return IncrementalFlavor.UNSUPPORTED; + } + if (flavor == null) { + flavor = volumeFlavor; + } else if (!flavor.equals(volumeFlavor)) { + LOG.debug("VM {} has volumes on both checkpoint-capable and RBD storage; a single backup " + + "cannot mix the two mechanisms — forcing legacy full", vm.getInstanceName()); + return IncrementalFlavor.UNSUPPORTED; } } - return true; + return flavor; } /** @@ -421,8 +532,8 @@ private String readDetail(Backup backup, String key) { * 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 /root..qcow2} - * others -> {@code /datadisk..qcow2} + * first disk -> {@code /root..} + * others -> {@code /datadisk..} * * 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; @@ -431,7 +542,7 @@ private String readDetail(Backup backup, String key) { * identities — a count mismatch is the only way to reach this branch with a non-null * active_checkpoint_id. */ - private List composeParentBackupPaths(Backup parent, long vmId) { + private List composeParentBackupPaths(Backup parent, long vmId, String extension) { // backupPath is stored as externalId by createBackupObject — e.g. // "i-2-1234-VM/2026.04.27.13.45.00". String dir = parent.getExternalId(); @@ -462,7 +573,7 @@ private List composeParentBackupPaths(Backup parent, long vmId) { 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"); + paths.add(dir + "/" + prefix + "." + volPath + "." + extension); } return paths; } @@ -482,6 +593,12 @@ private void persistChainMetadata(Backup backup, ChainDecision decision, String backupDetailsDao.persist(new BackupDetailVO(backup.getId(), NASBackupChainKeys.BITMAP_NAME, bitmapFromAgent, true)); } backupDetailsDao.persist(new BackupDetailVO(backup.getId(), NASBackupChainKeys.CHAIN_ID, decision.chainId, true)); + // Restore has to know how to read the files back: rebased qcow2, or an ordered replay of + // rbd export-diff streams. Only recorded for RBD chains — no detail means qcow2. + if (NASBackupChainKeys.FORMAT_RBD_DIFF.equals(decision.format)) { + backupDetailsDao.persist(new BackupDetailVO(backup.getId(), NASBackupChainKeys.CHAIN_FORMAT, + NASBackupChainKeys.FORMAT_RBD_DIFF, 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 @@ -588,7 +705,10 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce command.setBitmapParent(decision.bitmapParent); command.setParentPaths(decision.parentPaths); - if (VirtualMachine.State.Stopped.equals(vm.getState())) { + // The agent needs the per-volume pool + path to reach the disks itself whenever it cannot + // read them out of a running libvirt domain: for a stopped VM, and for every RBD backup + // (the RBD path drives `rbd` against the images directly, running or not). + if (VirtualMachine.State.Stopped.equals(vm.getState()) || decision.isRbd()) { List vmVolumes = volumeDao.findByInstance(vm.getId()); vmVolumes.sort(Comparator.comparing(Volume::getDeviceId)); Pair, List> volumePoolsAndPaths = getVolumePoolsAndPaths(vmVolumes); @@ -619,7 +739,7 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce // backup as a full and start a new chain. ChainDecision effective = decision; if (answer.getIncrementalFallback()) { - effective = ChainDecision.fullStart(decision.bitmapNew); + effective = decision.asFullRestart(); backupVO.setType("FULL"); } List volumes = new ArrayList<>(volumeDao.findByInstance(vm.getId())); @@ -719,6 +839,7 @@ private Pair restoreVMBackup(VirtualMachine vm, Backup backup) restoreCommand.setRestoreVolumePools(volumePoolsAndPaths.first()); restoreCommand.setRestoreVolumePaths(volumePoolsAndPaths.second()); restoreCommand.setBackupFiles(getBackupFiles(backedVolumes)); + applyBackupFormat(restoreCommand, backup); restoreCommand.setVmExists(vm.getRemoved() == null); restoreCommand.setVmState(vm.getState()); restoreCommand.setMountTimeout(NASBackupRestoreMountTimeout.value()); @@ -742,6 +863,48 @@ private Pair restoreVMBackup(VirtualMachine vm, Backup backup) return new Pair<>(answer.getResult(), answer.getDetails()); } + /** + * Tell the agent how to read {@code backup} back. qcow2 chains are self-describing — every + * incremental was rebased onto its parent, so handing the agent the leaf file is enough. RBD + * chains are not: an {@code rbd export-diff} stream carries no pointer to the diff before it, + * so the whole chain from the full anchor up to this backup travels with the command and the + * agent replays it in order. + */ + private void applyBackupFormat(RestoreBackupCommand restoreCommand, Backup backup) { + if (!NASBackupChainKeys.FORMAT_RBD_DIFF.equals(chainFormatOf(backup))) { + return; + } + restoreCommand.setBackupFormat(NASBackupChainKeys.FORMAT_RBD_DIFF); + restoreCommand.setBackupChainPaths(composeRestoreChainDirs(backup)); + } + + /** + * The backup directories to replay to reconstruct {@code backup}, ordered root-first: the + * chain's full anchor, every incremental after it, and {@code backup} itself. + * + *

Ancestors that a user asked to delete are only tombstoned (Hidden) while descendants are + * alive, precisely so their files survive on the NAS for this walk. A gap here therefore means + * the chain really was broken, and replaying what is left would silently restore a mix of two + * points in time — so we refuse instead.

+ */ + private List composeRestoreChainDirs(Backup backup) { + int targetPosition = chainPosition(backup); + List dirs = new ArrayList<>(); + for (Backup member : getChainOrderedLeafToRoot(backup)) { + if (chainPosition(member) <= targetPosition) { + dirs.add(member.getExternalId()); + } + } + // getChainOrderedLeafToRoot sorts by descending chain position; restore replays root-first. + Collections.reverse(dirs); + if (targetPosition != Integer.MAX_VALUE && dirs.size() != targetPosition + 1) { + throw new CloudRuntimeException(String.format( + "Cannot restore backup %s: its chain is incomplete (%d of the %d backups it depends on are present).", + backup.getUuid(), dirs.size(), targetPosition + 1)); + } + return dirs; + } + private List getBackupFiles(List backedVolumes) { List backupFiles = new ArrayList<>(); for (Backup.VolumeInfo backedVolume : backedVolumes) { @@ -848,6 +1011,7 @@ public Pair restoreBackedUpVolume(Backup backup, Backup.VolumeI restoreCommand.setVmState(vmNameAndState.second()); restoreCommand.setMountTimeout(NASBackupRestoreMountTimeout.value()); restoreCommand.setBackupFiles(Collections.singletonList(matchingVolume.getPath())); + applyBackupFormat(restoreCommand, backup); BackupAnswer answer; try { 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 f1d5613ab7fc..fc11b53d488e 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 @@ -450,6 +450,91 @@ public void decideChainReturnsFullWhenVmHasNoActiveCheckpoint() { Assert.assertEquals(0, decision.chainPosition); } + /** Enable the zone-level incremental master switch for a decideChain test. */ + private void enableIncrementalBackups() { + 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)); + } + + /** Give {@code vmId} a single volume on a Ceph-RBD pool. */ + private void givenVmVolumesOnRbd(Long vmId) { + VolumeVO rbdVol = mock(VolumeVO.class); + Mockito.when(rbdVol.getPoolId()).thenReturn(9L); + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(rbdVol)); + StoragePoolVO rbd = mock(StoragePoolVO.class); + Mockito.when(rbd.getPoolType()).thenReturn(Storage.StoragePoolType.RBD); + Mockito.when(storagePoolDao.findById(9L)).thenReturn(rbd); + } + + /** + * A VM entirely on Ceph-RBD no longer drops to the legacy full-only path: it anchors its chain + * on an RBD snapshot and its backups are written as rbd export-diff streams. + */ + @Test + public void decideChainStartsAnRbdChainForRbdBackedVm() { + Long vmId = 60L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.lenient().when(vm.getState()).thenReturn(VMInstanceVO.State.Running); + enableIncrementalBackups(); + givenVmVolumesOnRbd(vmId); + Mockito.when(vmInstanceDetailsDao.findDetail(vmId, NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID)).thenReturn(null); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + + Assert.assertEquals(NASBackupChainKeys.TYPE_RBD_FULL, decision.mode); + Assert.assertEquals(NASBackupChainKeys.FORMAT_RBD_DIFF, decision.format); + Assert.assertTrue("RBD chains anchor on an RBD snapshot, not a QEMU bitmap", + decision.bitmapNew.startsWith("csbackup-")); + Assert.assertNull("a full reads nothing from a parent anchor", decision.bitmapParent); + Assert.assertNull("RBD backups are replayed in order, not rebased onto a parent file", decision.parentPaths); + Assert.assertEquals(0, decision.chainPosition); + } + + /** + * RBD snapshots are taken inside Ceph, so a stopped VM is no different from a running one. It + * must stay on the RBD path rather than being pushed onto a qcow2 full the way a stopped VM on + * file-based storage is (which has no live QEMU to hold a dirty bitmap). + */ + @Test + public void decideChainKeepsStoppedRbdVmOnTheRbdPath() { + Long vmId = 61L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.lenient().when(vm.getState()).thenReturn(VMInstanceVO.State.Stopped); + enableIncrementalBackups(); + givenVmVolumesOnRbd(vmId); + Mockito.when(vmInstanceDetailsDao.findDetail(vmId, NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID)).thenReturn(null); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + + Assert.assertEquals(NASBackupChainKeys.TYPE_RBD_FULL, decision.mode); + Assert.assertEquals(NASBackupChainKeys.FORMAT_RBD_DIFF, decision.format); + } + + /** + * When the agent reports it completed a full after failing to proceed as an incremental, the + * backup is re-registered as the start of a new chain — in the same storage flavour, so an RBD + * backup is not accidentally recorded as a qcow2 one. + */ + @Test + public void asFullRestartKeepsTheRbdFlavour() { + NASBackupProvider.ChainDecision incremental = + NASBackupProvider.ChainDecision.rbdIncremental("csbackup-2", "csbackup-1", "chain-1", 3); + + NASBackupProvider.ChainDecision restarted = incremental.asFullRestart(); + + Assert.assertEquals(NASBackupChainKeys.TYPE_RBD_FULL, restarted.mode); + Assert.assertEquals(NASBackupChainKeys.FORMAT_RBD_DIFF, restarted.format); + Assert.assertEquals("csbackup-2", restarted.bitmapNew); + Assert.assertEquals(0, restarted.chainPosition); + } + // -- incremental storage-capability guard (Ceph-RBD / Linstor stay on legacy full) ---- /** @@ -457,7 +542,7 @@ public void decideChainReturnsFullWhenVmHasNoActiveCheckpoint() { * volume sits on NFS / HOST-scope local / SharedMountPoint is checkpoint-capable. */ @Test - public void allVolumesOnCheckpointCapableStorageTrueForNfsHostAndSharedMount() { + public void getIncrementalFlavorIsQcow2ForNfsHostAndSharedMount() { Long vmId = 55L; VMInstanceVO vm = mock(VMInstanceVO.class); Mockito.when(vm.getId()).thenReturn(vmId); @@ -480,15 +565,15 @@ public void allVolumesOnCheckpointCapableStorageTrueForNfsHostAndSharedMount() { Mockito.when(storagePoolDao.findById(2L)).thenReturn(host); Mockito.when(storagePoolDao.findById(3L)).thenReturn(smp); - Assert.assertTrue(nasBackupProvider.allVolumesOnCheckpointCapableStorage(vm)); + Assert.assertEquals(NASBackupProvider.IncrementalFlavor.QCOW2, nasBackupProvider.getIncrementalFlavor(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. + * The two mechanisms produce different on-NAS files, so a VM straddling file-based storage and + * Ceph-RBD cannot use either and stays on the legacy full-only path. */ @Test - public void allVolumesOnCheckpointCapableStorageFalseWhenAnyVolumeOnRbd() { + public void getIncrementalFlavorIsUnsupportedForMixedFileAndRbdStorage() { Long vmId = 56L; VMInstanceVO vm = mock(VMInstanceVO.class); Mockito.when(vm.getId()).thenReturn(vmId); @@ -506,12 +591,54 @@ public void allVolumesOnCheckpointCapableStorageFalseWhenAnyVolumeOnRbd() { Mockito.when(storagePoolDao.findById(1L)).thenReturn(nfs); Mockito.when(storagePoolDao.findById(9L)).thenReturn(rbd); - Assert.assertFalse(nasBackupProvider.allVolumesOnCheckpointCapableStorage(vm)); + Assert.assertEquals(NASBackupProvider.IncrementalFlavor.UNSUPPORTED, nasBackupProvider.getIncrementalFlavor(vm)); + } + + /** + * Every volume on Ceph-RBD selects the Ceph-native mechanism: an RBD snapshot per cycle plus + * {@code rbd export-diff}, instead of a libvirt checkpoint that a raw RBD image cannot hold. + */ + @Test + public void getIncrementalFlavorIsRbdWhenEveryVolumeIsOnRbd() { + Long vmId = 58L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + + VolumeVO rootVol = mock(VolumeVO.class); + Mockito.when(rootVol.getPoolId()).thenReturn(9L); + VolumeVO dataVol = mock(VolumeVO.class); + Mockito.when(dataVol.getPoolId()).thenReturn(10L); + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(rootVol, dataVol)); + + StoragePoolVO rbd = mock(StoragePoolVO.class); + Mockito.when(rbd.getPoolType()).thenReturn(Storage.StoragePoolType.RBD); + StoragePoolVO otherRbd = mock(StoragePoolVO.class); + Mockito.when(otherRbd.getPoolType()).thenReturn(Storage.StoragePoolType.RBD); + Mockito.when(storagePoolDao.findById(9L)).thenReturn(rbd); + Mockito.when(storagePoolDao.findById(10L)).thenReturn(otherRbd); + + Assert.assertEquals(NASBackupProvider.IncrementalFlavor.RBD, nasBackupProvider.getIncrementalFlavor(vm)); + } + + /** Linstor supports neither mechanism, so it keeps the legacy full-only behaviour. */ + @Test + public void getIncrementalFlavorIsUnsupportedForLinstor() { + Long vmId = 59L; + 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)); + StoragePoolVO linstor = mock(StoragePoolVO.class); + Mockito.when(linstor.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + Mockito.when(storagePoolDao.findById(1L)).thenReturn(linstor); + + Assert.assertEquals(NASBackupProvider.IncrementalFlavor.UNSUPPORTED, nasBackupProvider.getIncrementalFlavor(vm)); } /** A volume whose storage pool can no longer be resolved is treated as incapable (safe). */ @Test - public void allVolumesOnCheckpointCapableStorageFalseWhenPoolUnresolvable() { + public void getIncrementalFlavorIsUnsupportedWhenPoolUnresolvable() { Long vmId = 57L; VMInstanceVO vm = mock(VMInstanceVO.class); Mockito.when(vm.getId()).thenReturn(vmId); @@ -520,7 +647,7 @@ public void allVolumesOnCheckpointCapableStorageFalseWhenPoolUnresolvable() { Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(vol)); Mockito.when(storagePoolDao.findById(1L)).thenReturn(null); - Assert.assertFalse(nasBackupProvider.allVolumesOnCheckpointCapableStorage(vm)); + Assert.assertEquals(NASBackupProvider.IncrementalFlavor.UNSUPPORTED, nasBackupProvider.getIncrementalFlavor(vm)); } // -- restore clears active_checkpoint_id --------------------------------------------- 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 ccd0ec634525..b43e6cffe1f4 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 @@ -24,11 +24,16 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermissions; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.cloudstack.backup.BackupAnswer; import org.apache.cloudstack.backup.RestoreBackupCommand; @@ -62,6 +67,56 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper RBD_IMAGE_FEATURES = + List.of("layering", "exclusive-lock", "object-map", "fast-diff", "deep-flatten"); + + /** + * How the backup being restored is laid out on the NAS. + * + *

A qcow2 backup is a single self-describing file per volume: incrementals were rebased onto + * their parent, so following the backing chain from the leaf is enough. An {@code rbddiff} + * backup is a chain of {@code rbd export-diff} streams with no link between them, so the whole + * chain travels with the command and every diff is replayed in order.

+ * + *

Held as a value object rather than wrapper state: command wrappers are shared singletons + * and must stay stateless.

+ */ + private static final class BackupLayout { + private final String format; + private final List chainDirs; // root-first; exactly one entry for qcow2 + + private BackupLayout(RestoreBackupCommand command) { + this.format = FORMAT_RBD_DIFF.equals(command.getBackupFormat()) ? FORMAT_RBD_DIFF : FORMAT_QCOW2; + List dirs = command.getBackupChainPaths(); + this.chainDirs = (dirs == null || dirs.isEmpty()) + ? Collections.singletonList(command.getBackupPath()) + : dirs; + } + + private boolean isRbdDiff() { + return FORMAT_RBD_DIFF.equals(format); + } + + /** + * Every on-NAS file that has to be read for one volume, oldest first. For qcow2 that is the + * single leaf file; for rbddiff, the full anchor followed by each incremental. + */ + private List filesFor(String mountDirectory, String backupFile, String diskType) { + String fileName = String.format("%s.%s.%s", diskType.toLowerCase(Locale.ROOT), backupFile, format); + List files = new ArrayList<>(chainDirs.size()); + for (String dir : chainDirs) { + files.add(String.format(FILE_PATH_PLACEHOLDER, String.format(FILE_PATH_PLACEHOLDER, mountDirectory, dir), fileName)); + } + return files; + } + } private String getVolumeUuidFromPath(String volumePath, PrimaryDataStoreTO volumePool) { if (Storage.StoragePoolType.Linstor.equals(volumePool.getPoolType())) { @@ -80,7 +135,6 @@ private String getVolumeUuidFromPath(String volumePath, PrimaryDataStoreTO volum @Override public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) { String vmName = command.getVmName(); - String backupPath = command.getBackupPath(); String backupRepoAddress = command.getBackupRepoAddress(); String backupRepoType = command.getBackupRepoType(); String mountOptions = command.getMountOptions(); @@ -93,6 +147,7 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser int timeout = command.getWait() > 0 ? command.getWait() * 1000 : serverResource.getCmdsTimeout(); KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr(); List backupFiles = command.getBackupFiles(); + BackupLayout layout = new BackupLayout(command); String newVolumeId = null; try { @@ -103,12 +158,12 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser String backupFile = backupFiles.get(0); newVolumeId = getVolumeUuidFromPath(volumePath, volumePool); Long size = command.getRestoreVolumeSizes().get(0); - restoreVolume(storagePoolMgr, backupPath, volumePool, volumePath, diskType, backupFile, size, - new Pair<>(vmName, command.getVmState()), mountDirectory, timeout, mountTimeout); + restoreVolume(storagePoolMgr, volumePool, volumePath, diskType, backupFile, size, + new Pair<>(vmName, command.getVmState()), mountDirectory, layout, timeout, mountTimeout); } else if (Boolean.TRUE.equals(vmExists)) { - restoreVolumesOfExistingVM(storagePoolMgr, restoreVolumePools, restoreVolumePaths, backedVolumeUUIDs, backupPath, backupFiles, mountDirectory, timeout, mountTimeout); + restoreVolumesOfExistingVM(storagePoolMgr, restoreVolumePools, restoreVolumePaths, backedVolumeUUIDs, backupFiles, mountDirectory, layout, timeout, mountTimeout); } else { - restoreVolumesOfDestroyedVMs(storagePoolMgr, restoreVolumePools, restoreVolumePaths, backupPath, backupFiles, mountDirectory, timeout, mountTimeout); + restoreVolumesOfDestroyedVMs(storagePoolMgr, restoreVolumePools, restoreVolumePaths, backupFiles, mountDirectory, layout, timeout, mountTimeout); } } catch (CloudRuntimeException e) { String errorMessage = e.getMessage() != null ? e.getMessage() : ""; @@ -118,18 +173,30 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser return new BackupAnswer(command, true, newVolumeId); } - private void verifyBackupFile(String backupPath, String volUuid) { - if (!checkBackupPathExists(backupPath)) { - throw new CloudRuntimeException(String.format("Backup file for the volume [%s] does not exist.", volUuid)); + /** + * Check every file the volume will be rebuilt from. qcow2 leaves are additionally run through + * {@code qemu-img check}; an {@code rbd export-diff} stream is not a disk image, so there the + * check is existence only — a truncated or corrupt diff is caught by {@code rbd import-diff}, + * which refuses to apply it. + */ + private void verifyBackupFiles(List backupFiles, BackupLayout layout, String volUuid) { + for (String file : backupFiles) { + if (!checkBackupPathExists(file)) { + throw new CloudRuntimeException(String.format("Backup file for the volume [%s] does not exist: %s", volUuid, file)); + } } - if (!checkBackupFileImage(backupPath)) { + if (layout.isRbdDiff()) { + return; + } + String leaf = backupFiles.get(backupFiles.size() - 1); + if (!checkBackupFileImage(leaf)) { throw new CloudRuntimeException(String.format("Backup qcow2 file for the volume [%s] is corrupt.", volUuid)); } } private void restoreVolumesOfExistingVM(KVMStoragePoolManager storagePoolMgr, List restoreVolumePools, List restoreVolumePaths, List backedVolumesUUIDs, - String backupPath, List backupFiles, String mountDirectory, int timeout, Integer mountTimeout) { + List backupFiles, String mountDirectory, BackupLayout layout, int timeout, Integer mountTimeout) { String diskType = "root"; try { for (int idx = 0; idx < restoreVolumePaths.size(); idx++) { @@ -137,11 +204,11 @@ private void restoreVolumesOfExistingVM(KVMStoragePoolManager storagePoolMgr, Li String restoreVolumePath = restoreVolumePaths.get(idx); String backupFile = backupFiles.get(idx); String backupVolumeUuid = backedVolumesUUIDs.get(idx); - String fullPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType); + List volumeBackupFiles = layout.filesFor(mountDirectory, backupFile, diskType); diskType = "datadisk"; - verifyBackupFile(fullPath, backupVolumeUuid); - if (!replaceVolumeWithBackup(storagePoolMgr, restoreVolumePool, restoreVolumePath, fullPath, timeout)) { + verifyBackupFiles(volumeBackupFiles, layout, backupVolumeUuid); + if (!replaceVolumeWithBackup(storagePoolMgr, restoreVolumePool, restoreVolumePath, volumeBackupFiles, layout, timeout)) { throw new CloudRuntimeException(String.format("Unable to restore contents from the backup volume [%s].", backupVolumeUuid)); } } @@ -152,18 +219,19 @@ private void restoreVolumesOfExistingVM(KVMStoragePoolManager storagePoolMgr, Li } private void restoreVolumesOfDestroyedVMs(KVMStoragePoolManager storagePoolMgr, List volumePools, - List volumePaths, String backupPath, List backupFiles, String mountDirectory, int timeout, Integer mountTimeout) { + List volumePaths, List backupFiles, String mountDirectory, + BackupLayout layout, int timeout, Integer mountTimeout) { String diskType = "root"; try { for (int i = 0; i < volumePaths.size(); i++) { PrimaryDataStoreTO volumePool = volumePools.get(i); String volumePath = volumePaths.get(i); String backupFile = backupFiles.get(i); - String bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType); + List volumeBackupFiles = layout.filesFor(mountDirectory, backupFile, diskType); String volumeUuid = getVolumeUuidFromPath(volumePath, volumePool); diskType = "datadisk"; - verifyBackupFile(bkpPath, volumeUuid); - if (!replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, bkpPath, timeout)) { + verifyBackupFiles(volumeBackupFiles, layout, volumeUuid); + if (!replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, volumeBackupFiles, layout, timeout)) { throw new CloudRuntimeException(String.format("Unable to restore contents from the backup volume [%s].", volumeUuid)); } } @@ -173,15 +241,15 @@ private void restoreVolumesOfDestroyedVMs(KVMStoragePoolManager storagePoolMgr, } } - private void restoreVolume(KVMStoragePoolManager storagePoolMgr, String backupPath, PrimaryDataStoreTO volumePool, String volumePath, String diskType, String backupFile, - Long size, Pair vmNameAndState, String mountDirectory, int timeout, Integer mountTimeout) { - String bkpPath; + private void restoreVolume(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String diskType, String backupFile, + Long size, Pair vmNameAndState, String mountDirectory, BackupLayout layout, int timeout, Integer mountTimeout) { + List volumeBackupFiles; String volumeUuid; try { - bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType); + volumeBackupFiles = layout.filesFor(mountDirectory, backupFile, diskType); volumeUuid = getVolumeUuidFromPath(volumePath, volumePool); - verifyBackupFile(bkpPath, volumeUuid); - if (!replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, bkpPath, timeout, true, size)) { + verifyBackupFiles(volumeBackupFiles, layout, volumeUuid); + if (!replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, volumeBackupFiles, layout, timeout, true, size)) { throw new CloudRuntimeException(String.format("Unable to restore contents from the backup volume [%s].", volumeUuid)); } @@ -275,13 +343,6 @@ private void deleteTemporaryDirectory(String backupDirectory) { } } - private String getBackupPath(String mountDirectory, String backupPath, String backupFile, String diskType) { - String bkpPath = String.format(FILE_PATH_PLACEHOLDER, mountDirectory, backupPath); - String backupFileName = String.format("%s.%s.qcow2", diskType.toLowerCase(Locale.ROOT), backupFile); - bkpPath = String.format(FILE_PATH_PLACEHOLDER, bkpPath, backupFileName); - return bkpPath; - } - private boolean checkBackupFileImage(String backupPath) { int exitValue = Script.runSimpleBashScriptForExitValue(String.format("qemu-img check %s", backupPath)); return exitValue == 0; @@ -292,11 +353,18 @@ private boolean checkBackupPathExists(String backupPath) { return exitValue == 0; } - private boolean replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String backupPath, int timeout) { - return replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, backupPath, timeout, false, null); + private boolean replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, + List backupFiles, BackupLayout layout, int timeout) { + return replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, backupFiles, layout, timeout, false, null); } - private boolean replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String backupPath, int timeout, boolean createTargetVolume, Long size) { + private boolean replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, + List backupFiles, BackupLayout layout, int timeout, boolean createTargetVolume, Long size) { + if (layout.isRbdDiff()) { + return restoreRbdImageFromDiffChain(storagePoolMgr, volumePool, volumePath, backupFiles, size); + } + + String backupPath = backupFiles.get(backupFiles.size() - 1); if (List.of(Storage.StoragePoolType.RBD, Storage.StoragePoolType.Linstor).contains(volumePool.getPoolType())) { return replaceBlockDeviceWithBackup(storagePoolMgr, volumePool, volumePath, backupPath, timeout, createTargetVolume, size); } @@ -381,6 +449,173 @@ private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMg return true; } + /** + * Rebuild an RBD image by replaying a chain of {@code rbd export-diff} streams into it: the + * chain's full anchor first, then each incremental in the order it was taken. + * + *

RBD images are raw and cannot hold a backing-file reference, so — unlike the qcow2 path — + * a diff carries no pointer to the one before it and the chain cannot be flattened by + * {@code qemu-img}. Ceph applies the deltas natively instead: each diff ends at the snapshot + * the backup was taken from, {@code import-diff} recreates that snapshot on the target, and the + * next diff in the chain is validated against it. A truncated or out-of-order diff is therefore + * rejected by Ceph rather than silently producing a half-restored image.

+ * + *

The image is recreated first. A full diff only carries the extents that were allocated + * when it was taken, so any block still allocated in the current image would otherwise show + * through the restore as stale data. Recreating the image also drops any RBD snapshots it + * carried, including CloudStack volume snapshots kept on primary storage — a restore rewinds + * the volume wholesale, and there is no way to guarantee the first requirement while keeping + * snapshots of content that is being replaced.

+ */ + private boolean restoreRbdImageFromDiffChain(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, + String volumePath, List diffFiles, Long size) { + if (!Storage.StoragePoolType.RBD.equals(volumePool.getPoolType())) { + logger.error("Backup [{}] holds RBD diffs but volume [{}] is on {} storage; it cannot be restored there.", + diffFiles.get(0), volumePath, volumePool.getPoolType()); + return false; + } + KVMStoragePool pool = storagePoolMgr.getStoragePool(volumePool.getPoolType(), volumePool.getUuid()); + Path keyFile = null; + try { + keyFile = writeCephKeyFile(pool.getAuthSecret()); + List connection = rbdConnectionArgs(pool, keyFile); + // Volume paths of RBD volumes are already "/". + String image = volumePath; + + // Prefer the current image's own size: it is what the VM's disk was sized to. Only a + // restore that creates a brand new volume has no image to read it from. + Long imageSize = rbdImageSize(connection, image); + if (imageSize == null) { + imageSize = size; + } + if (imageSize == null || imageSize <= 0) { + logger.error("Unable to determine the size to recreate RBD image {} with", image); + return false; + } + + if (!recreateRbdImage(connection, image, imageSize, rbdDataPool(pool))) { + return false; + } + + for (String diff : diffFiles) { + logger.debug("Applying backup diff {} to RBD image {}", diff, image); + if (Script.executeCommandForExitValue(rbdCommand(connection, "import-diff", diff, image)) != 0) { + logger.error("Failed to apply backup diff {} to RBD image {}", diff, image); + return false; + } + } + + // The snapshots import-diff recreated only existed to chain the diffs together; the + // restored volume should look like any other, so drop them. + if (Script.executeCommandForExitValue(rbdCommand(connection, "snap", "purge", image)) != 0) { + logger.warn("Failed to purge the snapshots left behind by the restore of RBD image {}", image); + } + logger.debug("Successfully restored RBD image {} from {} backup diff(s)", image, diffFiles.size()); + return true; + } catch (IOException e) { + logger.error("Failed to prepare the Ceph credentials needed to restore volume [{}]", volumePath, e); + return false; + } finally { + deleteCephKeyFile(keyFile); + } + } + + /** + * Hand the cephx secret to the {@code rbd} CLI through a 0600 temporary file rather than an + * argument, so it never shows up in the host's process list. + */ + private Path writeCephKeyFile(String authSecret) throws IOException { + if (StringUtils.isBlank(authSecret)) { + return null; + } + Path keyFile = Files.createTempFile("csrestore-ceph-", ".key", + PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------"))); + Files.write(keyFile, authSecret.getBytes(StandardCharsets.UTF_8)); + return keyFile; + } + + private void deleteCephKeyFile(Path keyFile) { + if (keyFile == null) { + return; + } + try { + Files.deleteIfExists(keyFile); + } catch (IOException e) { + logger.warn("Failed to delete the temporary Ceph key file [{}].", keyFile, e); + } + } + + private List rbdConnectionArgs(KVMStoragePool pool, Path keyFile) { + List args = new ArrayList<>(); + if (StringUtils.isNotBlank(pool.getSourceHost())) { + List monitors = new ArrayList<>(); + for (String host : pool.getSourceHost().split(",")) { + host = host.trim().replace("[", "").replace("]", ""); + monitors.add(pool.getSourcePort() > 0 ? host + ":" + pool.getSourcePort() : host); + } + args.add("-m"); + args.add(String.join(",", monitors)); + } + if (StringUtils.isNotBlank(pool.getAuthUserName())) { + args.add("--id"); + args.add(pool.getAuthUserName()); + if (keyFile != null) { + args.add("--keyfile"); + args.add(keyFile.toString()); + } + } + return args; + } + + private String[] rbdCommand(List connection, String... arguments) { + List command = new ArrayList<>(); + command.add(Script.getExecutableAbsolutePath("rbd")); + command.addAll(connection); + command.addAll(Arrays.asList(arguments)); + return command.toArray(new String[0]); + } + + /** Size of {@code image} in bytes, or {@code null} when the image does not exist. */ + private Long rbdImageSize(List connection, String image) { + Pair result = Script.executePipedCommands( + Collections.singletonList(rbdCommand(connection, "info", image, "--format", "json")), 0); + if (result.first() == null || result.first() != 0 || result.second() == null) { + return null; + } + Matcher matcher = RBD_INFO_SIZE.matcher(result.second()); + return matcher.find() ? Long.valueOf(matcher.group(1)) : null; + } + + private boolean recreateRbdImage(List connection, String image, long sizeBytes, String dataPool) { + // `rbd rm` refuses to delete an image that still has snapshots, and a restore target + // normally carries the anchor snapshot of the backup chain it belongs to. + Script.executeCommandForExitValue(rbdCommand(connection, "snap", "purge", image)); + if (Script.executeCommandForExitValue(rbdCommand(connection, "rm", image)) != 0 + && rbdImageSize(connection, image) != null) { + logger.error("Failed to remove RBD image {} before restoring it", image); + return false; + } + List create = new ArrayList<>(Arrays.asList("create", image, "--size", sizeBytes + "B")); + for (String feature : RBD_IMAGE_FEATURES) { + create.add("--image-feature"); + create.add(feature); + } + if (StringUtils.isNotBlank(dataPool)) { + create.add("--data-pool"); + create.add(dataPool); + } + if (Script.executeCommandForExitValue(rbdCommand(connection, create.toArray(new String[0]))) != 0) { + logger.error("Failed to recreate RBD image {} with size {} bytes", image, sizeBytes); + return false; + } + return true; + } + + private String rbdDataPool(KVMStoragePool pool) { + Map details = pool.getDetails(); + return details == null ? null : details.get(KVMPhysicalDisk.RBD_DEFAULT_DATA_POOL); + } + private boolean attachVolumeToVm(KVMStoragePoolManager storagePoolMgr, String vmName, PrimaryDataStoreTO volumePool, String volumePath) { String deviceToAttachDiskTo = getDeviceToAttachDisk(vmName); if (Storage.StoragePoolType.RBD.equals(volumePool.getPoolType())) { 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 e76c9a2a3871..56c0f3deb7c7 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 @@ -52,6 +52,11 @@ public class LibvirtTakeBackupCommandWrapper extends CommandWrapperLines that do not start with a number are skipped rather than aborting the parse: the + * script's stdout also carries whatever the mount produced, and losing an otherwise complete + * backup over a stray warning line would be worse than reporting a size of 0.

*/ private long parseBackupSize(String stdout, List diskPaths) { long backupSize = 0L; @@ -223,7 +245,12 @@ private long parseBackupSize(String stdout, List diskPaths) { } } else { for (String line : stdout.split("\n")) { - backupSize = backupSize + Long.parseLong(line.split(" ")[0].trim()); + String sizeToken = line.trim().split("\\s+")[0]; + if (!sizeToken.matches("\\d+")) { + logger.debug("Ignoring non-size line in nasbackup.sh output: {}", line); + continue; + } + backupSize = backupSize + Long.parseLong(sizeToken); } } 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 3bdc23d27a15..e7deae417ab0 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 @@ -115,6 +115,98 @@ public void testExecuteWithVmExistsNull() throws Exception { } } + /** + * An RBD backup is a chain of `rbd export-diff` streams with no link between them, so the + * agent has to recreate the image and replay every diff from the chain's full anchor up to the + * backup being restored, in that order. + */ + @Test + public void testExecuteRestoresRbdVolumeByReplayingTheWholeDiffChain() throws Exception { + when(command.getVmName()).thenReturn("test-vm"); + // The provider sets both; the chain paths are what an rbddiff restore actually reads. + lenient().when(command.getBackupPath()).thenReturn("test-vm/inc1"); + when(command.getBackupFormat()).thenReturn("rbddiff"); + when(command.getBackupChainPaths()).thenReturn(Arrays.asList("test-vm/full", "test-vm/inc1")); + when(command.getBackupRepoAddress()).thenReturn("192.168.1.100:/backup"); + when(command.getBackupRepoType()).thenReturn("nfs"); + when(command.getMountOptions()).thenReturn("rw"); + when(command.isVmExists()).thenReturn(true); + when(command.getWait()).thenReturn(60); + when(command.getMountTimeout()).thenReturn(30); + PrimaryDataStoreTO primaryDataStore = Mockito.mock(PrimaryDataStoreTO.class); + when(primaryDataStore.getPoolType()).thenReturn(Storage.StoragePoolType.RBD); + when(primaryDataStore.getUuid()).thenReturn("ceph-pool-uuid"); + when(command.getRestoreVolumePools()).thenReturn(Arrays.asList(primaryDataStore)); + when(command.getRestoreVolumePaths()).thenReturn(Arrays.asList("cephpool/vol-uuid")); + when(command.getBackupVolumesUUIDs()).thenReturn(Arrays.asList("vol-uuid")); + when(command.getBackupFiles()).thenReturn(Arrays.asList("vol-uuid")); + + KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class); + KVMStoragePool pool = Mockito.mock(KVMStoragePool.class); + when(libvirtComputingResource.getStoragePoolMgr()).thenReturn(storagePoolMgr); + when(storagePoolMgr.getStoragePool(any(), anyString())).thenReturn(pool); + lenient().when(pool.getSourceHost()).thenReturn("10.0.1.2"); + lenient().when(pool.getSourcePort()).thenReturn(6789); + + List executed = new java.util.ArrayList<>(); + + try (MockedStatic filesMock = mockStatic(Files.class)) { + Path tempPath = Mockito.mock(Path.class); + when(tempPath.toString()).thenReturn("/tmp/csbackup.abc123"); + filesMock.when(() -> Files.createTempDirectory(anyString())).thenReturn(tempPath); + filesMock.when(() -> Files.deleteIfExists(any(Path.class))).thenReturn(true); + + try (MockedStatic