I think a goal of this state file is to be able to restart a VM (when interactively used). In theory, this should work well, but this is not what I'm observing in practice! So, I always have to remove it.
Another approach could be to remove this file (by default) if it already exists. If a user wants to use it, a flag could be set to not remove it.
This would also avoid the /tmp pollution (which is not a real issue I guess).

No opinion on this matter, however if a change is going to be made the documentation in nixos/doc/manual/development/running-nixos-tests-interactively.xml should be updated accordingly. Also the Perl test driver in nixos/lib/test-driver/Machine.pm.

The use of a deterministic name is intentional so we can keep state across reboots. This is useful both for interactive use and in some tests to see whether state it kept correctly across reboots (e.g. uid assignments).

@edolstra I think a lot of people find this pretty confusing – a lot of tests fail when the state is persisted, and it's easy to chalk that up to a different change and spend a long time debugging.

@edolstra well it is obvious that it is intentional right now.

But instead of just stating this and closing the issue you could have at least tried to relate to the problems that multiple people are stating here - which i have also run into when debugging something.

You don't seem to even acknowledge the possibility that there is an issue that people are running into.

The fact that it hasn't been a problem for you doesn't mean it doesn't exist. I would appreciate if in scenarios like this there could at least be some sort of discourse:

What are the issue that people are facing? What is the suggested solution and how does it collide with existing goals? Is there something that can be done about this that you or others are less opposed to but still addresses the issue described?

I think this would give a less hostile and more encouraging impression.

PS: Just to clarify: i am not saying you are wrong, my concern is purely about the communication and not ignoring concerns:

This is intended behavior. Ticket closed.

versus

I see how this can be problematic but the reason for this is that we want these to be deterministic across reboots. This is all gone with your change. How are we supposed to deal with that? Are you saying we should just drop determinism? This would be a problem because <....>

I for one would appreciate a --impure (or similar named) flag to enable that feature. In most cases I don't want shared state but sometimes I need it. It should be opt-in. Most impure things are (moving towards) opt-in in the Nix world.

@edolstra is a flag something that you could find acceptable? Where the default behavior is the current behavior and --impure would be something similar to what this PR implements? D'OH that was the wrong way round 😅 but you get the idea..

Personally I would just add a warning like using existing state directory /tmp/vm-state-bla. That should alert interactive users that they're reusing an existing VM. IMHO that's better than silently filling up the disk with virtual machine images.

BTW, since NixOS deployments should generally be congruent (i.e. not depend on the previous state of the system), if a test fails because the previous contents of the disk, then there might be an issue with the test or the service that it's testing.

BTW, since NixOS deployments should generally be congruent (i.e. not depend on the previous state of the system), if a test fails because the previous contents of the disk, then there might be an issue with the test or the service that it's testing.

@edolstra This won't work. If you terminate an instance during the test, you might end up with a screwed up file system, and NixOS shouldn't try to automatically fix this and break it further.

If have been bitten by some state on the machines in other occurences as well.

IMHO, The sane default here is to put the qcow image in a tmpdir that's cleaned up after invocation - with some flags to alternate behaviour.

Personally I would just add a warning like using existing state directory /tmp/vm-state-bla. That should alert interactive users that they're reusing an existing VM. IMHO that's better than silently filling up the disk with virtual machine images.

Stdout gets flooded by the VMs boot sequence when starting up the test driver. I'm not sure I'd notice such a warning. On top of that I'm generally more in enclined to prevent people from using a footgun altogether rather than warn them they're about to use a footgun.

IMHO that's better than silently filling up the disk with virtual machine images.

That's a fair point!

I could add a --impure flag to this PR allowing you to keep the state after the VM run (ie. the current behavior). On a default run, we could erase the VM state altogether to prevent /tmp to get unnecessarily filled.

^ Does that sound like a fair middle-ground to you? Would you be up to merge such PR?

I would name that something more specific, like --keep-vm-state but 👍 for that over debugging time

Implemented this solution there: picnoir@052e4f5

@domenkozar good suggestion, I used this together with -K to reflect nix-build's behaviour.

@mmilata good point regarding the doc. Updated. I also added a line to the release note about this behaviour's change.

Could we re-open this PR to talk about these changes?

@NinjaTrappeur github doesn't allow to reopen this PR, as the branch was deleted. But yes, please open a new PR, and cross-link it from here.

👍

See #91046

You can run with TMPDIR=/path/to/state-dir and handle the creation/cleanup of the tmpdir yourself. Hopefully that helps with some use cases.

@ali-abrar, #91046 has been merged. It'll delete the VMs state on the test runner startup.