Luis Henriques <luis.henriques@xxxxxxxxx> writes:
> Andreas Dilger <adilger@xxxxxxxxx> writes:
>
>> On Mar 28, 2024, at 11:29 AM, Luis Henriques (SUSE) <luis.henriques@xxxxxxxxx> wrote:
>>>
>>> This new test validates e2fsck by verifying that quota data is updated
>>> after a directory optimization is performed. It mimics fstest ext4/014
>>> by including a filesystem image where a file is created inside a new
>>> directory on the filesystem root and then root block 0 is wiped:
>>>
>>> # debugfs -w -R 'zap -f / 0' f_testnew/image
>>
>> I appreciate the test case, and I hate to be difficult, but IMHO this
>> test case is not ideal. It is *still* reporting quota inconsistency
>> at the end, so it is difficult to see whether the patch is actually
>> improving anything or not?
>
> Maybe I misunderstood how the tests really work. Here's what I
> understood:
>
> e2fsck is run twice. During the first run the filesystem is recovered.
> And that's the output of expect.1 -- it reports the quota inconsistency
> because quota data needs to be fixed. And it is fixed in that first run,
> where e2fsck returns '1' ("File system errors corrected"). The second
> time e2fsck is run (expect.2) it will do nothing, and '0' is returned
> because the filesystem hasn't been modified.
>
> Without the first patch in this series the second time e2fsck is executed
> it will still fail and report inconsistencies because the first time the
> fix wasn't correct. (And after this second time the filesystem should
> actually be corrected, a third run of e2fsck should return '0'.)
>
>> This is because the image is testing a number of different things at
>> once (repairing the root inode, superblock, etc). IMHO, it would be
>> better to have this test be specific to the directory shrink issue
>> (e.g. a large directory is created, many files are deleted from it,
>> then optimized), and ideally have a non-root user, group, and project
>> involved so that it is verifying that all of the quotas are updated.
>
> Right, that makes sense. However, I'm failing to narrow the test to that
> specific case. I've tried to create a bunch of files in a directory and
> used the debugfs 'kill_file' to remove files from that directory.
> However, in that case e2fsck isn't reporting quota inconsistencies as I
> would expect. Which may hint at yet more quota-related bugs. But I'm
> still looking.
OK, I _may_ have found a simple way to generate an image to test my patch.
Here's what I came up with:
make testnew
tune2fs -O quota f_testnew/image
debugfs -w -R "ln lost+found foo" f_testnew/image
debugfs -w -R "unlink lost+found" f_testnew/image
echo "update quota on directory optimization" > f_testnew/name
make testend
mv f_testnew f_quota_shrinkdir
This will trigger a directory optimization after the recreation of the
lost+found directory. Do you think this would be good enough?
Cheers,
--
Luis
