Quoting Warner Losh <i...@bsdimp.com> (from Mon, 7 Nov 2022 14:23:11 -0700):On Mon, Nov 7, 2022 at 4:15 AM Alexander Leidinger <alexan...@leidinger.net> wrote:Quoting Li-Wen Hsu <lw...@freebsd.org> (from Mon, 7 Nov 2022 03:39:19 GMT):The branch main has been updated by lwhsu: URL: https://cgit.FreeBSD.org/src/commit/?id=72a1cb05cd230ce0d12a7180ae65ddbba2e0cb6d commit 72a1cb05cd230ce0d12a7180ae65ddbba2e0cb6d Author: Li-Wen Hsu <lw...@freebsd.org> AuthorDate: 2022-11-07 03:30:09 +0000 Commit: Li-Wen Hsu <lw...@freebsd.org> CommitDate: 2022-11-07 03:30:09 +0000 rc(8): Add a zpoolupgrade rc.d script If a zpool is created by makefs(8), its version is 5000, i.e., all feature flags are off. Introduce an rc script to run `zpool upgrade` over the assigned zpools on the first boot. This is useful to the ZFS based VM images built from release(7).diff --git a/share/man/man5/rc.conf.5 b/share/man/man5/rc.conf.5 index f9ceabc83120..43fa44a5f1cb 100644 --- a/share/man/man5/rc.conf.5 +++ b/share/man/man5/rc.conf.5 @@ -24,7 +24,7 @@ .\" .\" $FreeBSD$ .\" -.Dd August 28, 2022 +.Dd November 7, 2022 .Dt RC.CONF 5 .Os .Sh NAME @@ -2109,6 +2109,13 @@ A space-separated list of ZFS pool names for which new pool GUIDs should be assigned upon first boot. This is useful when using a ZFS pool copied from a template, such as a virtual machine image. +.It Va zpool_upgrade +.Pq Vt str +A space-separated list of ZFS pool names for which version should be upgraded +upon first boot. +This is useful when using a ZFS pool generated by +.Xr makefs 8 +utility.For someone who knows ZFS well, it is clear that only a zpool upgrade is done. Not so experienced people may assume there is a combination of zpool upgrade and zfs upgrade (more so for people which do not know what the difference is). Maybe you want to add some explicit documentation, that zfs upgrade + feature flags needs to be done by hand. And this brings me to a second topic, we don't have an explicit list of features which are supported by the bootloader (I had a look at the zfs and the boot related man pages, if I overlooked a place, then the other places should reference this important part with some text).There is a fixed list of features we support in the boot loader: /* * List of ZFS features supported for read */ static const char *features_for_read[] = { "org.illumos:lz4_compress", "com.delphix:hole_birth", "com.delphix:extensible_dataset", "com.delphix:embedded_data", "org.open-zfs:large_blocks", "org.illumos:sha512", "org.illumos:skein", "org.zfsonlinux:large_dnode", "com.joyent:multi_vdev_crash_dump", "com.delphix:spacemap_histogram", "com.delphix:zpool_checkpoint", "com.delphix:spacemap_v2", "com.datto:encryption", "com.datto:bookmark_v2", "org.zfsonlinux:allocation_classes", "com.datto:resilver_defer", "com.delphix:device_removal", "com.delphix:obsolete_counts", "com.intel:allocation_classes", "org.freebsd:zstd_compress", "com.delphix:bookmark_written", "com.delphix:head_errlog", "org.openzfs:blake3", NULL };Any feature not on this list will cause the boot loader to reject the pool.Whether or not it should do that by default, always, or never is an openquestion. I've thought there should be a 'shoot footing' override that isn'tthere today.Thanks for the list. For those interested, it is in $SRC/stand/libsa/zfs/zfsimpl.cJust to make my opinion expressed before explicit again, this should be documented in a boot / bootloader related man-page, but isn't.Should the above list be sorted in some way? Maybe in the same order as the zpool-features lists them (sort by feature name after the colon), or alphabetical?
Is it OK if I commit this alphabetical sorting?
---snip---
diff --git a/stand/libsa/zfs/zfsimpl.c b/stand/libsa/zfs/zfsimpl.c
index 6b961f3110a..36c90613e82 100644
--- a/stand/libsa/zfs/zfsimpl.c
+++ b/stand/libsa/zfs/zfsimpl.c
@@ -118,29 +118,29 @@ static vdev_list_t zfs_vdevs;
* List of ZFS features supported for read
*/
static const char *features_for_read[] = {
- "org.illumos:lz4_compress",
- "com.delphix:hole_birth",
- "com.delphix:extensible_dataset",
- "com.delphix:embedded_data",
- "org.open-zfs:large_blocks",
- "org.illumos:sha512",
- "org.illumos:skein",
- "org.zfsonlinux:large_dnode",
- "com.joyent:multi_vdev_crash_dump",
- "com.delphix:spacemap_histogram",
- "com.delphix:zpool_checkpoint",
- "com.delphix:spacemap_v2",
- "com.datto:encryption",
"com.datto:bookmark_v2",
- "org.zfsonlinux:allocation_classes",
+ "com.datto:encryption",
"com.datto:resilver_defer",
+ "com.delphix:bookmark_written",
"com.delphix:device_removal",
+ "com.delphix:embedded_data",
+ "com.delphix:extensible_dataset",
+ "com.delphix:head_errlog",
+ "com.delphix:hole_birth",
"com.delphix:obsolete_counts",
+ "com.delphix:spacemap_histogram",
+ "com.delphix:spacemap_v2",
+ "com.delphix:zpool_checkpoint",
"com.intel:allocation_classes",
+ "com.joyent:multi_vdev_crash_dump",
"org.freebsd:zstd_compress",
- "com.delphix:bookmark_written",
- "com.delphix:head_errlog",
+ "org.illumos:lz4_compress",
+ "org.illumos:sha512",
+ "org.illumos:skein",
+ "org.open-zfs:large_blocks",
"org.openzfs:blake3",
+ "org.zfsonlinux:allocation_classes",
+ "org.zfsonlinux:large_dnode",
NULL
};
---snip---
As Mark already mentioned some flags, I checked the features marked as read only (I checked in the zpool-features man page, including the dependencies documented there) and here are those not listed in zfsimpl.c. I would assume as they are read-only compatible, we should add them:com.delphix:async_destroy com.delphix:bookmarks org.openzfs:device_rebuild com.delphix:empty_bpobj com.delphix:enable_txg com.joyent:filesystem_limits com.delphix:livelist com.delphix:log_spacemap com.zfsonlinux:project_quota com.zfsonlinux:userobj_accounting com.openzfs:zilsaxattr
If my understanding is correct that the read-only compatible parts (according to the zpool-features man page) are safe to add to the zfs boot, here is what I have only build-tested (relative to the above alphabetical sorting):
---snip---
--- zfsimpl.c_sorted 2022-11-09 12:55:06.346083000 +0100
+++ zfsimpl.c 2022-11-09 13:01:24.083364000 +0100
@@ -121,24 +121,35 @@
"com.datto:bookmark_v2",
"com.datto:encryption",
"com.datto:resilver_defer",
+ "com.delphix:async_destroy",
"com.delphix:bookmark_written",
+ "com.delphix:bookmarks",
"com.delphix:device_removal",
"com.delphix:embedded_data",
+ "com.delphix:empty_bpobj",
+ "com.delphix:enable_txg",
"com.delphix:extensible_dataset",
"com.delphix:head_errlog",
"com.delphix:hole_birth",
+ "com.delphix:livelist",
+ "com.delphix:log_spacemap",
"com.delphix:obsolete_counts",
"com.delphix:spacemap_histogram",
"com.delphix:spacemap_v2",
"com.delphix:zpool_checkpoint",
"com.intel:allocation_classes",
+ "com.joyent:filesystem_limits",
"com.joyent:multi_vdev_crash_dump",
+ "com.openzfs:zilsaxattr",
+ "com.zfsonlinux:project_quota",
+ "com.zfsonlinux:userobj_accounting",
"org.freebsd:zstd_compress",
"org.illumos:lz4_compress",
"org.illumos:sha512",
"org.illumos:skein",
"org.open-zfs:large_blocks",
"org.openzfs:blake3",
+ "org.openzfs:device_rebuild",
"org.zfsonlinux:allocation_classes",
"org.zfsonlinux:large_dnode",
NULL
---snip---
Anyone able to test some of those or confirms my understanding is
correct and would sign-off on a "reviewed by" level?
Bye, Alexander. -- http://www.Leidinger.net alexan...@leidinger.net: PGP 0x8F31830F9F2772BF http://www.FreeBSD.org netch...@freebsd.org : PGP 0x8F31830F9F2772BF
pgp_PNYtWTPMV.pgp
Description: Digitale PGP-Signatur
