From 2c92601aade421b4d9a643fb02782b34f3b604d0 Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Wed, 26 Aug 2026 13:00:08 +0300 Subject: [PATCH] Backup: support StorPool volumes in NAS backup provider on KVM Volume pool/path info is now sent to the agent whenever a VM has a StorPool volume, not just when the VM looks Stopped, since nasbackup.sh re-checks the VM's actual liveness itself right before acting and needs the StorPool path to clone a backup source disk if it finds the VM already stopped. - StorPoolStorageAdaptor.createPhysicalDisk() now creates and attaches a StorPool volume (previously a no-op returning null), used when restoring a backup provisions a new volume. - nasbackup.sh clones the live StorPool volume into a point-in-time volume before reading from it for a cold backup, and cleans it up afterwards (with an EXIT trap as a safety net). - Backup size is now reported via an explicit BACKUP_SIZE_TOTAL= marker on stdout instead of being inferred from output position/shape, which broke down once StorPool added a third code shape; take-backup errors are now always returned as a BackupAnswer rather than letting an uncaught exception surface as a plain Answer. - Restore reports back the volume path StorPool actually assigned (BackupAnswer.restoredVolumePath) instead of the path CloudStack guessed, since StorPool controls the device path itself. - Backup file identifiers are normalized to the qcow2 basename so StorPool's full device path matches what nasbackup.sh expects. --- .../cloudstack/backup/BackupAnswer.java | 12 + .../cloudstack/backup/NASBackupProvider.java | 60 +- .../backup/NASBackupProviderTest.java | 159 +++++ .../LibvirtRestoreBackupCommandWrapper.java | 68 +- .../LibvirtTakeBackupCommandWrapper.java | 136 ++-- .../kvm/storage/StorPoolStorageAdaptor.java | 60 +- scripts/vm/hypervisor/kvm/nasbackup.sh | 173 ++++- .../plugins/storpool/TestNasBackupStorPool.py | 650 ++++++++++++++++++ test/integration/plugins/storpool/sp_util.py | 70 +- 9 files changed, 1270 insertions(+), 118 deletions(-) create mode 100644 test/integration/plugins/storpool/TestNasBackupStorPool.py diff --git a/core/src/main/java/org/apache/cloudstack/backup/BackupAnswer.java b/core/src/main/java/org/apache/cloudstack/backup/BackupAnswer.java index abe78ee5553d..9ffc4dd04462 100644 --- a/core/src/main/java/org/apache/cloudstack/backup/BackupAnswer.java +++ b/core/src/main/java/org/apache/cloudstack/backup/BackupAnswer.java @@ -35,6 +35,10 @@ public class BackupAnswer extends Answer { // Set when an incremental was requested but the agent had to fall back to a full // (e.g. VM was stopped). Provider should record this backup as type=full. private Boolean incrementalFallback; + // Set when restore provisions a volume on storage that assigns its own path/ID (e.g. + // StorPool), so the provider doesn't have to guess it from the CloudStack volume UUID. + // Null when the guessed path is already correct (e.g. RBD/Linstor/NFS). + private String restoredVolumePath; public BackupAnswer(final Command command, final boolean success, final String details) { super(command, success, details); @@ -91,4 +95,12 @@ public void setIncrementalFallback(Boolean incrementalFallback) { this.incrementalFallback = incrementalFallback; } + public String getRestoredVolumePath() { + return restoredVolumePath; + } + + public void setRestoredVolumePath(String restoredVolumePath) { + this.restoredVolumePath = restoredVolumePath; + } + } 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..05cb20e16b06 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 @@ -383,6 +383,24 @@ protected boolean allVolumesOnCheckpointCapableStorage(VirtualMachine vm) { return true; } + /** + * True when any of the VM's volumes sits on a StorPool pool. Used to decide whether volume + * pool/path info must be sent to the agent even though the VM currently looks Running — see + * the caller in {@link #takeBackup}. + */ + private boolean hasStorPoolVolume(List volumes) { + if (volumes == null) { + return false; + } + for (VolumeVO volume : volumes) { + StoragePoolVO pool = primaryDataStoreDao.findById(volume.getPoolId()); + if (pool != null && Storage.StoragePoolType.StorPool.equals(pool.getPoolType())) { + return true; + } + } + return false; + } + /** * Read the {@code nas.active_checkpoint_id} VM detail. Returns {@code null} when no detail * exists (post-restore, first backup, or after explicit reset). @@ -588,8 +606,20 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce command.setBitmapParent(decision.bitmapParent); command.setParentPaths(decision.parentPaths); - if (VirtualMachine.State.Stopped.equals(vm.getState())) { - List vmVolumes = volumeDao.findByInstance(vm.getId()); + // Always sent (not just for a Stopped VM) when any volume is on StorPool: nasbackup.sh + // re-checks the VM's actual liveness itself right before acting, and that fresh check can + // disagree with this state read taken here. If it does and the VM turns out to be + // stopped, nasbackup.sh needs the StorPool volume path to clone a backup source disk from + // — without it there is nothing to derive that from. + boolean vmStopped = VirtualMachine.State.Stopped.equals(vm.getState()); + List vmVolumes = volumeDao.findByInstance(vm.getId()); + boolean hasStorPoolVolume = hasStorPoolVolume(vmVolumes); + if (vmStopped || hasStorPoolVolume) { + if (!vmStopped) { + logger.debug("VM {} looks {} but has a StorPool volume — sending volume pool/path info to the agent anyway, " + + "in case nasbackup.sh's own liveness check finds it already stopped by execution time", + vm.getInstanceName(), vm.getState()); + } vmVolumes.sort(Comparator.comparing(Volume::getDeviceId)); Pair, List> volumePoolsAndPaths = getVolumePoolsAndPaths(vmVolumes); command.setVolumePools(volumePoolsAndPaths.first()); @@ -745,11 +775,18 @@ private Pair restoreVMBackup(VirtualMachine vm, Backup backup) private List getBackupFiles(List backedVolumes) { List backupFiles = new ArrayList<>(); for (Backup.VolumeInfo backedVolume : backedVolumes) { - backupFiles.add(backedVolume.getPath()); + backupFiles.add(getBackupFileIdentifier(backedVolume.getPath())); } return backupFiles; } + // nasbackup.sh names each qcow2 after the basename of the volume's disk path. No-op for + // most drivers (bare volume path already); reduces StorPool's full device path to match. + private String getBackupFileIdentifier(String volumePath) { + int idx = volumePath.lastIndexOf('/'); + return idx >= 0 ? volumePath.substring(idx + 1) : volumePath; + } + private Pair, List> getVolumePoolsAndPaths(List volumes) { List volumePools = new ArrayList<>(); List volumePaths = new ArrayList<>(); @@ -762,9 +799,13 @@ private Pair, List> getVolumePoolsAndPaths(List DataStore dataStore = dataStoreMgr.getDataStore(storagePool.getId(), DataStoreRole.Primary); volumePools.add(dataStore != null ? (PrimaryDataStoreTO)dataStore.getTO() : null); - String volumePathPrefix = getVolumePathPrefix(storagePool); - String volumePathSuffix = getVolumePathSuffix(storagePool); - volumePaths.add(String.format("%s%s%s", volumePathPrefix, volume.getPath(), volumePathSuffix)); + if (Storage.StoragePoolType.StorPool.equals(storagePool.getPoolType())) { + volumePaths.add(volume.getPath()); + } else { + String volumePathPrefix = getVolumePathPrefix(storagePool); + String volumePathSuffix = getVolumePathSuffix(storagePool); + volumePaths.add(String.format("%s%s%s", volumePathPrefix, volume.getPath(), volumePathSuffix)); + } } return new Pair<>(volumePools, volumePaths); } @@ -777,6 +818,8 @@ private String getVolumePathPrefix(StoragePoolVO storagePool) { volumePathPrefix = storagePool.getPath() + "/"; } else if (Storage.StoragePoolType.Linstor.equals(storagePool.getPoolType())) { volumePathPrefix = "/dev/drbd/by-res/cs-"; + } else if (Storage.StoragePoolType.StorPool.equals(storagePool.getPoolType())) { + volumePathPrefix = storagePool.getPath(); } else { // Should be Storage.StoragePoolType.NetworkFilesystem volumePathPrefix = String.format("/mnt/%s/", storagePool.getUuid()); @@ -847,7 +890,7 @@ public Pair restoreBackedUpVolume(Backup backup, Backup.VolumeI restoreCommand.setVmExists(null); restoreCommand.setVmState(vmNameAndState.second()); restoreCommand.setMountTimeout(NASBackupRestoreMountTimeout.value()); - restoreCommand.setBackupFiles(Collections.singletonList(matchingVolume.getPath())); + restoreCommand.setBackupFiles(Collections.singletonList(getBackupFileIdentifier(matchingVolume.getPath()))); BackupAnswer answer; try { @@ -859,6 +902,9 @@ public Pair restoreBackedUpVolume(Backup backup, Backup.VolumeI } if (answer.getResult()) { + if (answer.getRestoredVolumePath() != null) { + restoredVolume.setPath(answer.getRestoredVolumePath()); + } try { volumeDao.persist(restoredVolume); } catch (Exception e) { 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..05178da7b29a 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 @@ -23,6 +23,7 @@ import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.UUID; import com.cloud.vm.snapshot.dao.VMSnapshotDao; import org.junit.Assert; @@ -574,6 +575,164 @@ public void restoreClearsActiveCheckpointDetail() throws AgentUnavailableExcepti Mockito.verify(vmInstanceDetailsDao).removeDetail(vmId, NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID); } + private VMInstanceVO mockActiveVm(Long vmId, Long hostId, String name) { + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getLastHostId()).thenReturn(hostId); + Mockito.when(vm.getRemoved()).thenReturn(null); + Mockito.when(vm.getName()).thenReturn(name); + return vm; + } + + private void mockActiveHostById(Long hostId) { + HostVO host = mock(HostVO.class); + Mockito.when(host.getStatus()).thenReturn(Status.Up); + Mockito.when(host.getId()).thenReturn(hostId); + Mockito.when(hostDao.findById(hostId)).thenReturn(host); + } + + private void mockHostByIp(String hostIp, Long hostId) { + HostVO host = mock(HostVO.class); + Mockito.when(host.getId()).thenReturn(hostId); + Mockito.when(hostDao.findByIp(hostIp)).thenReturn(host); + } + + private BackupVO mockBackup(Long vmId, Long backupOfferingId, String externalId, Long id) { + BackupVO backup = new BackupVO(); + backup.setVmId(vmId); + backup.setBackupOfferingId(backupOfferingId); + backup.setExternalId(externalId); + ReflectionTestUtils.setField(backup, "id", id); + return backup; + } + + private BackupVO mockBackupWithVolume(Long vmId, Long backupOfferingId, String externalId, + long size, Backup.VolumeInfo backedUp, Long id) { + BackupVO backup = mockBackup(vmId, backupOfferingId, externalId, id); + backup.setSize(size); + backup.setBackedUpVolumes(new Gson().toJson(Collections.singletonList(backedUp))); + return backup; + } + + private void mockNasRepository(Long backupOfferingId) { + BackupRepositoryVO repo = new BackupRepositoryVO(1L, "nas", "test-repo", + "nfs", "address", "sync", 1024L, null); + Mockito.when(backupRepositoryDao.findByBackupOfferingId(backupOfferingId)).thenReturn(repo); + } + + private void mockStorPoolRootVolume(Long vmId, Long poolId, String devicePath) { + StoragePoolVO pool = mock(StoragePoolVO.class); + Mockito.when(pool.getId()).thenReturn(poolId); + Mockito.when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.StorPool); + Mockito.when(storagePoolDao.findById(poolId)).thenReturn(pool); + + VolumeVO rootVolume = mock(VolumeVO.class); + Mockito.when(rootVolume.getPoolId()).thenReturn(poolId); + Mockito.when(rootVolume.getPath()).thenReturn(devicePath); + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(Collections.singletonList(rootVolume)); + } + + private void mockSourceVolume(String volUuid, String name) { + VolumeVO srcVolume = mock(VolumeVO.class); + Mockito.when(srcVolume.getUuid()).thenReturn(volUuid); + Mockito.when(srcVolume.getName()).thenReturn(name); + Mockito.when(volumeDao.findByUuid(volUuid)).thenReturn(srcVolume); + } + + private void mockThinDiskOffering(Long diskOfferingId) { + DiskOfferingVO diskOffering = mock(DiskOfferingVO.class); + Mockito.when(diskOffering.getId()).thenReturn(diskOfferingId); + Mockito.when(diskOffering.getProvisioningType()).thenReturn(Storage.ProvisioningType.THIN); + Mockito.when(diskOfferingDao.findByUuid(Mockito.anyString())).thenReturn(diskOffering); + } + + private void mockStorPoolPoolByUuid(String dsUuid, Long poolId) { + StoragePoolVO pool = mock(StoragePoolVO.class); + Mockito.when(pool.getId()).thenReturn(poolId); + Mockito.when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.StorPool); + Mockito.when(storagePoolDao.findByUuid(dsUuid)).thenReturn(pool); + } + + private BackupAnswer mockSuccessfulAgentSend() throws AgentUnavailableException, OperationTimedoutException { + BackupAnswer answer = mock(BackupAnswer.class); + Mockito.when(answer.getResult()).thenReturn(true); + Mockito.when(agentManager.send(Mockito.anyLong(), Mockito.any(RestoreBackupCommand.class))).thenReturn(answer); + return answer; + } + + private void assertRestoreVolumePaths(List expectedPaths) throws AgentUnavailableException, OperationTimedoutException { + ArgumentCaptor captor = ArgumentCaptor.forClass(RestoreBackupCommand.class); + Mockito.verify(agentManager).send(Mockito.anyLong(), captor.capture()); + Assert.assertEquals(expectedPaths, captor.getValue().getRestoreVolumePaths()); + } + + private void assertPersistedVolume(Storage.ImageFormat expectedFormat, String expectedPath) { + ArgumentCaptor volumeCaptor = ArgumentCaptor.forClass(VolumeVO.class); + Mockito.verify(volumeDao).persist(volumeCaptor.capture()); + Assert.assertEquals(expectedFormat, volumeCaptor.getValue().getFormat()); + Assert.assertEquals(expectedPath, volumeCaptor.getValue().getPath()); + } + + /** + * StorPool stores the full device path in the volume's path column, so it must be sent as-is. + */ + @Test + public void restoreVMBackupDoesNotDoublePrefixStorPoolVolumePath() + throws AgentUnavailableException, OperationTimedoutException { + Long vmId = 21L; + Long hostId = 22L; + Long backupOfferingId = 23L; + Long poolId = 24L; + String storPoolDevicePath = "/dev/storpool-byid/t.t.t"; + + VMInstanceVO vm = mockActiveVm(vmId, hostId, "vm21"); + mockActiveHostById(hostId); + BackupVO backup = mockBackup(vmId, backupOfferingId, "i-2-21-VM/2026.06.01.10.00.00", 210L); + mockNasRepository(backupOfferingId); + mockStorPoolRootVolume(vmId, poolId, storPoolDevicePath); + mockSuccessfulAgentSend(); + + boolean ok = nasBackupProvider.restoreVMFromBackup(vm, backup, false, null); + Assert.assertTrue(ok); + + assertRestoreVolumePaths(Collections.singletonList(storPoolDevicePath)); + } + + /** + * StorPool provisions/names new volumes itself (a globalId), so the persisted path must + * come from BackupAnswer#getRestoredVolumePath, not the guessed UUID-based path. + */ + @Test + public void restoreBackedUpVolumeUsesQcow2FormatAndRealPathForStorPool() + throws AgentUnavailableException, OperationTimedoutException { + Long backupOfferingId = 33L; + Long poolId = 34L; + String volUuid = UUID.randomUUID().toString(); + String hostIp = "10.0.0.9"; + String dsUuid = UUID.randomUUID().toString(); + String realStorPoolPath = "/dev/storpool-byid/t.t.t"; + + mockSourceVolume(volUuid, "data-sp"); + mockThinDiskOffering(6L); + mockStorPoolPoolByUuid(dsUuid, poolId); + mockHostByIp(hostIp, 8L); + mockNasRepository(backupOfferingId); + + Backup.VolumeInfo backedUp = new Backup.VolumeInfo(volUuid, "i-2-99-VM/2026/data-sp.qcow2", + Volume.Type.DATADISK, 2048L, 1L, "disk-offering-uuid", null, null); + BackupVO backup = mockBackupWithVolume(99L, backupOfferingId, "i-2-99-VM/2026.06.22.10.00.00", + 2048L, backedUp, 330L); + + BackupAnswer answer = mockSuccessfulAgentSend(); + Mockito.when(answer.getRestoredVolumePath()).thenReturn(realStorPoolPath); + + Pair result = nasBackupProvider.restoreBackedUpVolume( + backup, backedUp, hostIp, dsUuid, new Pair<>("i-2-42-VM", VirtualMachine.State.Stopped), null, false); + Assert.assertTrue(result.first()); + + assertPersistedVolume(Storage.ImageFormat.QCOW2, realStorPoolPath); + } + /** * Single-volume restore (restoreBackedUpVolume) must also clear the target VM's * active_checkpoint_id, so the next backup of that VM is a fresh full — the restored 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..171d396f8efa 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 @@ -95,6 +95,7 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser List backupFiles = command.getBackupFiles(); String newVolumeId = null; + String actualRestoredVolumePath = null; try { String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions, mountTimeout); if (Objects.isNull(vmExists)) { @@ -103,8 +104,11 @@ 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, + String resolvedVolumePath = restoreVolume(storagePoolMgr, backupPath, volumePool, volumePath, diskType, backupFile, size, new Pair<>(vmName, command.getVmState()), mountDirectory, timeout, mountTimeout); + if (Storage.StoragePoolType.StorPool.equals(volumePool.getPoolType())) { + actualRestoredVolumePath = resolvedVolumePath; + } } else if (Boolean.TRUE.equals(vmExists)) { restoreVolumesOfExistingVM(storagePoolMgr, restoreVolumePools, restoreVolumePaths, backedVolumeUUIDs, backupPath, backupFiles, mountDirectory, timeout, mountTimeout); } else { @@ -115,7 +119,9 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser return new BackupAnswer(command, false, errorMessage); } - return new BackupAnswer(command, true, newVolumeId); + BackupAnswer answer = new BackupAnswer(command, true, newVolumeId); + answer.setRestoredVolumePath(actualRestoredVolumePath); + return answer; } private void verifyBackupFile(String backupPath, String volUuid) { @@ -173,7 +179,12 @@ private void restoreVolumesOfDestroyedVMs(KVMStoragePoolManager storagePoolMgr, } } - private void restoreVolume(KVMStoragePoolManager storagePoolMgr, String backupPath, PrimaryDataStoreTO volumePool, String volumePath, String diskType, String backupFile, + /** + * @return the volume path the backup was actually written to. Equal to {@code volumePath} + * except when the pool assigns the created volume's identity itself (StorPool), in which + * case it reflects the real device path. + */ + private String 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; String volumeUuid; @@ -181,15 +192,17 @@ private void restoreVolume(KVMStoragePoolManager storagePoolMgr, String backupPa bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType); volumeUuid = getVolumeUuidFromPath(volumePath, volumePool); verifyBackupFile(bkpPath, volumeUuid); - if (!replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, bkpPath, timeout, true, size)) { + Pair restoreResult = replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, bkpPath, timeout, true, size); + if (!restoreResult.first()) { throw new CloudRuntimeException(String.format("Unable to restore contents from the backup volume [%s].", volumeUuid)); - } + volumePath = restoreResult.second(); if (VirtualMachine.State.Running.equals(vmNameAndState.second())) { if (!attachVolumeToVm(storagePoolMgr, vmNameAndState.first(), volumePool, volumePath)) { throw new CloudRuntimeException(String.format("Failed to attach volume to VM: %s", vmNameAndState.first())); } } + return volumePath; } finally { unmountBackupDirectory(mountDirectory, mountTimeout); deleteTemporaryDirectory(mountDirectory); @@ -293,11 +306,16 @@ private boolean checkBackupPathExists(String backupPath) { } private boolean replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String backupPath, int timeout) { - return replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, backupPath, timeout, false, null); + return replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, backupPath, timeout, false, null).first(); } - private boolean replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String backupPath, int timeout, boolean createTargetVolume, Long size) { - if (List.of(Storage.StoragePoolType.RBD, Storage.StoragePoolType.Linstor).contains(volumePool.getPoolType())) { + /** + * @return (success, the volume path the backup was actually written to). The path only + * differs from the input {@code volumePath} for pools that assign the created volume's + * identity themselves instead of accepting the one the caller proposed (StorPool). + */ + private Pair replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String backupPath, int timeout, boolean createTargetVolume, Long size) { + if (List.of(Storage.StoragePoolType.RBD, Storage.StoragePoolType.Linstor, Storage.StoragePoolType.StorPool).contains(volumePool.getPoolType())) { return replaceBlockDeviceWithBackup(storagePoolMgr, volumePool, volumePath, backupPath, timeout, createTargetVolume, size); } @@ -310,12 +328,12 @@ private boolean replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, Pr if (hasBackingChain(backupPath)) { String[] qemuImgCmd = new String[] { Script.getExecutableAbsolutePath("qemu-img"), "convert", "-O", "qcow2", backupPath, volumePath }; int flattenExit = Script.executeCommandForExitValue(qemuImgCmd); - return flattenExit == 0; + return new Pair<>(flattenExit == 0, volumePath); } String[] rsyncCmd = new String[] { Script.getExecutableAbsolutePath("rsync"), "-az", backupPath, volumePath }; int exitValue = Script.executeCommandForExitValue(timeout, rsyncCmd); - return exitValue == 0; + return new Pair<>(exitValue == 0, volumePath); } private boolean hasBackingChain(String qcow2Path) { @@ -323,23 +341,32 @@ private boolean hasBackingChain(String qcow2Path) { String.format(QEMU_IMG_HAS_BACKING_COMMAND, qcow2Path)) == 0; } - private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String backupPath, int timeout, boolean createTargetVolume, Long size) { + private Pair replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String backupPath, int timeout, boolean createTargetVolume, Long size) { KVMStoragePool volumeStoragePool = storagePoolMgr.getStoragePool(volumePool.getPoolType(), volumePool.getUuid()); + Storage.StoragePoolType poolType = volumePool.getPoolType(); QemuImg qemu; try { qemu = new QemuImg(timeout, true, false); String volumeUuid = getVolumeUuidFromPath(volumePath, volumePool); KVMPhysicalDisk disk = null; if (createTargetVolume) { - if (Storage.StoragePoolType.Linstor.equals(volumePool.getPoolType())) { + if (Storage.StoragePoolType.Linstor.equals(poolType) || Storage.StoragePoolType.StorPool.equals(poolType)) { if (size == null) { - throw new CloudRuntimeException("Restore volume size is required for Linstor pool when creating target volume"); + throw new CloudRuntimeException(String.format("Restore volume size is required for %s pool when creating target volume", poolType)); } disk = volumeStoragePool.createPhysicalDisk(volumeUuid, QemuImg.PhysicalDiskFormat.RAW, Storage.ProvisioningType.THIN, size, null); + if (disk == null) { + throw new CloudRuntimeException(String.format("Failed to provision a %s volume for restore [%s]", poolType, volumeUuid)); + } + if (Storage.StoragePoolType.StorPool.equals(poolType)) { + volumePath = disk.getPath(); + } } } else { - if (Storage.StoragePoolType.Linstor.equals(volumePool.getPoolType())) { - storagePoolMgr.connectPhysicalDisk(volumePool.getPoolType(), volumePool.getUuid(), volumeUuid, null); + if (Storage.StoragePoolType.Linstor.equals(poolType)) { + storagePoolMgr.connectPhysicalDisk(poolType, volumePool.getUuid(), volumeUuid, null); + } else if (Storage.StoragePoolType.StorPool.equals(poolType)) { + storagePoolMgr.connectPhysicalDisk(poolType, volumePool.getUuid(), volumePath, null); } else { disk = volumeStoragePool.getPhysicalDisk(volumePath); } @@ -349,7 +376,7 @@ private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMg logger.debug("Restoring volume: {}", disk.toString()); } } catch (LibvirtException ex) { - throw new CloudRuntimeException(String.format("Failed to create qemu-img command to restore %s volume with backup", volumePool.getPoolType()), ex); + throw new CloudRuntimeException(String.format("Failed to create qemu-img command to restore %s volume with backup", poolType), ex); } QemuImgFile srcBackupFile = null; @@ -357,15 +384,16 @@ private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMg try { srcBackupFile = new QemuImgFile(backupPath, QemuImg.PhysicalDiskFormat.QCOW2); String destVolume; - switch(volumePool.getPoolType()) { + switch(poolType) { case Linstor: + case StorPool: destVolume = volumePath; break; case RBD: destVolume = KVMPhysicalDisk.RBDStringBuilder(volumeStoragePool, volumePath); break; default: - throw new CloudRuntimeException(String.format("Unsupported storage pool type [%s] for block device restore with backup.", volumePool.getPoolType())); + throw new CloudRuntimeException(String.format("Unsupported storage pool type [%s] for block device restore with backup.", poolType)); } destVolumeFile = new QemuImgFile(destVolume, QemuImg.PhysicalDiskFormat.RAW); logger.debug("Starting convert backup {} to volume {}", backupPath, volumePath); @@ -375,10 +403,10 @@ private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMg String srcFilename = srcBackupFile != null ? srcBackupFile.getFileName() : null; String destFilename = destVolumeFile != null ? destVolumeFile.getFileName() : null; logger.error("Failed to convert backup {} to volume {}, the error was: {}", srcFilename, destFilename, e.getMessage()); - return false; + return new Pair<>(false, volumePath); } - return true; + return new Pair<>(true, volumePath); } private boolean attachVolumeToVm(KVMStoragePoolManager storagePoolMgr, String vmName, PrimaryDataStoreTO volumePool, String volumePath) { 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..4c0e002a82c3 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 @@ -46,6 +46,10 @@ public class LibvirtTakeBackupCommandWrapper extends CommandWrapper diskPaths = new ArrayList<>(); - if (Objects.nonNull(volumePaths)) { - for (int idx = 0; idx < volumePaths.size(); idx++) { - PrimaryDataStoreTO volumePool = volumePools.get(idx); - String volumePath = volumePaths.get(idx); - if (volumePool.getPoolType() != Storage.StoragePoolType.RBD) { + try { + if (Objects.nonNull(volumePaths)) { + for (int idx = 0; idx < volumePaths.size(); idx++) { + PrimaryDataStoreTO volumePool = volumePools.get(idx); + String volumePath = volumePaths.get(idx); + if (volumePool.getPoolType() == Storage.StoragePoolType.RBD) { + KVMStoragePool volumeStoragePool = storagePoolMgr.getStoragePool(volumePool.getPoolType(), volumePool.getUuid()); + String rbdDestVolumeFile = KVMPhysicalDisk.RBDStringBuilder(volumeStoragePool, volumePath); + diskPaths.add(rbdDestVolumeFile); + continue; + } + // StorPool (among others) is passed through as-is: nasbackup.sh checks the + // VM's actual liveness itself right before acting, and only then — if the VM + // turns out to be stopped — clones this into a point-in-time backup source + // volume. Doing that here instead would rely on the same stale state read + // nasbackup.sh's own check exists to correct for. diskPaths.add(volumePath); - } else { - KVMStoragePool volumeStoragePool = storagePoolMgr.getStoragePool(volumePool.getPoolType(), volumePool.getUuid()); - String rbdDestVolumeFile = KVMPhysicalDisk.RBDStringBuilder(volumeStoragePool, volumePath); - diskPaths.add(rbdDestVolumeFile); } } - } - Pair result = runBackupScript(libvirtComputingResource, command, vmName, backupRepoType, backupRepoAddress, - mountOptions, backupPath, diskPaths, command.getMode(), - command.getBitmapNew(), command.getBitmapParent(), command.getParentPaths(), timeout); + Pair result = runBackupScript(libvirtComputingResource, command, vmName, backupRepoType, backupRepoAddress, + mountOptions, backupPath, diskPaths, command.getMode(), + command.getBitmapNew(), command.getBitmapParent(), command.getParentPaths(), timeout); - if (result.first() != 0) { - logger.debug("Failed to take VM backup: " + result.second()); - BackupAnswer answer = new BackupAnswer(command, false, StringUtils.trimToEmpty(result.second())); - if (EXIT_CLEANUP_FAILED.equals(result.first())) { - logger.debug("Backup cleanup failed"); - answer.setNeedsCleanup(true); + if (result.first() != 0) { + logger.debug("Failed to take VM backup: " + result.second()); + BackupAnswer answer = new BackupAnswer(command, false, StringUtils.trimToEmpty(result.second())); + if (EXIT_CLEANUP_FAILED.equals(result.first())) { + logger.debug("Backup cleanup failed"); + answer.setNeedsCleanup(true); + } + return answer; } + + // The script self-heals to a full backup when an incremental can't proceed (e.g. the + // parent checkpoint can't be re-registered) and signals it with INCREMENTAL_FALLBACK + // on stdout. Detect it and the reported size from the raw output, then strip both + // marker lines before using stdout as the answer's human-facing details. + String rawStdout = result.second(); + boolean incrementalFallback = StringUtils.contains(rawStdout, INCREMENTAL_FALLBACK_MARKER); + long backupSize = extractBackupSize(rawStdout); + String stdout = stripMarkerLines(rawStdout).trim(); + + BackupAnswer answer = new BackupAnswer(command, true, stdout); + answer.setSize(backupSize); + // A successful run always created command.getBitmapNew() (full and incremental both do; + // it is null for legacy-full, which the orchestrator treats as "no bitmap"). + answer.setBitmapCreated(command.getBitmapNew()); + answer.setIncrementalFallback(incrementalFallback); return answer; + } catch (RuntimeException e) { + logger.error("Failed to take VM backup: " + e.getMessage(), e); + return new BackupAnswer(command, false, e.getMessage()); } - - // The script self-heals to a full backup when an incremental can't proceed (e.g. the - // parent checkpoint can't be re-registered) and signals it with INCREMENTAL_FALLBACK - // on stdout. Detect it, then strip the marker line before parsing the backup size. - String rawStdout = result.second(); - boolean incrementalFallback = StringUtils.contains(rawStdout, INCREMENTAL_FALLBACK_MARKER); - String stdout = stripMarkerLines(rawStdout).trim(); - long backupSize = parseBackupSize(stdout, diskPaths); - - BackupAnswer answer = new BackupAnswer(command, true, stdout); - answer.setSize(backupSize); - // A successful run always created command.getBitmapNew() (full and incremental both do; - // it is null for legacy-full, which the orchestrator treats as "no bitmap"). - answer.setBitmapCreated(command.getBitmapNew()); - answer.setIncrementalFallback(incrementalFallback); - return answer; } - /** Remove nasbackup.sh's stdout signalling marker lines so they don't pollute size parsing. */ + /** Remove nasbackup.sh's stdout signalling marker lines so they don't pollute the answer details. */ private String stripMarkerLines(String stdout) { if (StringUtils.isBlank(stdout)) { return ""; } StringBuilder sb = new StringBuilder(); for (String line : stdout.split("\n", -1)) { - if (line.contains(INCREMENTAL_FALLBACK_MARKER)) { + if (line.contains(INCREMENTAL_FALLBACK_MARKER) || line.startsWith(BACKUP_SIZE_MARKER_PREFIX)) { continue; } if (sb.length() > 0) { @@ -136,6 +151,33 @@ private String stripMarkerLines(String stdout) { return sb.toString(); } + /** + * Find nasbackup.sh's {@code BACKUP_SIZE_TOTAL=} marker line. Unlike the old + * position/shape-based parsing this replaced, it doesn't need to know or guess which of + * nasbackup.sh's code paths actually ran. + *

+ * The marker is only metadata: a missing/unparseable marker does not mean the backup (which + * already exited 0) failed, so this logs a warning and reports an unknown size (0) rather + * than throwing — the caller's generic RuntimeException handler would otherwise turn an + * on-disk-successful backup into a reported failure. + */ + private long extractBackupSize(String rawStdout) { + if (rawStdout != null) { + for (String line : rawStdout.split("\n", -1)) { + if (line.startsWith(BACKUP_SIZE_MARKER_PREFIX)) { + try { + return Long.parseLong(line.substring(BACKUP_SIZE_MARKER_PREFIX.length()).trim()); + } catch (NumberFormatException e) { + logger.warn("nasbackup.sh reported an unparseable {} marker: {}", BACKUP_SIZE_MARKER_PREFIX, line); + return 0L; + } + } + } + } + logger.warn("nasbackup.sh did not report a {} marker in its output; backup succeeded but its size is unknown", BACKUP_SIZE_MARKER_PREFIX); + return 0L; + } + /** * Run nasbackup.sh once with the given args. Returns the exit code + captured stdout. */ @@ -208,24 +250,4 @@ private String validateBackupArgs(TakeBackupCommand command) { } return "Unknown backup mode: " + mode; } - - /** - * Sum the per-disk size lines emitted by nasbackup.sh. Single-volume mode emits one - * line containing just the byte count; multi-volume mode emits one line per disk - * whose first whitespace-separated token is the byte count. - */ - private long parseBackupSize(String stdout, List diskPaths) { - long backupSize = 0L; - if (CollectionUtils.isEmpty(diskPaths)) { - List outputLines = Arrays.asList(stdout.split("\n")); - if (!outputLines.isEmpty()) { - backupSize = Long.parseLong(outputLines.get(outputLines.size() - 1).trim()); - } - } else { - for (String line : stdout.split("\n")) { - backupSize = backupSize + Long.parseLong(line.split(" ")[0].trim()); - } - } - return backupSize; - } } diff --git a/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java b/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java index 545f7b33c5fd..0d471323c297 100644 --- a/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java +++ b/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java @@ -454,7 +454,47 @@ public boolean createFolder(String uuid, String path, String localPath) { @Override public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, PhysicalDiskFormat format, ProvisioningType provisioningType, long size, byte[] passphrase) { - return null; + SP_LOG("StorPoolStorageAdaptor.createPhysicalDisk: name=%s, pool=%s, size=%d", name, pool.getUuid(), size); + + if (passphrase != null) { + throw new CloudRuntimeException("Encrypted StorPool volumes are not supported"); + } + + if (size <= 0) { + throw new CloudRuntimeException("Size must be greater than 0 to create a StorPool volume"); + } + + String[] uuidParts = pool.getUuid() != null ? pool.getUuid().split(";") : new String[0]; + if (uuidParts.length == 0 || StringUtils.isBlank(uuidParts[0])) { + throw new CloudRuntimeException("Unable to resolve the StorPool template for pool " + pool.getUuid()); + } + String template = uuidParts[0]; + + Map tags = new HashMap<>(); + tags.put("cs", "volume"); + tags.put("uuid", name); + + OutputInterpreter.AllLinesParser parser = createStorPoolVolume(template, size, tags, + String.format(" for %s", name)); + + String globalId = getNameFromResponse(parser.getLines(), false, false); + if (StringUtils.isBlank(globalId)) { + LOGGER.warn(String.format("StorPool volume for %s was created but its globalId could not be parsed from the response; " + + "it must be found (tag uuid=%s) and deleted manually to avoid an orphaned volume", name, name)); + throw new CloudRuntimeException(String.format("StorPool did not return a volume name/globalId when creating %s", name)); + } + + String volumePath = StorPoolUtil.devPath(globalId); + if (!attachOrDetachVolume("attach", "volume", volumePath)) { + volumeDelete(globalId); + throw new CloudRuntimeException(String.format("Could not attach newly created StorPool volume %s", volumePath)); + } + + KVMPhysicalDisk disk = new KVMPhysicalDisk(volumePath, name, pool); + disk.setFormat(PhysicalDiskFormat.RAW); + disk.setSize(size); + disk.setVirtualSize(size); + return disk; } @Override @@ -466,21 +506,25 @@ public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, String n private OutputInterpreter.AllLinesParser createStorPoolVolume(KVMStoragePool destPool, QemuImgFile srcFile, QemuImg qemu, String templateUuid) throws QemuImgException, LibvirtException { Map info = qemu.info(srcFile); - Map reqParams = new HashMap<>(); - reqParams.put("template", templateUuid); - reqParams.put("size", info.get("virtual_size")); Map tags = new HashMap<>(); tags.put("cs", "template"); + return createStorPoolVolume(templateUuid, info.get("virtual_size"), tags, ""); + } + + private OutputInterpreter.AllLinesParser createStorPoolVolume(String templateUuid, Object size, + Map tags, String errorContext) { + Map reqParams = new HashMap<>(); + reqParams.put("template", templateUuid); + reqParams.put("size", size); reqParams.put("tags", tags); - Gson gson = new Gson(); - String js = gson.toJson(reqParams); + String js = new Gson().toJson(reqParams); - Script sc = createStorPoolRequest(js, "VolumeCreate", null,true); + Script sc = createStorPoolRequest(js, "VolumeCreate", null, true); OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser(); String res = sc.execute(parser); if (res != null) { - throw new CloudRuntimeException("Could not create volume due to: " + res); + throw new CloudRuntimeException(String.format("Could not create StorPool volume%s due to: %s", errorContext, res)); } return parser; } diff --git a/scripts/vm/hypervisor/kvm/nasbackup.sh b/scripts/vm/hypervisor/kvm/nasbackup.sh index 216f5b5a4de1..e28837d5120b 100755 --- a/scripts/vm/hypervisor/kvm/nasbackup.sh +++ b/scripts/vm/hypervisor/kvm/nasbackup.sh @@ -33,6 +33,9 @@ MOUNT_OPTS="" BACKUP_DIR="" DISK_PATHS="" QUIESCE="" +# StorPool volumes cloned via sp_create_backup_source_disk during this run, so cleanup() can +# always release them even if the script exits before their normal per-disk cleanup runs. +SP_CLEANUP_VOLUMES=() # Incremental backup parameters (all optional; legacy callers omit them) MODE="" # "full" or "incremental"; empty => legacy full-only behavior (no checkpoint created) BITMAP_NEW="" # Bitmap/checkpoint name to create with this backup (e.g. "backup-1711586400") @@ -135,6 +138,129 @@ get_linstor_uuid_from_device() { return 1 } +# StorPool volume name (with the "~globalId" form the storpool/storpool_req CLIs expect) from +# /dev/storpool-byid/. +sp_volume_name_from_path() { + local fullpath="$1" + local name + if [[ "$fullpath" == /dev/storpool-byid/* ]]; then + name="${fullpath#/dev/storpool-byid/}" + name="~${name%%/*}" + else + return 1 + fi + echo "$name" +} + +# Poll for the device node to actually appear after a successful attach: udev creates the +# symlink asynchronously, so a successful "attach" CLI call doesn't guarantee the device path +# is usable yet. +sp_wait_for_device_symlink() { + local name="$1" + local devpath="/dev/storpool-byid/${name#\~}" + local tries=10 i + for ((i = 0; i < tries; i++)); do + if [[ -e "$devpath" ]] && [[ "$(blockdev --getsize64 "$devpath" 2>/dev/null || echo 0)" -gt 0 ]]; then + return 0 + fi + sleep 0.1 + done + return 1 +} + +# detach is retried (a live volume's detach can transiently fail while still in use), attach is +# not. A successful attach also waits for the device symlink. +sp_attach_detach_volume() { + local cmd="$1" name="$2" + local tries=10 i + for ((i = 0; i < tries; i++)); do + if [[ "$cmd" == "attach" ]]; then + storpool -M -B attach volume "$name" here onRemoteAttached export >>"$logFile" 2>&1 && { sp_wait_for_device_symlink "$name"; return $?; } + break + else + storpool -M -B detach volume "$name" here >>"$logFile" 2>&1 && return 0 + sleep 1 + fi + done + return 1 +} + +# Clones $1 (a live StorPool volume path) into a new, point-in-time StorPool volume via the +# VolumeCreate API's baseOn option, attaches it, and echoes the clone's device path. Only called +# once this script's own (fresh, right-here) VM liveness check has already routed to the cold +# path — see backup_stopped_vm. +sp_create_backup_source_disk() { + local volume_path="$1" + local base_on + if ! base_on=$(sp_volume_name_from_path "$volume_path"); then + echo "Could not resolve a StorPool volume name from path $volume_path to create a backup source volume" >&2 + return 1 + fi + + log -ne "StorPool: creating backup source volume based on $base_on (from $volume_path)" + + local resp + if ! resp=$(storpool_req -P -M --json "{\"baseOn\":\"$base_on\",\"tags\":{\"cs\":\"backup\"}}" VolumeCreate 2>>"$logFile"); then + log -ne "StorPool: VolumeCreate baseOn=$base_on failed, see above for storpool_req output" + echo "Could not create a backup source volume based on $base_on" >&2 + return 1 + fi + + local global_id + global_id=$(python3 -c ' +import sys, json +try: + data = json.load(sys.stdin) + gid = data.get("globalId") +except Exception: + gid = None +if not gid: + sys.exit(1) +print(gid) +' <<< "$resp" 2>>"$logFile") || { + log -ne "StorPool: VolumeCreate baseOn=$base_on returned no globalId, response: $resp -- the volume was created but its name could not be parsed; it must be found (tag cs=backup, baseOn=$base_on) and deleted manually to avoid an orphaned volume" + echo "StorPool did not return a volume name when cloning $base_on for backup" >&2 + return 1 + } + + local clone_path="/dev/storpool-byid/$global_id" + local clone_name="~$global_id" + log -ne "StorPool: created backup source volume $clone_name ($clone_path), attaching" + if ! sp_attach_detach_volume "attach" "$clone_name"; then + log -ne "StorPool: failed to attach backup source volume $clone_name, deleting it" + storpool_req -P -M VolumeDelete "$clone_name" >>"$logFile" 2>&1 || true + echo "Could not attach backup source volume $clone_path cloned from $base_on" >&2 + return 1 + fi + + SP_CLEANUP_VOLUMES+=("$clone_name") + log -ne "StorPool: backup source volume $clone_name attached at $clone_path, reading from it for this backup" + echo "$clone_path" +} + +# Detach + delete a volume previously returned by sp_create_backup_source_disk. Best-effort: +# logs on failure rather than aborting the backup, matching the wrapper's old cleanup semantics. +sp_delete_backup_source_disk() { + local clone_name="$1" + log -ne "StorPool: releasing backup source volume $clone_name" + sp_attach_detach_volume "detach" "$clone_name" || log -ne "Failed to detach StorPool backup source volume $clone_name" + storpool_req -P -M VolumeDelete "$clone_name" >>"$logFile" 2>&1 || log -ne "Failed to delete StorPool backup source volume $clone_name" +} + +# Safety net for any clone not already released by its own per-disk cleanup (e.g. the script +# exiting between creating it and reaching that point). A cleanup here means an earlier step +# didn't run its normal release — worth noticing, so it's logged even though it isn't fatal. +sp_cleanup_backup_source_disks() { + local clone_name + for clone_name in "${SP_CLEANUP_VOLUMES[@]:-}"; do + [[ -z "$clone_name" ]] && continue + log -ne "StorPool: cleaning up backup source volume $clone_name left over on script exit" + sp_delete_backup_source_disk "$clone_name" + done + SP_CLEANUP_VOLUMES=() +} +trap sp_cleanup_backup_source_disks EXIT + backup_running_vm() { mount_operation mkdir -p "$dest" || { echo "Failed to create backup directory $dest"; exit 1; } @@ -412,12 +538,12 @@ for dev in data.get("return", []) or []: ) fi - # Print statistics - virsh -c qemu:///system domjobinfo $VM --completed + # Print statistics (informational only — logged, not parsed; see BACKUP_SIZE_TOTAL below) + virsh -c qemu:///system domjobinfo $VM --completed >>"$logFile" 2>&1 backup_size=$(du -sb "$dest" 2>>"$logFile" | cut -f1) || { log -ne "WARNING: du failed for $dest, reporting size as 0"; backup_size=0; } timeout "$UNMOUNT_TIMEOUT" umount "$mount_point" 2>>"$logFile" || { log "WARNING: umount of $mount_point failed or timed out"; true; } rmdir "$mount_point" 2>>"$logFile" || { log "WARNING: rmdir of $mount_point failed"; true; } - echo "$backup_size" + echo "BACKUP_SIZE_TOTAL=$backup_size" } backup_stopped_vm() { @@ -426,10 +552,13 @@ backup_stopped_vm() { mount_operation mkdir -p "$dest" || { echo "Failed to create backup directory $dest"; exit 1; } - IFS="," + local -a disk_arr=() + IFS=',' read -ra disk_arr <<< "$DISK_PATHS" name="root" - for disk in $DISK_PATHS; do + for disk in "${disk_arr[@]}"; do + local read_disk="$disk" + local sp_clone_name="" if [[ "$disk" == rbd:* ]]; then volUuid=$(get_ceph_uuid_from_path "$disk") elif [[ "$disk" == /dev/drbd/by-res/* ]]; then @@ -440,25 +569,48 @@ backup_stopped_vm() { cleanup exit 1 fi + elif [[ "$disk" == /dev/storpool-byid/* ]]; then + volUuid="${disk##*/}" + # Clone before reading, so this backup never depends on (or interferes with) the live + # volume's attach state — safe even if the VM is started again before this finishes. + if ! read_disk=$(sp_create_backup_source_disk "$disk"); then + log -ne "Failed to create a StorPool backup source volume for $disk" + echo "Failed to create a StorPool backup source volume for $disk" + cleanup + exit 1 + fi + sp_clone_name=$(sp_volume_name_from_path "$read_disk") else volUuid="${disk##*/}" fi output="$dest/$name.$volUuid.qcow2" - if ! qemu-img convert -O qcow2 "$disk" "$output" >> "$logFile" 2> >(cat >&2); then - echo "qemu-img convert failed for $disk $output" + if ! qemu-img convert -O qcow2 "$read_disk" "$output" >> "$logFile" 2> >(cat >&2); then + log -ne "qemu-img convert failed for $read_disk $output" + echo "qemu-img convert failed for $read_disk $output" cleanup exit 1 fi + log -ne "Wrote $output from $read_disk" + + if [[ -n "$sp_clone_name" ]]; then + sp_delete_backup_source_disk "$sp_clone_name" + local -a remaining_clones=() + local tracked_clone + for tracked_clone in "${SP_CLEANUP_VOLUMES[@]}"; do + [[ "$tracked_clone" == "$sp_clone_name" ]] || remaining_clones+=("$tracked_clone") + done + SP_CLEANUP_VOLUMES=("${remaining_clones[@]}") + fi # Pre-seed a persistent bitmap on the source disk so the NEXT backup (taken # after this VM is started again) can be incremental against the qcow2 we # just wrote. Without this, every backup after a stopped-VM backup would # fall back to full because no parent bitmap exists on the host yet. - # Only applies to file-backed qcow2 sources — RBD/LINSTOR have their own + # Only applies to file-backed qcow2 sources — RBD/LINSTOR/StorPool have their own # snapshot mechanisms and qemu-img bitmap is not the right primitive there. # bitmap --add should not fail on a file-backed qcow2; if it does, fail the backup so the # underlying problem is surfaced rather than silently degrading future backups to full. - if [[ -n "$BITMAP_NEW" && "$disk" != rbd:* && "$disk" != /dev/drbd/by-res/* ]]; then + if [[ -n "$BITMAP_NEW" && "$disk" != rbd:* && "$disk" != /dev/drbd/by-res/* && "$disk" != /dev/storpool-byid/* ]]; then if ! qemu-img bitmap --add "$disk" "$BITMAP_NEW" 2>>"$logFile"; then echo "Failed to pre-seed bitmap $BITMAP_NEW on $disk" cleanup @@ -470,7 +622,8 @@ backup_stopped_vm() { done sync - find "$dest" -type f -exec stat -c '%s' {} + + backup_size=$(du -sb "$dest" 2>>"$logFile" | cut -f1) || { log -ne "WARNING: du failed for $dest, reporting size as 0"; backup_size=0; } + echo "BACKUP_SIZE_TOTAL=$backup_size" } delete_backup() { diff --git a/test/integration/plugins/storpool/TestNasBackupStorPool.py b/test/integration/plugins/storpool/TestNasBackupStorPool.py new file mode 100644 index 000000000000..4d171cea3b8f --- /dev/null +++ b/test/integration/plugins/storpool/TestNasBackupStorPool.py @@ -0,0 +1,650 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Smoke tests for NAS backup and recovery on StorPool primary storage. + +Assumes the environment already has NAS backup configured for the zone +(a NAS backup repository and an imported backup offering). +""" + +import math +import time +import uuid + +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.codes import FAILED, KVM +from marvin.lib.base import (Account, + Backup, + BackupOffering, + BackupRepository, + DiskOffering, + SecurityGroup, + ServiceOffering, + StoragePool, + VirtualMachine, + Volume) +from marvin.lib.common import (get_domain, + get_template, + list_disk_offering, + list_networks, + list_service_offering, + list_storage_pools, + list_zones) +from marvin.lib.utils import cleanup_resources, get_hypervisor_type +from nose.plugins.attrib import attr +from storpool import spapi + +from sp_util import StorPoolHelper, TestData + + +def _template_size_gb(template): + size_bytes = getattr(template, "size", None) + if not size_bytes: + return 1 + return max(1, int(math.ceil(float(size_bytes) / (1024 ** 3)))) + + +class TestNasBackupStorPool(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + super(TestNasBackupStorPool, cls).setUpClass() + try: + cls.setUpCloudStack() + except Exception: + cls.cleanUpCloudStack() + raise + + @classmethod + def setUpCloudStack(cls): + cls._setup_common() + if cls.unsupportedHypervisor: + return + + cls.vm = cls._create_test_vm("SP-NasBkp-%s" % uuid.uuid4(), cls.disk_offering.id) + cls._verify_setup_volumes(cls.vm) + + @classmethod + def _setup_common(cls): + """Build everything shared by the StorPool-only and mixed-storage NAS + backup suites: account, template, network, backup offering, StorPool + primary storage/service/disk offering. Does not create the test VM, + so subclasses can plug in a different disk offering for it.""" + config = cls.getClsConfig() + StorPoolHelper.logger = cls + + zone_cfg = config.zones[0] + assert zone_cfg is not None + cls.zone_cfg = zone_cfg + + testClient = super(TestNasBackupStorPool, cls).getClsTestClient() + cls.apiclient = testClient.getApiClient() + cls._cleanup = [] + cls.unsupportedHypervisor = False + cls.skip_reason = None + cls.nfs_storage_pool = None + cls.nfs_tag = None + + td = TestData() + cls.testdata = td.testdata + cls.helper = StorPoolHelper() + + cls.services = testClient.getParsedTestDataConfig() + cls.domain = get_domain(cls.apiclient) + cls.zone = list_zones(cls.apiclient, name=zone_cfg.name)[0] + assert cls.zone is not None + cls.services["mode"] = cls.zone.networktype + + cls.hypervisor = get_hypervisor_type(cls.apiclient) + if str(cls.hypervisor).lower() != KVM.lower(): + cls.unsupportedHypervisor = True + cls.skip_reason = "NAS backup on StorPool requires KVM" + return + + sp_pools = cls.helper.get_pool(zone_cfg) + if not sp_pools: + cls.unsupportedHypervisor = True + cls.skip_reason = "No StorPool primary storage configured in zone" + return + + cls.spapi = spapi.Api(host=zone_cfg.spEndpoint, port=zone_cfg.spEndpointPort, + auth=zone_cfg.spAuthToken, multiCluster=True) + + template = get_template(cls.apiclient, cls.zone.id, account="system") + if template == FAILED: + assert False, "get_template() failed to return a system template" + cls.template = template + + offerings = BackupOffering.listByZone(cls.apiclient, cls.zone.id) or [] + nas_offerings = [o for o in offerings + if getattr(o, "provider", None) and "nas" in str(o.provider).lower()] + if not nas_offerings: + cls.unsupportedHypervisor = True + cls.skip_reason = "No imported NAS backup offering found in the zone" + return + cls.backup_offering = BackupOffering(nas_offerings[0].__dict__) + + primarystorage = sp_pools[0] + storage_pools = list_storage_pools(cls.apiclient, name=primarystorage["name"]) + if not storage_pools: + cls.unsupportedHypervisor = True + cls.skip_reason = "StorPool primary storage pool %s not found" % primarystorage["name"] + return + cls.primary_storage = storage_pools[0] + + sp_tag = primarystorage["tags"] if primarystorage["tags"] else primarystorage["name"] + cls.helper.updateStoragePoolTags(cls.apiclient, cls.primary_storage.id, sp_tag) + cls.sp_tag = sp_tag + + cls.template_size_gb = _template_size_gb(cls.template) + cls.service_offering = ServiceOffering.create( + cls.apiclient, + { + "name": "sp-nas-so-%s" % uuid.uuid4(), + "displaytext": "StorPool NAS offering (template %s GiB)" % cls.template_size_gb, + "cpunumber": 1, + "cpuspeed": 500, + "memory": 512, + "storagetype": "shared", + "tags": sp_tag, + }) + cls._cleanup.append(cls.service_offering) + + cls.disk_offering = cls._get_or_create_disk_offering( + sp_tag, "StorPool NAS backup disk offering") + + cls.account = cls.helper.create_account( + cls.apiclient, + cls.services["account"], + accounttype=1, + domainid=cls.domain.id, + roleid=1) + cls._cleanup.append(cls.account) + + if cls.zone.securitygroupsenabled: + securitygroup = SecurityGroup.list( + cls.apiclient, account=cls.account.name, domainid=cls.account.domainid)[0] + cls.helper.set_securityGroups( + cls.apiclient, + account=cls.account.name, + domainid=cls.account.domainid, + id=securitygroup.id) + + cls.network_ids = None + if str(cls.zone.networktype).lower() == "advanced": + networks = list_networks(cls.apiclient, zoneid=cls.zone.id, listall=True) or [] + guest_networks = [n for n in networks + if str(getattr(n, "traffictype", "")).lower() == "guest"] + shared = [n for n in guest_networks + if str(getattr(n, "type", "")).lower() == "shared"] + picked = (shared or guest_networks)[0] if (shared or guest_networks) else None + if picked is None: + cls.unsupportedHypervisor = True + cls.skip_reason = "Advanced zone has no existing guest network to deploy on" + return + cls.network_ids = [picked.id] + + @classmethod + def _get_or_create_disk_offering(cls, tag, displaytext): + disk_offerings = list_disk_offering(cls.apiclient, name=tag) + if disk_offerings is None: + disk_offering = DiskOffering.create( + cls.apiclient, + { + "name": tag, + "displaytext": displaytext, + "disksize": 5, + "storagetype": "shared", + "tags": tag, + }) + cls._cleanup.append(disk_offering) + return disk_offering + return disk_offerings[0] + + @classmethod + def _create_test_vm(cls, name, diskofferingid): + vm = VirtualMachine.create( + cls.apiclient, + {"name": name}, + zoneid=cls.zone.id, + templateid=cls.template.id, + accountid=cls.account.name, + domainid=cls.account.domainid, + serviceofferingid=cls.service_offering.id, + diskofferingid=diskofferingid, + hypervisor=cls.hypervisor, + networkids=cls.network_ids, + rootdisksize=cls.template_size_gb, + mode=cls.services["mode"]) + cls._cleanup.insert(0, vm) + return vm + + @classmethod + def _verify_setup_volumes(cls, vm): + volumes = Volume.list(cls.apiclient, virtualmachineid=vm.id, listall=True) + assert isinstance(volumes, list) and len(volumes) >= 1 + for vol in volumes: + if cls.nfs_storage_pool and vol.storageid == cls.nfs_storage_pool.id: + continue + cls.helper.verify_storpool_volume(cls.spapi, vol) + return volumes + + @classmethod + def tearDownClass(cls): + cls.cleanUpCloudStack() + + @classmethod + def cleanUpCloudStack(cls): + try: + if hasattr(cls, "vm") and cls.vm is not None: + cls._remove_vm_backups_and_offering(cls.apiclient, cls.vm) + if hasattr(cls, "_cleanup") and cls._cleanup: + cleanup_resources(cls.apiclient, cls._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + + @classmethod + def _remove_vm_backups_and_offering(cls, apiclient, vm): + try: + backups = Backup.list(apiclient, vm.id) or [] + for backup in backups: + try: + Backup.delete(apiclient, backup.id, forced=True) + except Exception: + Backup.delete(apiclient, backup.id) + except Exception: + pass + try: + if hasattr(cls, "backup_offering") and cls.backup_offering is not None: + cls.backup_offering.removeOffering(apiclient, vm.id, forced=True) + except Exception: + pass + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.cleanup = [] + self.cleanup_backups = [] + if self.unsupportedHypervisor: + self.skipTest(self.skip_reason or "Skipping unsupported environment") + self._remove_vm_backups_and_offering(self.apiclient, self.vm) + + def tearDown(self): + try: + for obj in list(self.cleanup_backups): + try: + Backup.delete(self.apiclient, obj.id, forced=True) + except Exception: + pass + for obj in self.cleanup: + if isinstance(obj, VirtualMachine): + self._remove_vm_backups_and_offering(self.apiclient, obj) + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + + def _verify_vm_volumes_on_storpool(self, vm, spapi_client=None): + """Verify every volume of vm. A volume that lives on the NFS primary + storage pool (only possible in the mixed-storage suite, where + self.nfs_storage_pool is set) is checked against NFS instead of + StorPool, so the NAS backup lifecycle tests can run unmodified + against either an all-StorPool VM or a mixed StorPool+NFS one.""" + volumes = Volume.list(self.apiclient, virtualmachineid=vm.id, listall=True) + self.assertTrue(isinstance(volumes, list) and len(volumes) >= 1, + "VM should have at least one volume") + for vol in volumes: + if self.nfs_storage_pool and vol.storageid == self.nfs_storage_pool.id: + self.assertFalse(vol.path and vol.path.startswith("/dev/storpool-byid/"), + "Volume %s is on NFS storage %s, should not have a StorPool path, got %s" % + (vol.id, self.nfs_storage_pool.id, vol.path)) + continue + self.assertTrue(vol.path and vol.path.startswith("/dev/storpool-byid/"), + "Volume %s path should be /dev/storpool-byid/..., got %s" % + (vol.id, vol.path)) + self.helper.verify_storpool_volume(spapi_client or self.spapi, vol) + spvolume = self.helper.get_storpool_volume_by_path(spapi_client or self.spapi, vol.path) + sp_uuid_tag = (spvolume.tags or {}).get("uuid") + self.assertEqual(sp_uuid_tag, vol.id, + "StorPool volume uuid tag %s should match CloudStack volume id %s for %s" % + (sp_uuid_tag, vol.id, vol.path)) + return volumes + + def _write_guest_marker(self, vm, marker, ipaddress=None): + """Create a marker file in the guest and sync it to disk before backup.""" + try: + ssh = vm.get_ssh_client(ipaddress=ipaddress, reconnect=True) if ipaddress \ + else vm.get_ssh_client(reconnect=True) + ssh.execute("touch %s; sync" % marker) + except Exception as err: + self.fail("SSH failed writing marker on VM %s: %s" % + (ipaddress or getattr(vm, "ipaddress", vm.id), err)) + + def _assert_guest_reachable(self, vm, marker=None, ipaddress=None, msg=None): + if ipaddress is None: + listed = VirtualMachine.list( + self.apiclient, id=vm.id, listall=True) or [] + self.assertTrue(listed and listed[0].state == "Running", + "VM %s should be Running for SSH check" % vm.id) + ipaddress = listed[0].ipaddress + try: + ssh = vm.get_ssh_client(ipaddress=ipaddress, reconnect=True) + if marker: + result = ssh.execute("ls %s" % marker) + self.assertEqual(result[0], marker, + msg or ("Guest should retain marker %s" % marker)) + else: + result = ssh.execute("echo ok") + self.assertTrue(result and result[0].strip() == "ok", + msg or "Guest should answer SSH after restore") + except Exception as err: + self.fail("SSH failed for VM %s (%s): %s" % + (vm.id, ipaddress, err)) + + def _create_backup(self, vmid, name=None): + Backup.create(self.apiclient, vmid, name) + backups = Backup.list(self.apiclient, vmid) or [] + self.assertTrue(backups, "Backup.create should produce a listable backup") + return backups[0] + + def _backup_type(self, backup): + return getattr(backup, 'type', 'FULL') or 'FULL' + + @attr(tags=["advanced", "backup", "storpool"], required_hardware="true") + def test_01_vm_backup_lifecycle(self): + """Assign offering, create and delete an ad-hoc NAS backup on StorPool VM.""" + backups = Backup.list(self.apiclient, self.vm.id) + self.assertEqual(backups, None, "There should not exist any backup for the VM") + + self.backup_offering.assignOffering(self.apiclient, self.vm.id) + backup = self._create_backup(self.vm.id) + self.cleanup_backups.append(backup) + + self._verify_vm_volumes_on_storpool(self.vm) + + Backup.delete(self.apiclient, backup.id) + backups = Backup.list(self.apiclient, self.vm.id) + self.assertEqual(backups, None, "There should not exist any backup for the VM") + + self.backup_offering.removeOffering(self.apiclient, self.vm.id) + + @attr(tags=["advanced", "backup", "storpool"], required_hardware="true") + def test_02_create_vm_from_backup(self): + """Create a new VM from NAS backup and verify volumes via StorPool API. + """ + self.backup_offering.assignOffering(self.apiclient, self.vm.id) + + marker = "storpool_nas_backup_%s.txt" % int(time.time()) + self._write_guest_marker(self.vm, marker) + + time.sleep(5) + backup = self._create_backup(self.vm.id, "sp-nas-backup1") + self.cleanup_backups.append(backup) + + self.backup_offering.removeOffering(self.apiclient, self.vm.id) + + new_vm_name = "vm-from-sp-nas-%s" % int(time.time()) + vm_services = self.services.get("small", self.services) + new_vm = Backup.createVMFromBackup( + self.apiclient, + vm_services, + mode=self.services["mode"], + backupid=backup.id, + vmname=new_vm_name, + accountname=self.account.name, + domainid=self.account.domainid, + zoneid=self.zone.id, + networkids=self.network_ids) + self.cleanup.append(new_vm) + + self.assertEqual(new_vm.name, new_vm_name) + self.assertEqual(new_vm.state, "Running") + self.assertEqual(new_vm.zoneid, self.zone.id) + + volumes = self._verify_vm_volumes_on_storpool(new_vm) + self.assertEqual(2, len(volumes), + "New VM should have ROOT + DATADISK volumes on StorPool") + + self._assert_guest_reachable( + new_vm, marker=marker, ipaddress=new_vm.ipaddress, + msg="Instance created from backup should retain guest file") + + @attr(tags=["advanced", "backup", "storpool"], required_hardware="true") + def test_03_restore_vm_and_volume_from_backup(self): + """Restore VM in place and restore a volume attach; verify StorPool volumes.""" + self.backup_offering.assignOffering(self.apiclient, self.vm.id) + + marker = "storpool_nas_restore_%s.txt" % int(time.time()) + self._write_guest_marker(self.vm, marker) + time.sleep(5) + backup = self._create_backup(self.vm.id) + self.cleanup_backups.append(backup) + + self.vm.stop(self.apiclient, forced=True) + Backup.restoreVM(self.apiclient, backup.id) + self.vm.start(self.apiclient) + self._verify_vm_volumes_on_storpool(self.vm) + self._assert_guest_reachable( + self.vm, marker=marker, + msg="In-place restore should boot and retain guest file") + + target_vm = VirtualMachine.create( + self.apiclient, + {"name": "SP-NasAttach-%s" % uuid.uuid4()}, + zoneid=self.zone.id, + templateid=self.template.id, + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering.id, + hypervisor=self.hypervisor, + networkids=self.network_ids, + rootdisksize=self.template_size_gb, + mode=self.services["mode"], + startvm=False) + self.cleanup.append(target_vm) + + volumes = Volume.list(self.apiclient, virtualmachineid=self.vm.id, listall=True) + data_disk = next((v for v in volumes if v.type == "DATADISK"), None) + self.assertIsNotNone(data_disk, "Source VM should have a data disk") + + Backup.restoreVolumeFromBackupAndAttachToVM( + self.apiclient, + backupid=backup.id, + volumeid=data_disk.id, + virtualmachineid=target_vm.id) + target_vm.start(self.apiclient) + + target_volumes = self._verify_vm_volumes_on_storpool(target_vm) + self.assertEqual(2, len(target_volumes), + "Target VM should have ROOT + restored DATADISK volumes") + self._assert_guest_reachable( + target_vm, msg="VM with restored volume attached should boot and answer SSH") + + self.backup_offering.removeOffering(self.apiclient, self.vm.id) + + @attr(tags=["advanced", "backup", "storpool"], required_hardware="true") + def test_04_restore_destroyed_vm_from_backup(self): + """Destroy (no expunge) then restoreBackup — exercises restoreVolumesOfDestroyedVMs. + + Uses a ROOT-only VM: destroyVirtualMachine detaches data disks, which would + make volume-count validation fail against a ROOT+DATA backup. + """ + destroyed_vm = VirtualMachine.create( + self.apiclient, + {"name": "SP-NasDestroyed-%s" % uuid.uuid4()}, + zoneid=self.zone.id, + templateid=self.template.id, + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering.id, + hypervisor=self.hypervisor, + networkids=self.network_ids, + rootdisksize=self.template_size_gb, + mode=self.services["mode"]) + self.cleanup.append(destroyed_vm) + + self.backup_offering.assignOffering(self.apiclient, destroyed_vm.id) + + marker = "storpool_nas_destroyed_%s.txt" % int(time.time()) + self._write_guest_marker(destroyed_vm, marker) + time.sleep(5) + backup = self._create_backup(destroyed_vm.id) + self.cleanup_backups.append(backup) + + destroyed_vm.delete(self.apiclient, expunge=False) + + listed = VirtualMachine.list( + self.apiclient, id=destroyed_vm.id, listall=True) or [] + self.assertTrue(listed, "Destroyed VM should still be listable") + self.assertEqual(listed[0].state, "Destroyed") + + Backup.restoreVM(self.apiclient, backup.id) + + restored = VirtualMachine.list( + self.apiclient, id=destroyed_vm.id, listall=True) or [] + self.assertTrue(restored) + self.assertEqual(restored[0].state, "Stopped", + "restoreBackup should leave the Instance Stopped") + + destroyed_vm.start(self.apiclient) + volumes = self._verify_vm_volumes_on_storpool(destroyed_vm) + self.assertEqual(1, len(volumes), + "ROOT-only destroyed VM restore should have one StorPool volume") + self._assert_guest_reachable( + destroyed_vm, marker=marker, + msg="Restored destroyed VM should boot and retain guest file") + + self.backup_offering.removeOffering(self.apiclient, destroyed_vm.id) + + @attr(tags=["advanced", "backup", "storpool"], required_hardware="true") + def test_05_backup_stopped_vm(self): + """Restore a NAS backup that was taken while the StorPool VM was stopped. + """ + self.backup_offering.assignOffering(self.apiclient, self.vm.id) + + marker = "storpool_nas_stopped_restore_%s.txt" % int(time.time()) + self._write_guest_marker(self.vm, marker) + time.sleep(5) + + self.vm.stop(self.apiclient, forced=True) + + backup = self._create_backup(self.vm.id, "sp-nas-stopped-restore-backup") + self.cleanup_backups.append(backup) + self.assertEqual(self._backup_type(backup).upper(), "FULL", + "A stopped-VM NAS backup on StorPool should always be a FULL backup") + + Backup.restoreVM(self.apiclient, backup.id) + + self.vm.start(self.apiclient) + self._verify_vm_volumes_on_storpool(self.vm) + self._assert_guest_reachable( + self.vm, marker=marker, + msg="Restore of a stopped-VM StorPool backup should boot and retain the guest file") + + self.backup_offering.removeOffering(self.apiclient, self.vm.id) + + +class TestNasBackupMixedStorage(TestNasBackupStorPool): + """Repeats the NAS backup smoke tests (test_01 .. test_05, inherited + unchanged from TestNasBackupStorPool) against a VM with mixed-provider + volumes: ROOT on StorPool, DATADISK on NFS primary storage. + + The NFS primary storage pool, its name and storage tag are taken from + the Marvin cfg file (the "nfs://" primaryStorages entry for the zone). + Other suites assume that pool is left in Maintenance, so it is always + put into Maintenance before this suite touches it, taken out of + Maintenance only for the duration of creating volumes/VMs on it, and + put back into Maintenance once every test in this class has finished + (including when setup itself fails). + """ + + @classmethod + def setUpCloudStack(cls): + cls._setup_common() + if cls.unsupportedHypervisor: + return + + cls._setup_nfs_environment() + if cls.unsupportedHypervisor: + return + + cls.nfs_storage_pool = cls._ensure_maintenance(cls.nfs_storage_pool.id, enable=False) + + cls.vm = cls._create_test_vm("SP-NasMixed-%s" % uuid.uuid4(), cls.nfs_disk_offering.id) + cls._verify_setup_volumes(cls.vm) + + @classmethod + def _setup_nfs_environment(cls): + nfs_pools = cls.helper.get_nfs_pool(cls.zone_cfg) + if not nfs_pools: + cls.unsupportedHypervisor = True + cls.skip_reason = "No NFS primary storage configured in zone" + return + nfs_cfg = nfs_pools[0] + + storage_pools = list_storage_pools(cls.apiclient, name=nfs_cfg["name"]) + if not storage_pools: + cls.unsupportedHypervisor = True + cls.skip_reason = "NFS primary storage pool %s not found" % nfs_cfg["name"] + return + cls.nfs_storage_pool = storage_pools[0] + + cls.nfs_tag = nfs_cfg["tags"] if nfs_cfg["tags"] else nfs_cfg["name"] + cls.helper.updateStoragePoolTags(cls.apiclient, cls.nfs_storage_pool.id, cls.nfs_tag) + + # Other suites assume the NFS pool is unavailable; make sure that + # invariant holds before this suite starts using it. + cls.nfs_storage_pool = cls._ensure_maintenance(cls.nfs_storage_pool.id, enable=True) + + cls.nfs_disk_offering = cls._get_or_create_disk_offering( + cls.nfs_tag, "NFS NAS backup disk offering") + cls.nfs_service_offering = cls._get_or_create_nfs_service_offering() + + @classmethod + def _get_or_create_nfs_service_offering(cls): + service_offerings = list_service_offering(cls.apiclient, name=cls.nfs_tag) + if service_offerings is None: + service_offering = ServiceOffering.create( + cls.apiclient, + { + "name": cls.nfs_tag, + "displaytext": "NFS NAS offering (%s)" % cls.nfs_tag, + "cpunumber": 1, + "cpuspeed": 500, + "memory": 512, + "storagetype": "shared", + "tags": cls.nfs_tag, + }) + cls._cleanup.append(service_offering) + return service_offering + return service_offerings[0] + + @classmethod + def _ensure_maintenance(cls, pool_id, enable): + pool = list_storage_pools(cls.apiclient, id=pool_id)[0] + if enable and pool.state != "Maintenance": + pool = StoragePool.enableMaintenance(cls.apiclient, pool_id) + elif not enable and pool.state == "Maintenance": + pool = StoragePool.cancelMaintenance(cls.apiclient, pool_id) + return pool + + @classmethod + def cleanUpCloudStack(cls): + try: + super(TestNasBackupMixedStorage, cls).cleanUpCloudStack() + finally: + if getattr(cls, "nfs_storage_pool", None) is not None: + cls._ensure_maintenance(cls.nfs_storage_pool.id, enable=True) diff --git a/test/integration/plugins/storpool/sp_util.py b/test/integration/plugins/storpool/sp_util.py index 084a57ee954e..280a7797386f 100644 --- a/test/integration/plugins/storpool/sp_util.py +++ b/test/integration/plugins/storpool/sp_util.py @@ -21,6 +21,7 @@ from marvin.lib.base import (Account, Cluster, Configurations, + ImageStore, ServiceOffering, Snapshot, StoragePool, @@ -682,26 +683,50 @@ def destroy_vm(self, apiclient, virtualmachineid): apiclient.destroyVirtualMachine(cmd) @classmethod - def check_storpool_volume_size(cls, volume, spapi): - name = volume.path.split("/")[3] + def get_storpool_volume_by_path(cls, spapi, path): + """ + Resolve a CloudStack volume path (/dev/storpool-byid/) to a + StorPool volume. + """ + if not path: + raise Exception("Volume path is empty") + parts = path.rstrip("/").split("/") + if len(parts) < 4: + raise Exception("Unexpected StorPool volume path: %s" % path) + name = "~" + parts[3] try: - spvolume = spapi.volumeList(volumeName = "~" + name) - if spvolume[0].size != volume.size: - raise Exception("Storpool volume size is not the same as CloudStack db size") - except spapi.ApiError as err: - raise Exception(err) + volumes = spapi.volumeList(volumeName=name) + except Exception as err: + raise Exception("StorPool volume not found for path %s: %s" % (path, err)) + if not volumes: + raise Exception("StorPool volume not found for path %s" % path) + return volumes[0] + + @classmethod + def check_storpool_volume_size(cls, volume, spapi): + spvolume = cls.get_storpool_volume_by_path(spapi, volume.path) + if spvolume.size != volume.size: + raise Exception("Storpool volume size is not the same as CloudStack db size") @classmethod def check_storpool_volume_iops(cls, spapi, volume,): - name = volume.path.split("/")[3] - try: - spvolume = spapi.volumeList(volumeName = "~" + name) - logging.debug(spvolume[0].iops) - logging.debug(volume.maxiops) - if spvolume[0].iops != volume.maxiops: - raise Exception("Storpool volume size is not the same as CloudStack db size") - except spapi.ApiError as err: - raise Exception(err) + spvolume = cls.get_storpool_volume_by_path(spapi, volume.path) + logging.debug(spvolume.iops) + logging.debug(volume.maxiops) + if spvolume.iops != volume.maxiops: + raise Exception("Storpool volume size is not the same as CloudStack db size") + + @classmethod + def verify_storpool_volume(cls, spapi, volume, check_size=True): + """Assert the CloudStack volume exists on StorPool; optionally compare size.""" + if not volume.path or not volume.path.startswith("/dev/storpool"): + raise Exception("Volume %s does not have a StorPool device path: %s" % + (volume.id, volume.path)) + spvolume = cls.get_storpool_volume_by_path(spapi, volume.path) + if check_size and volume.size is not None and spvolume.size != volume.size: + raise Exception("StorPool volume size %s does not match CloudStack size %s for %s" % + (spvolume.size, volume.size, volume.path)) + return spvolume @classmethod def create_custom_disk(cls, apiclient, services, size = None, miniops = None, maxiops =None, diskofferingid=None, zoneid=None, account=None, domainid=None, snapshotid=None): @@ -926,6 +951,19 @@ def get_pool(cls, zone): return return sp_pools + @classmethod + def get_nfs_pool(cls, zone): + """Return the NFS primary storage entries configured for this zone in + the Marvin cfg file. NFS entries don't set "provider" (unlike + StorPool/RBD), so they're identified by their "nfs://" url instead.""" + storage_pools = zone.primaryStorages + nfs_pools = [] + for storage in storage_pools: + url = storage['url'] or "" + if not storage['provider'] and str(url).lower().startswith("nfs://"): + nfs_pools.append(storage) + return nfs_pools + @classmethod def create_snapshot_template(cls, apiclient, services, snapshot_id, zone_id): cmd = createTemplate.createTemplateCmd()