Skip to content

nixos: add boot.isVM option #390897

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

nixos: add boot.isVM option #390897

wants to merge 2 commits into from

Conversation

rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Mar 18, 2025

This adds an option, similar to boot.isContainer, to gate the
configuration of qemu-vm.nix behind, which is now imported by default.

This solves the longstanding issue that users would get errors about the
virtualisation.* not existing when used outside of the scope of
virtualisation.vmVariant, while at same time being listed in the NixOS
manual.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested with
    • nixosTests.simple
    • nixosTests.nixos-test-driver
    • nixosTests.nixos-rebuild-specialisations
    • nixosTests.nixos-rebuild-target-host
    • nixosTests.nixos-generate-config
    • nixosTests.nixos-rebuild-specialisations-ng
    • nixosTests.nixos-rebuild-install-bootloader
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

rnhmjoj added 2 commits March 18, 2025 09:17
This adds an option, similar to boot.isContainer, to gate the
configuration of qemu-vm.nix behind, which is now imported by default.

This solves the longstanding issue that users would get errors about the
`virtualisation.*` not existing when used outside of the scope of
`virtualisation.vmVariant`, while at same time being listed in the NixOS
manual.
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: testing Tooling for automated testing of packages and modules 8.has: documentation This PR adds or changes documentation labels Mar 18, 2025
@nix-owners nix-owners bot requested a review from RaitoBezarius March 18, 2025 09:15
@ElvishJerricco
Copy link
Contributor

This seems backwards compared to isContainer. With isContainer, it's set to true by the module that implements the container guest environment. The analogous module in this case is qemu-vm.nix. It's intended as a readonly variable that other modules can use to do certain things differently in a container. i.e. To be analogous to isContainer, qemu-vm.nix would set isVM = true; and then other modules would read isVM to determine whether to change behavior based on being in a VM. Other types of VM guest modules could also set isVM to give the same signal.

So the analogy is backwards. The option should be named something more indicative of the fact that users should set it to turn a module on. virtualisation.enable would be conventional, but that's probably too generic and might cause people to mistake it for "This is how I enable KVM hosting on my desktop".

Finally, I'm still not sure this is a good idea. It's really weird to create an option that results in the system.build.toplevel attribute being useless on its own, since you need to actually use the system.build.vm attribute. Seems like a big footgun. Something that so fundamentally changes the nature of the nixos configuration seems like something that should be an import, not an enable style option.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Mar 18, 2025

So the analogy is backwards. The option should be named something more indicative of the fact that users should set it to turn a module on. virtualisation.enable would be conventional, but that's probably too generic and might cause people to mistake it for "This is how I enable KVM hosting on my desktop".

An alternative would be to move the qemu-vm.nix options from the top-evel to something like virtualisation.nixos and then add an virtualisation.nixos.enable.

Seems like a big footgun. Something that so fundamentally changes the nature of the nixos configuration seems like something that should be an import, not an enable style option.

Ok, but I think the current situation is worse: the options are documented, interspersed with unrelated modules using the same namespace, not enabled by default and it's not clear that you have to import qemu-vm.nix to use them. I've see at least a dozen posts about this on the forum.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: testing Tooling for automated testing of packages and modules 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants