Date: Mon, 18 Sep 2023 20:39:29 -0700 From: Mark Millard <marklmi@yahoo.com> To: Alexander Motin <mav@FreeBSD.org> Cc: Current FreeBSD <freebsd-current@freebsd.org>, Warner Losh <imp@bsdimp.com>, Glen Barber <gjb@FreeBSD.org> Subject: Re: vfs.zfs.bclone_enabled (was: FreeBSD 14.0-BETA2 Now Available) [block_cloning and zilsaxattr missing from loader's features_for_read] Message-ID: <12D5CD6A-34ED-47E2-B43E-37AD5582206C@yahoo.com> In-Reply-To: <d0a85848-934d-9875-ebc6-2d34f0c60b9d@FreeBSD.org> References: <5C769ACC-F264-4BAB-AF7B-8C463A4BD99E@yahoo.com> <FBD2A7C3-E1F5-4B4A-B2A7-2268DCE2BAE5@yahoo.com> <d0a85848-934d-9875-ebc6-2d34f0c60b9d@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sep 18, 2023, at 17:05, Alexander Motin <mav@FreeBSD.org> wrote:
> On 18.09.2023 19:21, Mark Millard wrote:
>> On Sep 18, 2023, at 15:51, Mark Millard <marklmi@yahoo.com> wrote:
>>> Alexander Motin <mav_at_FreeBSD.org> wrote on
>>> Date: Mon, 18 Sep 2023 13:26:56 UTC :
>>>> block_cloning feature is marked as READONLY_COMPAT. It should not
>>>> require any special handling from the boot code.
>>>=20
>>> =46rom stand/libsa/zfs/zfsimpl.c but adding a comment about the
>>> read-only compatibility status of each entry:
>>>=20
>>> /*
>>> * List of ZFS features supported for read
>>> */
>> static const char *features_for_read[] =3D {
>>> "com.datto:bookmark_v2", // READ-ONLY COMPATIBLE no
>>> "com.datto:encryption", // READ-ONLY COMPATIBLE no
>>> "com.datto:resilver_defer", // READ-ONLY COMPATIBLE yes
>>> "com.delphix:bookmark_written", // READ-ONLY COMPATIBLE no
>>> "com.delphix:device_removal", // READ-ONLY COMPATIBLE no
>>> "com.delphix:embedded_data", // READ-ONLY COMPATIBLE no
>>> "com.delphix:extensible_dataset", // READ-ONLY COMPATIBLE no
>>> "com.delphix:head_errlog", // READ-ONLY COMPATIBLE no
>>> "com.delphix:hole_birth", // READ-ONLY COMPATIBLE no
>>> "com.delphix:obsolete_counts", // READ-ONLY COMPATIBLE yes
>>> "com.delphix:spacemap_histogram", // READ-ONLY COMPATIBLE yes
>>> "com.delphix:spacemap_v2", // READ-ONLY COMPATIBLE yes
>>> "com.delphix:zpool_checkpoint", // READ-ONLY COMPATIBLE yes
>>> "com.intel:allocation_classes", // READ-ONLY COMPATIBLE yes
>>> "com.joyent:multi_vdev_crash_dump", // READ-ONLY COMPATIBLE =
no
>>> "com.klarasystems:vdev_zaps_v2", // READ-ONLY COMPATIBLE no
>>> "org.freebsd:zstd_compress", // READ-ONLY COMPATIBLE no
>>> "org.illumos:lz4_compress", // READ-ONLY COMPATIBLE no
>>> "org.illumos:sha512", // READ-ONLY COMPATIBLE no
>>> "org.illumos:skein", // READ-ONLY COMPATIBLE no
>>> "org.open-zfs:large_blocks", // READ-ONLY COMPATIBLE no
>>> "org.openzfs:blake3", // READ-ONLY COMPATIBLE no
>>> "org.zfsonlinux:allocation_classes", // READ-ONLY COMPATIBLE =
yes
>>> "org.zfsonlinux:large_dnode", // READ-ONLY COMPATIBLE no
>>> NULL
>>> };
>>>=20
>>> So it appears that the design is that both "no" and "yes" ones
>>> that are known to be supported are listed and anything else is
>>> supposed to lead to rejection until explicitly added as
>>> known-compatibile.
>=20
> I don't think so. I think somebody by mistake added first featured =
that should not be here, and then others continued this irrelevant =
routine. My own development server/builder is happily running latest =
main with ZFS root without any patches and with block cloning not only =
enabled, but even active. So as I have told, it is not needed:
>=20
> mav@srv:/root# zpool get all | grep clon
> mavlab bcloneused 20.5M =
-
> mavlab bclonesaved 20.9M =
-
> mavlab bcloneratio 2.02x =
-
> mavlab feature@block_cloning active =
local
>=20
> Somebody should go through the list and clean in up from =
read-compatible features and document it, unless there are some features =
that were re-qualified at some point, I haven't checked if it could be.
>=20
>>> This matches up with stand/libsa/zfs/zfsimpl.c 's:
>>>=20
>>> static int
>>> nvlist_check_features_for_read(nvlist_t *nvl)
>>> {
> ...
>>> rc =3D nvlist_find(nvl, ZPOOL_CONFIG_FEATURES_FOR_READ,
>>> DATA_TYPE_NVLIST, NULL, &features, NULL);
>=20
> Take a note it reads ZPOOL_CONFIG_FEATURES_FOR_READ. Same time =
features declared as READONLY_COMPAT are stored in FEATURES_FOR_WRITE, =
that boot loader does not even care.
>=20
>>> I do not know if vfs.zfs.bclone_enabled=3D0 leads the loader
>>> to see vs. not-see a "com.fudosecurity:block_cloning".
>=20
> bclone_enabled=3D0 block copy_file_range() usage, that should keep the =
feature enabled, but not active. It could be related if the feature =
would be in FEATURES_FOR_WRITE, but here and now it is not.
>=20
>> It appears that 2 additions afeter opebzfas-2.1-freebsd are
>> missing from the above list:
>> com.fudosecurity:block_cloning
>> org.openzfs:zilsaxattr
>=20
> Nothing of ZIL is required for read-only import. So no, it is also =
not needed.
Thanks for the details in your notes, including that bclone_enabled=3D0
is not relevant.
As a result I found out about zhack feature stat POOLNAME that shows
the ZPOOL_CONFIG_FEATURES_FOR_READ separately from then ones for
write.
Looking at the pool that I used for testing block_cloning
via poudriere bulk build activity (and sorting the for_*_obj
lists produced) . . .
The for_read_obj list (with matching line added as a suffix):
com.datto:bookmark_v2 =3D 0 "com.datto:bookmark_v2", =
// READ-ONLY COMPATIBLE no
com.datto:encryption =3D 0 "com.datto:encryption", =
// READ-ONLY COMPATIBLE no
com.delphix:bookmark_written =3D 0 =
"com.delphix:bookmark_written", // READ-ONLY COMPATIBLE no
com.delphix:device_removal =3D 0 =
"com.delphix:device_removal", // READ-ONLY COMPATIBLE no
com.delphix:embedded_data =3D 1 =
"com.delphix:embedded_data", // READ-ONLY COMPATIBLE no
com.delphix:extensible_dataset =3D 87 =
"com.delphix:extensible_dataset", // READ-ONLY COMPATIBLE no
com.delphix:head_errlog =3D 1 =
"com.delphix:head_errlog", // READ-ONLY COMPATIBLE no
com.delphix:hole_birth =3D 1 =
"com.delphix:hole_birth", // READ-ONLY COMPATIBLE no
com.delphix:redacted_datasets =3D 0
com.delphix:redaction_bookmarks =3D 0
com.joyent:multi_vdev_crash_dump =3D 0 =
"com.joyent:multi_vdev_crash_dump", // READ-ONLY COMPATIBLE no
com.klarasystems:vdev_zaps_v2 =3D 1 =
"com.klarasystems:vdev_zaps_v2", // READ-ONLY COMPATIBLE no
org.freebsd:zstd_compress =3D 0 =
"org.freebsd:zstd_compress", // READ-ONLY COMPATIBLE no
org.illumos:edonr =3D 0
org.illumos:lz4_compress =3D 1 =
"org.illumos:lz4_compress", // READ-ONLY COMPATIBLE no
org.illumos:sha512 =3D 0 =
"org.illumos:sha512", // READ-ONLY COMPATIBLE no
org.illumos:skein =3D 0 "org.illumos:skein", // =
READ-ONLY COMPATIBLE no
org.open-zfs:large_blocks =3D 0 =
"org.open-zfs:large_blocks", // READ-ONLY COMPATIBLE no
org.openzfs:blake3 =3D 0 =
"org.openzfs:blake3", // READ-ONLY COMPATIBLE no
org.openzfs:draid =3D 0
org.zfsonlinux:large_dnode =3D 0 =
"org.zfsonlinux:large_dnode", // READ-ONLY COMPATIBLE no
I'll note that the following of the no-match examples
are listed in openzfs-2.2 :
redacted_datasets
redaction_bookmarks
edonr
draid
In fact, only edonr is new in openzfs-2.2 compared to
openzfs-2.1-freebsd .
So it appears that the 4 are missing from at least features_for_read
but might be missing what is required to have them supported if
something else is also needed. (The "=3D 0" might explain the lack of
a complaint from the loader? Or is there more classification that I
do not understand?)
The for_write_obj list:
com.datto:resilver_defer =3D 0 =
"com.datto:resilver_defer", // READ-ONLY COMPATIBLE yes
com.delphix:async_destroy =3D 0
com.delphix:bookmarks =3D 0
com.delphix:empty_bpobj =3D 44
com.delphix:enabled_txg =3D 34
com.delphix:livelist =3D 0
com.delphix:log_spacemap =3D 1
com.delphix:obsolete_counts =3D 0 =
"com.delphix:obsolete_counts", // READ-ONLY COMPATIBLE yes
com.delphix:spacemap_histogram =3D 110 =
"com.delphix:spacemap_histogram", // READ-ONLY COMPATIBLE yes
com.delphix:spacemap_v2 =3D 1 =
"com.delphix:spacemap_v2", // READ-ONLY COMPATIBLE yes
com.delphix:zpool_checkpoint =3D 0 =
"com.delphix:zpool_checkpoint", // READ-ONLY COMPATIBLE yes
com.fudosecurity:block_cloning =3D 0
com.joyent:filesystem_limits =3D 0
org.openzfs:device_rebuild =3D 0
org.openzfs:zilsaxattr =3D 3
org.zfsonlinux:allocation_classes =3D 0
org.zfsonlinux:project_quota =3D 44
org.zfsonlinux:userobj_accounting =3D 44
So: All those "READ-ONLY COMPATIBLE yes" examples do not
need to be listed in features_for_read .
The following have no matches in either for_*_obj list
that zhack produced:
"com.intel:allocation_classes", // READ-ONLY COMPATIBLE yes
"org.zfsonlinux:allocation_classes", // READ-ONLY COMPATIBLE yes
I'll note that openzfs-2.2 and openzfs-2.1-freebsd both list:
allocation_classes
But the "READ-ONLY COMPATIBLE yes" status suggests this is not
a loader functional problem but may be a waste: all the
"READ-ONLY COMPATIBLE yes" could be removed from
features_for_read .
Thanks again.
=3D=3D=3D
Mark Millard
marklmi at yahoo.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?12D5CD6A-34ED-47E2-B43E-37AD5582206C>