Hi Thanks for the comments, I agree with all the changes pointed but one inlined below. > > > +xfs_io -f -d -c 'pwrite -b 1m 0 120m' $mnt/f1 >/dev/null 2>&1 > > > + > > > +fsfreeze -f $mnt & > > > + > > > +#Wait fsfreeze to its job > > > +sleep 10 > > > > I think you could use 'wait' here rather than a sleep. But is there a > > reason we need to put the freeze in the background in the first place? > > It seems to me this test will either complete or hang regardless. ;P > > I had the same thought. not running fsfreeze in the background won't complete the test. fsfreeze needs some space in the device to complete, and with a full device it will hang until the device is extended with lvextend. The difference between a fixed filesystem and a buggy one, is whether the filesystem can be properly thawed or not, but both cases will hang in fsfreeze if the device isn't expanded right after freezing the fs. The main reason of the freeze, is to force xfsaild to run and push ail items, if no space is availale, xfsaild will stay stuck: ~ $ sudo cat /proc/4712/stack [<ffffffff813f7e16>] xfs_ail_push_all_sync+0xb6/0x100 [<ffffffff813e714f>] xfs_log_quiesce+0x4f/0x3a0 [<ffffffff813dd5cd>] xfs_quiesce_attr+0x6d/0xb0 [<ffffffff813dd660>] xfs_fs_freeze+0x50/0x80 [<ffffffff81246a5f>] freeze_super+0xcf/0x1a0 [<ffffffff81258ca3>] do_vfs_ioctl+0x4b3/0x5e0 [<ffffffff81258e49>] SyS_ioctl+0x79/0x90 [<ffffffff818da677>] entry_SYSCALL_64_fastpath+0x1a/0xa9 [<ffffffffffffffff>] 0xffffffffffffffff Other than that, I'll agree with everything. Thanks for the review guys > > > > + > > > > "extend the volume with sufficient space and unfreeze ..." > > > > > +lvextend -L $newpsize $vgname/$poolname >/dev/null 2>&1 > > > + > > > +fsfreeze -u $mnt > > > +echo "Test OK" > > > + > > > +status=0 > > > +exit > > > diff --git a/tests/xfs/999.out b/tests/xfs/999.out > > > new file mode 100644 > > > index 0000000..8c3c938 > > > --- /dev/null > > > +++ b/tests/xfs/999.out > > > @@ -0,0 +1,2 @@ > > > +QA output created by 999 > > > +Test OK > > > diff --git a/tests/xfs/group b/tests/xfs/group > > > index 792161a..2bde916 100644 > > > --- a/tests/xfs/group > > > +++ b/tests/xfs/group > > > @@ -416,3 +416,4 @@ > > > 416 dangerous_fuzzers dangerous_scrub dangerous_repair > > > 417 dangerous_fuzzers dangerous_scrub dangerous_online_repair > > > 418 dangerous_fuzzers dangerous_scrub dangerous_repair > > > +999 dangerous > > > > I think the auto group makes sense too. > > > > Also, since the discussion around having a couple tests here (one using > > error injection and this one using dm-thin) it seems like this test > > could be more suited to a generic test. Nothing in the test really > > depends on a particular filesystem. The header comment could simply be > > updated to explain the specific XFS issue as the inspiration and that > > the test generically exercises the ability to recover/continue after a > > dm-thin ENOPSPC and subsequent volume extend. Thoughts? > > I agree. :) > > (new laptop, let's see if this mail even makes it to the ml...) > > --D > > > Brian > > > > > -- > > > 2.9.4 > > > > > > -- > > > To unsubscribe from this list: send the line "unsubscribe fstests" in > > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > > To unsubscribe from this list: send the line "unsubscribe fstests" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Carlos -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
