xfs
[Top] [All Lists]

Re: [PATCH 3/3] xfstests generic 310: fix common file path and other cle

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/3] xfstests generic 310: fix common file path and other cleanups
From: Zhao Hongjiang <zhaohongjiang37@xxxxxxxxx>
Date: Tue, 09 Apr 2013 15:25:49 +0800
Cc: rjohnston@xxxxxxx, eguan@xxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=rRQcRQ5gpWZhXsLwNF8VccZO5jGozNA0ntzlcV5ylO0=; b=wIFthqX6IvD5Q2B1CXNmGF7nQRjqqBOT2DZZBXtLH9NvI/VN7Ns9u09Q1WGsKu9XZq xNaGzOCEH42sGkhGIFcJep4E2CWfVEVNqKKMc0NRKS5K+tyMV4zRfUASDl7EHfB8NaKm W1GMOyIYO3s3T1enzfgrfdv4/W6UaLO6Qje3q5qmE2A9OtABA8IButHvzYF1zg1edBbE E6Qh7jVKR+v47iGigXdRFlxxG/wLeJXc+RHG67ILdHev0rgrEenrLuLcf+zn9ePDmljE z0oPSdgGnp1kTREmljprT6kWYXhRRIFoYfxTSxym+ZYF9PB+HrSq4YRgeqhcbznK4iy7 iXlg==
In-reply-to: <20130409064030.GF17758@dastard>
References: <5163B257.4080809@xxxxxxxxx> <20130409064030.GF17758@dastard>
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130107 Thunderbird/17.0.2
On 2013-4-9 14:40, Dave Chinner wrote:
> On Tue, Apr 09, 2013 at 02:16:55PM +0800, Zhao Hongjiang wrote:
>> On 2013/4/8 22:05, Rich Johnston wrote:
>>> Hi Eryu,
>>>
>>> Thanks for this cleanup patch. I was going to revert patch "bbaf78c0" which 
>>> introduced test generic/310 but will wait and see if Zhao will provide more 
>>> information which could be added to this patch.
>>>
>>>
>>> On 04/07/2013 05:39 AM, Eryu Guan wrote:
>>>> 1. add one space between # and test description
>>>
>>> The rest of the changes look good, sorry I missed them when I reviewed .
>>>
>>>> 2. remove creator/owner info
>>>> 3. fix common/rc and common/filter path so they can be sourced correctly
>>>> 4. no need to remove $seq.full cause it's not used(or if verbose output
>>>>     is needed, $seqres.full should be used)
>>>>
>>>> Signed-off-by: Eryu Guan <eguan@xxxxxxxxxx>
>>>> ---
>>>>   tests/generic/310 | 12 +++++-------
>>>>   1 file changed, 5 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/tests/generic/310 b/tests/generic/310
>>>> index ef51422..35baa23 100644
>>>> --- a/tests/generic/310
>>>> +++ b/tests/generic/310
>>>> @@ -1,8 +1,8 @@
>>>>   #! /bin/bash
>>>>   # FS QA Test No. 310
>>>>   #
>>>> -#Check if there are two threads,one keeps calling read() or lseek(), and
>>>> -#the other calling readdir(), both on the same directory fd.
>>>> +# Check if there are two threads,one keeps calling read() or lseek(), and
>>>> +# the other calling readdir(), both on the same directory fd.
>>>>   #
>>>
>>> Hi Zhao,
>>>
>>> I did see both threads running at the same time, but the more I
>>> look at this, the more I am a loss as to what this test is
>>> doing.
>>>
>>> Will you expand this a little please.  I should have asked for
>>> more justification the first time I reviewed this. Please
>>> provide what bug this is testing or what failure/weakness this
>>> test exposes.  If there is a commit this is related to, please
>>> reference it.
>>>
>> When I ran it on ext2, ext3 and ext4 which has dir_index feature
>> disabled, I got something like this:
>>
>> EXT3-fs error (device loop1): ext3_readdir: bad entry in directory
>> #34817: rec_len is \ smaller than minimal - offset=993, inode=0,
>> rec_len=0, name_len=0 EXT3-fs error \ (device loop1):
>> ext3_readdir: bad entry in directory #34817: rec_len is smaller
>> than \ minimal - offset=1009, inode=0, rec_len=0, name_len=0
>> EXT3-fs error (device loop1): \ ext3_readdir: bad entry in
>> directory #34817: rec_len is smaller than minimal - \ offset=993,
>> inode=0, rec_len=0, name_len=0 EXT3-fs error (device loop1): \
>> ext3_readdir: bad entry in directory #34817: rec_len is smaller
>> than minimal - \ offset=1009, inode=0, rec_len=0, name_len=0 ...
>>
>> If we configured errors=remount-ro, the filesystem will become
>> read-only.
> 
> So what is the criteria for a test failure?  The test body is only
> reading from the filesystem, so a ro,remount won't cause an obvious
> failure of the test.

There haven't a obvious criteria for a test failure, you should see it
from dmesg while you run the test.

> 
> Perhaps the test should have more comments in it than "read this
> URL" to explain what it is doing and what constitutes a failure?

Yes, i'll add more comments to explain it!

Thanks,
Zhao Hongjiang
> 
> Cheers,
> 
> Dave.
> 

<Prev in Thread] Current Thread [Next in Thread>