* [PATCH] common/rc: Use -v option with findmnt
@ 2017-03-13 18:07 Goldwyn Rodrigues
2017-03-14 5:07 ` Eryu Guan
0 siblings, 1 reply; 4+ messages in thread
From: Goldwyn Rodrigues @ 2017-03-13 18:07 UTC (permalink / raw)
To: fstests; +Cc: eguan, Goldwyn Rodrigues
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>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] common/rc: Use -v option with findmnt
2017-03-13 18:07 [PATCH] common/rc: Use -v option with findmnt Goldwyn Rodrigues
@ 2017-03-14 5:07 ` Eryu Guan
2017-03-14 14:07 ` Goldwyn Rodrigues
0 siblings, 1 reply; 4+ messages in thread
From: Eryu Guan @ 2017-03-14 5:07 UTC (permalink / raw)
To: Goldwyn Rodrigues; +Cc: fstests, Goldwyn Rodrigues
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] common/rc: Use -v option with findmnt
2017-03-14 5:07 ` Eryu Guan
@ 2017-03-14 14:07 ` Goldwyn Rodrigues
2017-03-15 8:44 ` Eryu Guan
0 siblings, 1 reply; 4+ messages in thread
From: Goldwyn Rodrigues @ 2017-03-14 14:07 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests, Goldwyn Rodrigues
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.
--
Goldwyn
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] common/rc: Use -v option with findmnt
2017-03-14 14:07 ` Goldwyn Rodrigues
@ 2017-03-15 8:44 ` Eryu Guan
0 siblings, 0 replies; 4+ messages in thread
From: Eryu Guan @ 2017-03-15 8:44 UTC (permalink / raw)
To: Goldwyn Rodrigues; +Cc: fstests, Goldwyn Rodrigues
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-15 8:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-13 18:07 [PATCH] common/rc: Use -v option with findmnt Goldwyn Rodrigues
2017-03-14 5:07 ` Eryu Guan
2017-03-14 14:07 ` Goldwyn Rodrigues
2017-03-15 8:44 ` Eryu Guan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.