From: Luis Henriques <luis.henriques@linux.dev>
To: Andreas Dilger <adilger@dilger.ca>
Cc: Theodore Ts'o <tytso@mit.edu>, linux-ext4@vger.kernel.org
Subject: Re: [PATCH 3/4] tests: new test to check quota after directory optimization
Date: Wed, 03 Apr 2024 16:03:49 +0100 [thread overview]
Message-ID: <87r0fm5vze.fsf@brahms.olymp> (raw)
In-Reply-To: <87il0zhms9.fsf@brahms.olymp> (Luis Henriques's message of "Tue, 02 Apr 2024 15:17:10 +0100")
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
next prev parent reply other threads:[~2024-04-03 15:03 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-28 17:29 [PATCH e2fsprogs 0/4] quota-related e2fsck fixes and tests Luis Henriques (SUSE)
2024-03-28 17:29 ` [PATCH 1/4] e2fsck: update quota accounting after directory optimization Luis Henriques (SUSE)
2024-04-01 19:52 ` Andreas Dilger
2024-03-28 17:29 ` [PATCH 2/4] e2fsck: update quota when deallocating a bad inode Luis Henriques (SUSE)
2024-04-01 20:52 ` Andreas Dilger
2024-03-28 17:29 ` [PATCH 3/4] tests: new test to check quota after directory optimization Luis Henriques (SUSE)
2024-04-01 21:01 ` Andreas Dilger
2024-04-02 14:17 ` Luis Henriques
2024-04-03 15:03 ` Luis Henriques [this message]
2024-03-28 17:29 ` [PATCH 4/4] tests: new test to check quota after a bad inode deallocation Luis Henriques (SUSE)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r0fm5vze.fsf@brahms.olymp \
--to=luis.henriques@linux.dev \
--cc=adilger@dilger.ca \
--cc=linux-ext4@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).