Skip to content

Fix KvmFileBasedStorageVmSnapshotStrategy unit test failures #14135

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

Merged
weizhouapache merged 1 commit into from
Sep 10, 2026
Merged

Fix KvmFileBasedStorageVmSnapshotStrategy unit test failures #14135

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
2 changes: 1 addition & 1 deletion .../java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@
protected BackupOfferingDao backupOfferingDao;

@Inject
private InternalBackupService internalBackupService;
protected InternalBackupService internalBackupService;

@Inject
private InternalBackupStoragePoolDao internalBackupStoragePoolDao;
Expand All @@ -117,13 +117,13 @@
private SnapshotDao snapshotDao;


@Inject

Check warning on line 120 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this field injection and use constructor injection instead. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-l&open=AaCMflqZiGrYjGtJQG-l&pullRequest=14135
protected VMInstanceDetailsDao vmInstanceDetailsDao;

@Inject

Check warning on line 123 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this field injection and use constructor injection instead. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-m&open=AaCMflqZiGrYjGtJQG-m&pullRequest=14135
protected HostDetailsDao hostDetailsDao;

@Inject

Check warning on line 126 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this field injection and use constructor injection instead. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-n&open=AaCMflqZiGrYjGtJQG-n&pullRequest=14135
protected AlertManager alertManager;

@Override
Expand Down Expand Up @@ -156,7 +156,7 @@
logger.info("Starting VM snapshot delete process for snapshot [{}].", vmSnapshot.getUuid());
UserVmVO userVm = userVmDao.findById(vmSnapshot.getVmId());
VMSnapshotVO vmSnapshotBeingDeleted = (VMSnapshotVO) vmSnapshot;
Long hostId = pickHostForNvramSidecarCleanup(vmSnapshotBeingDeleted, userVm, "delete");

Check failure on line 159 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal "delete" 3 times. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-k&open=AaCMflqZiGrYjGtJQG-k&pullRequest=14135
validateHostSupportsNvramSidecarCleanup(vmSnapshotBeingDeleted, hostId, "delete");
long virtualSize = 0;
boolean isCurrent = vmSnapshotBeingDeleted.getCurrent();
Expand Down Expand Up @@ -438,7 +438,7 @@

protected void deleteNvramSnapshotIfNeeded(VMSnapshotVO vmSnapshotVO, Long hostId, PrimaryDataStoreTO primaryDataStore) {
String nvramSnapshotPath = getNvramSnapshotPath(vmSnapshotVO);
if (StringUtils.isBlank(nvramSnapshotPath) || primaryDataStore == null) {

Check failure on line 441 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use static access with "org.apache.commons.lang3.StringUtils" for "isBlank". 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-o&open=AaCMflqZiGrYjGtJQG-o&pullRequest=14135
return;
}

Expand Down Expand Up @@ -575,7 +575,7 @@

transitStateWithoutThrow(vmSnapshot, VMSnapshot.Event.CreateRequested);

Long hostId = pickHostForUefiNvramAwareDiskOnlySnapshot(userVm, "create");

Check failure on line 578 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal "create" 3 times. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-j&open=AaCMflqZiGrYjGtJQG-j&pullRequest=14135
validateHostSupportsUefiNvramAwareDiskOnlySnapshots(hostId, userVm, "create");
VMSnapshotVO vmSnapshotVO = (VMSnapshotVO) vmSnapshot;
List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(userVm.getId());
Expand Down Expand Up @@ -652,7 +652,7 @@
publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_CREATE, vmSnapshot, userVm, (VolumeObjectTO) volumeInfo.getTO());
}

