xfs: test xfs_info on block device and mountpoint

PATCH In review Zorro Lang 3 participants 7 messages Jun 22, 2019 – Jun 25, 2019

lore

Share & tools

lore

Zorro Lang

author

2019-06-22 15:38 UTC #0 lore raw

PATCH v2 1/1 xfs: test xfs_info on block device and mountpoint

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 <[email protected]>

---

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

tests/xfs/1000 new file +82 -0

new file mode 100755
index 00000000..721bcdf2
@@ -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

tests/xfs/1000.out new file +2 -0

new file mode 100644
index 00000000..681b3b48
@@ -0,0 +1,2 @@
+QA output created by 1000
+Silence is golden

tests/xfs/group +1 -0

index ffe4ae12..047fe332 100644
@@ -504,3 +504,4 @@
 504 auto quick mkfs label
 505 auto quick spaceman
 506 auto quick health
+1000 auto quick

Signature · 1 line

2.17.2

Darrick J. Wong

2019-06-24 01:41 UTC #1 lore raw

Quoted 5 lines · Zorro Lang

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

Quoted 7 lines

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 <[email protected]>

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

Quoted 130 lines

---

Thanks the reviewing from Darrick and Eryu,

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

tests/xfs/1000 new file +82 -0

new file mode 100755
index 00000000..721bcdf2
@@ -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

tests/xfs/1000.out new file +2 -0

new file mode 100644
index 00000000..681b3b48
@@ -0,0 +1,2 @@
+QA output created by 1000
+Silence is golden

tests/xfs/group +1 -0

index ffe4ae12..047fe332 100644
@@ -504,3 +504,4 @@
 504 auto quick mkfs label
 505 auto quick spaceman
 506 auto quick health
+1000 auto quick

Signature · 1 line

2.17.2

Zorro Lang

author

2019-06-24 01:54 UTC #2 lore raw

Quoted 10 lines · Darrick J. Wong

Quoted 9 lines · Zorro Lang
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:

Quoted 7 lines

Quoted 3 lines
bbb43745 xfs_info: use findmnt to handle mounted block devices
Signed-off-by Zorro Lang <[email protected]>
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

Quoted 133 lines

--D

Quoted 130 lines

---

Thanks the reviewing from Darrick and Eryu,

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

tests/xfs/1000 new file +82 -0

new file mode 100755
index 00000000..721bcdf2
@@ -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

tests/xfs/1000.out new file +2 -0

new file mode 100644
index 00000000..681b3b48
@@ -0,0 +1,2 @@
+QA output created by 1000
+Silence is golden

tests/xfs/group +1 -0

index ffe4ae12..047fe332 100644
@@ -504,3 +504,4 @@
 504 auto quick mkfs label
 505 auto quick spaceman
 506 auto quick health
+1000 auto quick

Signature · 1 line

2.17.2

Eryu Guan

2019-06-25 02:45 UTC #3 lore raw

Quoted 15 lines · Zorro Lang

Quoted 14 lines · Darrick J. Wong
Quoted 12 lines · Zorro Lang
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 <[email protected]>

Quoted 8 lines

Quoted 3 lines
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

Zorro Lang

author

2019-06-25 07:11 UTC #4 lore raw

Quoted 12 lines · Eryu Guan

Quoted 11 lines · Zorro Lang
Quoted 10 lines · Darrick J. Wong
Quoted 9 lines · Zorro Lang
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:

Quoted 7 lines

Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
bbb43745 xfs_info: use findmnt to handle mounted block devices
Signed-off-by Zorro Lang <[email protected]>
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?

Quoted 13 lines

Quoted 5 lines
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

Darrick J. Wong

2019-06-25 14:36 UTC #5 lore raw

Quoted 21 lines · Zorro Lang

Quoted 12 lines · Eryu Guan
Quoted 11 lines · Zorro Lang
Quoted 10 lines · Darrick J. Wong
Quoted 9 lines · Zorro Lang
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:
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
bbb43745 xfs_info: use findmnt to handle mounted block devices
Signed-off-by Zorro Lang <[email protected]>
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?

Quoted 5 lines

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

Quoted 13 lines

Quoted 13 lines
Quoted 5 lines
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

Zorro Lang

author

2019-06-25 17:07 UTC #6 lore raw

Quoted 26 lines · Darrick J. Wong

Quoted 25 lines · Zorro Lang
Quoted 12 lines · Eryu Guan
Quoted 11 lines · Zorro Lang
Quoted 10 lines · Darrick J. Wong
Quoted 9 lines · Zorro Lang
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:
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
bbb43745 xfs_info: use findmnt to handle mounted block devices
Signed-off-by Zorro Lang <[email protected]>
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.

Quoted 8 lines

Quoted 1 line
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

Quoted 16 lines

--D
Quoted 13 lines
Quoted 13 lines
Quoted 5 lines
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