| Message ID | 20170313180726.1967-1-rgoldwyn@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 13, 2017 at 01:07:26PM -0500, Goldwyn Rodrigues wrote: > From: Goldwyn Rodrigues <rgoldwyn@suse.com> > > Since btrfs puts in the subvol name in the device, we should > use -v option to not print the bind mounts or btrfs volumes. > > Fixes: 5e6892d ("common/rc: use findmnt to check mounted device") > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> Hmm, I mounted a btrfs snapshot of TEST_DEV to another dir and ran check, check reported: TEST_DEV=/dev/sdc1 is mounted but not on TEST_DIR=/mnt - aborting Already mounted result: /dev/sdc1[/snap] /mnt/scratch which seems fine and clear to me. Did you hit any particular failures without this fix? Thanks, Eryu > --- > common/rc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/common/rc b/common/rc > index d53f679..d7a5321 100644 > --- a/common/rc > +++ b/common/rc > @@ -1463,7 +1463,7 @@ _check_mounted_on() > local type=$5 > > # find $dev as the source, and print result in "$dev $mnt" format > - local mount_rec=`findmnt -rnc -S $dev -o SOURCE,TARGET` > + local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` > [ -n "$mount_rec" ] || return 1 # 1 = not mounted > > # if it's mounted, make sure its on $mnt > -- > 2.10.2 > -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/14/2017 12:07 AM, Eryu Guan wrote: > On Mon, Mar 13, 2017 at 01:07:26PM -0500, Goldwyn Rodrigues wrote: >> From: Goldwyn Rodrigues <rgoldwyn@suse.com> >> >> Since btrfs puts in the subvol name in the device, we should >> use -v option to not print the bind mounts or btrfs volumes. >> >> Fixes: 5e6892d ("common/rc: use findmnt to check mounted device") >> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > > Hmm, I mounted a btrfs snapshot of TEST_DEV to another dir and ran > check, check reported: > > TEST_DEV=/dev/sdc1 is mounted but not on TEST_DIR=/mnt - aborting > Already mounted result: > /dev/sdc1[/snap] /mnt/scratch > > which seems fine and clear to me. Did you hit any particular failures > without this fix? The problem is you hit this error even if /dev/sdc1 is mounted on TEST_DIR=/mnt because of the subvolume name appended to the device name. IOW, the test fails when it should not have. Here is an output of executing btrfs/021: --- tests/btrfs/021.out 2017-01-26 02:23:04.444778259 -0600 +++ /fiona/xfstests/results//btrfs/021.out.bad 2017-03-14 04:25:38.807212599 -0500 @@ -1,2 +1,4 @@ QA output created by 021 -Silence is golden +SCRATCH_DEV=/dev/vdc is mounted but not on SCRATCH_MNT=/scratch - aborting +Already mounted result: +/dev/vdc[/newvol] /scratch However, this fix is incorrect. I found that this was because of reminiscences of btrfs/009 Adding _scratch_umount to the end of btrfs/009 solves the issue, since the filesystem is created fresh in the next test. The filesystem was not created fresh in the next test because findmnt would fail in _require_scratch. I will shortly post a new patch which would fix this.
On Tue, Mar 14, 2017 at 09:07:05AM -0500, Goldwyn Rodrigues wrote: > > > On 03/14/2017 12:07 AM, Eryu Guan wrote: > > On Mon, Mar 13, 2017 at 01:07:26PM -0500, Goldwyn Rodrigues wrote: > >> From: Goldwyn Rodrigues <rgoldwyn@suse.com> > >> > >> Since btrfs puts in the subvol name in the device, we should > >> use -v option to not print the bind mounts or btrfs volumes. > >> > >> Fixes: 5e6892d ("common/rc: use findmnt to check mounted device") > >> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > > > > Hmm, I mounted a btrfs snapshot of TEST_DEV to another dir and ran > > check, check reported: > > > > TEST_DEV=/dev/sdc1 is mounted but not on TEST_DIR=/mnt - aborting > > Already mounted result: > > /dev/sdc1[/snap] /mnt/scratch > > > > which seems fine and clear to me. Did you hit any particular failures > > without this fix? > > > The problem is you hit this error even if /dev/sdc1 is mounted on > TEST_DIR=/mnt because of the subvolume name appended to the device name. > > IOW, the test fails when it should not have. Here is an output of > executing btrfs/021: > > --- tests/btrfs/021.out 2017-01-26 02:23:04.444778259 -0600 > +++ /fiona/xfstests/results//btrfs/021.out.bad 2017-03-14 > 04:25:38.807212599 -0500 > @@ -1,2 +1,4 @@ > QA output created by 021 > -Silence is golden > +SCRATCH_DEV=/dev/vdc is mounted but not on SCRATCH_MNT=/scratch - > aborting > +Already mounted result: > +/dev/vdc[/newvol] /scratch Ah, I see the problem. It's because btrfs/009 sets default volume to the newly created "newvol", then _scratch_mount mounts the newvol by default. And _check_mounted_on finds the inconsistency because of the subvolume name. > > However, this fix is incorrect. I think this patch is a clearer fix, it doesn't require test to do _scratch_umount at the end of test, and works for future tests that do the same setup. Thanks, Eryu > > I found that this was because of reminiscences of btrfs/009 > Adding _scratch_umount to the end of btrfs/009 solves the issue, since > the filesystem is created fresh in the next test. > > The filesystem was not created fresh in the next test because findmnt > would fail in _require_scratch. > > I will shortly post a new patch which would fix this. > > > > -- > Goldwyn -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/common/rc b/common/rc index d53f679..d7a5321 100644 --- a/common/rc +++ b/common/rc @@ -1463,7 +1463,7 @@ _check_mounted_on() local type=$5 # find $dev as the source, and print result in "$dev $mnt" format - local mount_rec=`findmnt -rnc -S $dev -o SOURCE,TARGET` + local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` [ -n "$mount_rec" ] || return 1 # 1 = not mounted # if it's mounted, make sure its on $mnt