On Mon, Apr 17, 2023, 5:37 PM Rick Macklem <rick.mack...@gmail.com> wrote:
> On Mon, Apr 17, 2023 at 4:29 PM Cy Schubert <cy.schub...@cschubert.com> > wrote: > > > > In message <b57b06bd-7e73-ae2d-2fba-bd226883f...@dawidek.net>, Pawel > Jakub > > Dawi > > dek writes: > > > On 4/18/23 05:14, Mateusz Guzik wrote: > > > > On 4/17/23, Pawel Jakub Dawidek <p...@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. > I would suggest going a step further and making the sysctl RO for > FreeBSD14. > (This could be changed for FreeBSD14.n if/when block_cloning is believed to > be debugged.) > > I would apply all 3 of the above to "main", since some that install "main" > will not know how "bleeding edge" this is unless the above is done. > (Yes, I know "main" is "bleeding edge", but some still expect a stable > test system will result from installing it.) > > Thanks go to all that tracked this problem down, rick > Related question: what zfs branch is stable/14 going to track? With 13 it was whatever the next stable branch was. Warner > > > 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.schub...@cschubert.com> > > FreeBSD UNIX: <c...@freebsd.org> Web: https://FreeBSD.org > > NTP: <c...@nwtime.org> Web: https://nwtime.org > > > > e^(i*pi)+1=0 > > > > > > > >
