Skip to content

MountTag hashing using ":" instead of "\0" separator #5081

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.

Sign up for GitHub

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
ciderale wants to merge 1 commit into
base: master
Choose a base branch
from
+1 −1
Open

MountTag hashing using ":" instead of "\0" separator #5081

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/limayaml/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func MACAddress(uniqueID string) string {
// MountTag generates a stable mount tag from location and mountPoint.
// Both paths are hashed to handle the same location mounted to multiple mount points.
func MountTag(location, mountPoint string) string {
sha := sha256.Sum256([]byte(location + "\x00" + mountPoint))
sha := sha256.Sum256([]byte(location + ":" + mountPoint))
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Jun 3, 2026

Choose a reason for hiding this comment

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

The hash is expected to be stable across versions, so I don't think we can accept this change.

Why does https://github.com/ciderale/nixos-lima need to compute the hash by itself?
If it is really necessary it may invoke another scripting language such as bash or Python

Copy link
Copy Markdown
Author

@ciderale ciderale Jun 3, 2026

Choose a reason for hiding this comment

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

Thank you for the quick reply. In NixOS, the aim is to declaratively define the system configuration using the nix language and its module system. My nixos-lima setup can essentially avoid configuration via cloud-init (except for writing the "lima-boot-done" files). Technically, there is a way to call external programs and read them back into the nix configuration, but that should generally be avoided and causes other issues in this MacOS/Linux setup.

My impression was, that the mountTag is an opaque implementation detail as cloud-init gets the tag as configuration and mounts them accordingly. I was more worried, that my implementation now depends on this hashing -- I have a locally patched lima version. Is there a possibility to change that in some (not too far) future version?

Another idea was: Would it make sense to allow to provide a mountTag in the configuration explicitly? The current hash computation would be used if no mountTag was specified. That would be a bit more of a change, but that way, there is no coupling to this hashing.

Copy link
Copy Markdown
Member

@jandubois jandubois Jun 3, 2026

Choose a reason for hiding this comment

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

I don't quite understand the issue: if you don't use the tag generated by Lima, then it shouldn't matter what kind of tag you calculate in Nix, as long as it is stable. Why do they have to match?

Copy link
Copy Markdown
Author

@ciderale ciderale Jun 3, 2026

Choose a reason for hiding this comment

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

Lima-vm is "registering" the tags that eventually are "mountable" in the guest -- the tag is kind of the interface.

The NixOS configuration only does the latter. With my lima-nixos configuration, I also generate the lima configuration yaml. However, since the mountTag is currently not configurable in the lima configuration, I have to match the hashing scheme.

With the idea of making the mountTag configurable (in lima.yaml), my nix configuration could define both sides. In that case, there would not be a need to match the hashing.

Making the mountTag (optionally) configurable would be backward compatible, I suppose. The hashing would be used if the configuration has no explicit mountTag. But it is slightly more complex than changing the separator and one still had to use some hashing to configure the mountTag.

May I ask, in which scenario would a change of the hashing (as in this PR) have a noticeable impact? I currently don't see it, but I guess you have a concrete example where it matters.

Copy link
Copy Markdown
Member

@jandubois jandubois Jun 3, 2026

Choose a reason for hiding this comment

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

The whole point of switching from an index to a hash (c30cc31) was to have a stable mapping across reboots, so fstab entries would remain correct early during the boot, even before cloud-init is being processed. Otherwise you would need 2 reboot cycles when the mounts changed.

Changing the hashing function would break existing instances when you upgrade Lima.

I still don't understand why Nix can't extract the tags from user-data, which should get written even if you don't use cloud-init. You should never need to recompute hash values in the guest; they should be opaque tags. The fact that they are hashes is an implementation detail.

Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Jun 4, 2026

Choose a reason for hiding this comment

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

Let me close this PR, feel free to open a new one for defining the tag in the yaml

Copy link
Copy Markdown
Member

@jandubois jandubois Jun 4, 2026

Choose a reason for hiding this comment

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

Given that changing the hashing algorithm is not going to break existing instance (they will all remount because all the tags have changed, so there should be no risk of mounting the wrong device), isn't it better not to expose this implementation detail to the end user (well, template author)?

Because now a Nix user would have to manually specify a tag for each mount point.

So unless I misunderstand that changing the hash doesn't break anything, I would prefer the fix from the current PR.

Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Jun 4, 2026

Choose a reason for hiding this comment

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

So unless I misunderstand that changing the hash doesn't break anything

Was it confirmed that it does not leave old entries in fstab?

Copy link
Copy Markdown
Member

@jandubois jandubois Jun 4, 2026

Choose a reason for hiding this comment

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

In short, my nixos-lima has a unified configuration.nix that generates the lima.yaml configuration and also the NixOS guest configuration.

Sorry, I totally misunderstood that this calculation happens again on the host and not inside the guest.

Copy link
Copy Markdown
Member

@jandubois jandubois Jun 5, 2026

Choose a reason for hiding this comment

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

Was it confirmed that it does not leave old entries in fstab?

It was not confirmed yet, and I had no chance to try, but it sounded convincing:

  • When the hash function changes, all tags change.
  • On reboot none of the old tags exist anymore, so all the entries will be removed from fstab.
  • During cloud-init all mounts will be re-created using the new tags.

This was a problem before because the tags were re-used and would point to the wrong location. The mounts were not removed during reboot because the tag, e.g. mount1 still existed, it just now pointed somewhere else.

But with a changed hash function, none of the old tags should exist anymore, so fstab should be clean by the time cloud-init starts.

It is like deleting all mounts and adding them all back in a single boot cycle.

return fmt.Sprintf("lima-%x", sha[0:8])
}

Expand Down
Loading