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
nixos/qemu-vm: Option to use squashfs Nix store closure instead of virtfs access to host's store #72354
Conversation
12cf6fe
to
588aaba
Compare
(Rebased on top of #72342 so the NixOS tests can run) |
@GrahamcOfBorg test qemu-private-store |
@GrahamcOfBorg test qemu-private-store |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks like a good idea, though I don't have enough familiarity to really judge whether this breaks anything. Requested a few more people to review.
588aaba
to
9a8668b
Compare
Updated to rebase on top of #55645. I confirm that login, upnp, installer, virtualbox, and the new test pass on x86_64-linux. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
9a8668b
to
6fc1303
Compare
Updated test from perl test framework to python test framework. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/157 |
virtualisation.shareExchangeDir = | ||
mkOption { | ||
default = true; | ||
type = types.bool; | ||
description = | ||
'' | ||
Share /tmp/xchg with the host. | ||
''; | ||
}; | ||
|
||
virtualisation.shareSharedDir = | ||
mkOption { | ||
default = true; | ||
type = types.bool; | ||
description = | ||
'' | ||
Share /tmp/shared with the host. | ||
''; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference of these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know.
By default they point at the same directory in the host. The test framework points them at different directories by setting SHARED_DIR
.
History:
- Aug 2011 d75efe4 edolstra: Introduced /tmp/xchg: "For security, don't mount the entire host filesystem."
- Aug 2012 a025e7e edolstra: Introduced /tmp/shared: "Provide a common share between VMs to allow easy communication"
- May 2020 d85dc4f erikarvstedt: Mentions "xchg is advertised as a bidirectional exchange dir."
Current uses I could find:
Mount | Purpose | Location |
---|---|---|
xchg | coverage-data | nixos/modules/testing/test-instrumentation.nix nixos/lib/test-driver/test-driver.pl |
xchg | runInMachine | nixos/lib/testing.nix |
shared | copy_from_host copy_from_vm | nixos/lib/test-driver/test-driver.py |
I just want a lever to turn them off. I guess it makes more sense to be one lever rather than two.
I combined these two options into one and added a note about what these directories are currently used for in the option description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@erikarvstedt @edolstra can you provide some insight here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably get rid of coverage-data
. AFAIK we're not doing coverage runs in VMs anymore (and IIRC it was only used in some toy examples like the Quake 3 test anyway).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably also eliminate copy_from_*
since it's only used in 3 places, which can be replaced by machine.execute("cat ...")
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs a rebase. |
When the store is not shared, the closure used in the VM it is packaged up in a squashfs, just like the ISO installer does.
Share /tmp/shared and /tmp/xchg with the host. These are used | ||
for runInMachine, copy_from_{host,vm}, and coverage-data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May want to use <literal>
markup for easier reading, e.g.
Share /tmp/shared and /tmp/xchg with the host. These are used | |
for runInMachine, copy_from_{host,vm}, and coverage-data. | |
Share <literal>/tmp/shared</literal> and <literal>/tmp/xchg</literal> with the host. | |
These are used for <literal>runInMachine</literal>, <literal>copy_from_{host,vm}</literal>, | |
and <literal>coverage-data</literal>. |
(Please check the manual builds after this, one can accidentlaly screw it up like I did recently.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nh2: ... or go even further and use <filename/>
and <function/>
:
Share /tmp/shared and /tmp/xchg with the host. These are used | |
for runInMachine, copy_from_{host,vm}, and coverage-data. | |
Share <filename class="directory">/tmp/shared</filename> and | |
<filename class="directory">/tmp/xchg</filename> with the host. | |
These are used for <function>runInMachine</function>, | |
<literal>copy_from_{host,vm}</literal>, and | |
<literal>coverage-data</literal>. |
this makes building the VM much faster requires less disk | ||
space. Disable to give the guest a separate, fresh store | ||
in a squashfs that contains only the closure of the guest's | ||
dependencies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the wording that contains only the closure of the guest's dependencies
is a bit misleading, because it may be understood as that if you shareNixStore = true
, then all of the hosts nix store is shared with the guest:
Even if you share the nix store with the host, only the closure of the guest's dependencies are "available" in the nix store; all the store paths are present as files in /nix/store
, but most of those are not valid
(registered in the sqlite DB).
This recently tricked me: https://discourse.nixos.org/t/how-to-build-a-nixos-vm-with-nix-in-which-nixos-rebuild-is-a-no-op/7937
Because of this ambiguity, I think we should add an explicit:
dependencies. | |
dependencies. | |
Note that even with <literal>virtualisation.shareNixStore = true;</literal>, | |
not the entire host Nix store is available in the guest Nix store: | |
While all <literal>/nix/store</literal> files will be present, only the ones | |
pulled in by <option>virtualisation.pathsInNixDB</option> | |
will be registered as valid in the Nix store database. |
appear in the guest Nix store as well, but are considered | ||
garbage (because they are not registered in the Nix | ||
database in the guest). | ||
database in the VM. If virtualisation.shareNixStore is true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
database in the VM. If virtualisation.shareNixStore is true, | |
database in the VM. If <option>virtualisation.shareNixStore</option> is true, |
shared.succeed("[[ $(mount | grep -c virt) -gt 0 ]]") | ||
private.succeed("[[ $(mount | grep -c virt) -eq 0 ]]") | ||
''; | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New tests are great!
Optionally, maybe also add a copy of this test that exercises the default virtualisation.shareNixStore = false;
?
This isn't used anymore as per NixOS#72354 (comment).
I marked this as stale due to inactivity. → More info |
now it works
Since ec6c604 from #127933 , this can be accomplished via configuration; it doesn't need to be explicitly supported in the qemu-vm module. Thanks, @rnhmjoj! The necessary configuration is now available at https://github.com/chkno/nixos-qemu-vm-isolation/blob/main/modules/qemu-vm-isolation.nix , which can be used as an example or directly as a library for folks that want this feature. |
Nice! It's good to know my cleanup has been useful. |
Motivation for this change
Allow VMs built with
nixos-rebuild build-vm
to work without any virtfs mounts into the host. In particular, allow them to use a nix store closure in a squashfs like the installer uses.This allows better isolation of guests. A host that runs multiple VMs may not wish the guests to be able to see each others' derivations.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review rev --branch staging HEAD"
-- It's zero packages, but this refactoring touches all the NixOS tests. I ran a handful of existing ones../result/bin/
)nix path-info -S
before and after) -- N/A?