Date: Tue, 08 Nov 2022 10:50:53 +0100 From: Alexander Leidinger <Alexander@leidinger.net> To: Warner Losh <imp@bsdimp.com>, marklmi@yahoo.com Cc: Li-Wen Hsu <lwhsu@freebsd.org>, 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 Message-ID: <20221108105053.Horde.eqgFiBJe2ngGAj6BkXcv5-Z@webmail.leidinger.net> In-Reply-To: <CANCZdfrdc%2BDbv6sDyDLcWNpXnWScEmpUsGu3q8%2BMbZRjDS8eig@mail.gmail.com> References: <202211070339.2A73dJlO027991@gitrepo.freebsd.org> <20221107121514.Horde.nulS9Wg1s3yzAsXXkuJRBa9@webmail.leidinger.net> <CANCZdfrdc%2BDbv6sDyDLcWNpXnWScEmpUsGu3q8%2BMbZRjDS8eig@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <imp@bsdimp.com> (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<Alexander@leidinger.net> wrote: > >> Quoting Li-Wen Hsu <lwhsu@freebsd.org> (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 <lwhsu@FreeBSD.org> >>> AuthorDate: 2022-11-07 03:30:09 +0000 >>> Commit:=C2=A0 =C2=A0 =C2=A0Li-Wen Hsu <lwhsu@FreeBSD.org> >>> 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 <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN" "http://www.w3.org/TR/REC-html40/loose.dtd">; <html> <head> <meta http-equiv=3D"content-type" content=3D"text/html; charset=3Dutf-8"> <title></title> </head> <body style=3D"font-family:Arial;font-size:14px"> <p>Quoting Warner Losh <<a href=3D"mailto:imp@bsdimp.com">imp@bsdimp.com= </a>> (from Mon, 7 Nov 2022 14:23:11 -0700):</p> <blockquote style=3D"border-left:2px solid blue;margin-left:2px;padding-lef= t:12px;" type=3D"cite"> <div dir=3D"ltr"> <div dir=3D"ltr"> </div> <br> <div class=3D"gmail_quote"> <div class=3D"gmail_attr" dir=3D"ltr">On Mon, Nov 7, 2022 at 4:15 AM Alexan= der Leidinger <<a href=3D"mailto:Alexander@leidinger.net">Alexander@leid= inger.net</a>> wrote:</div> <blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-= left:1px solid rgb(204,204,204);padding-left:1ex"> <p><br> Quoting Li-Wen Hsu <<a href=3D"mailto:lwhsu@freebsd.org" target=3D"_blan= k">lwhsu@freebsd.org</a>> (from Mon, 7 Nov 2022 03:39:19 GMT):<br> <br> > The branch main has been updated by lwhsu:<br> ><br> > URL: <br> > <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D72a1cb05cd230ce0d= 12a7180ae65ddbba2e0cb6d" rel=3D"noreferrer" target=3D"_blank">https://cgit.= FreeBSD.org/src/commit/?id=3D72a1cb05cd230ce0d12a7180ae65ddbba2e0cb6d</a><b= r> ><br> > commit 72a1cb05cd230ce0d12a7180ae65ddbba2e0cb6d<br> > Author: Li-Wen Hsu <lwhsu@FreeBSD.org><br> > AuthorDate: 2022-11-07 03:30:09 +0000<br> > Commit: Li-Wen Hsu <lwhsu@FreeBSD.org><br> > CommitDate: 2022-11-07 03:30:09 +0000<br> ><br> > rc(8): Add a zpoolupgrade rc.d script<br> ><br> > If a zpool is created by makefs(8), its version is = 5000, i.e., all<br> > feature flags are off. Introduce an rc script= to run `zpool upgrade`<br> > over the assigned zpools on the first boot. T= his is useful to the<br> > ZFS based VM images built from release(7).<br> <br> > diff --git a/share/man/man5/rc.conf.5 b/share/man/man5/rc.conf.5<br> > index f9ceabc83120..43fa44a5f1cb 100644<br> > --- a/share/man/man5/rc.conf.5<br> > +++ b/share/man/man5/rc.conf.5<br> > @@ -24,7 +24,7 @@<br> > .\"<br> > .\" $FreeBSD$<br> > .\"<br> > -.Dd August 28, 2022<br> > +.Dd November 7, 2022<br> > .Dt RC.CONF 5<br> > .Os<br> > .Sh NAME<br> > @@ -2109,6 +2109,13 @@ A space-separated list of ZFS pool names for&nb= sp;<br> > which new pool GUIDs should be<br> > assigned upon first boot.<br> > This is useful when using a ZFS pool copied from a template, suc= h <br> > as a virtual<br> > machine image.<br> > +.It Va zpool_upgrade<br> > +.Pq Vt str<br> > +A space-separated list of ZFS pool names for which version should&nbs= p;<br> > be upgraded<br> > +upon first boot.<br> > +This is useful when using a ZFS pool generated by<br> > +.Xr makefs 8<br> > +utility.<br> <br> For someone who knows ZFS well, it is clear that only a zpool upgrade = <br> 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 = ;<br> what the difference is). Maybe you want to add some explicit <br> documentation, that zfs upgrade + feature flags needs to be done by <b= r> hand.<br> <br> 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 = ;<br> zfs and the boot related man pages, if I overlooked a place, then the = <br> other places should reference this important part with some text).</p> </blockquote> <div> </div> <div>There is a fixed list of features we support in the boot loader:</div> <div> </div> <div>/*<br> * List of ZFS features supported for read<br> */<br> static const char *features_for_read[] =3D {<br> "org.illumos:lz4_compress",<br> "com.delphix:hole_birth",<br> "com.delphix:extensible_dataset",<br> "com.delphix:embedded_data",<br> "org.open-zfs:large_blocks",<br> "org.illumos:sha512",<br> "org.illumos:skein",<br> "org.zfsonlinux:large_dnode",<br> "com.joyent:multi_vdev_crash_dump",<br> "com.delphix:spacemap_histogram",<br> "com.delphix:zpool_checkpoint",<br> "com.delphix:spacemap_v2",<br> "com.datto:encryption",<br> "com.datto:bookmark_v2",<br> "org.zfsonlinux:allocation_classes",<br> "com.datto:resilver_defer",<br> "com.delphix:device_removal",<br> "com.delphix:obsolete_counts",<br> "com.intel:allocation_classes",<br> "org.freebsd:zstd_compress",<br> "com.delphix:bookmark_written",<br> "com.delphix:head_errlog",<br> "org.openzfs:blake3",<br> NULL<br> };</div> <div> </div> <div>Any feature not on this list will cause the boot loader to reject the = pool.</div> <div> </div> <div>Whether or not it should do that by default, always, or never is an op= en</div> <div>question. I've thought there should be a 'shoot footing' override that= isn't</div> <div>there today.</div> <div> </div> </div> </div> </blockquote> <p><br> Thanks for the list. For those interested, it is in<br> $SRC/stand/libsa/zfs/zfsimpl.c<br> <br> Just to make my opinion expressed before explicit again, this should be doc= umented in a boot / bootloader related man-page, but isn't.<br> <br> 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?<br> <br> 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:<br> com.delphix:async_destroy<br> com.delphix:bookmarks<br> org.openzfs:device_rebuild<br> com.delphix:empty_bpobj<br> com.delphix:enable_txg<br> com.joyent:filesystem_limits<br> com.delphix:livelist<br> com.delphix:log_spacemap<br> com.zfsonlinux:project_quota<br> com.zfsonlinux:userobj_accounting<br> com.openzfs:zilsaxattr<br> <br> Bye,<br> Alexander.<br> <br></p> <div><a href=3D"http://www.Leidinger.net" target=3D"_blank">http://www.Leid= inger.net</a> <a href=3D"mailto:Alexander@Leidinger.net">Alexander@Leidinge= r.net</a>: PGP 0x8F31830F9F2772BF<br> <a href=3D"http://www.FreeBSD.org" target=3D"_blank">http://www.FreeBSD.org= </a> <a href=3D"mailto:netchild@FreeBSD.org">netchild@FreeBSD.= org</a> : PGP 0x8F31830F9F2772BF</div> </body> </html> --=_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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20221108105053.Horde.eqgFiBJe2ngGAj6BkXcv5-Z>