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

From: Tomoaki AOKI <junchoon_at_dec.sakura.ne.jp>
Date: Mon, 18 Sep 2023 21:37:20 UTC
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.


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

-- 
Tomoaki AOKI    <junchoon@dec.sakura.ne.jp>