Re: [PATCH 0/2] btrfs: port to new mount api

[David Sterba <dsterba@suse.cz>]

On Tue, Jun 27, 2023 at 05:03:42PM +0200, Christian Brauner wrote:
> On Tue, Jun 27, 2023 at 04:08:09PM +0200, David Sterba wrote:
> > On Tue, Jun 27, 2023 at 11:51:01AM +0000, Johannes Thumshirn wrote:
> > > On 26.06.23 16:19, Christian Brauner wrote:
> > > > This whole thing ends up being close to a rewrite in some places. I
> > > > haven't found a really elegant way to split this into smaller chunks.
> > > 
> > > You'll probably hate me for this, but you could split it up a bit by 
> > > first doing the move of the old mount code into params.c and then do the
> > > rewrite for the new mount API.
> > 
> > The patch needs more finer split than just that. Replacing the entire
> > mount code like that will introduce bugs that users will hit for sure.
> > We have some weird mount option combinations or constraints, and we
> > don't have a 100% testsuite coverage.
> > 
> > The switch to the new API needs to be done in one patch, that's obvious,
> > however all the code does not need to be in one patch. I suggest to
> > split generic preparatory changes, add basic framework for the new API,
> > then add the easy options, then by one the complicated ones, then do the
> > switch, then do the cleanup and removal of the old code. Yes it's more
> 
> You can't support both apis. You either do a full switch or you have to
> have a lot of dead and duplicatd code around that isn't used until the
> switch is done. I might just miss what you mean though. So please
> provide more details how you envision this to be done.

Temporarily there will be unused code from one or the other part, this
is IMHO acceptable as it's supposed to make future debugging possible.
If it's not understandable from my description above then I'll need to
basically split the patch myself. I don't mind as the API conversion has
been done by you, only the patch separation is my concern. I'll get to
that eventually.

> > work but if we have to debug anything in the future it'll be narrowed
> > down to a few short patches.
> 
> I don't think you'll end up with a few short patches. That just not how
> that works but again, I might just not see what you're seeing.

Yeah, we'd need something more concrete. I'm basing my suggestion on
previous work in other areas where the first version was a big chunk of
code replacing another one, and then we'd have to fix that one big
commit repeatedly.  I've been burned too many times to let such things
happen again.  This costs more time and distracts any current work so
I'm taking the pessimistic attitude and try to do it right from the
beginning, at some small cost like additional intermediate changes.