From nobody Tue Nov 8 09:50:53 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4N63JX2S48z4hHfR; Tue, 8 Nov 2022 09:51:08 +0000 (UTC) (envelope-from Alexander@leidinger.net) Received: from mailgate.Leidinger.net (mailgate.leidinger.net [IPv6:2a00:1828:2000:313::1:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature ECDSA (P-256) client-digest SHA256) (Client CN "mailgate.leidinger.net", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4N63JW6QPQz42Ht; Tue, 8 Nov 2022 09:51:07 +0000 (UTC) (envelope-from Alexander@leidinger.net) Authentication-Results: mx1.freebsd.org; none Received: from outgoing.leidinger.net (p508d4280.dip0.t-ipconnect.de [80.141.66.128]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-256) server-digest SHA256 client-signature ECDSA (P-256) client-digest SHA256) (Client CN "outgoing.leidinger.net", Issuer "R3" (verified OK)) by mailgate.Leidinger.net (Postfix) with ESMTPSA id 9B852293F3; Tue, 8 Nov 2022 10:50:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=leidinger.net; s=outgoing-alex; t=1667901058; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=wDuT6jKeTjI+0M0rkl5cL39o+25rE7+XGWxrQZlLASY=; b=1BFUU+2LlHQrSsuqnlBBRzcLZ/QhPuiXcOeOCzbzPCOTwltibfZIcSOr5la9Jsn2NUzSCD S2peMcFywkLntO1E4MWWR+RKQO0ihZbt3lArargIJ8bgpyj0bgBzF1hIMDB5pPqOE4/dag qiRvVrkqH9w11WA1qxHl6WsqzEZM1+bGvJbMFKydgrTxXy86HDwUXFi1GlCzd6CVLGzz3k vaC5Q0ra5m6urtb3AKvT3iF9erjVD5tcBqcX0h24rR8fZUvs+jNVyS/NJK/5S/dn8/DuhU V8AJo9xpZ8hTgUV87/D6Vo5s6863wDxmETnViGoLZZSm6qt2tpcKlwkwNrgmZg== Received: from webmail.leidinger.net (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-256) server-digest SHA256) (Client did not present a certificate) by outgoing.leidinger.net (Postfix) with ESMTPS id 37C964FA1; Tue, 8 Nov 2022 10:50:55 +0100 (CET) Date: Tue, 08 Nov 2022 10:50:53 +0100 Message-ID: <20221108105053.Horde.eqgFiBJe2ngGAj6BkXcv5-Z@webmail.leidinger.net> From: Alexander Leidinger To: Warner Losh , marklmi@yahoo.com Cc: Li-Wen Hsu , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 72a1cb05cd23 - main - rc(8): Add a zpoolupgrade rc.d script References: <202211070339.2A73dJlO027991@gitrepo.freebsd.org> <20221107121514.Horde.nulS9Wg1s3yzAsXXkuJRBa9@webmail.leidinger.net> In-Reply-To: Accept-Language: de,en Content-Type: multipart/signed; boundary="=_thrjPkoMLuD2j4Rj9EGBFwk"; protocol="application/pgp-signature"; micalg=pgp-sha256 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 X-Rspamd-Queue-Id: 4N63JW6QPQz42Ht X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:34240, ipnet:2a00:1828::/32, country:DE] X-ThisMailContainsUnwantedMimeParts: N This message is in MIME format and has been PGP signed. --=_thrjPkoMLuD2j4Rj9EGBFwk Content-Type: multipart/alternative; boundary="=_v_NuznhOh5dfXKA4Ei6J_Hb" This message is in MIME format. --=_v_NuznhOh5dfXKA4Ei6J_Hb Content-Type: text/plain; charset=utf-8; format=flowed; DelSp=Yes Content-Description: Textnachricht Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Quoting Warner Losh (from Mon, 7 Nov 2022 14:23:11 -0700= ): > =C2=A0 > > On Mon, Nov 7, 2022 at 4:15 AM Alexander Leidinger=20=20 >=20 wrote: > >> Quoting Li-Wen Hsu (from Mon, 7 Nov 2022 03:39:19 GM= T): >> >>> The branch main has been updated by lwhsu: >>> >>> URL:=C2=A0 >>> https://cgit.FreeBSD.org/src/commit/?id=3D72a1cb05cd230ce0d12a7180ae65d= dbba2e0cb6d >>> >>> commit 72a1cb05cd230ce0d12a7180ae65ddbba2e0cb6d >>> Author:=C2=A0 =C2=A0 =C2=A0Li-Wen Hsu >>> AuthorDate: 2022-11-07 03:30:09 +0000 >>> Commit:=C2=A0 =C2=A0 =C2=A0Li-Wen Hsu >>> CommitDate: 2022-11-07 03:30:09 +0000 >>> >>> =C2=A0 =C2=A0 =C2=A0rc(8): Add a zpoolupgrade rc.d script >>> >>> =C2=A0 =C2=A0 =C2=A0If a zpool is created by makefs(8), its version is = 5000, i.e., all >>> =C2=A0 =C2=A0 =C2=A0feature flags are off.=C2=A0 Introduce an rc script= to run `zpool upgrade` >>> =C2=A0 =C2=A0 =C2=A0over the assigned zpools on the first boot.=C2=A0 T= his is useful to the >>> =C2=A0 =C2=A0 =C2=A0ZFS 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 @@ >>> =C2=A0 .\" >>> =C2=A0 .\" $FreeBSD$ >>> =C2=A0 .\" >>> -.Dd August 28, 2022 >>> +.Dd November 7, 2022 >>> =C2=A0 .Dt RC.CONF 5 >>> =C2=A0 .Os >>> =C2=A0 .Sh NAME >>> @@ -2109,6 +2109,13 @@ A space-separated list of ZFS pool names for=C2= =A0 >>> which new pool GUIDs should be >>> =C2=A0 assigned upon first boot. >>> =C2=A0 This is useful when using a ZFS pool copied from a template, suc= h=C2=A0 >>> as a virtual >>> =C2=A0 machine image. >>> +.It Va zpool_upgrade >>> +.Pq Vt str >>> +A space-separated list of ZFS pool names for which version should=C2= =A0 >>> 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=C2= =A0 >> is done. Not so experienced people may assume there is a combination=C2= =A0 >> of zpool upgrade and zfs upgrade (more so for people which do not know= =C2=A0 >> what the difference is). Maybe you want to add some explicit=C2=A0 >> documentation, that zfs upgrade + feature flags needs to be done by=C2= =A0 >> hand. >> >> And this brings me to a second topic, we don't have an explicit list=C2= =A0 >> of features which are supported by the bootloader (I had a look at the= =C2=A0 >> zfs and the boot related man pages, if I overlooked a place, then the=C2= =A0 >> other places should reference this important part with some text). > > =C2=A0 > There is a fixed list of features we support in the boot loader: > =C2=A0 > /* > =C2=A0* List of ZFS features supported for read > =C2=A0*/ > static const char *features_for_read[] =3D { > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "org.illumos:lz4_compress", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.delphix:hole_birth", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.delphix:extensible_dataset", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.delphix:embedded_data", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "org.open-zfs:large_blocks", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "org.illumos:sha512", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "org.illumos:skein", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "org.zfsonlinux:large_dnode", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.joyent:multi_vdev_crash_dump", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.delphix:spacemap_histogram", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.delphix:zpool_checkpoint", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.delphix:spacemap_v2", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.datto:encryption", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.datto:bookmark_v2", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "org.zfsonlinux:allocation_classes", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.datto:resilver_defer", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.delphix:device_removal", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.delphix:obsolete_counts", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.intel:allocation_classes", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "org.freebsd:zstd_compress", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.delphix:bookmark_written", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "com.delphix:head_errlog", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 "org.openzfs:blake3", > =C2=A0 =C2=A0 =C2=A0 =C2=A0 NULL > }; > =C2=A0 > Any feature not on this list will cause the boot loader to=20=20 >=20reject the pool. > =C2=A0 > Whether or not it should do that by default, always, or never is an o= pen > question. I've thought there should be a 'shoot footing'=20=20 >=20override that isn't > there today. > =C2=A0 Thanks for the list. For those interested, it is in =C2=A0 =C2=A0=C2=A0$SRC/stand/libsa/zfs/zfsimpl.c Just to make my opinion expressed before explicit again, this should=20=20 be=20documented in a boot / bootloader related man-page, but isn't. Should the above list be sorted in some way? Maybe in the same order=20=20 as=20the zpool-features lists them (sort by feature name after the=20=20 colon),=20or alphabetical? As Mark already mentioned some flags, I checked the features marked as=20= =20 read=20only (I checked in the zpool-features man page, including the=20=20 dependencies=20documented there) and here are those not listed in=20=20 zfsimpl.c.=20I would assume as they are read-only compatible, we should=20= =20 add=20them: =C2=A0 =C2=A0 com.delphix:async_destroy =C2=A0 =C2=A0 com.delphix:bookmarks =C2=A0 =C2=A0 org.openzfs:device_rebuild =C2=A0 =C2=A0 com.delphix:empty_bpobj =C2=A0 =C2=A0 com.delphix:enable_txg =C2=A0 =C2=A0 com.joyent:filesystem_limits =C2=A0 =C2=A0 com.delphix:livelist =C2=A0 =C2=A0 com.delphix:log_spacemap =C2=A0 =C2=A0 com.zfsonlinux:project_quota =C2=A0 =C2=A0 com.zfsonlinux:userobj_accounting =C2=A0 =C2=A0 com.openzfs:zilsaxattr Bye, Alexander. --=20 http://www.Leidinger.net=20Alexander@Leidinger.net: PGP 0x8F31830F9F2772BF http://www.FreeBSD.org netchild@FreeBSD.org : PGP 0x8F31830F9F2772BF --=_v_NuznhOh5dfXKA4Ei6J_Hb Content-Type: text/html; charset=utf-8 Content-Description: HTML-Nachricht Content-Disposition: inline Content-Transfer-Encoding: quoted-printable

