mount: Handle multi-device with 1 device node #245
Conversation
Instead of requiring the user to supply all the device nodes for a multi-device FS, allow them to specifiy 1 of them. We then fetch the UUID for the FS and then find all the disks on the system that match this UUID. This allows me to bring up a bcachefs FS in /etc/fstab by using a UUID. This works because it appears the mount command looks up the UUID, finds an entry in /dev/disk/by-uuid and then passes that found device node to mount.bcachefs which fails with `insufficient_devices_to_start` as bcachefs is expecting a list of devices with a ":" between them. This behavior is preserved if a user specifies a list of all the needed device nodes to bring up the FS. Signed-off-by: Tony Asleson <[email protected]>
|
Looks like this is similar to PR #142 , but different. |
src/commands/cmd_mount.rs
Outdated
| } | ||
| } | ||
| } | ||
| Ok(None) |
There was a problem hiding this comment.
Why do you return Ok() here?
If I'm reading the right we should only reach this if the device path provided does not match any of the disks in the udev scan. That should return an Err() IMO.
There was a problem hiding this comment.
I took the approach that the code should display the same error message when it can't find something that the user supplies. That message before this change is Fatal error: No device found from specified parameters. By returning Ok(None) we preserve the same error path progression which reports this error. We could certainly change this to return an error instead.
| if let Some(bcache_fs_uuid) = uuid { | ||
| devs_str_sbs_from_uuid(bcache_fs_uuid.to_string()) | ||
| } else { | ||
| Ok((String::new(), Vec::new())) |
There was a problem hiding this comment.
Similar to my other question. The way I read the rest of the code is it returns Err() when the function could not provide the desired result.
IOW, should you not return Err() instead of special empty variables?
Better yet, return the Err() provided by devs_str_sbs_from_uuid?
There was a problem hiding this comment.
When devs_str_sbs_from_uuid doesn't find block devices that match the specified uuid it doesn't return an error, it returns an empty tuple if I'm reading it correctly. This is preserving the same behavior.
Just verified, take existing code and try to mount a non-existent UUID, you end up in this:
if block_devices_to_mount.len() == 0 {
Err(anyhow::anyhow!("No device found from specified parameters"))?;
}
Signed-off-by: Tony Asleson <[email protected]>
Signed-off-by: Tony Asleson <[email protected]>
When blkid changes with bcachefs are ubiquitous, we can simply leverage the bcachefs information in the udev db instead of reading the superblock on every disk. Reduces the number of superblock reads and removes messages pertaining to superblock reads for devices which do not contain a bcachefs FS. Note: this shouldn't be included until blkid changes have been released and packaged. Signed-off-by: Tony Asleson <[email protected]>
|
I had to think about this, and - I think this actually a pretty slick solution. This should help with systemd integration as well; we're trying to avoid systemd having to know which devices it has to wait for, and this helps with that. |
|
And since systemd initrd still can not properly mount bcachefs on boot (systemd-udev can), I think it will fix that too. |
|
The first commit in this PR was merged already. |
Instead of requiring the user to supply all the device nodes for a multi-device FS, allow them to specifiy 1 of them. We then fetch the UUID for the FS and then find all the disks on the system that match this UUID.
This allows me to bring up a bcachefs FS in /etc/fstab by using a UUID. This works because it appears the mount command looks up the UUID, finds an entry in /dev/disk/by-uuid and then passes that found device node to mount.bcachefs which fails with
insufficient_devices_to_startas bcachefs is expecting a list of devices with a":"between them. This behavior is preserved if a user specifies a list of all the needed device nodes to bring up the FS.Notes:
6.7.5-200Fedora kernel.