Date: Wed, 09 Nov 2022 13:46:10 +0100 From: Alexander Leidinger <Alexander@leidinger.net> To: Warner Losh <imp@bsdimp.com>, marklmi@yahoo.com, tsoome@freebsd.org Cc: Li-Wen Hsu <lwhsu@freebsd.org>, current@freebsd.org Subject: changes to the zfs boot (was: Re: git: 72a1cb05cd23 - main - rc(8): Add a zpoolupgrade rc.d script) Message-ID: <20221109134610.Horde.JB7ibQTWprHbmIUfhg7JY7f@webmail.leidinger.net> In-Reply-To: <20221108105053.Horde.eqgFiBJe2ngGAj6BkXcv5-Z@webmail.leidinger.net> References: <202211070339.2A73dJlO027991@gitrepo.freebsd.org> <20221107121514.Horde.nulS9Wg1s3yzAsXXkuJRBa9@webmail.leidinger.net> <CANCZdfrdc%2BDbv6sDyDLcWNpXnWScEmpUsGu3q8%2BMbZRjDS8eig@mail.gmail.com> <20221108105053.Horde.eqgFiBJe2ngGAj6BkXcv5-Z@webmail.leidinger.net>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format and has been PGP signed. --=_bMpPAB6BU7wUH7sU0BGV0-m Content-Type: text/plain; charset=utf-8; format=flowed; DelSp=Yes Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Quoting Alexander Leidinger <Alexander@leidinger.net> (from Tue, 08=20=20 Nov=202022 10:50:53 +0100): > 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 G= MT): >>> >>>> The branch main has been updated by lwhsu: >>>> >>>> URL:=C2=A0 >>>> https://cgit.FreeBSD.org/src/commit/?id=3D72a1cb05cd230ce0d12a7180ae65= ddbba2e0cb6d >>>> >>>> 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 scrip= t to run `zpool upgrade` >>>> =C2=A0 =C2=A0 =C2=A0over the assigned zpools on the first boot.=C2=A0 = This 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, su= ch=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 >=20be 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=20= =20 >=20as the zpool-features lists them (sort by feature name after the=20=20 >=20colon), 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[] =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: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=20=20 >=20as read only (I checked in the zpool-features man page, including=20=20 >=20the dependencies documented there) and here are those not listed in=20= =20 >=20zfsimpl.c. I would assume as they are read-only compatible, we=20=20 >=20should add them: > =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 If my understanding is correct that the read-only compatible parts=20=20 (according=20to the zpool-features man page) are safe to add to the zfs=20= =20 boot,=20here is what I have only build-tested (relative to the above=20=20 alphabetical=20sorting): ---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=20=20 correct=20and would sign-off on a "reviewed by" level? Bye, Alexander. --=20 http://www.Leidinger.net=20Alexander@Leidinger.net: PGP 0x8F31830F9F2772BF http://www.FreeBSD.org netchild@FreeBSD.org : PGP 0x8F31830F9F2772BF --=_bMpPAB6BU7wUH7sU0BGV0-m Content-Type: application/pgp-signature Content-Description: Digitale PGP-Signatur Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIzBAABCAAdFiEER9UlYXp1PSd08nWXEg2wmwP42IYFAmNroRIACgkQEg2wmwP4 2IatPQ/9Ec9Cl+RsVO21ljwV/Ed0s1Wr33NjX8xYenpAnVOQ8CritxolkJYuTkUP 0ChSzMx7Pjjih2drT3X8UCtRLlAh5TpQBef1lu8DicurVbCMCu/K8C5RJiaHiS6G u6KmZR2oGNClh0tyy34i98DM39c7RBYyKk/bCZhHJMZ6HnGqytTwaCeOoRG2SAJM RAjFlq5/V0pzs6prGwCEcEXhHrdAbSw5ed/9UIKv7ulFIv73GK5g+9HB+SHbnHTJ tDPYtfVh0MmNqpfvOVnG1QbQ2JTNOj3nON3LLx46+kbIG0oDR/wI6zMwQPsmZvEV LjvEZz3w8u+4Ca4AZv6WWCAdt1I/5at1byK1R6zSXdonfBuL7CcOKlt/jX3o++Ps ppVu9RVfP3Tg6zGfMx0S7G43TzQzligXfUyvM8KMj3uzc3bpgK5TJJYNPTv+Pu+U ur2IUAR9/AP9YwI/wSXtjckWGd586A8nf5QqcLW5DzxrO0ldc4p5nZQQNcJqJabx 9GCp618YqOfM6vKcCgFHyZ6DrqvaVen8uNIRSnO/PwWeD0KmxOIU6uxwfAlnvhzP CPGnVi8KhkhUhKDqkWE0dpUdHlKVvwZ1l4NUIAQEe9OoAXnQOfZHwShy2mBMbJQI GQBaKo2XLGjhzeKzhGMqdZdDYfCqoeS+m0P2jyc4xoP6C9FRO+8= =sKQf -----END PGP SIGNATURE----- --=_bMpPAB6BU7wUH7sU0BGV0-m--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20221109134610.Horde.JB7ibQTWprHbmIUfhg7JY7f>