Skip to content

Backup: support StorPool volumes in NAS backup provider on KVM #14203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
slavkap wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Backup: support StorPool volumes in NAS backup provider on KVM #14203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions core/src/main/java/org/apache/cloudstack/backup/BackupAnswer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

}
60 changes: 53 additions & 7 deletions plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<VolumeVO> volumes) {

@weizhouapache weizhouapache Sep 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to pass pooltype as a parameter of the method

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).
Expand Down Expand Up @@ -588,8 +606,20 @@ public Pair<Boolean, Backup> takeBackup(final VirtualMachine vm, Boolean quiesce
command.setBitmapParent(decision.bitmapParent);
command.setParentPaths(decision.parentPaths);

if (VirtualMachine.State.Stopped.equals(vm.getState())) {
List<VolumeVO> 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<VolumeVO> 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<PrimaryDataStoreTO>, List<String>> volumePoolsAndPaths = getVolumePoolsAndPaths(vmVolumes);
command.setVolumePools(volumePoolsAndPaths.first());
Expand Down Expand Up @@ -745,11 +775,18 @@ private Pair<Boolean, String> restoreVMBackup(VirtualMachine vm, Backup backup)
private List<String> getBackupFiles(List<Backup.VolumeInfo> backedVolumes) {
List<String> 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<PrimaryDataStoreTO>, List<String>> getVolumePoolsAndPaths(List<VolumeVO> volumes) {
List<PrimaryDataStoreTO> volumePools = new ArrayList<>();
List<String> volumePaths = new ArrayList<>();
Expand All @@ -762,9 +799,13 @@ private Pair<List<PrimaryDataStoreTO>, List<String>> 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);
}
Expand All @@ -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());
Expand Down Expand Up @@ -847,7 +890,7 @@ public Pair<Boolean, String> 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 {
Expand All @@ -859,6 +902,9 @@ public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, Backup.VolumeI
}

if (answer.getResult()) {
if (answer.getRestoredVolumePath() != null) {
restoredVolume.setPath(answer.getRestoredVolumePath());
}
try {
volumeDao.persist(restoredVolume);
} catch (Exception e) {
Expand Down
159 changes: 159 additions & 0 deletions plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String> expectedPaths) throws AgentUnavailableException, OperationTimedoutException {
ArgumentCaptor<RestoreBackupCommand> 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<VolumeVO> 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<Boolean, String> 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
Expand Down
Loading
Loading