[sled-agent] Make new mountpoints immutable #7888
Conversation
|
I'm going to take this for a spin on Dublin. |
illumos-utils/src/zfs.rs
Outdated
| // property on it. This prevents the mountpoint from being used as anything other than a | ||
| // mountpoint. | ||
| // | ||
| // NOTE: This must only be called on mountpoints for datasets which ARE NOT MOUNTED. |
There was a problem hiding this comment.
What happens if this is called and the dataset is mounted? Do we need to guard against that?
Edit: after reading below, I think if we did this, we'd make the mounted directory immutable, make a temporary directory, then fail in the rename() calls (since rename won't cross filesystems, right?). The renames failing is good, but making the mounted directory immutable seems pretty bad...
There was a problem hiding this comment.
I think this is always a little bit racy, but I can do something better in this case. It's easy to query zfs get -Hpo value mountpoint <mountpoint>, and bail if things already look mounted.
I'll do that.
| if !mounted && want_to_mount { | ||
| Self::mount_dataset(name)?; | ||
| if !zoned && !mounted { | ||
| if let (CanMount::On, Mountpoint::Path(path)) = |
There was a problem hiding this comment.
What other kinds of Mountpoints are there? (And what would it mean if we had CanMount::On, MountPoint::SomethingElse(_)?)
There was a problem hiding this comment.
mountpoint=path|none|legacy
Controls the mount point used for this file system. See the Mount Points
section for more information on how this property is used.
When the mountpoint property is changed for a file system, the file
system and any children that inherit the mount point are unmounted. If
the new value is legacy, then they remain unmounted. Otherwise, they are
automatically remounted in the new location if the property was
previously legacy or none, or if they were mounted before the property
was changed. In addition, any shared file systems are unshared and
shared in the new location.
My expectation is that Path is the only value where we actually want to do this mountpoint setup.
After installing r13, I created some resources on the rack., then rebooted each node by itself. I then created and deleted more resources on the rack. The reboot caused the crypt/debug datasets to not be mounted any longer. The additional activities resulted in logs being rotated to crypt/debug directories (not the dataset, just the directory). After mupdate to this PR and a schema update, things came back online. I can see the results here where crypt/debug directories were moved aside, and crypt/debug datasets are all now mounted. |
leftwo
left a comment
leftwo
left a comment
There was a problem hiding this comment.
github really struggled with the diff here. I believe I've looked at the most important parts and it all looks good. If you hid some easter egg in nexus_db_schema then good for you, I can't find it.
| } | ||
|
|
||
| // If it doesn't exist, make it. | ||
| // If the dataset doesn't exist, create it. |
There was a problem hiding this comment.
Is it worth a comment here that zoned datasets are mounted elsewhere?
I went ahead and rebased, hopefully this is easier to read now! |
Partial fix of #7874 and #4203
This PR causes new points to be created as immutable directories, preventing files from begin created
in the "parent filesystem", underneath the intended mountpoint.