Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 09 Nov 2022 20:39:49 +0100
From:      Alexander Leidinger <Alexander@leidinger.net>
To:        Warner Losh <imp@bsdimp.com>
Cc:        marklmi@yahoo.com, tsoome@freebsd.org, Li-Wen Hsu <lwhsu@freebsd.org>, current@freebsd.org
Subject:   Re: changes to the zfs boot (was: Re: git: 72a1cb05cd23 - main - rc(8): Add a zpoolupgrade rc.d script)
Message-ID:  <20221109203949.Horde.qIUauGgF-S3rZjw2-w-VIZT@webmail.leidinger.net>
In-Reply-To: <CANCZdfptmipq%2BsS0AQ1%2B7EmLT-7YdKv8s%2BnCV7ON1Qy6-C6N9A@mail.gmail.com>
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> <20221109134610.Horde.JB7ibQTWprHbmIUfhg7JY7f@webmail.leidinger.net> <CANCZdfptmipq%2BsS0AQ1%2B7EmLT-7YdKv8s%2BnCV7ON1Qy6-C6N9A@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.

--=_iRyGHLeLeeS0LMDvtdHFkCg
Content-Type: text/plain; charset=utf-8; format=flowed; DelSp=Yes
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Quoting Warner Losh <imp@bsdimp.com> (from Wed, 9 Nov 2022 08:54:33 -0700):

> On Wed, Nov 9, 2022 at 5:46 AM Alexander Leidinger <Alexander@leidinger.n=
et>
> wrote:
>
>> Quoting Alexander Leidinger <Alexander@leidinger.net> (from Tue, 08
>> Nov 2022 10:50:53 +0100):

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

[diff of feature-sorting]

>
> This patch looks good because it's a nop and just tidies things up a bit.
>
> Reviewed by: imp

Will do later.

>> > 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?
>>
>
> I'm inclined to strongly NAK this patch, absent some way to test it.
> There's no issues today with any of them being absent causing
> problems on boot that have been reported. The ZFS that's in the
> boot loader is a reduced copy of what's in base and not everything is
> supported. There's no urgency here to rush into this. The ones that
> are on the list already are for things that we know we support in the
> boot loader because we've gone to the trouble to put blake3 or sha512
> into it (note: Not all boot loaders will support all ZFS features in the
> future... x86 BIOS booting likely is going to have to be frozen at its
> current ZFS feature set due to code size issues).
>
> While most of these options look OK on the surface, I'd feel a lot better
> if there were tests for these to prove they work. I'd also feel better if
> the ZFS experts could explain how those come to be set on a zpool
> as well. I'd settle for a good script that could be run as root (better
> would be not as root) that would take a filesystem that was created
> by makefs -t zfs and turn on these features after an zpool upgrade.
> I have the vague outlines of a test suite for the boot loader that I
> could see about integrating something like that into, but most of my
> time these days is chasing after 'the last bug' in some kboot stuff I'm
> working on (which includes issues with our ZFS in the boot loader
> integration).
>
> So not a hard no, but I plea for additional scripts to create images
> that can be tested.

I didn't want to commit untested or unverified stuff. I fully agree=20=20
with=20your reasoning.

Bye,
Alexander.

--=20
http://www.Leidinger.net=20Alexander@Leidinger.net: PGP 0x8F31830F9F2772BF
http://www.FreeBSD.org    netchild@FreeBSD.org  : PGP 0x8F31830F9F2772BF

--=_iRyGHLeLeeS0LMDvtdHFkCg
Content-Type: application/pgp-signature
Content-Description: Digitale PGP-Signatur
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIzBAABCAAdFiEER9UlYXp1PSd08nWXEg2wmwP42IYFAmNsAgQACgkQEg2wmwP4
2IZnLg//U34KrUAmwXUtGme8g9fJAZgrXMVotbGsyQVGRZPjKSGDh4W+pc38K2TW
FUMV/h/WAbUdhtv6tRZAJmJ1mkrP01RmfRMSsea6L9hBoO5E+0QZ/32EMTBbHYtT
awgrbQXgF9TGRKonQgxvTzBCo9NQcdXHz5V9OLl8SlgSwLKbjyVyKRyWfG6ysaF/
9vZ97VZ9OOa6NRUzWCZ/R5ZDILYuLPV8fUpTFpLQ5tOPY41KpJd/KE9qc9HZw61y
jWwbHlNdhCs5kvwLiJqoZ99z7wisSGjXqAdQHBkarNct3I2TPD5AthClJ7ffwUP0
WQ0wVtOMemtTDeo2qnX4I7wH7XWXtPuVgaKQcWsr39GVIq8KjY8ctlUMh2+5GqGW
aUvkat+W2/wEO8e702k+Gd528z1eluQScYyNIzZLPnHGlFw+qwz44Lw7ETA5cxLc
C4l2aIsBQ5b8wA5MFlvvrhHsc0vnz671kIiwqqObfJvXBMSwFCgH5Yhi1pAvD0DF
l6m+JVm5SS7eRe7+/yj5FtVGJxXpcIHggMazmCacREtSE1VYrYYHBTDCrcjTnET2
3FWBTbITBWQGq3hO2cxJPs8MxkMh7PFCrOd1YUThav3lHwKSl2G7MhVqtAMopQYs
lClJjrVd11Tvt7O08O11/oc7UdiMX2k+3aW+VoexVfsGH0R1hVo=
=y0JE
-----END PGP SIGNATURE-----

--=_iRyGHLeLeeS0LMDvtdHFkCg--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20221109203949.Horde.qIUauGgF-S3rZjw2-w-VIZT>