Re: vfs.zfs.bclone_enabled (was: FreeBSD 14.0-BETA2 Now Available)

On 9/18/23 16:37, Tomoaki AOKI wrote:
> At least, if I read the code correctly,
>      "com.fudosecurity:block_cloning",
> should be added to array
>      *features_for_read[]
> of stand/libsa/zfs/zfsimpl.c.
> 
> There are check codes like below, so without it, boot codes would
> reject to boot from any pool having block_cloning feature enabled.
> Am I missing something?
> 
>> 		for (i = 0; features_for_read[i] != NULL; i++) {
>> 			if (memcmp(nvp_name->nv_data,
> features_for_read[i], nvp_name->nv_size) == 0) {
>> 				found = 1;
>> 				break;
>> 			}
>> 		}
>>
>> 		if (!found) {
>> 			printf("ZFS: unsupported feature: %.*s\n",
>> 			    nvp_name->nv_size, nvp_name->nv_data);
>> 			rc = EIO;
>> 		}
> 
> Regards.
> 

I'm pretty sure what he's trying to tell you is that this feature won't 
show up in the list to be compared against this in the first place. 
Presumably the inquiry it uses either just looks for `ZFEATURE_FLAG_MOS` 
or filters out `ZFEATURE_FLAG_READONLY_COMPAT`, which is described as:

  90         /* Can open pool readonly even if this feature is not 
supported. */

Thanks,

Kyle Evans

> 
> On Mon, 18 Sep 2023 09:26:56 -0400
> Alexander Motin <mav@FreeBSD.org> wrote:
> 
>> block_cloning feature is marked as READONLY_COMPAT.  It should not
>> require any special handling from the boot code.
>>
>> On 18.09.2023 07:22, Tomoaki AOKI wrote:
>>> Really OK?
>>>
>>> I cannot find block_cloning in array *features_for_read[] of
>>> stand/libsa/zfs/zfsimpl.c, which possibly mean boot codes (including
>>> loader) cannot boot from Root-on-ZFS pool having block_cloning active.
>>>
>>> Not sure adding '"com.fudosecurity:block_cloning",' here is sufficient
>>> or not. Possibly more works are needed.
>>>
>>> IMHO, all default-enabled features should be safe for booting.
>>> Implement features with disalded, impement boot codes to support them,
>>> then finally enable them by default should be the only valid route.
>>>
>>>
>>> [1] https://cgit.freebsd.org/src/tree/stand/libsa/zfs/zfsimpl.c
>>>
>>>
>>> On Mon, 18 Sep 2023 07:31:46 +0200
>>> Martin Matuska <mm@FreeBSD.org> wrote:
>>>
>>>> I vote for enabling block cloning on main :-)
>>>>
>>>> mm
>>>>
>>>> On 16. 9. 2023 19:14, Alexander Motin wrote:
>>>>> On 16.09.2023 01:25, Graham Perrin wrote:
>>>>>> On 16/09/2023 01:28, Glen Barber wrote:
>>>>>>> o A fix for the ZFS block_cloning feature has been implemented.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> I see
>>>>>> <https://github.com/openzfs/zfs/commit/5cc1876f14f90430b24f1ad2f231de936691940f>,
>>>>>> with
>>>>>> <https://github.com/freebsd/freebsd-src/commit/9dcf00aa404bb62052433c45aaa5475e2760f5ed>
>>>>>> in stable/14.
>>>>>>
>>>>>> As vfs.zfs.bclone_enabled is still 0 (at least, with 15.0-CURRENT
>>>>>> n265350-72d97e1dd9cc): should we assume that additional fixes, not
>>>>>> necessarily in time for 14.0-RELEASE, will be required before
>>>>>> vfs.zfs.bclone_enabled can default to 1?
>>>>>
>>>>> I am not aware of any block cloning issues now.  All this thread about
>>>>> bclone_enabled actually started after I asked why it is still
>>>>> disabled. Thanks to Mark Millard for spotting this issue I could fix,
>>>>> but now we are back at the point of re-enabling it again.  Since the
>>>>> tunable does not even exist anywhere outside of FreeBSD base tree, I'd
>>>>> propose to give this code another try here too.  I see no point to
>>>>> have it disabled at least in main unless somebody needs time to run
>>>>> some specific tests first.
>>>
>>
>> -- 
>> Alexander Motin
>