[PATCH] btrfs_show_devname don't traverse into the seed fsid

[PATCH] btrfs_show_devname don't traverse into the seed fsid

[Anand Jain]

->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 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)
-- 
2.25.1

Re: [PATCH] btrfs_show_devname don't traverse into the seed fsid

[Josef Bacik]

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

Re: [PATCH] btrfs_show_devname don't traverse into the seed fsid

[Anand Jain]

> 
> This needs to come with an xfstest so we do not regress this in the 
> future.  Thanks,

  Oh yes. Makes sense. An xfstests test case is on its way.

Thanks, Anand

> 
> Josef

Re: [PATCH] btrfs_show_devname don't traverse into the seed fsid

[Martin K. Petersen]

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

[PATCH] fstests: btrfs test if show_devname returns sprout device

[Anand Jain]

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

Re: [PATCH] fstests: btrfs test if show_devname returns sprout device

[Nikolay Borisov]

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
> 

[PATCH v2] fstests: btrfs test if show_devname returns sprout device

[Anand Jain]

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

Re: [PATCH v2] fstests: btrfs test if show_devname returns sprout device

[Nikolay Borisov]

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
> 

Re: [PATCH v2] fstests: btrfs test if show_devname returns sprout device

[Anand Jain]

>> +# 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

Re: [PATCH] btrfs_show_devname don't traverse into the seed fsid

[Nikolay Borisov]

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

Re: [PATCH] btrfs_show_devname don't traverse into the seed fsid

[David Sterba]

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.

Re: [PATCH v2] fstests: btrfs test if show_devname returns sprout device

[Eryu Guan]

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

[PATCH v3] fstests: btrfs test if show_devname returns sprout device

[Anand Jain]

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