On Fri, Jun 09, 2023 at 10:03:07AM +0200, Christian Brauner wrote: > On Fri, Jun 09, 2023 at 10:38:15AM +0300, Amir Goldstein wrote: > > On Fri, Jun 9, 2023 at 10:28 AM Christian Brauner <brauner@xxxxxxxxxx> wrote: > > > On Thu, Jun 08, 2023 at 09:29:39PM +0300, Amir Goldstein wrote: > > > > > fs/overlayfs/super.c | 896 ++++++++++++++++++++++++++++++++------------------- > > > > > 1 file changed, 568 insertions(+), 328 deletions(-) > > > > > > > > > > > > > Very big patch - Not so easy to review. > > > > It feels like a large refactoring mixed with the api change. > > > > Can it easily be split to just the refactoring patch > > > > and the api change patch? or any other split that will be > > > > easier to review. > > > > > > I don't really think so because you can't do a piecemeal conversion > > > unfortunately. But if you have concrete ideas I'm happy to hear them. > > > > > > > To me it looks like besides using new api you changed the order > > of config parsing to: > > - fill ovl_config and sanitize path arguments > > (replacing string with path in case of upper/workdir) > > Afaict this only makes sense if you cane have a sensible split between > option parsing and superblock creation. While the new mount api does > have that the old one doesn't. So ovl_fill_super() does option parsing, > verification, and superblock creation. This is the same problem we had with XFS and ext4 conversions, and to a lesser extent all the other simpler ones that have been done. There is no real intermediate step - the change from old parser to new parser as an atomic unit of change comes down to a single large patch. Yes, we whittled away at the edges to make it a bit smaller, but in the end the core API changeover was still a single large patch... > So I honestly thing this might end up being churn for churn. But I'll do > it if you insist. Yup, based on the experience of converting other filesytsems, I think it is largely a waste of time and effort to try to significantly rework this into smaller chunks. The bulk of the work has been done, reworking individual patches doesn't change the end result, so just review it as is, merge it and move on with more important stuff.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
