Date: Mon, 18 Sep 2023 20:05:37 -0400 From: Alexander Motin <mav@FreeBSD.org> To: Mark Millard <marklmi@yahoo.com>, Current FreeBSD <freebsd-current@freebsd.org>, Warner Losh <imp@bsdimp.com> 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: <d0a85848-934d-9875-ebc6-2d34f0c60b9d@FreeBSD.org> In-Reply-To: <FBD2A7C3-E1F5-4B4A-B2A7-2268DCE2BAE5@yahoo.com> References: <5C769ACC-F264-4BAB-AF7B-8C463A4BD99E@yahoo.com> <FBD2A7C3-E1F5-4B4A-B2A7-2268DCE2BAE5@yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
--
Alexander Motin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?d0a85848-934d-9875-ebc6-2d34f0c60b9d>