Re: [PATCH 3/4] tests: new test to check quota after directory optimization

[Luis Henriques <luis.henriques@linux.dev>]

Luis Henriques <luis.henriques@linux.dev> writes:

> Andreas Dilger <adilger@dilger.ca> writes:
>
>> On Mar 28, 2024, at 11:29 AM, Luis Henriques (SUSE) <luis.henriques@linux.dev> 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