On 2022/12/8 05:48, Dave Chinner wrote:
> 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.
Hi, David
I have written new versions, please see:
https://lore.kernel.org/fstests/20221207093147.1634425-1-ZiyangZhang@xxxxxxxxxxxxxxxxx/
Regards,
Zhang
