Re: [PATCH] generic/411: change sub-path name that's duplicate of TEST_DIR

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Thu, Mar 09, 2017 at 03:16:38PM +0200, Amir Goldstein wrote:
> On Thu, Mar 9, 2017 at 1:28 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> > On Thu, Mar 9, 2017 at 11:59 AM, Eryu Guan <eguan@xxxxxxxxxx> wrote:
> >> On Wed, Mar 08, 2017 at 04:26:22PM +0200, Amir Goldstein wrote:
> >>
> >> [snip]
> >>
> >>> From f365bd5d60d9d5b82e02db17e615380bf37a74de Mon Sep 17 00:00:00 2001
> >>> From: Amir Goldstein <amir73il@xxxxxxxxx>
> >>> Date: Wed, 8 Mar 2017 12:38:22 +0200
> >>> Subject: [PATCH] filter: match $TEST_* $SCRATCH_* in beginning of path string
> >>>
> >>> For example, if $TEST_DIR=/mnt, only replace instacnes of /mnt that
> >>> are in the beginning of a path string, e.g.:
> >>>
> >>> "/mnt/mntA/mntB:/mnt/mntC" => "TEST_DIR/mntA/mntB:TEST_DIR/mntC"
> >>>
> >>> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> >>
> >> I'm testing this patch now, with the following config:
> >>
> >> TEST_DEV=/dev/vda5
> >> TEST_DIR=/vda5
> >> SCRATCH_DEV=/dev/vda6
> >> SCRATCH_MNT=/vda6
> >> FSTYP=xfs
> >> MKFS_OPTIONS="-m crc=1,reflink=1"
> >>
> >> First I run this test on xfs, and everything went well. Then I ran test
> >> with -overlay option, and found generic/171 generic/172 in this order
> >> confused _check_mounted_on().
> >>
> >> ./check -overlay generic/17[1-2]
> >>
> >> I got this diff
> >>
> >> --- tests/generic/172.out       2016-12-30 14:13:24.076000000 +0800
> >> +++ /root/xfstests/results//xfs_4k_reflink/generic/172.out.bad  2017-03-09 17:27:12.203000000 +0800
> >> @@ -1,9 +1,4 @@
> >>  QA output created by 172
> >> -Format and mount
> >> -Reformat with appropriate size
> >> -Create a big file and reflink it
> >> -Allocate the rest of the space
> >> -CoW the big file
> >> -pwrite: No space left on device
> >> -Remount and try CoW again
> >> -pwrite: No space left on device
> >> +SCRATCH_DEV=/vda6 is mounted but not on SCRATCH_MNT=/vda6/ovl-mnt - aborting
> >> +Already mounted result:
> >> +/dev/vda6 on /vda6 type xfs (rw,relatime,context=system_u:object_r:nfs_t:s0,attr2,inode64,noquota)
> >>
> >> Seems the 'grep -F "$dev on ' check isn't that accurate either. This
> >> also happens without this patch.
> >>
> >
> > Oops. Good catch.
> > I have this sort of setup with kvm-xfstests, but only tested it with
> > old overlay config
> > (because I need to change kvm-xfstests scripts to new overlay config)
> 
> So I changed kvm-xfstests over to new overlay config and got the error
> you reported.
> Your fix also works, but see a few minor comments below.
> 
> >
> >> I'm trying a draft patch like below, this seems to fix the problem for
> >> me, and it works for NFS too (both ipv4 and ipv6). I'm testing your
> >> patch + my local fix now.  Will see how it goes, if everything goes well
> >> I'll post it as a seperate patch.
> >
> > Are you planning to keep my patch as is or drop the else statement in
> > the filters?

I was testing your attached patch as is, if you can send a formal patch
with the else statement dropped that'd be great.

> >
> >>
> >> diff --git a/common/rc b/common/rc
> >> index 109325d..e3169d5 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -1440,11 +1440,13 @@ _check_mounted_on()
> >>         # IPv6 server as a regular expression.  Because of that, we cannot use
> >>         # ^$dev so we use "$dev on " to avoid matching $dev to mount point field
> >>         # for overlay case, where $dev is a directory.
> 
> The comment above is no longer relevant

Yes, will remove it in final patch, this one is a draft patch.

> 
> >> -       local mount_rec=`_mount | grep -F "$dev on "`
> >> +#      local mount_rec=`_mount | grep -F "$dev on "`
> >> +       local mount_rec=`findmnt -rnc -S $dev -o SOURCE,TARGET`
> >>         [ -n "$mount_rec" ] || return 1 # 1 = not mounted
> >>
> >>         # if it's mounted, make sure its on $mnt
> >> -       if ! (echo $mount_rec | grep -q "$dev on $mnt")
> >> +#      if ! (echo $mount_rec | grep -q "$dev on $mnt")
> >> +       if ! (echo $mount_rec | grep -Fq "$dev $mnt")
> 
> Why bother with grep -F (and having to keep the comment above).
> I used the following instead:
> +       if [ "$mount_rec" != "$dev $mnt" ]

Yes, this looks better, thanks!

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



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux