* Re: [PATCH] btrfs_show_devname don't traverse into the seed fsid
2020-07-10 6:37 [PATCH] btrfs_show_devname don't traverse into the seed fsid Anand Jain
@ 2020-07-10 14:32 ` Josef Bacik
2020-07-10 15:16 ` Anand Jain
2020-07-10 15:54 ` Martin K. Petersen
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Josef Bacik @ 2020-07-10 14:32 UTC (permalink / raw)
To: Anand Jain, linux-btrfs
On 7/10/20 2:37 AM, Anand Jain wrote:
> ->show_devname currently shows the lowest devid in the list. As the seed
> devices have the lowest devid in the sprouted filesystem, the userland
> tool such as findmnt end up seeing seed device instead of the device from
> the read-writable sprouted filesystem. As shown below.
>
> mount /dev/sda /btrfs
> mount: /btrfs: WARNING: device write-protected, mounted read-only.
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sda /btrfs 899f7027-3e46-4626-93e7-7d4c9ad19111
>
> btrfs dev add -f /dev/sdb /btrfs
>
> umount /btrfs
> mount /dev/sdb /btrfs
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sda /btrfs 899f7027-3e46-4626-93e7-7d4c9ad19111
>
>
> All sprouts from a single seed will show the same seed device and the
> same fsid. That's messy.
> This is causing problems in our prototype as there isn't any reference
> to the sprout file-system(s) which is being used for actual read and
> write.
>
> This was added in the patch which implemented the show_devname in btrfs
> commit 9c5085c14798 (Btrfs: implement ->show_devname).
> I tried to look for any particular reason that we need to show the seed
> device, there isn't any.
>
> So instead, do not traverse through the seed devices, just show the
> lowest devid in the sprouted fsid.
>
> After the patch:
>
> mount /dev/sda /btrfs
> mount: /btrfs: WARNING: device write-protected, mounted read-only.
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sda /btrfs 899f7027-3e46-4626-93e7-7d4c9ad19111
>
> btrfs dev add -f /dev/sdb /btrfs
> mount -o rw,remount /dev/sdb /btrfs
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sdb /btrfs 595ca0e6-b82e-46b5-b9e2-c72a6928be48
>
> mount /dev/sda /btrfs1
> mount: /btrfs1: WARNING: device write-protected, mounted read-only.
>
> btrfs dev add -f /dev/sdc /btrfs1
>
> findmnt --output SOURCE,TARGET,UUID /btrfs1
> SOURCE TARGET UUID
> /dev/sdc /btrfs1 ca1dbb7a-8446-4f95-853c-a20f3f82bdbb
>
> cat /proc/self/mounts | grep btrfs
> /dev/sdb /btrfs btrfs rw,relatime,noacl,space_cache,subvolid=5,subvol=/ 0 0
> /dev/sdc /btrfs1 btrfs ro,relatime,noacl,space_cache,subvolid=5,subvol=/ 0 0
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
This needs to come with an xfstest so we do not regress this in the future. Thanks,
Josef
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] btrfs_show_devname don't traverse into the seed fsid
2020-07-10 6:37 [PATCH] btrfs_show_devname don't traverse into the seed fsid Anand Jain
2020-07-10 14:32 ` Josef Bacik
@ 2020-07-10 15:54 ` Martin K. Petersen
2020-07-13 5:48 ` [PATCH] fstests: btrfs test if show_devname returns sprout device Anand Jain
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Martin K. Petersen @ 2020-07-10 15:54 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs
Anand,
> So instead, do not traverse through the seed devices, just show the
> lowest devid in the sprouted fsid.
Reported-by: Martin K. Petersen <martin.petersen@oracle.com>
Tested-by: Martin K. Petersen <martin.petersen@oracle.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH] fstests: btrfs test if show_devname returns sprout device
2020-07-10 6:37 [PATCH] btrfs_show_devname don't traverse into the seed fsid Anand Jain
2020-07-10 14:32 ` Josef Bacik
2020-07-10 15:54 ` Martin K. Petersen
@ 2020-07-13 5:48 ` Anand Jain
2020-07-13 7:03 ` Nikolay Borisov
2020-07-13 14:04 ` [PATCH] btrfs_show_devname don't traverse into the seed fsid Nikolay Borisov
2020-07-15 10:21 ` David Sterba
4 siblings, 1 reply; 13+ messages in thread
From: Anand Jain @ 2020-07-13 5:48 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, josef
Test if the show_devname() returns sprout device instead of seed device.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
tests/btrfs/215 | 59 +++++++++++++++++++++++++++++++++++++++++++++
tests/btrfs/215.out | 2 ++
tests/btrfs/group | 1 +
3 files changed, 62 insertions(+)
create mode 100755 tests/btrfs/215
create mode 100644 tests/btrfs/215.out
diff --git a/tests/btrfs/215 b/tests/btrfs/215
new file mode 100755
index 000000000000..19eb68437567
--- /dev/null
+++ b/tests/btrfs/215
@@ -0,0 +1,59 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 Oracle. All Rights Reserved.
+#
+# FS QA Test 215
+#
+# Test if the show_devname() returns sprout device instead of seed device.
+#
+# Requires kernel patch:
+# btrfs: btrfs_show_devname don't traverse into the seed fsid
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch_dev_pool 2
+
+_scratch_dev_pool_get 2
+
+seed=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
+sprout=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
+
+_mkfs_dev $seed
+$BTRFS_TUNE_PROG -S 1 $seed
+_mount $seed $SCRATCH_MNT >> $seqres.full 2>&1
+cat /proc/self/mounts | grep $seed >> $seqres.full
+$BTRFS_UTIL_PROG device add -f $sprout $SCRATCH_MNT
+cat /proc/self/mounts | grep $sprout >> $seqres.full
+
+#must fail
+cat /proc/self/mounts | grep $seed
+
+_scratch_dev_pool_put
+
+echo "Silence is golden"
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/215.out b/tests/btrfs/215.out
new file mode 100644
index 000000000000..0a11773bbb32
--- /dev/null
+++ b/tests/btrfs/215.out
@@ -0,0 +1,2 @@
+QA output created by 215
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 505665b54d61..76c8b78d08f9 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -217,3 +217,4 @@
212 auto balance dangerous
213 auto balance dangerous
214 auto quick send snapshot
+215 auto quick seed
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH] fstests: btrfs test if show_devname returns sprout device
2020-07-13 5:48 ` [PATCH] fstests: btrfs test if show_devname returns sprout device Anand Jain
@ 2020-07-13 7:03 ` Nikolay Borisov
2020-07-13 11:00 ` [PATCH v2] " Anand Jain
0 siblings, 1 reply; 13+ messages in thread
From: Nikolay Borisov @ 2020-07-13 7:03 UTC (permalink / raw)
To: Anand Jain, fstests; +Cc: linux-btrfs, josef
On 13.07.20 г. 8:48 ч., Anand Jain wrote:
> Test if the show_devname() returns sprout device instead of seed device.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> tests/btrfs/215 | 59 +++++++++++++++++++++++++++++++++++++++++++++
> tests/btrfs/215.out | 2 ++
> tests/btrfs/group | 1 +
> 3 files changed, 62 insertions(+)
> create mode 100755 tests/btrfs/215
> create mode 100644 tests/btrfs/215.out
>
> diff --git a/tests/btrfs/215 b/tests/btrfs/215
> new file mode 100755
> index 000000000000..19eb68437567
> --- /dev/null
> +++ b/tests/btrfs/215
> @@ -0,0 +1,59 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 215
> +#
> +# Test if the show_devname() returns sprout device instead of seed device.
> +#
> +# Requires kernel patch:
> +# btrfs: btrfs_show_devname don't traverse into the seed fsid
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch_dev_pool 2
> +
> +_scratch_dev_pool_get 2
> +
> +seed=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
> +sprout=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
> +
> +_mkfs_dev $seed
> +$BTRFS_TUNE_PROG -S 1 $seed
> +_mount $seed $SCRATCH_MNT >> $seqres.full 2>&1
> +cat /proc/self/mounts | grep $seed >> $seqres.full
> +$BTRFS_UTIL_PROG device add -f $sprout $SCRATCH_MNT
> +cat /proc/self/mounts | grep $sprout >> $seqres.full
> +
> +#must fail
> +cat /proc/self/mounts | grep $seed
Checking for presence of specific sprout device will make the test more
robust rather than checking for the absence of the seed device.
> +
> +_scratch_dev_pool_put
> +
> +echo "Silence is golden"
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/215.out b/tests/btrfs/215.out
> new file mode 100644
> index 000000000000..0a11773bbb32
> --- /dev/null
> +++ b/tests/btrfs/215.out
> @@ -0,0 +1,2 @@
> +QA output created by 215
> +Silence is golden
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index 505665b54d61..76c8b78d08f9 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -217,3 +217,4 @@
> 212 auto balance dangerous
> 213 auto balance dangerous
> 214 auto quick send snapshot
> +215 auto quick seed
>
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2] fstests: btrfs test if show_devname returns sprout device
2020-07-13 7:03 ` Nikolay Borisov
@ 2020-07-13 11:00 ` Anand Jain
2020-07-13 11:15 ` Nikolay Borisov
0 siblings, 1 reply; 13+ messages in thread
From: Anand Jain @ 2020-07-13 11:00 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, josef, nborisov
Test if the show_devname() returns sprout device instead of seed device.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: check for presence of needed sprout device.
common/filter | 8 ++++++
tests/btrfs/215 | 59 +++++++++++++++++++++++++++++++++++++++++++++
tests/btrfs/215.out | 2 ++
tests/btrfs/group | 1 +
4 files changed, 70 insertions(+)
create mode 100755 tests/btrfs/215
create mode 100644 tests/btrfs/215.out
diff --git a/common/filter b/common/filter
index 2477f3860151..992783aba187 100644
--- a/common/filter
+++ b/common/filter
@@ -284,6 +284,14 @@ _filter_test_dir()
-e "s,\B$TEST_DEV,TEST_DEV,g"
}
+_filter_devs()
+{
+ local filter_devs
+
+ filter_devs=$(echo $1 | sed -e 's/\s\+/\\\|/g')
+ sed -e "s,$filter_devs,SCRATCH_DEV,g"
+}
+
_filter_scratch()
{
# SCRATCH_DEV may be a prefix of SCRATCH_MNT (e.g. /mnt, /mnt/ovl-mnt)
diff --git a/tests/btrfs/215 b/tests/btrfs/215
new file mode 100755
index 000000000000..cf5e360d14b1
--- /dev/null
+++ b/tests/btrfs/215
@@ -0,0 +1,59 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 Oracle. All Rights Reserved.
+#
+# FS QA Test 215
+#
+# Test if the show_devname() returns sprout device instead of seed device.
+#
+# Fixed in kernel patch:
+# btrfs: btrfs_show_devname don't traverse into the seed fsid
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch_dev_pool 2
+
+_scratch_dev_pool_get 2
+
+seed=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
+sprout=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
+
+_mkfs_dev $seed
+$BTRFS_TUNE_PROG -S 1 $seed
+_mount $seed $SCRATCH_MNT >> $seqres.full 2>&1
+cat /proc/self/mounts | grep $seed >> $seqres.full
+$BTRFS_UTIL_PROG device add -f $sprout $SCRATCH_MNT
+cat /proc/self/mounts | grep $sprout >> $seqres.full
+
+# check if the show_devname() returns the sprout device instead of seed device.
+cat /proc/self/mounts | grep $SCRATCH_MNT | awk '{print $1}' | \
+ _filter_devs $sprout
+
+_scratch_dev_pool_put
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/215.out b/tests/btrfs/215.out
new file mode 100644
index 000000000000..ed3207851653
--- /dev/null
+++ b/tests/btrfs/215.out
@@ -0,0 +1,2 @@
+QA output created by 215
+SCRATCH_DEV
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 505665b54d61..76c8b78d08f9 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -217,3 +217,4 @@
212 auto balance dangerous
213 auto balance dangerous
214 auto quick send snapshot
+215 auto quick seed
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2] fstests: btrfs test if show_devname returns sprout device
2020-07-13 11:00 ` [PATCH v2] " Anand Jain
@ 2020-07-13 11:15 ` Nikolay Borisov
2020-07-13 11:32 ` Anand Jain
0 siblings, 1 reply; 13+ messages in thread
From: Nikolay Borisov @ 2020-07-13 11:15 UTC (permalink / raw)
To: Anand Jain, fstests; +Cc: linux-btrfs, josef
On 13.07.20 г. 14:00 ч., Anand Jain wrote:
> Test if the show_devname() returns sprout device instead of seed device.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v2: check for presence of needed sprout device.
>
> common/filter | 8 ++++++
> tests/btrfs/215 | 59 +++++++++++++++++++++++++++++++++++++++++++++
> tests/btrfs/215.out | 2 ++
> tests/btrfs/group | 1 +
> 4 files changed, 70 insertions(+)
> create mode 100755 tests/btrfs/215
> create mode 100644 tests/btrfs/215.out
>
> diff --git a/common/filter b/common/filter
> index 2477f3860151..992783aba187 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -284,6 +284,14 @@ _filter_test_dir()
> -e "s,\B$TEST_DEV,TEST_DEV,g"
> }
>
> +_filter_devs()
> +{
> + local filter_devs
> +
> + filter_devs=$(echo $1 | sed -e 's/\s\+/\\\|/g')
> + sed -e "s,$filter_devs,SCRATCH_DEV,g"
> +}
> +
> _filter_scratch()
> {
> # SCRATCH_DEV may be a prefix of SCRATCH_MNT (e.g. /mnt, /mnt/ovl-mnt)
> diff --git a/tests/btrfs/215 b/tests/btrfs/215
> new file mode 100755
> index 000000000000..cf5e360d14b1
> --- /dev/null
> +++ b/tests/btrfs/215
> @@ -0,0 +1,59 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 215
> +#
> +# Test if the show_devname() returns sprout device instead of seed device.
> +#
> +# Fixed in kernel patch:
> +# btrfs: btrfs_show_devname don't traverse into the seed fsid
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch_dev_pool 2
> +
> +_scratch_dev_pool_get 2
> +
> +seed=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
> +sprout=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
> +
> +_mkfs_dev $seed
> +$BTRFS_TUNE_PROG -S 1 $seed
> +_mount $seed $SCRATCH_MNT >> $seqres.full 2>&1
> +cat /proc/self/mounts | grep $seed >> $seqres.full
> +$BTRFS_UTIL_PROG device add -f $sprout $SCRATCH_MNT
> +cat /proc/self/mounts | grep $sprout >> $seqres.full
> +
> +# check if the show_devname() returns the sprout device instead of seed device.
> +cat /proc/self/mounts | grep $SCRATCH_MNT | awk '{print $1}' | \
> + _filter_devs $sprout
Why does this have to be so complicated - 4 chained program executions,
1 additional function...
dev=$(grep $SCRATCH_MOUNT /proc/mounts | awk '{printf $1}')
if [ $sprout != $dev ]; then
_fail "Unexpected device"
fi
> +
> +_scratch_dev_pool_put
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/215.out b/tests/btrfs/215.out
> new file mode 100644
> index 000000000000..ed3207851653
> --- /dev/null
> +++ b/tests/btrfs/215.out
> @@ -0,0 +1,2 @@
> +QA output created by 215
> +SCRATCH_DEV
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index 505665b54d61..76c8b78d08f9 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -217,3 +217,4 @@
> 212 auto balance dangerous
> 213 auto balance dangerous
> 214 auto quick send snapshot
> +215 auto quick seed
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] fstests: btrfs test if show_devname returns sprout device
2020-07-13 11:15 ` Nikolay Borisov
@ 2020-07-13 11:32 ` Anand Jain
2020-07-19 15:19 ` Eryu Guan
0 siblings, 1 reply; 13+ messages in thread
From: Anand Jain @ 2020-07-13 11:32 UTC (permalink / raw)
To: Nikolay Borisov, fstests; +Cc: linux-btrfs, josef, guaneryu
>> +# check if the show_devname() returns the sprout device instead of seed device.
>> +cat /proc/self/mounts | grep $SCRATCH_MNT | awk '{print $1}' | \
>> + _filter_devs $sprout
>
> Why does this have to be so complicated - 4 chained program executions,
> 1 additional function...
>
For example:
/dev/sdb /btrfs btrfs
ro,relatime,noacl,space_cache,subvolid=5,subvol=/ 0 0
$1 to $3 remain constant, but $4 options might vary. So to avoid
unnecessary breakage of test case due to kernel updates or mount
options, I just used $1.
> dev=$(grep $SCRATCH_MOUNT /proc/mounts | awk '{printf $1}')
>
> if [ $sprout != $dev ]; then
> _fail "Unexpected device"
> fi
fstests prefers use of .out file to look for the expected string.
Will wait for Eryu comments.
Thanks, Anand
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] fstests: btrfs test if show_devname returns sprout device
2020-07-13 11:32 ` Anand Jain
@ 2020-07-19 15:19 ` Eryu Guan
2020-07-20 3:55 ` [PATCH v3] " Anand Jain
0 siblings, 1 reply; 13+ messages in thread
From: Eryu Guan @ 2020-07-19 15:19 UTC (permalink / raw)
To: Anand Jain; +Cc: Nikolay Borisov, fstests, linux-btrfs, josef, guaneryu
On Mon, Jul 13, 2020 at 07:32:57PM +0800, Anand Jain wrote:
>
>
>
> > > +# check if the show_devname() returns the sprout device instead of seed device.
> > > +cat /proc/self/mounts | grep $SCRATCH_MNT | awk '{print $1}' | \
> > > + _filter_devs $sprout
> >
> > Why does this have to be so complicated - 4 chained program executions,
> > 1 additional function...
> >
>
> For example:
> /dev/sdb /btrfs btrfs ro,relatime,noacl,space_cache,subvolid=5,subvol=/ 0 0
>
> $1 to $3 remain constant, but $4 options might vary. So to avoid
> unnecessary breakage of test case due to kernel updates or mount
> options, I just used $1.
>
> > dev=$(grep $SCRATCH_MOUNT /proc/mounts | awk '{printf $1}')
This looks simpler, just use $AWK_PROG instead of bare awk.
> >
> > if [ $sprout != $dev ]; then
> > _fail "Unexpected device"
> > fi
> fstests prefers use of .out file to look for the expected string.
> Will wait for Eryu comments.
Even $dev is not constant, we don't have to filter the device to
"SCRATCH_DEV" by _filter_devs. Just do
echo "Silence is golden"
if [ "$sprout" != "$dev" ]; then
echo "Unexpected device: $dev"
fi
Thanks,
Eryu
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v3] fstests: btrfs test if show_devname returns sprout device
2020-07-19 15:19 ` Eryu Guan
@ 2020-07-20 3:55 ` Anand Jain
0 siblings, 0 replies; 13+ messages in thread
From: Anand Jain @ 2020-07-20 3:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: fstests, guan, nborisov
Test if the show_devname() returns sprout device instead of seed device.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: check for presence of needed sprout device.
v3: check for not presence of needed sprout device and break the
silence.
tests/btrfs/215 | 63 +++++++++++++++++++++++++++++++++++++++++++++
tests/btrfs/215.out | 2 ++
tests/btrfs/group | 1 +
3 files changed, 66 insertions(+)
create mode 100755 tests/btrfs/215
create mode 100644 tests/btrfs/215.out
diff --git a/tests/btrfs/215 b/tests/btrfs/215
new file mode 100755
index 000000000000..5be6d0f60192
--- /dev/null
+++ b/tests/btrfs/215
@@ -0,0 +1,63 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 Oracle. All Rights Reserved.
+#
+# FS QA Test 215
+#
+# Test if the show_devname() returns sprout device instead of seed device.
+#
+# Fixed in kernel patch:
+# btrfs: btrfs_show_devname don't traverse into the seed fsid
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch_dev_pool 2
+
+_scratch_dev_pool_get 2
+
+seed=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
+sprout=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
+
+_mkfs_dev $seed
+$BTRFS_TUNE_PROG -S 1 $seed
+_mount $seed $SCRATCH_MNT >> $seqres.full 2>&1
+cat /proc/self/mounts | grep $seed >> $seqres.full
+$BTRFS_UTIL_PROG device add -f $sprout $SCRATCH_MNT
+cat /proc/self/mounts | grep $sprout >> $seqres.full
+
+# check if the show_devname() returns the sprout device instead of seed device.
+dev=$(grep $SCRATCH_MNT /proc/self/mounts | $AWK_PROG '{print $1}')
+
+if [ "$sprout" != "$dev" ]; then
+ echo "Unexpected device: $dev"
+fi
+echo "Silence is golden"
+
+_scratch_dev_pool_put
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/215.out b/tests/btrfs/215.out
new file mode 100644
index 000000000000..0a11773bbb32
--- /dev/null
+++ b/tests/btrfs/215.out
@@ -0,0 +1,2 @@
+QA output created by 215
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 505665b54d61..76c8b78d08f9 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -217,3 +217,4 @@
212 auto balance dangerous
213 auto balance dangerous
214 auto quick send snapshot
+215 auto quick seed
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] btrfs_show_devname don't traverse into the seed fsid
2020-07-10 6:37 [PATCH] btrfs_show_devname don't traverse into the seed fsid Anand Jain
` (2 preceding siblings ...)
2020-07-13 5:48 ` [PATCH] fstests: btrfs test if show_devname returns sprout device Anand Jain
@ 2020-07-13 14:04 ` Nikolay Borisov
2020-07-15 10:21 ` David Sterba
4 siblings, 0 replies; 13+ messages in thread
From: Nikolay Borisov @ 2020-07-13 14:04 UTC (permalink / raw)
To: Anand Jain, linux-btrfs
On 10.07.20 г. 9:37 ч., Anand Jain wrote:
> ->show_devname currently shows the lowest devid in the list. As the seed
> devices have the lowest devid in the sprouted filesystem, the userland
> tool such as findmnt end up seeing seed device instead of the device from
> the read-writable sprouted filesystem. As shown below.
>
> mount /dev/sda /btrfs
> mount: /btrfs: WARNING: device write-protected, mounted read-only.
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sda /btrfs 899f7027-3e46-4626-93e7-7d4c9ad19111
>
> btrfs dev add -f /dev/sdb /btrfs
>
> umount /btrfs
> mount /dev/sdb /btrfs
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sda /btrfs 899f7027-3e46-4626-93e7-7d4c9ad19111
>
>
> All sprouts from a single seed will show the same seed device and the
> same fsid. That's messy.
> This is causing problems in our prototype as there isn't any reference
> to the sprout file-system(s) which is being used for actual read and
> write.
>
> This was added in the patch which implemented the show_devname in btrfs
> commit 9c5085c14798 (Btrfs: implement ->show_devname).
> I tried to look for any particular reason that we need to show the seed
> device, there isn't any.
>
> So instead, do not traverse through the seed devices, just show the
> lowest devid in the sprouted fsid.
>
> After the patch:
>
> mount /dev/sda /btrfs
> mount: /btrfs: WARNING: device write-protected, mounted read-only.
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sda /btrfs 899f7027-3e46-4626-93e7-7d4c9ad19111
>
> btrfs dev add -f /dev/sdb /btrfs
> mount -o rw,remount /dev/sdb /btrfs
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sdb /btrfs 595ca0e6-b82e-46b5-b9e2-c72a6928be48
>
> mount /dev/sda /btrfs1
> mount: /btrfs1: WARNING: device write-protected, mounted read-only.
>
> btrfs dev add -f /dev/sdc /btrfs1
>
> findmnt --output SOURCE,TARGET,UUID /btrfs1
> SOURCE TARGET UUID
> /dev/sdc /btrfs1 ca1dbb7a-8446-4f95-853c-a20f3f82bdbb
>
> cat /proc/self/mounts | grep btrfs
> /dev/sdb /btrfs btrfs rw,relatime,noacl,space_cache,subvolid=5,subvol=/ 0 0
> /dev/sdc /btrfs1 btrfs ro,relatime,noacl,space_cache,subvolid=5,subvol=/ 0 0
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Tested-by: Nikolay Borisov <nborisov@suse.com>
> ---
> This has passed the xfstests/btrfs sucessfully with no new
> regressions. Thanks.
>
> fs/btrfs/super.c | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index c7ee119cffa1..1e2a36f5c792 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2377,9 +2377,7 @@ static int btrfs_unfreeze(struct super_block *sb)
> static int btrfs_show_devname(struct seq_file *m, struct dentry *root)
> {
> struct btrfs_fs_info *fs_info = btrfs_sb(root->d_sb);
> - struct btrfs_fs_devices *cur_devices;
> struct btrfs_device *dev, *first_dev = NULL;
> - struct list_head *head;
>
> /*
> * Lightweight locking of the devices. We should not need
> @@ -2389,18 +2387,13 @@ static int btrfs_show_devname(struct seq_file *m, struct dentry *root)
> * least until the rcu_read_unlock.
> */
> rcu_read_lock();
> - cur_devices = fs_info->fs_devices;
> - while (cur_devices) {
> - head = &cur_devices->devices;
> - list_for_each_entry_rcu(dev, head, dev_list) {
> - if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state))
> - continue;
> - if (!dev->name)
> - continue;
> - if (!first_dev || dev->devid < first_dev->devid)
> - first_dev = dev;
> - }
> - cur_devices = cur_devices->seed;
> + list_for_each_entry_rcu(dev, &fs_info->fs_devices->devices, dev_list) {
> + if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state))
> + continue;
> + if (!dev->name)
> + continue;
> + if (!first_dev || dev->devid < first_dev->devid)
> + first_dev = dev;
> }
>
> if (first_dev)
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] btrfs_show_devname don't traverse into the seed fsid
2020-07-10 6:37 [PATCH] btrfs_show_devname don't traverse into the seed fsid Anand Jain
` (3 preceding siblings ...)
2020-07-13 14:04 ` [PATCH] btrfs_show_devname don't traverse into the seed fsid Nikolay Borisov
@ 2020-07-15 10:21 ` David Sterba
4 siblings, 0 replies; 13+ messages in thread
From: David Sterba @ 2020-07-15 10:21 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs
On Fri, Jul 10, 2020 at 02:37:38PM +0800, Anand Jain wrote:
> ->show_devname currently shows the lowest devid in the list. As the seed
> devices have the lowest devid in the sprouted filesystem, the userland
> tool such as findmnt end up seeing seed device instead of the device from
> the read-writable sprouted filesystem. As shown below.
>
> mount /dev/sda /btrfs
> mount: /btrfs: WARNING: device write-protected, mounted read-only.
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sda /btrfs 899f7027-3e46-4626-93e7-7d4c9ad19111
>
> btrfs dev add -f /dev/sdb /btrfs
>
> umount /btrfs
> mount /dev/sdb /btrfs
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sda /btrfs 899f7027-3e46-4626-93e7-7d4c9ad19111
>
>
> All sprouts from a single seed will show the same seed device and the
> same fsid. That's messy.
> This is causing problems in our prototype as there isn't any reference
> to the sprout file-system(s) which is being used for actual read and
> write.
>
> This was added in the patch which implemented the show_devname in btrfs
> commit 9c5085c14798 (Btrfs: implement ->show_devname).
> I tried to look for any particular reason that we need to show the seed
> device, there isn't any.
>
> So instead, do not traverse through the seed devices, just show the
> lowest devid in the sprouted fsid.
>
> After the patch:
>
> mount /dev/sda /btrfs
> mount: /btrfs: WARNING: device write-protected, mounted read-only.
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sda /btrfs 899f7027-3e46-4626-93e7-7d4c9ad19111
>
> btrfs dev add -f /dev/sdb /btrfs
> mount -o rw,remount /dev/sdb /btrfs
>
> findmnt --output SOURCE,TARGET,UUID /btrfs
> SOURCE TARGET UUID
> /dev/sdb /btrfs 595ca0e6-b82e-46b5-b9e2-c72a6928be48
>
> mount /dev/sda /btrfs1
> mount: /btrfs1: WARNING: device write-protected, mounted read-only.
>
> btrfs dev add -f /dev/sdc /btrfs1
>
> findmnt --output SOURCE,TARGET,UUID /btrfs1
> SOURCE TARGET UUID
> /dev/sdc /btrfs1 ca1dbb7a-8446-4f95-853c-a20f3f82bdbb
>
> cat /proc/self/mounts | grep btrfs
> /dev/sdb /btrfs btrfs rw,relatime,noacl,space_cache,subvolid=5,subvol=/ 0 0
> /dev/sdc /btrfs1 btrfs ro,relatime,noacl,space_cache,subvolid=5,subvol=/ 0 0
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
Added to misc-next, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread