Re: [PATCH] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format

[Dave Chinner <david@fromorbit.com>]

On Wed, Dec 07, 2022 at 10:11:49AM +0800, Gao Xiang wrote:
> On Wed, Dec 07, 2022 at 10:34:17AM +1100, Dave Chinner wrote:
> > On Fri, Dec 02, 2022 at 10:23:07AM +0800, Gao Xiang wrote:
> > > > > +			[ "$nexts" -gt "$(((isize - 176) / 16))" ] && break
> > > > 
> > > > Only need to calculate this once if you declare this at the top:
> > > > 
> > > > 	# We need enough extents to guarantee that the data fork is in
> > > > 	# btree format.  Cycling the mount to use xfs_db is too slow, so
> > > > 	# watch for when the extent count exceeds the space after the
> > > > 	# inode core.
> > > > 	local max_nextents="$(((isize - 176) / 16))"
> > > > 
> > > > and then you can do:
> > > > 
> > > > 			[[ $nexts -gt $max_nextents ]] && break
> > > > 
> > > > Also not a fan of hardcoding 176 around fstests, but I don't know how
> > > > we'd detect that at all.
> > > > 
> > > > # Number of bytes reserved for only the inode record, excluding the
> > > > # immediate fork areas.
> > > > _xfs_inode_core_bytes()
> > > > {
> > > > 	echo 176
> > > > }
> > > > 
> > > > I guess?  Or extract it from tests/xfs/122.out?
> > > 
> > > Thanks for your comments.
> > > 
> > > I guess hard-coded 176 in _xfs_inode_core_bytes() is fine for now
> > > (It seems a bit weird to extract a number from a test expected result..)
> > 
> > Which is wrong when testing a v4 filesystem - in that case the inode
> > core size is 96 bytes and so max extents may be larger on v4
> > filesystems than v5 filesystems....
> 
> Do we really care v4 fs for now since it's deprecated?...

Yes, there are still lots of v4 filesystems in production
environments. There shouldn't be many new ones, but there is a long
tail of existing storage containing v4 filesystems that we must not
break.

We have to support v4 filesystems for another few years yet, hence
we still need solid test coverage on them to ensure we don't
accidentally break something that is going to bite users before they
migrate to newer filesystems....

> Darrick once also 
> suggested using (isize / 16) but it seems it could take unnecessary time to
> prepare.. Or we could just use (isize - 96) / 16 to keep v4 work.

It's taken me longer to write this email than it does to write the
code to make it work properly. e.g.:

	xfs_info $scratch | sed -ne 's/.*crc=\([01]\).*/\1/p'

And now we have 0 = v4, 1 = v5, and it's trivial to return the
correct inode size.

You can even do this trivially with grep:

	xfs_info $scratch | grep -wq "crc=1"
	if [ $? -eq 0 ]; then
		echo 176
	else
		echo 96
	fi

and now the return value tells us if we have a v4 or v5 filesystem.

-Dave.
-- 
Dave Chinner
david@fromorbit.com