xfstests generic 310: fix common file path and other cleanups

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.

Quoted 1 line · Eryu Guan

1. add one space between # and test description

The rest of the changes look good, sorry I missed them when I reviewed .

Quoted 15 lines

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 <[email protected]>

---
  tests/generic/310 | 12 +++++-------
  1 file changed, 5 insertions(+), 7 deletions(-)

tests/generic/310 +0 -0

index ef51422..35baa23 100644

tests/generic/310 line 6

Quoted 8 lines

tests/generic/310 +2 -2

@@ -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.

Thanks
--Rich

Quoted 40 lines · Rich Johnston

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.

Quoted 1 line · Eryu Guan

1. add one space between # and test description

The rest of the changes look good, sorry I missed them when I reviewed .

Quoted 15 lines

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 <[email protected]>

---
  tests/generic/310 | 12 +++++-------
  1 file changed, 5 insertions(+), 7 deletions(-)

tests/generic/310 +0 -0

index ef51422..35baa23 100644

tests/generic/310 line 6

Quoted 8 lines

tests/generic/310 +2 -2

@@ -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.

Quoted 6 lines

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.

Quoted 61 lines · Zhao Hongjiang

Quoted 40 lines · Rich Johnston

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.

Quoted 1 line · Eryu Guan

1. add one space between # and test description

The rest of the changes look good, sorry I missed them when I reviewed .

Quoted 15 lines

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 <[email protected]>

---
  tests/generic/310 | 12 +++++-------
  1 file changed, 5 insertions(+), 7 deletions(-)

tests/generic/310 +0 -0

index ef51422..35baa23 100644

tests/generic/310 line 6

Quoted 8 lines

tests/generic/310 +2 -2

@@ -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.

Quoted 6 lines

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 ...

Quoted 2 lines

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.

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

Cheers,

Dave.

Quoted 65 lines · Dave Chinner

Quoted 63 lines · Zhao Hongjiang

Quoted 40 lines · Rich Johnston

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.

Quoted 1 line · Eryu Guan

1. add one space between # and test description

The rest of the changes look good, sorry I missed them when I reviewed .

Quoted 15 lines

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 <[email protected]>

---
  tests/generic/310 | 12 +++++-------
  1 file changed, 5 insertions(+), 7 deletions(-)

tests/generic/310 +0 -0

index ef51422..35baa23 100644

tests/generic/310 line 6

Quoted 8 lines

tests/generic/310 +2 -2

@@ -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.

Quoted 6 lines

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.

Quoted 3 lines

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.

Quoted 2 lines

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!