On Fri, Jul 07, 2017 at 11:24:31AM +0200, Carlos Maiolino wrote: > 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: > Ok, I see.. that makes sense. Could you update the comments a bit here just to explain why the background task is necessary and that the freeze is going to block on the lvextend due to the state of the fs? I also think a wait after the lvextend couldn't hurt so it's more clear that the freeze is what is blocked, but not a big deal either way. Brian > ~ $ 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 -- 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