Quoting Warner Losh <imp@bsdimp.com= > (from Mon, 7 Nov 2022 14:23:11 -0700):

 

On Mon, Nov 7, 2022 at 4:15 AM Alexan= der Leidinger <Alexander@leid= inger.net> wrote:


Quoting Li-Wen Hsu <lwhsu@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=3D72a1cb05cd230ce0d12a7180ae65ddbba2e0cb6d >
> commit 72a1cb05cd230ce0d12a7180ae65ddbba2e0cb6d
> Author:     Li-Wen Hsu <lwhsu@FreeBSD.org>
> AuthorDate: 2022-11-07 03:30:09 +0000
> Commit:     Li-Wen Hsu <lwhsu@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.  T= his 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&nb= sp;
> which new pool GUIDs should be
>  assigned upon first boot.
>  This is useful when using a ZFS pool copied from a template, suc= h 
> as a virtual
>  machine image.
> +.It Va zpool_upgrade
> +.Pq Vt str
> +A space-separated list of ZFS pool names for which version should&nbs= p;
> 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 <= br> 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 <= br> 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[] =3D {
        "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 op= en
question. I've thought there should be a 'shoot footing' override that= isn't
there today.
 


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

Just to make my opinion expressed before explicit again, this should be doc= umented 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 alpha= betical?

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 assu= me 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

Bye,
Alexander.

--=_v_NuznhOh5dfXKA4Ei6J_Hb-- --=_thrjPkoMLuD2j4Rj9EGBFwk Content-Type: application/pgp-signature Content-Description: Digitale PGP-Signatur Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIzBAABCAAdFiEER9UlYXp1PSd08nWXEg2wmwP42IYFAmNqJn0ACgkQEg2wmwP4 2IYIbg//R/S8wiUlRnXlGrPhnHJ//rbUpETW6wtjH2zs3nMNPQIys4/dNlR7gtOf F3EniivXAr0BxkDNGdjO+XA2l1l6A7JjQn3g4DTg861yv5rWMpSk8bOMHd8P07N1 jTQWxCEIHnWcuzzjdNfjK5VYKDddcTeLjBfFc9mTnA7KfCTYdc4f5hIypHHY6VQ/ 6pcZ0jZTloih5AHePV/ZIdX4zD8d0MNjxmOW2DdO782qKgQVBVZloZa472j0Nm4X rF+q61t2JzQi5y3OF1aEEKYR7tgUKtWzpgaS+eb0ySixACmev9e/WvI+LvOPNzRG vzG7scWyGJ2/jl61MqNZ0d8y/OWDudP3qM32KgBo8cPY/ZQFOeiLP+Zers75ADW3 IOG3VunB0cZA55PPeWXvNYL+HwKDOkghPJEmikuVCcBb8xh7BCxNcU4rhvsSmTDx WG2pk7TTl0MWsGjL7h+jb+Pb4tKEOTKuYX7Fyy6/9gpHU7RQEqdAG0X56Jv2ufmE qTPParsoOY1dyYMWisLrD0ec4PEE0Co9KHjtp/yHE1mgx8IibqhQG+T4f63z8kpD NSB0HVozb2cFyh3CI3yh4Ew4Ic8wBVGEcK8/0bstleGVdTXhbZDj8RZqA52cPxEa 9QLi6T1gZMukVRXxdr9v4bROyN1jni0BPfodLYvepcWYXe7m914= =KayF -----END PGP SIGNATURE----- --=_thrjPkoMLuD2j4Rj9EGBFwk--