if (StringUtils.isNotBlank(answer.getNvramSnapshotPath())) {

Check failure on line 655 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use static access with "org.apache.commons.lang3.StringUtils" for "isNotBlank". 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-p&open=AaCMflqZiGrYjGtJQG-p&pullRequest=14135
vmSnapshotDetailsDao.addDetail(vmSnapshot.getId(), KVM_FILE_BASED_STORAGE_SNAPSHOT_NVRAM, answer.getNvramSnapshotPath(), false);
} else if (isUefiVm(userVm)) {
logger.warn("Disk-only snapshot [{}] for UEFI VM [{}] was created without an NVRAM sidecar and cannot be safely reverted. "
Expand Down Expand Up @@ -864,7 +864,7 @@

protected Long pickHostForNvramSidecarCleanup(VMSnapshotVO vmSnapshotVO, UserVm userVm, String operation) {
Long selectedHostId = vmSnapshotHelper.pickRunningHost(vmSnapshotVO.getVmId());
if (StringUtils.isBlank(getNvramSnapshotPath(vmSnapshotVO))) {

Check failure on line 867 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use static access with "org.apache.commons.lang3.StringUtils" for "isBlank". 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-q&open=AaCMflqZiGrYjGtJQG-q&pullRequest=14135
return selectedHostId;
}

Expand Down Expand Up @@ -947,7 +947,7 @@
}

protected void validateHostSupportsNvramSidecarCleanup(VMSnapshotVO vmSnapshotVO, Long hostId, String operation) {
if (StringUtils.isBlank(getNvramSnapshotPath(vmSnapshotVO))) {

Check failure on line 950 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use static access with "org.apache.commons.lang3.StringUtils" for "isBlank". 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-r&open=AaCMflqZiGrYjGtJQG-r&pullRequest=14135
return;
}

Expand All @@ -959,7 +959,7 @@
}

protected void notifyGuestRecoveryIssueIfNeeded(CreateDiskOnlyVmSnapshotAnswer answer, UserVm userVm, VMSnapshotVO vmSnapshot) {
if (StringUtils.isBlank(answer.getDetails())) {

Check failure on line 962 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use static access with "org.apache.commons.lang3.StringUtils" for "isBlank". 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-s&open=AaCMflqZiGrYjGtJQG-s&pullRequest=14135
return;
}

Expand All @@ -968,7 +968,7 @@
vmSnapshot.getUuid(), userVm.getUuid(), answer.getDetails());
logger.error(message);
try {
alertManager.sendAlert(AlertManager.AlertType.ALERT_TYPE_VM_SNAPSHOT, userVm.getDataCenterId(), userVm.getPodIdToDeployIn(), subject, message);

Check failure on line 971 in engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use static access with "org.apache.cloudstack.alert.AlertService" for "AlertType". 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflqZiGrYjGtJQG-t&open=AaCMflqZiGrYjGtJQG-t&pullRequest=14135
} catch (Exception e) {
logger.warn("Failed to send post-snapshot guest recovery alert for VM snapshot [{}].", vmSnapshot.getUuid(), e);
}
Expand Down
6 changes: 3 additions & 3 deletions ...a/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.Collections;
import java.util.List;

import org.apache.cloudstack.backup.InternalBackupService;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
Expand Down Expand Up @@ -97,7 +98,7 @@

@Before
public void setup() {
strategy = Mockito.spy(new KvmFileBasedStorageVmSnapshotStrategy());

Check warning on line 101 in engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategyTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "spy". 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaCMflu3iGrYjGtJQG-u&open=AaCMflu3iGrYjGtJQG-u&pullRequest=14135
vmSnapshotDetailsDao = mock(VMSnapshotDetailsDao.class);
vmSnapshotDao = mock(VMSnapshotDao.class);
vmSnapshotHelper = mock(VMSnapshotHelper.class);
Expand All @@ -121,6 +122,7 @@
strategy.vmInstanceDetailsDao = mock(VMInstanceDetailsDao.class);
strategy.hostDetailsDao = hostDetailsDao;
strategy.alertManager = mock(AlertManager.class);
strategy.internalBackupService = mock(InternalBackupService.class);
doNothing().when(strategy).publishUsageEvent(anyString(), any(VMSnapshot.class), any(UserVm.class), anyLong(), anyLong());
doNothing().when(strategy).publishUsageEvent(anyString(), any(VMSnapshot.class), any(UserVm.class), any(VolumeObjectTO.class));
}
Expand Down Expand Up @@ -343,12 +345,10 @@
SnapshotInfo rootSnapshotInfo = mock(SnapshotInfo.class);
SnapshotObjectTO rootSnapshotObjectTo = mock(SnapshotObjectTO.class);
VolumeObjectTO rootSnapshotVolume = mock(VolumeObjectTO.class);
VMSnapshotDetailsVO volumeSnapshotDetail = new VMSnapshotDetailsVO(vmSnapshotId, "kvmFileBasedStorageSnapshot", String.valueOf(rootSnapshotId), true);

when(vmSnapshot.getId()).thenReturn(vmSnapshotId);
when(vmSnapshot.getUuid()).thenReturn("vm-snapshot");
when(vmSnapshotDetailsDao.findDetails(vmSnapshotId, "kvmFileBasedStorageSnapshot")).thenReturn(List.of(volumeSnapshotDetail));
when(snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(rootSnapshotId, DataStoreRole.Primary)).thenReturn(rootSnapshotDataStore);
when(vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(vmSnapshotId)).thenReturn(List.of(rootSnapshotDataStore));
when(rootSnapshotDataStore.getSnapshotId()).thenReturn(rootSnapshotId);
when(rootSnapshotDataStore.getDataStoreId()).thenReturn(dataStoreId);
when(strategy.snapshotDataFactory.getSnapshot(rootSnapshotId, dataStoreId, DataStoreRole.Primary)).thenReturn(rootSnapshotInfo);
Expand Down
Loading