Skip to content
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.

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

Jump to bottom

nixos: fix qemu_test being used in normal VMs #101246

Merged
merged 1 commit into from Oct 22, 2020
Merged

nixos: fix qemu_test being used in normal VMs #101246

merged 1 commit into from Oct 22, 2020

Conversation

rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Oct 21, 2020 •
edited

Motivation for this change

It looks like since PR #49403 the VMs built by nixos-rebuild built-vm are using the stripped-down qemu package, which lacks gtk and spice support.

Things done
  • Tested that nixosTests uses qemu_test package
  • Tested that build-vm uses the normal qemu package
  • Test I have not broken more things by removing config.system.build.qemu

@rnhmjoj rnhmjoj requested review from flokli and andir October 21, 2020 11:09
@gravndal gravndal mentioned this pull request Oct 21, 2020
Open
andir
andir requested changes Oct 21, 2020
View reviewed changes
Copy link
Member

@andir andir left a comment

Choose a reason for hiding this comment

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

I just wrote a very similar patch before I found this.

We should probably add a virtualisation.qemu.package = pkgs.qemu_test; line to nixos/modules/testing/test-instrumentation.nix to ensure the tests are still using that.

Otherwise it looks fine. I haven't executed this code yet tho..

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Oct 21, 2020

We should probably add a virtualisation.qemu.package = pkgs.qemu_test; line to nixos/modules/testing/test-instrumentation.nix to ensure the tests are still using that.

Wait, isn't this exactly what I'm doing in this PR?

@andir
Copy link
Member

andir commented Oct 21, 2020

We should probably add a virtualisation.qemu.package = pkgs.qemu_test; line to nixos/modules/testing/test-instrumentation.nix to ensure the tests are still using that.

Wait, isn't this exactly what I'm doing in this PR?

Yes, sorry. The line breaking on the (small) GitHub Window didn't really make that obvious. It looked more like you did just reflow some of the code but indeed you are doing exactly that. :)

@andir
Copy link
Member

andir commented Oct 21, 2020 

* [ ]  Test I have not broken more things by removing `config.system.build.qemu`

There is one case of system.build.qemu in nixos/lib/build-vms.nix.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Oct 21, 2020

Thank you, I'll take a look at build-vms now.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Oct 21, 2020

It looks like that line can simply be removed, since the test package is already being configured by test-instrumentation.

@andir
Copy link
Member

andir commented Oct 22, 2020

I am building all the tests over night and will merge this in the morning when everything looks fine.

@andir andir merged commit 8935152 into NixOS:master Oct 22, 2020
@xaverdh
Copy link
Contributor

xaverdh commented Oct 25, 2020

Ah so I think this pr reintroduced the eval error which was supposed to be fixed by rnhmjoj@2578557#diff-ec9255f81e919804497f4a2ae13abd965816f4c519b08d037be851055ac53b4e ?

@xaverdh xaverdh mentioned this pull request Oct 25, 2020
Merged
@rnhmjoj rnhmjoj deleted the vm-fix branch July 10, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andir andir andir approved these changes

@flokli flokli Awaiting requested review from flokli

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rnhmjoj @andir @xaverdh