Re: git: 72a1cb05cd23 - main - rc(8): Add a zpoolupgrade rc.d script

[Alexander Leidinger]

 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
};
     

     
    Whether or not it should do that by default, always, or never is an open

    there today.
     

Any feature not on this list will cause the boot loader to reject the pool. question. I've thought there should be a 'shoot footing' override that isn't

Thanks for the list. For those interested, it is in
    $SRC/stand/libsa/zfs/zfsimpl.c






    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

Bye,
Alexander.
--
http://www.Leidinger.net alexan...@leidinger.net: PGP 0x8F31830F9F2772BF
http://www.FreeBSD.org    netch...@freebsd.org  : PGP 0x8F31830F9F2772BF

Just 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? 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:

pgp7ZpjmbBmnx.pgp
Description: Digitale PGP-Signatur