Re: vfs.zfs.bclone_enabled (was: FreeBSD 14.0-BETA2 Now Available) [block_cloning and zilsaxattr missing from loader's features_for_read]

From: Mark Millard <marklmi_at_yahoo.com>
Date: Tue, 19 Sep 2023 03:39:29 UTC
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.
>>> 
>>> 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