On Sep 18, 2023, at 17:05, Alexander Motin <m...@freebsd.org> wrote:
> On 18.09.2023 19:21, Mark Millard wrote:
>> On Sep 18, 2023, at 15:51, Mark Millard <mark...@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.
>>>
>>> From stand/libsa/zfs/zfsimpl.c but adding a comment about the
>>> read-only compatibility status of each entry:
>>>
>>> /*
>>> * List of ZFS features supported for read
>>> */
>> static const char *features_for_read[] = {
>>> "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
>>> };
>>>
>>> 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.
>
> 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:
>
> 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
>
> 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.
>
>>> This matches up with stand/libsa/zfs/zfsimpl.c 's:
>>>
>>> static int
>>> nvlist_check_features_for_read(nvlist_t *nvl)
>>> {
> ...
>>> rc = nvlist_find(nvl, ZPOOL_CONFIG_FEATURES_FOR_READ,
>>> DATA_TYPE_NVLIST, NULL, &features, NULL);
>
> 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.
>
>>> I do not know if vfs.zfs.bclone_enabled=0 leads the loader
>>> to see vs. not-see a "com.fudosecurity:block_cloning".
>
> bclone_enabled=0 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.
>
>> It appears that 2 additions afeter opebzfas-2.1-freebsd are
>> missing from the above list:
>> com.fudosecurity:block_cloning
>> org.openzfs:zilsaxattr
>
> 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=0
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 = 0 "com.datto:bookmark_v2", //
READ-ONLY COMPATIBLE no
com.datto:encryption = 0 "com.datto:encryption", //
READ-ONLY COMPATIBLE no
com.delphix:bookmark_written = 0 "com.delphix:bookmark_written",
// READ-ONLY COMPATIBLE no
com.delphix:device_removal = 0 "com.delphix:device_removal",
// READ-ONLY COMPATIBLE no
com.delphix:embedded_data = 1 "com.delphix:embedded_data", //
READ-ONLY COMPATIBLE no
com.delphix:extensible_dataset = 87
"com.delphix:extensible_dataset", // READ-ONLY COMPATIBLE no
com.delphix:head_errlog = 1 "com.delphix:head_errlog", //
READ-ONLY COMPATIBLE no
com.delphix:hole_birth = 1 "com.delphix:hole_birth", //
READ-ONLY COMPATIBLE no
com.delphix:redacted_datasets = 0
com.delphix:redaction_bookmarks = 0
com.joyent:multi_vdev_crash_dump = 0
"com.joyent:multi_vdev_crash_dump", // READ-ONLY COMPATIBLE no
com.klarasystems:vdev_zaps_v2 = 1
"com.klarasystems:vdev_zaps_v2", // READ-ONLY COMPATIBLE no
org.freebsd:zstd_compress = 0 "org.freebsd:zstd_compress", //
READ-ONLY COMPATIBLE no
org.illumos:edonr = 0
org.illumos:lz4_compress = 1 "org.illumos:lz4_compress", //
READ-ONLY COMPATIBLE no
org.illumos:sha512 = 0 "org.illumos:sha512", //
READ-ONLY COMPATIBLE no
org.illumos:skein = 0 "org.illumos:skein", //
READ-ONLY COMPATIBLE no
org.open-zfs:large_blocks = 0 "org.open-zfs:large_blocks", //
READ-ONLY COMPATIBLE no
org.openzfs:blake3 = 0 "org.openzfs:blake3", //
READ-ONLY COMPATIBLE no
org.openzfs:draid = 0
org.zfsonlinux:large_dnode = 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 "= 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 = 0 "com.datto:resilver_defer", //
READ-ONLY COMPATIBLE yes
com.delphix:async_destroy = 0
com.delphix:bookmarks = 0
com.delphix:empty_bpobj = 44
com.delphix:enabled_txg = 34
com.delphix:livelist = 0
com.delphix:log_spacemap = 1
com.delphix:obsolete_counts = 0 "com.delphix:obsolete_counts",
// READ-ONLY COMPATIBLE yes
com.delphix:spacemap_histogram = 110
"com.delphix:spacemap_histogram", // READ-ONLY COMPATIBLE yes
com.delphix:spacemap_v2 = 1 "com.delphix:spacemap_v2", //
READ-ONLY COMPATIBLE yes
com.delphix:zpool_checkpoint = 0 "com.delphix:zpool_checkpoint",
// READ-ONLY COMPATIBLE yes
com.fudosecurity:block_cloning = 0
com.joyent:filesystem_limits = 0
org.openzfs:device_rebuild = 0
org.openzfs:zilsaxattr = 3
org.zfsonlinux:allocation_classes = 0
org.zfsonlinux:project_quota = 44
org.zfsonlinux:userobj_accounting = 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.
===
Mark Millard
marklmi at yahoo.com
