On 26.10.18 г. 18:34 ч., Anand Jain wrote: > > > On 10/26/2018 11:02 PM, Nikolay Borisov wrote: >> >> >> On 8.10.18 г. 21:28 ч., Anand Jain wrote: >>> We have a known bug in btrfs, that we let the device path be changed >>> after the device has been mounted. So using this loop hole the new >>> copied device would appears as if its mounted immediately after its >>> been copied. So this test case reproduces this issue. >>> >>> For example: >>> >>> Initially.. /dev/mmcblk0p4 is mounted as / >>> >>> lsblk >>> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT >>> mmcblk0 179:0 0 29.2G 0 disk >>> |-mmcblk0p4 179:4 0 4G 0 part / >>> |-mmcblk0p2 179:2 0 500M 0 part /boot >>> |-mmcblk0p3 179:3 0 256M 0 part [SWAP] >>> `-mmcblk0p1 179:1 0 256M 0 part /boot/efi >>> >>> btrfs fi show >>> Label: none uuid: 07892354-ddaa-4443-90ea-f76a06accaba >>> Total devices 1 FS bytes used 1.40GiB >>> devid 1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4 >>> >>> Copy mmcblk0 to sda >>> dd if=/dev/mmcblk0 of=/dev/sda >>> >>> And immediately after the copy completes the change in the device >>> superblock is notified which the automount scans using >>> btrfs device scan and the new device sda becomes the mounted root >>> device. >>> >>> lsblk >>> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT >>> sda 8:0 1 14.9G 0 disk >>> |-sda4 8:4 1 4G 0 part / >>> |-sda2 8:2 1 500M 0 part >>> |-sda3 8:3 1 256M 0 part >>> `-sda1 8:1 1 256M 0 part >>> mmcblk0 179:0 0 29.2G 0 disk >>> |-mmcblk0p4 179:4 0 4G 0 part >>> |-mmcblk0p2 179:2 0 500M 0 part /boot >>> |-mmcblk0p3 179:3 0 256M 0 part [SWAP] >>> `-mmcblk0p1 179:1 0 256M 0 part /boot/efi >>> btrfs fi show / >>> Label: none uuid: 07892354-ddaa-4443-90ea-f76a06accaba >>> Total devices 1 FS bytes used 1.40GiB >>> devid 1 size 4.00GiB used 3.00GiB path /dev/sda4 >>> >>> The bug is quite nasty that you can't either unmount /dev/sda4 or >>> /dev/mmcblk0p4. And the problem does not get solved until you take >>> the sda out of the system on to another system to change its fsid using >>> the 'btrfstune -u' command. >> >> Is there a pending fix for this? > > Yes. > https://patchwork.kernel.org/patch/10641041/ > > Test case header mentioned it. > >> >>> >>> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx> >>> --- >>> v1->v2: >>> dont play around with dev patch use it as it is. >>> do not use SCRATCH_MNT instead create it at the TEST_DIR and its >>> related >>> changes. >>> golden out changes >>> tests/btrfs/173 | 88 >>> +++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> tests/btrfs/173.out | 6 ++++ >>> tests/btrfs/group | 1 + >>> 3 files changed, 95 insertions(+) >>> create mode 100755 tests/btrfs/173 >>> create mode 100644 tests/btrfs/173.out >>> >>> diff --git a/tests/btrfs/173 b/tests/btrfs/173 >>> new file mode 100755 >>> index 000000000000..b466ae921e19 >>> --- /dev/null >>> +++ b/tests/btrfs/173 >>> @@ -0,0 +1,88 @@ >>> +#! /bin/bash >>> +# SPDX-License-Identifier: GPL-2.0 >>> +# Copyright (c) 2018 Oracle. All Rights Reserved. >>> +# >>> +# FS QA Test 173 >>> +# >>> +# Fuzzy test for FS image duplication. >>> +# Could be fixed by >>> +# [patch] btrfs: harden agaist duplicate 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 >>> + >>> +mnt=$TEST_DIR/$seq.mnt >>> +_cleanup() >>> +{ >>> + rm -rf $mnt > /dev/null 2>&1 >>> + 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 >>> + >>> +# Modify as appropriate. >>> +_supported_fs btrfs >>> +_supported_os Linux >>> +_require_scratch_dev_pool 2 >>> +_scratch_dev_pool_get 2 >>> + >>> +dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}') >>> +dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}') >> >> Naming devices _foo and _bar just shows you could not care less about >> the test. > > Hmm. Will make it device_1 and device_2. > >> Either use sane names - primary_dev/secondary dev or device_1 >> and device_2. > >>> + >>> +echo dev_foo=$dev_foo >> $seqres.full >>> +echo dev_bar=$dev_bar >> $seqres.full >>> +echo | tee -a $seqres.full >>> + >>> +rm -rf $mnt > /dev/null 2>&1 >> >> So what is $mnt? I can see it's used by very few tests and it's not >> obvious? Generally you should either define it as a local variable or >> use one of the SCRATCH_MNT/TEST_MNT global variables. Also I checked >> with Eric re. the use of $mnt in tests and his conclusion is : >> >> "<sandeen> looks like a bug" > > No its not. As few lines above, I have assigned it as.. > mnt=$TEST_DIR/$seq.mnt I missed that, I will recommend moving the assignment near where you set the devices > > >>> +mkdir $mnt >>> +_mkfs_dev $dev_foo >>> +_mount $dev_foo $mnt >>> + >>> +check_btrfs_mount() >>> +{ >>> + local x=$(findmnt $mnt | grep -v TARGET | awk '{print $2}') >>> + [[ $x == $dev_foo ]] && echo DEV_FOO >>> + [[ $x == $dev_bar ]] && echo DEV_BAR >>> +} >> >> Same thing here re. DEV_(FOO|BAR). >> >>> + >>> +echo MNT $(check_btrfs_mount) >>> + >>> +for sb_bytenr in 65536 67108864 >>> +do >>> + echo -n "dd status=none if=$dev_foo of=$dev_bar bs=1 "\ >>> + "seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full >>> + dd status=none if=$dev_foo of=$dev_bar bs=1 seek=$sb_bytenr \ >>> + skip=$sb_bytenr count=4096 >> $seqres.full 2>&1 >>> + echo ..:$? >> $seqres.full >> >> This is an overkill, just use dd ... >/dev/null 2>&1, no one cares about >> the output of that. Also use DIO so that sb's are directly written to >> the devices. > > Yeah right will fix. > >>> +done >>> + >>> +#Original device is mounted, scan of its clone should fail >>> +$BTRFS_UTIL_PROG device scan $dev_bar >> $seqres.full 2>&1 >>> +echo btrfs device scan dev_bar ...:$?| tee -a $seqres.full >>> + >>> +echo MNT $(check_btrfs_mount) >>> + >>> +#Original device scan should be successful >>> +$BTRFS_UTIL_PROG device scan $dev_foo >> $seqres.full 2>&1 >>> +echo btrfs device scan dev_foo ...:$?| tee -a $seqres.full >>> + >>> +umount $mnt > /dev/null 2>&1 >>> +_scratch_dev_pool_put >>> + >>> +# success, all done >>> +status=0 >>> +exit >>> diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out >>> new file mode 100644 >>> index 000000000000..3c7e3fb4e3f7 >>> --- /dev/null >>> +++ b/tests/btrfs/173.out >>> @@ -0,0 +1,6 @@ >>> +QA output created by 173 >>> + >>> +MNT DEV_FOO >>> +btrfs device scan dev_bar ...:1 >>> +MNT DEV_FOO >>> +btrfs device scan dev_foo ...:0 >> >> It seems the output is really pointless it should just be "Silence is >> golden". Then in the test you would do all your run-time checks of >> what's the source of the mount etc etc and if your expectations don't >> match you fail the test and don't output "Silence is golden" which would >> signal success. Check for example > > I could do that. Will fix. > > Thanks, Anand > >>> diff --git a/tests/btrfs/group b/tests/btrfs/group >>> index 45782565c3b7..b2f1393f3e97 100644 >>> --- a/tests/btrfs/group >>> +++ b/tests/btrfs/group >>> @@ -175,3 +175,4 @@ >>> 170 auto quick snapshot >>> 171 auto quick qgroup >>> 172 auto quick punch >>> +173 volume >>> >
