bll: Allow resuming Paused/Suspended VMs with older custom compatibil… - #1178
Open
dupondje wants to merge 1 commit into
Open
bll: Allow resuming Paused/Suspended VMs with older custom compatibil…#1178dupondje wants to merge 1 commit into
dupondje wants to merge 1 commit into
Conversation
dupondje
requested review from
JasperB-TeamBlue and
peter-boden
as code owners
August 17, 2026 12:26
peter-boden
approved these changes
Aug 17, 2026
peter-boden
left a comment
peter-boden
left a comment
Member
There was a problem hiding this comment.
LGTM apart from a small nit 😅
| // Skip the compatibility version check when resuming a Paused or Suspended VM, | ||
| // since the VM was already running with its custom compatibility version before | ||
| // the data center was upgraded and is only being resumed, not started fresh. | ||
| if (getVm().getCustomCompatibilityVersion() != null && |
Member
There was a problem hiding this comment.
The old code also did it, but the mixed use of getVm() and vm is confusing.
dupondje
force-pushed
the
fix_resume_compat
branch
2 times, most recently
from
August 17, 2026 12:49
a3a6a6d to
4a731cf
Compare
…ity version When a data center is upgraded to a newer compatibility version, VMs that were hibernated (Suspended) or Paused with an older custom compatibility version could no longer be resumed. The validation in RunVmCommand rejected them with ACTION_TYPE_FAILED_VM_COMPATIBILITY_VERSION_NOT_SUPPORTED, even though these VMs were already running with that version before the upgrade and are only being resumed, not started fresh. Skip the custom compatibility version check when the VM status is Paused or Suspended, so that the RESUME_PAUSE and RESUME_HIBERNATE flows can proceed. The check is still enforced for VMs being started from Down status, preserving the existing protection against running a VM with an unsupported compatibility version. Added tests covering: - Down VM with older custom compatibility version still fails validation - Suspended VM with older custom compatibility version now passes - Paused VM with older custom compatibility version now passes Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
dupondje
force-pushed
the
fix_resume_compat
branch
from
August 17, 2026 13:20
4a731cf to
c5ad6ea
Compare
Collaborator
|
/ost |
|
⏳ Running ost suite 'basic-suite-master' on distro 'centos9'. Follow the progress here. |
|
😎💪 ost suite 'basic-suite-master' on distro 'centos9' finished successfully. (details) |
JasperB-TeamBlue
requested changes
Aug 18, 2026
JasperB-TeamBlue
left a comment
JasperB-TeamBlue
left a comment
Collaborator
There was a problem hiding this comment.
Functionality wise looking good, just a slight improvement that could be made on the test side of things.
Comment on lines
+439
to
+481
| @Test | ||
| @MockedConfig("mockConfiguration") | ||
| public void testValidateSucceedsOnOlderCustomCompatibilityVersionWhenVmSuspended() { | ||
| final VM vm = new VM(); | ||
| vm.setStatus(VMStatus.Suspended); | ||
| vm.setCustomCompatibilityVersion(new Version(4, 7)); | ||
| command.setVm(vm); | ||
| StoragePool storagePool = new StoragePool(); | ||
| storagePool.setCompatibilityVersion(new Version(4, 8)); | ||
| command.setStoragePool(storagePool); | ||
| doReturn(true).when(command).checkRngDeviceClusterCompatibility(); | ||
| doReturn(true).when(command).checkPayload(any()); | ||
| doReturn(ValidationResult.VALID).when(command).checkDisksInBackupStorage(); | ||
| doReturn(false).when(command).isVmDuringBackup(); | ||
| doNothing().when(command).checkVmLeaseStorageDomain(); | ||
| Cluster cluster = new Cluster(); | ||
| cluster.setArchitecture(ArchitectureType.x86_64); | ||
| cluster.setCompatibilityVersion(new Version(4, 8)); | ||
| command.setCluster(cluster); | ||
| ValidateTestUtils.runAndAssertValidateSuccess(command); | ||
| } | ||
|
|
||
| @Test | ||
| @MockedConfig("mockConfiguration") | ||
| public void testValidateSucceedsOnOlderCustomCompatibilityVersionWhenVmPaused() { | ||
| final VM vm = new VM(); | ||
| vm.setStatus(VMStatus.Paused); | ||
| vm.setCustomCompatibilityVersion(new Version(4, 7)); | ||
| command.setVm(vm); | ||
| StoragePool storagePool = new StoragePool(); | ||
| storagePool.setCompatibilityVersion(new Version(4, 8)); | ||
| command.setStoragePool(storagePool); | ||
| doReturn(true).when(command).checkRngDeviceClusterCompatibility(); | ||
| doReturn(true).when(command).checkPayload(any()); | ||
| doReturn(ValidationResult.VALID).when(command).checkDisksInBackupStorage(); | ||
| doReturn(false).when(command).isVmDuringBackup(); | ||
| doNothing().when(command).checkVmLeaseStorageDomain(); | ||
| Cluster cluster = new Cluster(); | ||
| cluster.setArchitecture(ArchitectureType.x86_64); | ||
| cluster.setCompatibilityVersion(new Version(4, 8)); | ||
| command.setCluster(cluster); | ||
| ValidateTestUtils.runAndAssertValidateSuccess(command); | ||
| } |
Collaborator
There was a problem hiding this comment.
Instead of writing two tests that share the same result and a lot of repetitive code this could be rewritten as a parameterized test
@ParameterizedTest
@EnumSource(value = VMStatus.class, names = {"Suspended", "Paused"})
@MockedConfig("mockConfiguration")
public void testValidateSucceedsOnOlderCustomCompatibilityVersionWhenVmSuspendedOrPaused(VMStatus vmStatus) {
final VM vm = new VM();
vm.setStatus(vmStatus);
vm.setCustomCompatibilityVersion(new Version(4, 7));
command.setVm(vm);
StoragePool storagePool = new StoragePool();
storagePool.setCompatibilityVersion(new Version(4, 8));
command.setStoragePool(storagePool);
doReturn(true).when(command).checkRngDeviceClusterCompatibility();
doReturn(true).when(command).checkPayload(any());
doReturn(ValidationResult.VALID).when(command).checkDisksInBackupStorage();
doReturn(false).when(command).isVmDuringBackup();
doNothing().when(command).checkVmLeaseStorageDomain();
Cluster cluster = new Cluster();
cluster.setArchitecture(ArchitectureType.x86_64);
cluster.setCompatibilityVersion(new Version(4, 8));
command.setCluster(cluster);
ValidateTestUtils.runAndAssertValidateSuccess(command);
}This results in the same vm states being tested, but less repetitive code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ity version
When a data center is upgraded to a newer compatibility version, VMs that were hibernated (Suspended) or Paused with an older custom compatibility version could no longer be resumed. The validation in RunVmCommand rejected them with ACTION_TYPE_FAILED_VM_COMPATIBILITY_VERSION_NOT_SUPPORTED, even though these VMs were already running with that version before the upgrade and are only being resumed, not started fresh.
Skip the custom compatibility version check when the VM status is Paused or Suspended, so that the RESUME_PAUSE and RESUME_HIBERNATE flows can proceed. The check is still enforced for VMs being started from Down status, preserving the existing protection against running a VM with an unsupported compatibility version.
Added tests covering:
Are you the owner of the code you are sending in, or do you have permission of the owner?
[y]