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

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

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

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

+ */ + protected ChainDecision decideContentChain(VirtualMachine vm) { + // Stopped VMs are backed up straight from the disk with qemu-img convert; that path has no + // point-in-time export to diff against a parent, so it is always a full. + if (VirtualMachine.State.Stopped.equals(vm.getState())) { + return ChainDecision.contentFull(); + } + + Integer fullEvery = NASBackupFullEvery.valueIn(vm.getDataCenterId()); + if (fullEvery == null || fullEvery <= 1) { + return ChainDecision.contentFull(); + } + + Backup parent = findLatestBackedUpBackup(vm.getId()); + if (parent == null) { + return ChainDecision.contentFull(); + } + + String parentChainId = readDetail(parent, NASBackupChainKeys.CHAIN_ID); + int parentChainPosition = chainPosition(parent); + if (parentChainId == null || parentChainPosition == Integer.MAX_VALUE) { + return ChainDecision.contentFull(); + } + + // Force a fresh full when the chain has reached the configured length. + if (parentChainPosition + 1 >= fullEvery) { + return ChainDecision.contentFull(); + } + + List parentPaths = composeParentBackupPaths(parent, vm.getId()); + if (parentPaths == null) { + LOG.debug("VM {} parent backup {} volume layout no longer matches current VM — forcing full", + vm.getInstanceName(), parent.getUuid()); + return ChainDecision.contentFull(); + } + return ChainDecision.contentIncremental(parentPaths, parentChainId, parentChainPosition + 1, + parent.getUuid()); + } + + /** + * True when EVERY volume of the VM sits on storage whose backups must be derived by content + * comparison instead of dirty bitmaps — currently LINSTOR, whose volumes are raw DRBD block + * devices and so cannot persist a bitmap (persistence is a qcow2-only feature). A VM with a + * mix of such volumes and others is not eligible, since one backup run uses a single mode. + */ + protected boolean allVolumesOnContentDiffCapableStorage(VirtualMachine vm) { + List volumes = volumeDao.findByInstance(vm.getId()); + if (CollectionUtils.isEmpty(volumes)) { + return false; + } + for (VolumeVO volume : volumes) { + StoragePoolVO pool = primaryDataStoreDao.findById(volume.getPoolId()); + if (pool == null || !Storage.StoragePoolType.Linstor.equals(pool.getPoolType())) { + return false; + } + } + return true; + } + /** * Incremental backups require QEMU dirty bitmaps / libvirt checkpoints, which are only * possible on file-based qcow2 storage. Returns {@code true} only when EVERY volume of the @@ -502,7 +611,10 @@ private void persistChainMetadata(Backup backup, ChainDecision decision, String // source of truth. Not duplicated into backup_details. if (decision.isIncremental()) { // Resolve the parent backup's UUID so restore can walk the chain by id, not by path. - String parentUuid = lookupParentBackupUuid(backup.getVmId(), decision.bitmapParent); + // Content-based decisions carry the parent uuid directly (no bitmap to look it up by). + String parentUuid = decision.parentBackupUuid != null + ? decision.parentBackupUuid + : lookupParentBackupUuid(backup.getVmId(), decision.bitmapParent); if (parentUuid != null) { backupDetailsDao.persist(new BackupDetailVO(backup.getId(), NASBackupChainKeys.PARENT_BACKUP_ID, parentUuid, true)); } @@ -593,11 +705,14 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce command.setQuiesce(quiesceVM); command.setQuiesceTimeout(NASBackupQuiesceAgentTimeout.valueIn(vm.getDataCenterId())); command.setMode(decision.mode); + command.setMethod(decision.method); command.setBitmapNew(decision.bitmapNew); command.setBitmapParent(decision.bitmapParent); command.setParentPaths(decision.parentPaths); - if (VirtualMachine.State.Stopped.equals(vm.getState())) { + // A stopped VM is copied straight from its volumes. A running VM backed up with the content method + // takes the scratch space of its pull-mode backup from the volumes' primary storage. + if (VirtualMachine.State.Stopped.equals(vm.getState()) || decision.isContentBased()) { List vmVolumes = volumeDao.findByInstance(vm.getId()); vmVolumes.sort(Comparator.comparing(Volume::getDeviceId)); Pair, List> volumePoolsAndPaths = getVolumePoolsAndPaths(vmVolumes); @@ -628,7 +743,9 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce // backup as a full and start a new chain. ChainDecision effective = decision; if (answer.getIncrementalFallback()) { - effective = ChainDecision.fullStart(decision.bitmapNew); + effective = decision.isContentBased() + ? ChainDecision.contentFull() + : ChainDecision.fullStart(decision.bitmapNew); backupVO.setType("FULL"); } List volumes = new ArrayList<>(volumeDao.findByInstance(vm.getId())); @@ -642,11 +759,15 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce // created — the only valid parent for the next incremental (see decideChain). // If the agent reports no bitmap (bitmapCreated=null), clear any stale detail // so the next backup starts a fresh full. - String confirmedBitmap = answer.getBitmapCreated(); - if (confirmedBitmap != null) { - upsertVmActiveCheckpoint(vm.getId(), confirmedBitmap); - } else { - clearVmActiveCheckpoint(vm.getId()); + // The content-based path creates no bitmaps, so there is no active + // checkpoint to track — its chain is anchored purely on backup history. + if (!decision.isContentBased()) { + String confirmedBitmap = answer.getBitmapCreated(); + if (confirmedBitmap != null) { + upsertVmActiveCheckpoint(vm.getId(), confirmedBitmap); + } else { + clearVmActiveCheckpoint(vm.getId()); + } } } return new Pair<>(true, backupVO); diff --git a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java index 787d3ff94567..104604e042f2 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 @@ -19,6 +19,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.mock; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Objects; @@ -49,6 +50,7 @@ import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; +import java.util.Date; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; @@ -274,6 +276,37 @@ public void takeBackupSuccessfully() throws AgentUnavailableException, Operation Mockito.verify(agentManager).send(anyLong(), Mockito.any(TakeBackupCommand.class)); } + @Test + public void takeBackupSendsVolumePoolsForContentMethodOnRunningVm() throws AgentUnavailableException, OperationTimedoutException { + VMInstanceVO vm = mockRunningVmForBackup(1L, 2L, 3L, 4L, 5L, 6L); + Mockito.doReturn(NASBackupProvider.ChainDecision.contentFull()).when(nasBackupProvider).decideChain(vm); + VolumeVO volume = volumeDao.findByInstance(1L).get(0); + Mockito.when(volume.getPoolId()).thenReturn(11L); + Mockito.when(volume.getPath()).thenReturn("vol-uuid"); + Mockito.when(volumeDao.findByInstance(1L)).thenReturn(new ArrayList<>(List.of(volume))); + StoragePoolVO pool = mock(StoragePoolVO.class); + Mockito.when(pool.getId()).thenReturn(11L); + Mockito.when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + Mockito.when(storagePoolDao.findById(11L)).thenReturn(pool); + DataStore dataStore = mock(DataStore.class); + PrimaryDataStoreTO dataStoreTO = mock(PrimaryDataStoreTO.class); + Mockito.when(dataStore.getTO()).thenReturn(dataStoreTO); + Mockito.when(dataStoreMgr.getDataStore(11L, DataStoreRole.Primary)).thenReturn(dataStore); + + BackupAnswer answer = mock(BackupAnswer.class); + Mockito.when(answer.getResult()).thenReturn(false); + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(TakeBackupCommand.class); + Mockito.when(agentManager.send(anyLong(), commandCaptor.capture())).thenReturn(answer); + Mockito.when(backupDao.persist(Mockito.any(BackupVO.class))).thenAnswer(invocation -> invocation.getArgument(0)); + + nasBackupProvider.takeBackup(vm, false); + + TakeBackupCommand command = commandCaptor.getValue(); + Assert.assertEquals("content", command.getMethod()); + Assert.assertEquals(List.of(dataStoreTO), command.getVolumePools()); + Assert.assertEquals(List.of("/dev/drbd/by-res/cs-vol-uuid/0"), command.getVolumePaths()); + } + @Test public void takeBackupFailureNeedingCleanupReturnsErrorBackup() throws AgentUnavailableException, OperationTimedoutException { VMInstanceVO vm = mockRunningVmForBackup(1L, 2L, 3L, 4L, 5L, 6L); @@ -504,6 +537,7 @@ public void decideChainReturnsFullWhenVmHasNoActiveCheckpoint() { NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); Assert.assertNotNull(decision); Assert.assertEquals(NASBackupChainKeys.TYPE_FULL, decision.mode); + Assert.assertEquals(NASBackupChainKeys.METHOD_QCOW2, decision.method); Assert.assertNull(decision.bitmapParent); Assert.assertEquals(0, decision.chainPosition); } @@ -1099,4 +1133,196 @@ public void sweepContinuesPastFailedTombstoneDelete() Mockito.verify(backupDao, Mockito.never()).remove(51L); Mockito.verify(backupDao).remove(50L); } + + // -- content-based (pull mode) chain decisions for raw block storage ----------------- + + /** + * With the incremental master switch off, a LINSTOR-backed VM keeps the legacy push-mode full + * like every other VM: enabling incrementals is the opt-in to the content-based path and its + * host-local scratch space. The storage is not even looked at. + */ + @Test + public void decideChainReturnsLegacyFullForLinstorWhenIncrementalDisabled() { + Long vmId = 70L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.lenient().when(vm.getId()).thenReturn(vmId); + Mockito.lenient().when(vm.getDataCenterId()).thenReturn(1L); + + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupIncrementalEnabled", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Boolean.class, + "nas.backup.incremental.enabled", "false", + "test override — disabled", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertTrue(decision.isLegacyFull()); + Assert.assertEquals(NASBackupChainKeys.METHOD_QCOW2, decision.method); + Assert.assertFalse(decision.isContentBased()); + Assert.assertNull("legacy full anchors no chain", decision.chainId); + Mockito.verify(volumeDao, Mockito.never()).findByInstance(vmId); + } + + /** + * Stopped VMs are imaged straight from the disk by qemu-img, with no point-in-time export to + * diff against a parent, so a LINSTOR VM that is stopped must take a content full. + */ + @Test + public void decideChainReturnsContentFullForStoppedLinstorVm() { + Long vmId = 71L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Stopped); + stubAllVolumesOnLinstor(vmId, 1); + enableIncrementals(); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertEquals(NASBackupChainKeys.TYPE_FULL, decision.mode); + Assert.assertEquals(NASBackupChainKeys.METHOD_CONTENT, decision.method); + } + + /** + * Running LINSTOR VM with a healthy parent inside the cadence => content incremental, carrying + * the parent's chain id, the next position, the parent uuid (resolved directly since there is + * no bitmap to look it up by) and the per-volume parent paths the script rebases onto. + */ + @Test + public void decideChainReturnsContentIncrementalForLinstorWithValidParent() { + Long vmId = 72L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Running); + stubAllVolumesOnLinstor(vmId, 1); + enableIncrementals(); + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupFullEvery", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Integer.class, + "nas.backup.full.every", "3", "test override", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + + Backup parent = mock(Backup.class); + Mockito.when(parent.getId()).thenReturn(900L); + Mockito.when(parent.getStatus()).thenReturn(Backup.Status.BackedUp); + Mockito.lenient().when(parent.getDate()).thenReturn(new Date()); + Mockito.when(parent.getUuid()).thenReturn("parent-uuid"); + Mockito.when(parent.getExternalId()).thenReturn("i-2-3-VM/2026.01.01.00.00.00"); + Backup.VolumeInfo pv = mock(Backup.VolumeInfo.class); + Mockito.when(pv.getPath()).thenReturn("volpath1"); + Mockito.>when(parent.getBackedUpVolumes()).thenReturn(List.of(pv)); + Mockito.when(backupDao.listByVmId(null, vmId)).thenReturn(List.of(parent)); + + BackupDetailVO chainId = mock(BackupDetailVO.class); + Mockito.when(chainId.getValue()).thenReturn("chain-1"); + BackupDetailVO chainPos = mock(BackupDetailVO.class); + Mockito.when(chainPos.getValue()).thenReturn("0"); + Mockito.when(backupDetailsDao.findDetail(900L, NASBackupChainKeys.CHAIN_ID)).thenReturn(chainId); + Mockito.when(backupDetailsDao.findDetail(900L, NASBackupChainKeys.CHAIN_POSITION)).thenReturn(chainPos); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertEquals(NASBackupChainKeys.TYPE_INCREMENTAL, decision.mode); + Assert.assertEquals(NASBackupChainKeys.METHOD_CONTENT, decision.method); + Assert.assertTrue(decision.isIncremental()); + Assert.assertTrue(decision.isContentBased()); + Assert.assertNull("a content incremental carries no bitmap", decision.bitmapNew); + Assert.assertNull(decision.bitmapParent); + Assert.assertEquals("chain-1", decision.chainId); + Assert.assertEquals(1, decision.chainPosition); + Assert.assertEquals("parent-uuid", decision.parentBackupUuid); + Assert.assertEquals(List.of("i-2-3-VM/2026.01.01.00.00.00/root.volpath1.qcow2"), + decision.parentPaths); + } + + /** + * Once the chain reaches nas.backup.full.every, the next LINSTOR backup must anchor a new + * chain with a content full rather than extending the existing one. + */ + @Test + public void decideChainReturnsContentFullForLinstorAtChainEnd() { + Long vmId = 73L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Running); + stubAllVolumesOnLinstor(vmId, 1); + enableIncrementals(); + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupFullEvery", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Integer.class, + "nas.backup.full.every", "2", "test override", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + + Backup parent = mock(Backup.class); + Mockito.when(parent.getId()).thenReturn(901L); + Mockito.when(parent.getStatus()).thenReturn(Backup.Status.BackedUp); + Mockito.lenient().when(parent.getDate()).thenReturn(new Date()); + Mockito.when(backupDao.listByVmId(null, vmId)).thenReturn(List.of(parent)); + BackupDetailVO chainId = mock(BackupDetailVO.class); + Mockito.when(chainId.getValue()).thenReturn("chain-9"); + BackupDetailVO chainPos = mock(BackupDetailVO.class); + Mockito.when(chainPos.getValue()).thenReturn("1"); // 1 + 1 >= 2 => new full + Mockito.when(backupDetailsDao.findDetail(901L, NASBackupChainKeys.CHAIN_ID)).thenReturn(chainId); + Mockito.when(backupDetailsDao.findDetail(901L, NASBackupChainKeys.CHAIN_POSITION)).thenReturn(chainPos); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertEquals(NASBackupChainKeys.TYPE_FULL, decision.mode); + Assert.assertEquals(NASBackupChainKeys.METHOD_CONTENT, decision.method); + } + + /** + * A VM straddling LINSTOR and file-based storage is not content-diff capable: one backup run + * uses a single mode, so it must not be routed down the content path. + */ + @Test + public void allVolumesOnContentDiffCapableStorageFalseForMixedStorage() { + Long vmId = 74L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + + VolumeVO linVol = mock(VolumeVO.class); + Mockito.when(linVol.getPoolId()).thenReturn(1L); + VolumeVO nfsVol = mock(VolumeVO.class); + Mockito.when(nfsVol.getPoolId()).thenReturn(2L); + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(linVol, nfsVol)); + + StoragePoolVO lin = mock(StoragePoolVO.class); + Mockito.when(lin.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + StoragePoolVO nfs = mock(StoragePoolVO.class); + Mockito.when(nfs.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + Mockito.when(storagePoolDao.findById(1L)).thenReturn(lin); + Mockito.when(storagePoolDao.findById(2L)).thenReturn(nfs); + + Assert.assertFalse(nasBackupProvider.allVolumesOnContentDiffCapableStorage(vm)); + } + + /** A VM with no volumes is not content-diff capable (safe default). */ + @Test + public void allVolumesOnContentDiffCapableStorageFalseForNoVolumes() { + Long vmId = 75L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of()); + Assert.assertFalse(nasBackupProvider.allVolumesOnContentDiffCapableStorage(vm)); + } + + private void enableIncrementals() { + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupIncrementalEnabled", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Boolean.class, + "nas.backup.incremental.enabled", "true", + "test override — enabled", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + } + + /** Point every volume of {@code vmId} at a LINSTOR pool. */ + private void stubAllVolumesOnLinstor(Long vmId, int count) { + List vols = new java.util.ArrayList<>(); + StoragePoolVO lin = mock(StoragePoolVO.class); + Mockito.lenient().when(lin.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + for (int i = 0; i < count; i++) { + VolumeVO v = mock(VolumeVO.class); + long poolId = 100L + i; + Mockito.when(v.getPoolId()).thenReturn(poolId); + Mockito.when(storagePoolDao.findById(poolId)).thenReturn(lin); + vols.add(v); + } + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(vols); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java index 92de04b1b95a..08a27710674d 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 @@ -63,7 +63,7 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper { @@ -53,13 +62,18 @@ public class LibvirtTakeBackupCommandWrapper extends CommandWrapper volumePools = command.getVolumePools(); final List volumePaths = command.getVolumePaths(); KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr(); @@ -87,12 +101,67 @@ public Answer execute(TakeBackupCommand command, LibvirtComputingResource libvir } } - Pair result = runBackupScript(libvirtComputingResource, command, vmName, backupRepoType, backupRepoAddress, - mountOptions, backupPath, diskPaths, command.getMode(), - command.getBitmapNew(), command.getBitmapParent(), command.getParentPaths(), timeout); + // A stopped VM is copied straight from its volumes, but a LINSTOR volume only has a device on + // the hosts that hold a resource of it, and once the VM is stopped its last host may hold none. + List> connectedVolumes; + try { + connectedVolumes = connectMissingLinstorVolumes(storagePoolMgr, volumePools, volumePaths); + } catch (CloudRuntimeException e) { + logger.warn("Failed to make the volumes of VM {} available for its backup", vmName, e); + return new BackupAnswer(command, false, + String.format("Failed to make the volumes of VM %s available: %s", vmName, e.getMessage())); + } + + KVMStoragePool scratchPool = null; + Pair> scratch = null; + try { + // A pull-mode backup of a running VM needs a copy-before-write scratch device per disk, provided by + // the VM's primary storage. A stopped VM is copied straight from its disks and needs none. + if (METHOD_CONTENT.equals(command.getMethod())) { + try { + List diskSizes = getRunningVmDiskSizes(libvirtComputingResource, vmName); + if (diskSizes != null) { + if (CollectionUtils.isEmpty(volumePools)) { + return new BackupAnswer(command, false, "No volumes to back up for VM " + vmName); + } + PrimaryDataStoreTO scratchPoolTO = volumePools.get(0); + scratchPool = storagePoolMgr.getStoragePool(scratchPoolTO.getPoolType(), scratchPoolTO.getUuid()); + scratch = scratchPool.createBackupScratch(vmName, + diskSizes.stream().map(LibvirtTakeBackupCommandWrapper::scratchSize).collect(Collectors.toList())); + } + } catch (LibvirtException | CloudRuntimeException e) { + logger.warn("Failed to create the backup scratch space for VM {}", vmName, e); + return new BackupAnswer(command, false, + String.format("Failed to create the backup scratch space for VM %s: %s", vmName, e.getMessage())); + } + } + + return runBackup(libvirtComputingResource, command, diskPaths, scratch != null ? scratch.second() : null, timeout); + } finally { + if (scratch != null) { + try { + scratchPool.deleteBackupScratch(scratch.first()); + } catch (CloudRuntimeException e) { + logger.warn("Failed to delete the backup scratch space {} of VM {}, delete it manually: {}", + scratch.first(), vmName, e.getMessage()); + } + } + disconnectVolumes(storagePoolMgr, connectedVolumes); + } + } + + private Answer runBackup(LibvirtComputingResource libvirtComputingResource, TakeBackupCommand command, + List diskPaths, List scratchDevices, int timeout) { + final String vmName = command.getVmName(); + Pair result = runBackupScript(libvirtComputingResource, command, vmName, command.getBackupRepoType(), + command.getBackupRepoAddress(), command.getMountOptions(), command.getBackupPath(), diskPaths, command.getMode(), + command.getBitmapNew(), command.getBitmapParent(), command.getParentPaths(), scratchDevices, timeout); if (result.first() != 0) { logger.debug("Failed to take VM backup: " + result.second()); + if (Script.ERR_TIMEOUT.equals(result.second()) && METHOD_CONTENT.equals(command.getMethod())) { + abortPullBackupJob(vmName); + } BackupAnswer answer = new BackupAnswer(command, false, StringUtils.trimToEmpty(result.second())); if (EXIT_CLEANUP_FAILED.equals(result.first())) { logger.debug("Backup cleanup failed"); @@ -118,6 +187,104 @@ public Answer execute(TakeBackupCommand command, LibvirtComputingResource libvir return answer; } + /** + * Makes each LINSTOR volume whose device does not exist on this host available here; connecting + * returns once the device exists. A volume of a running VM, or one this host holds a replica of, + * already has its device and is left alone. Returns the pool and uuid of every volume it connected, + * for {@link #disconnectVolumes}. + */ + private List> connectMissingLinstorVolumes(KVMStoragePoolManager storagePoolMgr, + List volumePools, List volumePaths) { + List> connected = new ArrayList<>(); + if (volumePaths == null) { + return connected; + } + try { + for (int idx = 0; idx < volumePaths.size(); idx++) { + PrimaryDataStoreTO volumePool = volumePools.get(idx); + String volumePath = volumePaths.get(idx); + if (volumePool.getPoolType() != Storage.StoragePoolType.Linstor || deviceExists(volumePath)) { + continue; + } + String volumeUuid = LibvirtRestoreBackupCommandWrapper.getVolumeUuidFromPath(volumePath, volumePool); + logger.debug("Making LINSTOR volume {} available for the backup", volumeUuid); + storagePoolMgr.connectPhysicalDisk(volumePool.getPoolType(), volumePool.getUuid(), volumeUuid, null); + connected.add(new Pair<>(volumePool, volumeUuid)); + } + } catch (CloudRuntimeException e) { + disconnectVolumes(storagePoolMgr, connected); + throw e; + } + return connected; + } + + private void disconnectVolumes(KVMStoragePoolManager storagePoolMgr, List> volumes) { + for (Pair volume : volumes) { + try { + storagePoolMgr.disconnectPhysicalDisk(volume.first().getPoolType(), volume.first().getUuid(), volume.second()); + } catch (CloudRuntimeException e) { + logger.warn("Failed to disconnect volume {} after the backup: {}", volume.second(), e.getMessage()); + } + } + } + + boolean deviceExists(String path) { + return Files.exists(Paths.get(path)); + } + + /** + * A pull-mode backup job never ends on its own: nasbackup.sh ends it once the data is read, + * and its EXIT trap does so on failure. A timed-out script is killed with SIGKILL, which + * no trap can catch, so end the job here. Otherwise it keeps the NBD server and the scratch + * devices busy and blocks every later backup or migration of the VM. + */ + private void abortPullBackupJob(String vmName) { + logger.warn("Backup of VM {} timed out, aborting its pull-mode backup job", vmName); + int exitValue = Script.executeCommandForExitValue(Script.getExecutableAbsolutePath("virsh"), + "-c", "qemu:///system", "domjobabort", vmName); + if (exitValue != 0) { + logger.warn("Failed to abort the backup job of VM {} (exit code {})", vmName, exitValue); + } + } + + /** + * The capacity of each disk of the VM in domain order, which is the order nasbackup.sh lists them in, or + * {@code null} if the VM is not running here. nasbackup.sh takes a stopped VM's backup without pull mode. + */ + private List getRunningVmDiskSizes(LibvirtComputingResource libvirtComputingResource, String vmName) + throws LibvirtException { + Connect conn = libvirtComputingResource.getLibvirtUtilitiesHelper().getConnectionByVmName(vmName); + Domain dm; + try { + dm = libvirtComputingResource.getDomain(conn, vmName); + } catch (LibvirtException e) { + logger.debug("VM {} is not defined on this host: {}", vmName, e.getMessage()); + return null; + } + try { + if (dm.getInfo().state != DomainInfo.DomainState.VIR_DOMAIN_RUNNING) { + return null; + } + List sizes = new ArrayList<>(); + for (DiskDef disk : libvirtComputingResource.getDisks(conn, vmName)) { + if (disk.getDeviceType() == DiskDef.DeviceType.DISK) { + sizes.add(dm.blockInfo(disk.getDiskLabel()).getCapacity()); + } + } + return sizes; + } finally { + dm.free(); + } + } + + /** + * Size of a scratch device for a disk of {@code diskSize} bytes. libvirt formats it as a qcow2 that may + * end up holding every block of the disk, plus metadata: about 0.015% of the disk with 64 KiB clusters. + */ + static long scratchSize(long diskSize) { + return diskSize + diskSize / 4096 + SCRATCH_EXTRA_BYTES; + } + /** Remove nasbackup.sh's stdout signalling marker lines so they don't pollute size parsing. */ private String stripMarkerLines(String stdout) { if (StringUtils.isBlank(stdout)) { @@ -142,7 +309,7 @@ private String stripMarkerLines(String stdout) { private Pair runBackupScript(LibvirtComputingResource libvirtComputingResource, TakeBackupCommand command, String vmName, String backupRepoType, String backupRepoAddress, String mountOptions, String backupPath, List diskPaths, String mode, - String bitmapNew, String bitmapParent, List parentPaths, int timeout) { + String bitmapNew, String bitmapParent, List parentPaths, List scratchDevices, int timeout) { boolean quiesce = command.getQuiesce() != null && command.getQuiesce(); List argv = new ArrayList<>(Arrays.asList( libvirtComputingResource.getNasBackupPath(), @@ -164,6 +331,10 @@ private Pair runBackupScript(LibvirtComputingResource libvirtCo argv.add("-M"); argv.add(mode); } + if (StringUtils.isNotBlank(command.getMethod())) { + argv.add("--method"); + argv.add(command.getMethod()); + } if (StringUtils.isNotBlank(bitmapNew)) { argv.add("--bitmap-new"); argv.add(bitmapNew); @@ -176,6 +347,10 @@ private Pair runBackupScript(LibvirtComputingResource libvirtCo argv.add("--parent-paths"); argv.add(String.join(",", parentPaths)); } + if (CollectionUtils.isNotEmpty(scratchDevices)) { + argv.add("--scratch-devs"); + argv.add(String.join(",", scratchDevices)); + } List commands = new ArrayList<>(); commands.add(argv.toArray(new String[0])); @@ -188,6 +363,13 @@ private Pair runBackupScript(LibvirtComputingResource libvirtCo */ private String validateBackupArgs(TakeBackupCommand command) { String mode = command.getMode(); + String method = StringUtils.defaultIfBlank(command.getMethod(), METHOD_QCOW2); + if (METHOD_CONTENT.equals(method)) { + return validateContentBackupArgs(command); + } + if (!METHOD_QCOW2.equals(method)) { + return "Unknown backup method: " + method; + } if (StringUtils.isBlank(mode)) { return null; // legacy full-only, no extra args expected } @@ -215,6 +397,21 @@ private String validateBackupArgs(TakeBackupCommand command) { return "Unknown backup mode: " + mode; } + private String validateContentBackupArgs(TakeBackupCommand command) { + String mode = command.getMode(); + if (MODE_FULL.equals(mode)) { + return null; // pull-mode full — no bitmap, no parent + } + if (MODE_INCREMENTAL.equals(mode)) { + // No bitmap is involved: the delta comes from comparing the disk against the parent. + if (CollectionUtils.isEmpty(command.getParentPaths())) { + return "content incremental mode requires parentPaths"; + } + return null; + } + return "Unsupported mode for the content backup method: " + mode; + } + /** * Sum the per-disk size lines emitted by nasbackup.sh. Single-volume mode emits one * line containing just the byte count; multi-volume mode emits one line per disk diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java index 3e35ed9476b1..46e57d701a03 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java @@ -30,6 +30,8 @@ import com.cloud.hypervisor.kvm.resource.KVMHABase.HAStoragePool; import com.cloud.storage.Storage; import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; public interface KVMStoragePool { @@ -125,4 +127,35 @@ default LibvirtVMDef.DiskDef.BlockIOSize getSupportedPhysicalBlockSize() { default void customizeLibvirtDiskDef(LibvirtVMDef.DiskDef disk) { } + + /** + * Whether the volume {@code volumeUuid}, freshly created in this pool, already reads as zeroes, + * so a copy onto it may skip zero regions (qemu-img --target-is-zero). Says nothing about a + * volume that has been written to: that still holds its old data. + */ + default boolean newVolumeReadsAsZero(String volumeUuid) { + return false; + } + + /** + * Creates the scratch space for a pull-mode backup of the running VM {@code vmName} on this host: one + * block device per entry of {@code sizes}, at least that many bytes large. libvirt formats each device + * as the copy-before-write target of one disk, keeping the guest data the backup has not read yet. + * + * @return the name to pass to {@link #deleteBackupScratch(String)} and the device paths, in the order + * of {@code sizes} + */ + default Pair> createBackupScratch(String vmName, List sizes) { + throw new CloudRuntimeException(String.format("Storage pool %s of type %s cannot provide backup scratch space", + getUuid(), getType())); + } + + /** + * Removes scratch space created by {@link #createBackupScratch(String, List)}. Only valid once the + * backup job has ended and libvirt has released the devices. + */ + default void deleteBackupScratch(String name) { + throw new CloudRuntimeException(String.format("Storage pool %s of type %s cannot provide backup scratch space", + getUuid(), getType())); + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java index 3bdc23d27a15..5adf0eb04241 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java @@ -21,6 +21,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mockConstruction; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.when; @@ -34,10 +35,13 @@ import org.apache.cloudstack.backup.BackupAnswer; import org.apache.cloudstack.backup.RestoreBackupCommand; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.utils.qemu.QemuImg; +import org.apache.cloudstack.utils.qemu.QemuImgFile; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.MockedConstruction; import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; @@ -812,4 +816,67 @@ public void testUnmountIsBoundedByTheConfiguredTimeout() throws Exception { } } } + + /** + * Restores a backup onto a LINSTOR volume through replaceBlockDeviceWithBackup and returns + * the QemuImg instance used for the copy together with its constructor arguments. + */ + private Pair> restoreOntoLinstorVolume(boolean createTargetVolume, boolean newVolumeReadsAsZero) + throws Exception { + Method method = LibvirtRestoreBackupCommandWrapper.class.getDeclaredMethod("replaceBlockDeviceWithBackup", + KVMStoragePoolManager.class, PrimaryDataStoreTO.class, String.class, String.class, int.class, + boolean.class, Long.class); + method.setAccessible(true); + + PrimaryDataStoreTO volumePool = Mockito.mock(PrimaryDataStoreTO.class); + when(volumePool.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + when(volumePool.getUuid()).thenReturn("pool-uuid"); + KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class); + KVMStoragePool linstorPool = Mockito.mock(KVMStoragePool.class); + lenient().when(linstorPool.newVolumeReadsAsZero("volume-123")).thenReturn(newVolumeReadsAsZero); + when(storagePoolMgr.getStoragePool(Storage.StoragePoolType.Linstor, "pool-uuid")).thenReturn(linstorPool); + + final List[] ctorArgs = new List[1]; + try (MockedConstruction qemuMock = mockConstruction(QemuImg.class, + (mock, context) -> ctorArgs[0] = context.arguments())) { + boolean result = (boolean) method.invoke(wrapper, storagePoolMgr, volumePool, + "/dev/drbd/by-res/cs-volume-123/0", "/mnt/backup/root.volume-123.qcow2", 60, + createTargetVolume, 1024L); + + Assert.assertTrue(result); + Assert.assertEquals(1, qemuMock.constructed().size()); + QemuImg qemu = qemuMock.constructed().get(0); + Mockito.verify(qemu).convert(any(QemuImgFile.class), any(QemuImgFile.class)); + if (!createTargetVolume) { + Mockito.verify(linstorPool, Mockito.never()).newVolumeReadsAsZero(anyString()); + } + return new Pair<>(qemu, ctorArgs[0]); + } + } + + @Test + public void testInPlaceLinstorRestoreDoesNotSkipZeroes() throws Exception { + // Even on a thin pool: the volume still holds its old data, so the backup's zero regions + // must be written. + Pair> restore = restoreOntoLinstorVolume(false, true); + + Assert.assertEquals(Boolean.FALSE, restore.second().get(1)); + Mockito.verify(restore.first()).setSkipTargetVolumeCreation(true); + } + + @Test + public void testLinstorRestoreToNewThickVolumeDoesNotSkipZeroes() throws Exception { + Pair> restore = restoreOntoLinstorVolume(true, false); + + Assert.assertEquals(Boolean.FALSE, restore.second().get(1)); + Mockito.verify(restore.first()).setSkipTargetVolumeCreation(true); + } + + @Test + public void testLinstorRestoreToNewThinVolumeSkipsZeroes() throws Exception { + Pair> restore = restoreOntoLinstorVolume(true, true); + + Assert.assertEquals(Boolean.TRUE, restore.second().get(1)); + Mockito.verify(restore.first()).setSkipTargetVolumeCreation(true); + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java index 7ef64fd53b50..2feaba135283 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java @@ -18,25 +18,41 @@ import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.when; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.apache.cloudstack.backup.BackupAnswer; import org.apache.cloudstack.backup.TakeBackupCommand; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.libvirt.Connect; +import org.libvirt.Domain; +import org.libvirt.DomainBlockInfo; +import org.libvirt.DomainInfo; +import org.libvirt.LibvirtException; import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef; +import com.cloud.hypervisor.kvm.storage.KVMStoragePool; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.storage.Storage; import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; @RunWith(MockitoJUnitRunner.class) @@ -99,4 +115,268 @@ public void executeOmitsQuiesceTimeoutWhenNotPositive() { Assert.assertFalse(runAndCaptureArgv().contains("--quiesce-timeout")); } + + private void runFailing(Pair result, boolean expectJobAbort) { + try (MockedStatic