Open jacobgreenleaf opened 1 year ago
In theory, all "read-only compatible" properties should be added to the compatibility.d/grub2 file. I have an open PR to add livelist
and zpool_checkpoint
. Is there any reason why the other two shouldn't also be added? Why do you say "possibly" here?
In theory, all "read-only compatible" properties should be added to the compatibility.d/grub2 file. I have an open PR to add
livelist
andzpool_checkpoint
. Is there any reason why the other two shouldn't also be added? Why do you say "possibly" here?
... ah, this is explained in the existing doc. @rlaager let me know if you think we should just add these to compatibility.d/grub2
upstream (or add to my PR yourself); it's probably a good idea to minimize the differences in features enabled on grub2 pools and no-compatibility pools?
I'm all for providing more features on the GRUB pool. But since it is meant to be compatible with GRUB, compatibility with GRUB should be the main focus. Did you test the features with GRUB? Note that GRUB support for ZFS is indeed not robust, and there are instances where read-only compatible features which are not compatible with GRUB, such as the userobj_accounting feature. See this page:
Colm @.***> writes:
In theory, all "read-only compatible" properties should be added to the compatibility.d/grub2 file. I have an open PR to add
livelist
andzpool_checkpoint
. Is there any reason why the other two shouldn't also be added? Why do you say "possibly" here?... ah, this is explained in the existing doc. @rlaager let me know if you think we should just add these to
compatibility.d/grub2
upstream (or add to my PR yourself); it's probably a good idea to minimize the differences in features enabled on grub2 pools and no-compatibility pools?-- Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs-docs/pull/433#issuecomment-1562314706 You are receiving this because you are subscribed to this thread.
Message ID: @.***>
Even more, grub from Suse doesn't work well with most of features, didn't have time to investigate further, unfortunately.
But I'm all to use compats with actual tests on selected distros.
I commented on the PR: This seems fine to me. I've been listing those features in the Debian Root-on-ZFS HOWTO. Note that I explicitly warn against userobj_accounting, since I have seen errors with that, even though it should be compatible.
But I'm all to use compats with actual tests on selected distros.
We can do that with the grub-probe command, which will use the ZFS driver shipped with GRUB to read the partition. If any unsupported features were enabled, the command will fail. Maybe I should add this test to the Root on ZFS guides.
Relevant source code
...
grub-core/fs/zfs/zfs.c:1135: str=com.delphix:hole_birth
grub-core/fs/zfs/zfs.c:1135: str=com.delphix:embedded_data
grub-core/fs/zfs/zfs.c:1144: check 12 passed (feature flags)
grub-core/fs/zfs/zfs.c:1884: zio_read: E 0: size 4096/4096
grub-core/fs/zfs/zfs.c:1906: endian = -1
grub-core/fs/zfs/zfs.c:597: dva=8, 2809a0
grub-core/fs/zfs/zfs.c:2503: looking for 'features_for_read'
grub-core/osdep/hostdisk.c:399: reusing open device /dev/nvme0n1p2' grub-core/fs/zfs/zfs.c:2117: zap: name = org.illumos:lz4_compress, value = 1, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = com.delphix:hole_birth, value = 1, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = com.delphix:extensible_dataset, value = 0, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = com.delphix:embedded_data, value = 1, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = org.open-zfs:large_blocks, value = 0, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = , value = 0, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = , value = 0, cd = 0 zfs grub-core/kern/disk.c:295: Closing
hostdisk//dev/nvme0n1'.
... grub-core/fs/zfs/zfs.c:2117: zap: name = org.illumos:lz4_compress, value = 1, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = com.joyent:multi_vdev_crash_dump, value = 0, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = com.delphix:hole_birth, value = 1, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = com.delphix:extensible_dataset, value = 10, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = com.delphix:embedded_data, value = 1, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = org.open-zfs:large_blocks, value = 0, cd = 0 grub-core/fs/zfs/zfs.c:2117: zap: name = org.zfsonlinux:large_dnode, value = 8, cd = 0 grub-core/kern/fs.c:78: zfs detection failed. grub-probe: error: unknown filesystem.
I'm all for providing more features on the GRUB pool. But since it is meant to be compatible with GRUB, compatibility with GRUB should be the main focus.
I absolutely agree; and to be honest I don't think I'm the best person to decide on which features should be present in which compatibility files - all I did was implement it.
Did you test the features with GRUB? Note that GRUB support for ZFS is indeed not robust, and there are instances where read-only compatible features which are not compatible with GRUB, such as the userobj_accounting feature. See this page.
I did test that GRUB worked ok with livelist
and zpool_checkpoint
, both of which provide features which are useful enough that I can envisage their being used on a boot pool.
The balance I think we should draw is "works with GRUB, has no reported problems, and is conceivably useful on a small /boot pool". userobj_accounting
doesn't make the cut. @rlaager's "root on ZFS" page (this page, also the one you refer to) seems to draw a good balance - but I would be inclined to add known-good features to the compatibility.d/grub2
file.
Even more, grub from Suse doesn't work well with most of features, didn't have time to investigate further, unfortunately.
But I'm all to use compats with actual tests on selected distros.
This concerns me. What version of GRUB2 does SUSE ship with? Is there any conversation about these problems we could dig into?
Let me pose the question a different way:
Assuming the main use of -o compatibility=grub2
is going to be "installing /boot on a ZFS partition, per the instructions in this document", which features should we consider including in the GRUB2 compatibility file?
I see several main categories of features:
My question is: would any of the features in the final group here be actively harmful to include in compatibility.d/grub2
(and therefore probably being enabled by default in any installations which follow this guide)?
Approaches:
My personal opinion favors maximal liberalism; I think there's a benefit to enabling as many features as possible on the boot pool and only excluding features which are known to break GRUB. Features being enabled is almost definitely the most common case (that's why they're called features, and why zpool status
flags them as being available).
But I feel that @rlaager has put the most effort into figuring out what works well with GRUB and what doesn't, so I'm inclined to defer to his judgement.
WDYT?
- Known compatible with GRUB, low-risk, useful for boot pools:
One could potentially split a subset of this list into:
Then these remain in the next category:
- async_destroy
- bookmarks
- empty_bpobj
- enabled_txg
- filesystem_limits
- livelist
- spacemap_histogram
- zpool_checkpoint
I'm not sure all of these are "useful for boot pools". I'm sure we could bikeshed that to death. But, for example, I think "hole_birth" is effectively moot these days, after the bug was found in it.
I say this not to pick on you, but because this has been something I've actually thought about. Should the recommendations in the Root on ZFS HOWTOs be limited to just those that are demonstrably useful for a boot pool? That's a bit of a different question of whether a given feature should be part of compatibility=grub2. (Though your assumption, which I think is reasonable, that compatibilty=grub2 is for /boot on ZFS makes them essentially the same.)
Probably compatible with GRUB, not currently recommended for boot pool:
- allocation_classes
- device_rebuild
- log_spacemap
- project_quota
- resilver_defer
My question is: would any of the features in the final group here be actively harmful to include in
compatibility.d/grub2
(and therefore probably being enabled by default in any installations which follow this guide)?
Those should be safe. But I can't remember what all I've tested and what the results were. That's why I try to document these things, like I did with userobj_accounting.
Also, when testing features, it's easy to test with them "enabled", but we really need to test with them "active". So you'd have to make a boot pool and put a "special" device in it to test allocation_classes (which I think I might have done and would likely be fine), setup a deferred resilver situation (however that works) for "resilver_defer", etc.
My personal opinion favors maximal liberalism; I think there's a benefit to enabling as many features as possible on the boot pool and only excluding features which are known to break GRUB. Features being enabled is almost definitely the most common case (that's why they're called features, and why
zpool status
flags them as being available).
I've been taking the "maximal liberalism" approach. Like you, my thinking is that we only exclude them if they're known to not work. On the other hand, this leads to enabling a long list of features of questionable utility. The work of hand-enabling them goes away with the "compatibility" option though. We are still taking some risk by enabling features. This is what is making me start leaning towards more conservative.
I am absolutely inclined to defer to your expertise and judgement on this; so I suggest leaving the contents of the compatibility file alone for now, post- adding livelist
and zpool-checkpoint
. There's nothing stopping a given system administrator from using a different compatibility file if they feel the need for an additional feature (or opening a PR against openzfs and fighting the good fight there...)
Hopefully we can go with just -o compatibility=grub2
for the bookworm version of this document. Not sure whether my recent PR will make the cut for the release (probably not), but it'll be included soon enough I suspect. Either way, the impact will be negligible.
On Bookworm:
root@debian:~# zpool create \
-o ashift=12 \
-o autotrim=on -d \
-o cachefile=/etc/zfs/zpool.cache \
-o feature@livelist=enabled \
-o feature@zpool_checkpoint=enabled \
-o compatibility=grub2 \
-O devices=off \
-O acltype=posixacl -O xattr=sa \
-O compression=lz4 \
-O normalization=formD \
-O relatime=on \
-O canmount=off -O mountpoint=/boot -R /mnt \
bpool ${DISK}3
Warning: feature "zpool_checkpoint" enabled but is not in specified 'compatibility' feature set.
Warning: feature "livelist" enabled but is not in specified 'compatibility' feature set.
cannot create 'bpool': operation not supported on this type of pool
"-d" is an error; it should not be in this command line.
Makes a small note available both for existing users of
bullseye-backports
and the soon to be releasedbookworm
release that thecompatibility
option is available and we can usegrub2
.A future version of this guide referencing bookworm may want to continue enabling some of the features that are read-compatible but not on that list. That is:
And possibly
device_rebuild
,resilver_defer
to be added.