blockdev: handle ZFS dataset sources in list_dev_by_dir#2138
blockdev: handle ZFS dataset sources in list_dev_by_dir#2138hanthor wants to merge 1 commit intobootc-dev:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for identifying block devices backing ZFS datasets by introducing the list_dev_for_zfs_dataset function and updating list_dev_by_dir. The review feedback suggests improving the robustness of the ZFS pool name extraction and narrowing the filesystem detection heuristic to prevent incorrect matches with network filesystems like NFS.
| let pool = dataset | ||
| .split('/') | ||
| .next() | ||
| .ok_or_else(|| anyhow!("Invalid ZFS dataset: {dataset}"))?; |
There was a problem hiding this comment.
The split('/').next() call on a string will always return Some (even for an empty string), making the ok_or_else block unreachable. Furthermore, if the dataset string is empty or only contains the ZFS= prefix, pool will be an empty string, which will cause the zpool command to fail later. It's safer to filter out empty strings to ensure a valid pool name is extracted.
| let pool = dataset | |
| .split('/') | |
| .next() | |
| .ok_or_else(|| anyhow!("Invalid ZFS dataset: {dataset}"))?; | |
| let pool = dataset | |
| .split('/') | |
| .next() | |
| .filter(|s| !s.is_empty()) | |
| .ok_or_else(|| anyhow!("Invalid ZFS dataset: {dataset}"))?; |
| let fsinfo = bootc_mount::inspect_filesystem_of_dir(dir)?; | ||
| list_dev(&Utf8PathBuf::from(&fsinfo.source)) | ||
| let source = &fsinfo.source; | ||
| if fsinfo.fstype == "zfs" || (!source.starts_with('/') && source.contains('/')) { |
There was a problem hiding this comment.
The heuristic (!source.starts_with('/') && source.contains('/')) is too broad and will incorrectly match network filesystems like NFS (e.g., server:/export). This would cause bootc to attempt running zpool on non-ZFS systems, which might fail if the zpool command is missing or the source is not a valid pool. It is recommended to rely primarily on fsinfo.fstype == "zfs" or a more specific prefix like ZFS=.
| if fsinfo.fstype == "zfs" || (!source.starts_with('/') && source.contains('/')) { | |
| if fsinfo.fstype == "zfs" || source.starts_with("ZFS=") { |
|
I don't know if this is wanted since ZFS doesn't support fs very as-is but it is a fun way to maintain an is lifecycle without it still. ZFS on root is fun and cool, but I'm more working on this as a proof of concept Ubuntu 26.04 Desktop |
ZFS filesystems report their mount source as a dataset name (e.g. 'rpool/root') rather than a block device path. Passing this to lsblk causes it to fail with 'not a block device', breaking bootc status and bootc upgrade on ZFS-rooted systems. Add list_dev_for_zfs_dataset() which extracts the pool name and queries 'zpool list -H -v -P <pool>' to find the underlying block device path, then delegates to list_dev() as normal. Detect ZFS sources in list_dev_by_dir() via fstype=='zfs' or by a non-absolute path containing '/' (dataset name form). Fixes: bootc-dev#1240 Signed-off-by: James Reilly <jreilly1821@gmail.com> Assisted-by: GitHub Copilot (claude-sonnet-4-6)
f94ee90 to
71fcbe5
Compare
ZFS filesystems report their mount source as a dataset name (e.g.
rpool/root) rather than a block device path. Passing this tolsblkcauses it to fail withnot a block device, breakingbootc statusandbootc upgradeon ZFS-rooted systems.Add
list_dev_for_zfs_dataset()which extracts the pool name and querieszpool list -H -v -P <pool>to find the underlying block device path, then delegates tolist_dev()as normal.Detect ZFS sources in
list_dev_by_dir()viafstype == "zfs"or by a non-absolute path containing/(dataset name form).Testing
Tested on an Ubuntu 26.04 system installed to ZFS via
bootc install to-filesystem. The installed system usesrpool/rootas the rootfs.bootc status— previously failed withlsblk: rpool/root: not a block device, now returns full status output ✅bootc upgrade --check— previously failed at the same point, now successfully queries the registry ✅Reproduced against v1.15.0 and verified fix against current main.
Fixes: #1240
Assisted-by: GitHub Copilot (claude-sonnet-4-6)