| Message ID | 20190622153827.4448-1-zlang@redhat.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | [v2] xfs: test xfs_info on block device and mountpoint | expand |
On Sat, Jun 22, 2019 at 11:38:27PM +0800, Zorro Lang wrote: > There was a bug, xfs_info fails on a mounted block device: > > # xfs_info /dev/mapper/testdev > xfs_info: /dev/mapper/testdev contains a mounted filesystem > > fatal error -- couldn't initialize XFS library > > xfsprogs has fixed it by: > > bbb43745 xfs_info: use findmnt to handle mounted block devices > > Signed-off-by: Zorro Lang <zlang@redhat.com> Aha! I remembered something -- xfs/449 already checks for consistency in the various xfs geometry reports that each command provides, so why not just add the $XFS_INFO_PROG $SCRATCH_DEV case at the end? --D > --- > > Thanks the reviewing from Darrick and Eryu, > > V2 did below changes: > 1) Compare the contents between the two xfs_info invocations in test_xfs_info() > 2) document the commit that the case cover > 3) Add more comments > 4) Move the test on unmounted device to the end > > Sorry Eryu, I'll keep the case number next time :) > > Thanks, > Zorro > > tests/xfs/1000 | 82 ++++++++++++++++++++++++++++++++++++++++++++++ > tests/xfs/1000.out | 2 ++ > tests/xfs/group | 1 + > 3 files changed, 85 insertions(+) > create mode 100755 tests/xfs/1000 > create mode 100644 tests/xfs/1000.out > > diff --git a/tests/xfs/1000 b/tests/xfs/1000 > new file mode 100755 > index 00000000..721bcdf2 > --- /dev/null > +++ b/tests/xfs/1000 > @@ -0,0 +1,82 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2019 Red Hat, Inc. All Rights Reserved. > +# > +# FS QA Test No. 1000 > +# > +# test xfs_info on block device and mountpoint, uncover xfsprogs commit: > +# bbb43745 xfs_info: use findmnt to handle mounted block devices > +# > +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 > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# real QA test starts here > +_supported_fs xfs > +_supported_os Linux > +_require_scratch > + > +info_file=$tmp.$seq.info > + > +test_xfs_info() > +{ > + local target="$1" > + local tmpfile=$tmp.$seq.info.tmp > + local need_cmp=0 > + > + # save the *old* xfs_info file, to compare with the new one later > + if [ -f $info_file ]; then > + cat $info_file > $tmpfile > + need_cmp=1 > + fi > + > + $XFS_INFO_PROG $target > $info_file 2>&1 > + if [ $? -ne 0 ];then > + echo "$XFS_INFO_PROG $target fails:" > + cat $info_file > + else > + cat $info_file >> $seqres.full > + fi > + # compare the contents between the two xfs_info invocations > + if [ $need_cmp -eq 1 ]; then > + diff $tmpfile $info_file > + fi > +} > + > +_scratch_mkfs > $seqres.full 2>&1 > +_scratch_mount > +# test mounted block device and mountpoint > +test_xfs_info $SCRATCH_DEV > +test_xfs_info $SCRATCH_MNT > + > +# test on unmounted block device > +_scratch_unmount > +# Due to new xfsprogs use xfs_db 'info' command to get the information of > +# offline XFS, it supports running on a unmounted device. But old xfsprogs > +# doesn't support it, so skip that. > +$XFS_DB_PROG -c "info" $SCRATCH_DEV | grep -q "command info not found" > +if [ $? -ne 0 ]; then > + test_xfs_info $SCRATCH_DEV > +fi > + > +echo "Silence is golden" > +# success, all done > +status=0 > +exit > diff --git a/tests/xfs/1000.out b/tests/xfs/1000.out > new file mode 100644 > index 00000000..681b3b48 > --- /dev/null > +++ b/tests/xfs/1000.out > @@ -0,0 +1,2 @@ > +QA output created by 1000 > +Silence is golden > diff --git a/tests/xfs/group b/tests/xfs/group > index ffe4ae12..047fe332 100644 > --- a/tests/xfs/group > +++ b/tests/xfs/group > @@ -504,3 +504,4 @@ > 504 auto quick mkfs label > 505 auto quick spaceman > 506 auto quick health > +1000 auto quick > -- > 2.17.2 >
On Sun, Jun 23, 2019 at 02:49:19PM -0700, Darrick J. Wong wrote: > On Sat, Jun 22, 2019 at 11:38:27PM +0800, Zorro Lang wrote: > > There was a bug, xfs_info fails on a mounted block device: > > > > # xfs_info /dev/mapper/testdev > > xfs_info: /dev/mapper/testdev contains a mounted filesystem > > > > fatal error -- couldn't initialize XFS library > > > > xfsprogs has fixed it by: > > > > bbb43745 xfs_info: use findmnt to handle mounted block devices > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > Aha! I remembered something -- xfs/449 already checks for consistency > in the various xfs geometry reports that each command provides, so why > not just add the $XFS_INFO_PROG $SCRATCH_DEV case at the end? Wow, there're so many cases, can't sure what we've covered now:) Sure, I can do this change on xfs/449, if Eryu thinks it's fine to increase the test coverage of a known case. Thanks, Zorro > > --D > > > --- > > > > Thanks the reviewing from Darrick and Eryu, > > > > V2 did below changes: > > 1) Compare the contents between the two xfs_info invocations in test_xfs_info() > > 2) document the commit that the case cover > > 3) Add more comments > > 4) Move the test on unmounted device to the end > > > > Sorry Eryu, I'll keep the case number next time :) > > > > Thanks, > > Zorro > > > > tests/xfs/1000 | 82 ++++++++++++++++++++++++++++++++++++++++++++++ > > tests/xfs/1000.out | 2 ++ > > tests/xfs/group | 1 + > > 3 files changed, 85 insertions(+) > > create mode 100755 tests/xfs/1000 > > create mode 100644 tests/xfs/1000.out > > > > diff --git a/tests/xfs/1000 b/tests/xfs/1000 > > new file mode 100755 > > index 00000000..721bcdf2 > > --- /dev/null > > +++ b/tests/xfs/1000 > > @@ -0,0 +1,82 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2019 Red Hat, Inc. All Rights Reserved. > > +# > > +# FS QA Test No. 1000 > > +# > > +# test xfs_info on block device and mountpoint, uncover xfsprogs commit: > > +# bbb43745 xfs_info: use findmnt to handle mounted block devices > > +# > > +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 > > + > > +# remove previous $seqres.full before test > > +rm -f $seqres.full > > + > > +# real QA test starts here > > +_supported_fs xfs > > +_supported_os Linux > > +_require_scratch > > + > > +info_file=$tmp.$seq.info > > + > > +test_xfs_info() > > +{ > > + local target="$1" > > + local tmpfile=$tmp.$seq.info.tmp > > + local need_cmp=0 > > + > > + # save the *old* xfs_info file, to compare with the new one later > > + if [ -f $info_file ]; then > > + cat $info_file > $tmpfile > > + need_cmp=1 > > + fi > > + > > + $XFS_INFO_PROG $target > $info_file 2>&1 > > + if [ $? -ne 0 ];then > > + echo "$XFS_INFO_PROG $target fails:" > > + cat $info_file > > + else > > + cat $info_file >> $seqres.full > > + fi > > + # compare the contents between the two xfs_info invocations > > + if [ $need_cmp -eq 1 ]; then > > + diff $tmpfile $info_file > > + fi > > +} > > + > > +_scratch_mkfs > $seqres.full 2>&1 > > +_scratch_mount > > +# test mounted block device and mountpoint > > +test_xfs_info $SCRATCH_DEV > > +test_xfs_info $SCRATCH_MNT > > + > > +# test on unmounted block device > > +_scratch_unmount > > +# Due to new xfsprogs use xfs_db 'info' command to get the information of > > +# offline XFS, it supports running on a unmounted device. But old xfsprogs > > +# doesn't support it, so skip that. > > +$XFS_DB_PROG -c "info" $SCRATCH_DEV | grep -q "command info not found" > > +if [ $? -ne 0 ]; then > > + test_xfs_info $SCRATCH_DEV > > +fi > > + > > +echo "Silence is golden" > > +# success, all done > > +status=0 > > +exit > > diff --git a/tests/xfs/1000.out b/tests/xfs/1000.out > > new file mode 100644 > > index 00000000..681b3b48 > > --- /dev/null > > +++ b/tests/xfs/1000.out > > @@ -0,0 +1,2 @@ > > +QA output created by 1000 > > +Silence is golden > > diff --git a/tests/xfs/group b/tests/xfs/group > > index ffe4ae12..047fe332 100644 > > --- a/tests/xfs/group > > +++ b/tests/xfs/group > > @@ -504,3 +504,4 @@ > > 504 auto quick mkfs label > > 505 auto quick spaceman > > 506 auto quick health > > +1000 auto quick > > -- > > 2.17.2 > >
On Mon, Jun 24, 2019 at 09:21:03AM +0800, Zorro Lang wrote: > On Sun, Jun 23, 2019 at 02:49:19PM -0700, Darrick J. Wong wrote: > > On Sat, Jun 22, 2019 at 11:38:27PM +0800, Zorro Lang wrote: > > > There was a bug, xfs_info fails on a mounted block device: > > > > > > # xfs_info /dev/mapper/testdev > > > xfs_info: /dev/mapper/testdev contains a mounted filesystem > > > > > > fatal error -- couldn't initialize XFS library > > > > > > xfsprogs has fixed it by: > > > > > > bbb43745 xfs_info: use findmnt to handle mounted block devices > > > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > > > Aha! I remembered something -- xfs/449 already checks for consistency > > in the various xfs geometry reports that each command provides, so why > > not just add the $XFS_INFO_PROG $SCRATCH_DEV case at the end? > > Wow, there're so many cases, can't sure what we've covered now:) > > Sure, I can do this change on xfs/449, if Eryu thinks it's fine to increase > the test coverage of a known case. Given that we're having more and more tests and the test time grows quickly, I'm fine now with adding such small & similar test to existing test case to reuse the test setups, especially when XFS maintainer agrees to do so :) Thanks, Eryu
On Tue, Jun 25, 2019 at 10:45:46AM +0800, Eryu Guan wrote: > On Mon, Jun 24, 2019 at 09:21:03AM +0800, Zorro Lang wrote: > > On Sun, Jun 23, 2019 at 02:49:19PM -0700, Darrick J. Wong wrote: > > > On Sat, Jun 22, 2019 at 11:38:27PM +0800, Zorro Lang wrote: > > > > There was a bug, xfs_info fails on a mounted block device: > > > > > > > > # xfs_info /dev/mapper/testdev > > > > xfs_info: /dev/mapper/testdev contains a mounted filesystem > > > > > > > > fatal error -- couldn't initialize XFS library > > > > > > > > xfsprogs has fixed it by: > > > > > > > > bbb43745 xfs_info: use findmnt to handle mounted block devices > > > > > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > > > > > Aha! I remembered something -- xfs/449 already checks for consistency > > > in the various xfs geometry reports that each command provides, so why > > > not just add the $XFS_INFO_PROG $SCRATCH_DEV case at the end? Hmm... But I hope the case can keep running xfs_info test even there're not xfs_spaceman -c "info" or xfs_db -c "info", just skip these two steps. Due to RHEL-7 has old xfsprogs, we'd like to cover bug on RHEL-7. What do you think? > > > > Wow, there're so many cases, can't sure what we've covered now:) > > > > Sure, I can do this change on xfs/449, if Eryu thinks it's fine to increase > > the test coverage of a known case. > > Given that we're having more and more tests and the test time grows > quickly, I'm fine now with adding such small & similar test to existing > test case to reuse the test setups, especially when XFS maintainer > agrees to do so :) > > Thanks, > Eryu
On Tue, Jun 25, 2019 at 03:16:39PM +0800, Zorro Lang wrote: > On Tue, Jun 25, 2019 at 10:45:46AM +0800, Eryu Guan wrote: > > On Mon, Jun 24, 2019 at 09:21:03AM +0800, Zorro Lang wrote: > > > On Sun, Jun 23, 2019 at 02:49:19PM -0700, Darrick J. Wong wrote: > > > > On Sat, Jun 22, 2019 at 11:38:27PM +0800, Zorro Lang wrote: > > > > > There was a bug, xfs_info fails on a mounted block device: > > > > > > > > > > # xfs_info /dev/mapper/testdev > > > > > xfs_info: /dev/mapper/testdev contains a mounted filesystem > > > > > > > > > > fatal error -- couldn't initialize XFS library > > > > > > > > > > xfsprogs has fixed it by: > > > > > > > > > > bbb43745 xfs_info: use findmnt to handle mounted block devices > > > > > > > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > > > > > > > Aha! I remembered something -- xfs/449 already checks for consistency > > > > in the various xfs geometry reports that each command provides, so why > > > > not just add the $XFS_INFO_PROG $SCRATCH_DEV case at the end? > > Hmm... But I hope the case can keep running xfs_info test even there're not > xfs_spaceman -c "info" or xfs_db -c "info", just skip these two steps. Due > to RHEL-7 has old xfsprogs, we'd like to cover bug on RHEL-7. > > What do you think? If there isn't an xfs_db -c info command then xfs_info <blockev> won't work because that's what it does internally. Sooo unless you're backporting the new xfs_db info command to rhel7 xfsprogs as well as the new xfs_info wrapper, the test ought to just _notrun on rhel7. --D > > > > > > Wow, there're so many cases, can't sure what we've covered now:) > > > > > > Sure, I can do this change on xfs/449, if Eryu thinks it's fine to increase > > > the test coverage of a known case. > > > > Given that we're having more and more tests and the test time grows > > quickly, I'm fine now with adding such small & similar test to existing > > test case to reuse the test setups, especially when XFS maintainer > > agrees to do so :) > > > > Thanks, > > Eryu
On Tue, Jun 25, 2019 at 07:36:16AM -0700, Darrick J. Wong wrote: > On Tue, Jun 25, 2019 at 03:16:39PM +0800, Zorro Lang wrote: > > On Tue, Jun 25, 2019 at 10:45:46AM +0800, Eryu Guan wrote: > > > On Mon, Jun 24, 2019 at 09:21:03AM +0800, Zorro Lang wrote: > > > > On Sun, Jun 23, 2019 at 02:49:19PM -0700, Darrick J. Wong wrote: > > > > > On Sat, Jun 22, 2019 at 11:38:27PM +0800, Zorro Lang wrote: > > > > > > There was a bug, xfs_info fails on a mounted block device: > > > > > > > > > > > > # xfs_info /dev/mapper/testdev > > > > > > xfs_info: /dev/mapper/testdev contains a mounted filesystem > > > > > > > > > > > > fatal error -- couldn't initialize XFS library > > > > > > > > > > > > xfsprogs has fixed it by: > > > > > > > > > > > > bbb43745 xfs_info: use findmnt to handle mounted block devices > > > > > > > > > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > > > > > > > > > Aha! I remembered something -- xfs/449 already checks for consistency > > > > > in the various xfs geometry reports that each command provides, so why > > > > > not just add the $XFS_INFO_PROG $SCRATCH_DEV case at the end? > > > > Hmm... But I hope the case can keep running xfs_info test even there're not > > xfs_spaceman -c "info" or xfs_db -c "info", just skip these two steps. Due > > to RHEL-7 has old xfsprogs, we'd like to cover bug on RHEL-7. > > > > What do you think? > > If there isn't an xfs_db -c info command then xfs_info <blockev> won't > work because that's what it does internally. > > Sooo unless you're backporting the new xfs_db info command to rhel7 > xfsprogs as well as the new xfs_info wrapper, the test ought to just > _notrun on rhel7. No, I'm not trying to test an unmount <blockdev>. I'm trying to run xfs_info on a mounted <blockdev>. There was a bug. RHEL-7 xfsprogs can get a mounted device info: # xfs_growfs -p xfs_info -n /dev/mapper/home meta-data=/dev/mapper/home isize=512 agcount=4, agsize=47341056 blks = sectsz=512 attr=2, projid32bit=1 = crc=1 finobt=0 spinodes=0 data = bsize=4096 blocks=189364224, imaxpct=25 = sunit=0 swidth=0 blks naming =version 2 bsize=4096 ascii-ci=0 ftype=1 log =internal bsize=4096 blocks=92463, version=2 = sectsz=512 sunit=0 blks, lazy-count=1 realtime =none extsz=4096 blocks=0, rtextents=0 But RHEL-8.0 xfsprogs can't do that: # xfs_info /dev/mapper/xfshome xfs_info: /dev/mapper/xfshome contains a mounted filesystem fatal error -- couldn't initialize XFS library Thanks, Zorro > > --D > > > > > > > > > Wow, there're so many cases, can't sure what we've covered now:) > > > > > > > > Sure, I can do this change on xfs/449, if Eryu thinks it's fine to increase > > > > the test coverage of a known case. > > > > > > Given that we're having more and more tests and the test time grows > > > quickly, I'm fine now with adding such small & similar test to existing > > > test case to reuse the test setups, especially when XFS maintainer > > > agrees to do so :) > > > > > > Thanks, > > > Eryu
diff --git a/tests/xfs/1000 b/tests/xfs/1000 new file mode 100755 index 00000000..721bcdf2 --- /dev/null +++ b/tests/xfs/1000 @@ -0,0 +1,82 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2019 Red Hat, Inc. All Rights Reserved. +# +# FS QA Test No. 1000 +# +# test xfs_info on block device and mountpoint, uncover xfsprogs commit: +# bbb43745 xfs_info: use findmnt to handle mounted block devices +# +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 + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here +_supported_fs xfs +_supported_os Linux +_require_scratch + +info_file=$tmp.$seq.info + +test_xfs_info() +{ + local target="$1" + local tmpfile=$tmp.$seq.info.tmp + local need_cmp=0 + + # save the *old* xfs_info file, to compare with the new one later + if [ -f $info_file ]; then + cat $info_file > $tmpfile + need_cmp=1 + fi + + $XFS_INFO_PROG $target > $info_file 2>&1 + if [ $? -ne 0 ];then + echo "$XFS_INFO_PROG $target fails:" + cat $info_file + else + cat $info_file >> $seqres.full + fi + # compare the contents between the two xfs_info invocations + if [ $need_cmp -eq 1 ]; then + diff $tmpfile $info_file + fi +} + +_scratch_mkfs > $seqres.full 2>&1 +_scratch_mount +# test mounted block device and mountpoint +test_xfs_info $SCRATCH_DEV +test_xfs_info $SCRATCH_MNT + +# test on unmounted block device +_scratch_unmount +# Due to new xfsprogs use xfs_db 'info' command to get the information of +# offline XFS, it supports running on a unmounted device. But old xfsprogs +# doesn't support it, so skip that. +$XFS_DB_PROG -c "info" $SCRATCH_DEV | grep -q "command info not found" +if [ $? -ne 0 ]; then + test_xfs_info $SCRATCH_DEV +fi + +echo "Silence is golden" +# success, all done +status=0 +exit diff --git a/tests/xfs/1000.out b/tests/xfs/1000.out new file mode 100644 index 00000000..681b3b48 --- /dev/null +++ b/tests/xfs/1000.out @@ -0,0 +1,2 @@ +QA output created by 1000 +Silence is golden diff --git a/tests/xfs/group b/tests/xfs/group index ffe4ae12..047fe332 100644 --- a/tests/xfs/group +++ b/tests/xfs/group @@ -504,3 +504,4 @@ 504 auto quick mkfs label 505 auto quick spaceman 506 auto quick health +1000 auto quick
There was a bug, xfs_info fails on a mounted block device: # xfs_info /dev/mapper/testdev xfs_info: /dev/mapper/testdev contains a mounted filesystem fatal error -- couldn't initialize XFS library xfsprogs has fixed it by: bbb43745 xfs_info: use findmnt to handle mounted block devices Signed-off-by: Zorro Lang <zlang@redhat.com> --- Thanks the reviewing from Darrick and Eryu, V2 did below changes: 1) Compare the contents between the two xfs_info invocations in test_xfs_info() 2) document the commit that the case cover 3) Add more comments 4) Move the test on unmounted device to the end Sorry Eryu, I'll keep the case number next time :) Thanks, Zorro tests/xfs/1000 | 82 ++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/1000.out | 2 ++ tests/xfs/group | 1 + 3 files changed, 85 insertions(+) create mode 100755 tests/xfs/1000 create mode 100644 tests/xfs/1000.out