Skip to content

nix: use previous version of libvirt in version migration tests#196

Merged
arctic-alpaca merged 4 commits intocyberus-technology:mainfrom
arctic-alpaca:libvirt-prev
Apr 10, 2026
Merged

nix: use previous version of libvirt in version migration tests#196
arctic-alpaca merged 4 commits intocyberus-technology:mainfrom
arctic-alpaca:libvirt-prev

Conversation

@arctic-alpaca
Copy link
Copy Markdown
Contributor

@arctic-alpaca arctic-alpaca commented Apr 2, 2026

Since libvirt and cloud-hypervisor are shipped as pairs, the cross-version migration test should also use the appropriate lbivirt version.

Libvirt PR: https://gitlab.cyberus-technology.de/cyberus/cloud/libvirt/-/merge_requests/166

Comment thread flake.nix Outdated
Comment thread tests/default.nix
@arctic-alpaca arctic-alpaca force-pushed the libvirt-prev branch 2 times, most recently from 4e17510 to 88cf4a4 Compare April 7, 2026 10:28
@arctic-alpaca arctic-alpaca marked this pull request as ready for review April 7, 2026 11:18
@arctic-alpaca arctic-alpaca marked this pull request as draft April 7, 2026 12:34
@arctic-alpaca arctic-alpaca force-pushed the libvirt-prev branch 3 times, most recently from c5345fe to fc4c9dd Compare April 7, 2026 14:52
@arctic-alpaca arctic-alpaca marked this pull request as ready for review April 8, 2026 05:50
Copy link
Copy Markdown
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

Great idea with the asserts!

I'm unsure if you also override the libvirt in the corresponding test suite.. something as you did here 0c55a77#diff-1cc580de297308d93d82f7b72446ae4b98832a8aae3378e9e134102519a0e33aR63

Please double-check!

PS: Apologies for the long review delay

Comment thread flake.nix
Comment thread flake.nix Outdated
Comment thread flake.nix Outdated
Comment thread flake.nix Outdated
Comment thread flake.nix
@arctic-alpaca arctic-alpaca force-pushed the libvirt-prev branch 2 times, most recently from 01ddda2 to fa7249b Compare April 9, 2026 05:45
@arctic-alpaca
Copy link
Copy Markdown
Contributor Author

I'm unsure if you also override the libvirt in the corresponding test suite.. something as you did here 0c55a77#diff-1cc580de297308d93d82f7b72446ae4b98832a8aae3378e9e134102519a0e33aR63

Please double-check!

The libvirt version for the test is set as a parameter (here) since, IIUC, libvirt needs to be passed to libvirt-test.nix.

@phip1611
Copy link
Copy Markdown
Member

phip1611 commented Apr 9, 2026

I'm unsure if you also override the libvirt in the corresponding test suite.. something as you did here 0c55a77#diff-1cc580de297308d93d82f7b72446ae4b98832a8aae3378e9e134102519a0e33aR63
Please double-check!

The libvirt version for the test is set as a parameter (here) since, IIUC, libvirt needs to be passed to libvirt-test.nix.

Oh I must have missed that during my review yesterday - thank you for pointing that out!

Comment thread tests/default.nix
Comment thread flake.nix Outdated
@arctic-alpaca arctic-alpaca force-pushed the libvirt-prev branch 2 times, most recently from 1848727 to 276140e Compare April 9, 2026 09:44
Copy link
Copy Markdown
Collaborator

@hertrste hertrste left a comment

Choose a reason for hiding this comment

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

I am happy with how this turned out now :)

Comment thread flake.nix
@arctic-alpaca
Copy link
Copy Markdown
Contributor Author

arctic-alpaca commented Apr 9, 2026

@amphi pointed out that it would be helpful to see the versions in CI to verify the expected behavior. (39cc440)
We currently cannot get the commit of cloud-hypervisor, but setting that via https://github.com/cyberus-technology/cloud-hypervisor/blob/gardenlinux/cloud-hypervisor/build.rs#L21-L25 in the flake shouldn't be an issue.
PR for setting the commit in the version info: cyberus-technology/cloud-hypervisor#140

Comparing the versions won't work for now since we reuse the same set of tests for the normal migration tests.

Comment thread tests/common.nix
@arctic-alpaca arctic-alpaca force-pushed the libvirt-prev branch 2 times, most recently from 98a8a81 to 5776739 Compare April 10, 2026 05:47
Copy link
Copy Markdown
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

I'm concerned this doesn't do the right thing (see my remarks). Please double-check. (In case I'm wrong, ignore all of the following)

You're definitely working here with a combination of Nix features that brings together some of the most complicated pieces, and the way we build our test suites only adds to that complexity.

  • Nix Flakes
  • Nix Overlays
  • Multiple nixpkgs instantiations
  • Passing parameters to functions that generate NixOS modules
  • ...

That's not a very pleasant experience. Feel free to setup a meeting with StefanH, SebastianE or me to brain storm this again - this is for sure pretty complicated.

Comment thread tests/common.nix
Comment thread tests/default.nix
Copy link
Copy Markdown
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

Loos like its working from your data - very nice!

arctic-alpaca and others added 4 commits April 10, 2026 09:22
Allows overriding `libvirt` via overlay, which is necessary for the next
commit.

On-behalf-of: SAP julian.schindel@sap.com
Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>
Since `libvirt` and `cloud-hypervisor` are shipped as a pair, the
cross-version migration test should also use the appropriate `lbivirt`
version.

On-behalf-of: SAP julian.schindel@sap.com
Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>
On-behalf-of: SAP julian.schindel@sap.com
Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>
Allows verifying the versions of `libvirt` and `cloud-hypervisor`
running in the respective VMs.

Co-authored-by: Sebastian Eydam <sebastian.eydam@cyberus-technology.de>
On-behalf-of: SAP julian.schindel@sap.com
Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>
@arctic-alpaca arctic-alpaca enabled auto-merge April 10, 2026 07:28
@arctic-alpaca arctic-alpaca added this pull request to the merge queue Apr 10, 2026
Merged via the queue into cyberus-technology:main with commit 561f183 Apr 10, 2026
3 checks passed
@arctic-alpaca arctic-alpaca deleted the libvirt-prev branch April 10, 2026 07:37
cyberus-renovate pushed a commit to cyberus-technology/libvirt that referenced this pull request Apr 10, 2026
Adds support for `libvirt` in cross-version migration testing introduced in cyberus-technology/libvirt-tests#196.

The change in `libvirt-tests` moved `libvirt` from a parameter to `pkgs`.
This commit adapts the override to use the `libvirt` version from the repo to match the change.

On-behalf-of: SAP julian.schindel@sap.com
Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants