Re: another crash and going forward with zfs

In message <b57b06bd-7e73-ae2d-2fba-bd226883ff34@dawidek.net>, Pawel Jakub 
Dawi
dek writes:
> On 4/18/23 05:14, Mateusz Guzik wrote:
> > On 4/17/23, Pawel Jakub Dawidek <pjd@freebsd.org> wrote:
> >> Correct me if I'm wrong, but from my understanding there were zero
> >> problems with block cloning when it wasn't in use or now disabled.
> >>
> >> The reason I've introduced vfs.zfs.bclone_enabled sysctl, was to exactly
> >> avoid mess like this and give us more time to sort all the problems out
> >> while making it easy for people to try it.
> >>
> >> If there is no plan to revert the whole import, I don't see what value
> >> removing just block cloning will bring if it is now disabled by default
> >> and didn't cause any problems when disabled.
> >>
> > 
> > The feature definitely was not properly stress tested and what not and
> > trying to do it keeps running into panics. Given the complexity of the
> > feature I would expect there are many bug lurking, some of which
> > possibly related to the on disk format. Not having to deal with any of
> > this is can be arranged as described above and is imo the most
> > sensible route given the timeline for 14.0
>
> Block cloning doesn't create, remove or modify any on-disk data until it 
> is in use.
>
> Again, if we are not going to revert the whole merge, I see no point in 
> reverting block cloning as until it is enabled, its code is not 
> executed. This allow people who upgraded the pools to do nothing special 
> and it will allow people to test it easily.

In this case zpool upgrade and zpool status should return no feature 
upgrades are available instead of enticing users to zpool upgrade. The 
userland zpool command should test for this sysctl and print nothing 
regarding block_cloning. I can see a scenario when a user zpool upgrades 
their pools, notices the sysctl and does the unthinkable. Not only would 
this fill the mailing lists with angry chatter but it would spawn a number 
of PRs plus give us a lot of bad press for data loss.

Should we keep the new ZFS in 14, we should:

1. Make sure that zpool(8) does not mention or offer block_cloning in any 
way if the sysctl is disabled.

2. Print a cautionary note in release notes advising people not to enable 
this experimental sysctl. Maybe even have it print "(experimental)" to warn 
users that it will hurt.

3. Update the man pages to caution that block_cloning is experimental and 
unstable.

It's not enough to have a sysctl without hiding block_cloning completely 
from view. Only expose it in zpool(8) when the sysctl is enabled. Let's 
avoid people mistakenly enabling it.


-- 
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy@nwtime.org>    Web:  https://nwtime.org

			e^(i*pi)+1=0