* [PATCH] xfs/444: add check for xfs_db write bno array @ 2018-04-12 10:00 yang xu 2018-04-12 12:09 ` Dave Chinner 0 siblings, 1 reply; 13+ messages in thread From: yang xu @ 2018-04-12 10:00 UTC (permalink / raw) To: fstests; +Cc: yang xu On RHEL7.5Alpha, the test case can trigger an xfsprogs bug that xfs_db writes bno array failed ,such as: $ xfs_db -x -c "agfl 0" -c "write bno[32] 78" /dev/sda9 bno[32] = 32:null 33:null 34:null 35:null 36:null 37:null ... the correct result as below: bno[32] = 78 On xfs/444 the following code triggers this bug: cat $tmp.remap | while read dest_pos bno junk; do _scratch_xfs_set_metadata_field "bno[$dest_pos]" "$bno" \ "agfl 0" >> $seqres.full done It is because xfs_db on RHEL7.5Alpha doesn't support write via array indexing. The problem has been fixed since xfprogs commit 4222d000ed ("db: write via array indexing doesn't work"). But the fix patch has not been merged into RHEL7.5Alpha. For users, they should know the reason for the failure of the test is a filesystem bug or an xfsprogs bug. So, we add check for xfs_db write bno array. Signed-off-by: yang xu <xuyang.jy@cn.fujitsu.com> --- tests/xfs/444 | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/xfs/444 b/tests/xfs/444 index 141be52..fe80502 100755 --- a/tests/xfs/444 +++ b/tests/xfs/444 @@ -58,6 +58,22 @@ _require_test_program "punch-alternating" # This is only a v5 filesystem problem _require_scratch_xfs_crc +check_xfs_db_write_bno_array() { + _scratch_xfs_set_metadata_field "bno[32]" "78" "agfl 0" >> $seqres.full + + # Before xfsprogs commit 0ebbf1d58898 ("db: limit AGFL bno + # arrayi printing), When asking for a single agfl entry, + # the result outputs the entire remainder of the array + # starting at the given index. + # It is difficult to extract single entry values. + # So filter them. + bno2=$(_scratch_xfs_get_metadata_field "bno[32]" \ + "agfl 0" |sed -e 's/ .*$//g' | sed -e 's/^.*://g') + echo "bno[32] set 78 get $bno2" >> $seqres.full + + [ "${bno2}" != "78" ] && _fail "xfs_db write can't support bno array" +} + mount_loop() { if ! _try_scratch_mount >> $seqres.full 2>&1; then echo "scratch mount failed" >> $seqres.full @@ -233,6 +249,10 @@ ENDL diff -u $tmp.repair $tmp.remount >> $seqres.full } +#Before xfsprogs commit 4222d000ed("db: write via array indexing doesn't work"). +#xfs_db command to write a specific AGFL index doesn't work. We should check it. +check_xfs_db_write_bno_array + runtest fix_end runtest fix_start runtest fix_wrap -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] xfs/444: add check for xfs_db write bno array 2018-04-12 10:00 [PATCH] xfs/444: add check for xfs_db write bno array yang xu @ 2018-04-12 12:09 ` Dave Chinner 2018-04-12 13:00 ` Eryu Guan 0 siblings, 1 reply; 13+ messages in thread From: Dave Chinner @ 2018-04-12 12:09 UTC (permalink / raw) To: yang xu; +Cc: fstests On Thu, Apr 12, 2018 at 06:00:30PM +0800, yang xu wrote: > On RHEL7.5Alpha, the test case can trigger an xfsprogs bug that xfs_db > writes bno array failed ,such as: > $ xfs_db -x -c "agfl 0" -c "write bno[32] 78" /dev/sda9 > bno[32] = 32:null 33:null 34:null 35:null 36:null 37:null ... > > the correct result as below: > bno[32] = 78 > > On xfs/444 the following code triggers this bug: > cat $tmp.remap | while read dest_pos bno junk; do > _scratch_xfs_set_metadata_field "bno[$dest_pos]" "$bno" \ > "agfl 0" >> $seqres.full > done > > It is because xfs_db on RHEL7.5Alpha doesn't support write via array > indexing. The problem has been fixed since xfprogs commit 4222d000ed > ("db: write via array indexing doesn't work"). But the fix patch has > not been merged into RHEL7.5Alpha. > > For users, they should know the reason for the failure of the test is > a filesystem bug or an xfsprogs bug. So, we add check for xfs_db > write bno array. > > Signed-off-by: yang xu <xuyang.jy@cn.fujitsu.com> > --- > tests/xfs/444 | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/tests/xfs/444 b/tests/xfs/444 > index 141be52..fe80502 100755 > --- a/tests/xfs/444 > +++ b/tests/xfs/444 > @@ -58,6 +58,22 @@ _require_test_program "punch-alternating" > # This is only a v5 filesystem problem > _require_scratch_xfs_crc > > +check_xfs_db_write_bno_array() { _require_xfs_db_write_array(), and move to common/xfs. > + _scratch_xfs_set_metadata_field "bno[32]" "78" "agfl 0" >> $seqres.full > + > + # Before xfsprogs commit 0ebbf1d58898 ("db: limit AGFL bno > + # arrayi printing), When asking for a single agfl entry, > + # the result outputs the entire remainder of the array > + # starting at the given index. > + # It is difficult to extract single entry values. > + # So filter them. > + bno2=$(_scratch_xfs_get_metadata_field "bno[32]" \ > + "agfl 0" |sed -e 's/ .*$//g' | sed -e 's/^.*://g') > + echo "bno[32] set 78 get $bno2" >> $seqres.full > + > + [ "${bno2}" != "78" ] && _fail "xfs_db write can't support bno array" test should _notrun is db doesn't have therequired support, not _fail. > +#Before xfsprogs commit 4222d000ed("db: write via array indexing doesn't work"). > +#xfs_db command to write a specific AGFL index doesn't work. We should check it. > +check_xfs_db_write_bno_array This comment belongs with the function, not the caller. Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] xfs/444: add check for xfs_db write bno array 2018-04-12 12:09 ` Dave Chinner @ 2018-04-12 13:00 ` Eryu Guan 2018-04-12 23:00 ` Dave Chinner 0 siblings, 1 reply; 13+ messages in thread From: Eryu Guan @ 2018-04-12 13:00 UTC (permalink / raw) To: Dave Chinner, yang xu; +Cc: fstests On Thu, Apr 12, 2018 at 10:09:52PM +1000, Dave Chinner wrote: > On Thu, Apr 12, 2018 at 06:00:30PM +0800, yang xu wrote: > > On RHEL7.5Alpha, the test case can trigger an xfsprogs bug that xfs_db > > writes bno array failed ,such as: > > $ xfs_db -x -c "agfl 0" -c "write bno[32] 78" /dev/sda9 > > bno[32] = 32:null 33:null 34:null 35:null 36:null 37:null ... > > > > the correct result as below: > > bno[32] = 78 > > > > On xfs/444 the following code triggers this bug: > > cat $tmp.remap | while read dest_pos bno junk; do > > _scratch_xfs_set_metadata_field "bno[$dest_pos]" "$bno" \ > > "agfl 0" >> $seqres.full > > done > > > > It is because xfs_db on RHEL7.5Alpha doesn't support write via array > > indexing. The problem has been fixed since xfprogs commit 4222d000ed > > ("db: write via array indexing doesn't work"). But the fix patch has > > not been merged into RHEL7.5Alpha. > > > > For users, they should know the reason for the failure of the test is > > a filesystem bug or an xfsprogs bug. So, we add check for xfs_db > > write bno array. > > > > Signed-off-by: yang xu <xuyang.jy@cn.fujitsu.com> (Please cc xfs list too for xfs-specific test cases in future. I didn't add xfs list in this thread as Dave already replied). > > --- > > tests/xfs/444 | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/tests/xfs/444 b/tests/xfs/444 > > index 141be52..fe80502 100755 > > --- a/tests/xfs/444 > > +++ b/tests/xfs/444 > > @@ -58,6 +58,22 @@ _require_test_program "punch-alternating" > > # This is only a v5 filesystem problem > > _require_scratch_xfs_crc > > > > +check_xfs_db_write_bno_array() { > > _require_xfs_db_write_array(), and move to common/xfs. > > > + _scratch_xfs_set_metadata_field "bno[32]" "78" "agfl 0" >> $seqres.full > > + > > + # Before xfsprogs commit 0ebbf1d58898 ("db: limit AGFL bno > > + # arrayi printing), When asking for a single agfl entry, > > + # the result outputs the entire remainder of the array > > + # starting at the given index. > > + # It is difficult to extract single entry values. > > + # So filter them. > > + bno2=$(_scratch_xfs_get_metadata_field "bno[32]" \ > > + "agfl 0" |sed -e 's/ .*$//g' | sed -e 's/^.*://g') > > + echo "bno[32] set 78 get $bno2" >> $seqres.full > > + > > + [ "${bno2}" != "78" ] && _fail "xfs_db write can't support bno array" > > test should _notrun is db doesn't have therequired support, not > _fail. If xfsprogs commit 4222d000ed ("db: write via array indexing doesn't work") introduced a new feature, I agreed that we need the _require rule and _notrun the test. But this issue looks like a bug in xfsprogs to me, not a missing feature in xfsprogs on RHEL7, so I tend to fail the test instead of adding a new _require rule & _notrun the test. And in this case, IMHO, I don't think it's necessary to do any update to the test, just leave the test as it is and file a new bug in Red Hat bugzilla. > > > +#Before xfsprogs commit 4222d000ed("db: write via array indexing doesn't work"). > > +#xfs_db command to write a specific AGFL index doesn't work. We should check it. > > +check_xfs_db_write_bno_array > > This comment belongs with the function, not the caller. Agreed. Thanks, Eryu > > Cheers, > > Dave. > > -- > Dave Chinner > david@fromorbit.com > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] xfs/444: add check for xfs_db write bno array 2018-04-12 13:00 ` Eryu Guan @ 2018-04-12 23:00 ` Dave Chinner 2018-04-13 4:36 ` Eryu Guan 0 siblings, 1 reply; 13+ messages in thread From: Dave Chinner @ 2018-04-12 23:00 UTC (permalink / raw) To: Eryu Guan; +Cc: yang xu, fstests On Thu, Apr 12, 2018 at 09:00:38PM +0800, Eryu Guan wrote: > On Thu, Apr 12, 2018 at 10:09:52PM +1000, Dave Chinner wrote: > > On Thu, Apr 12, 2018 at 06:00:30PM +0800, yang xu wrote: > > > --- > > > tests/xfs/444 | 20 ++++++++++++++++++++ > > > 1 file changed, 20 insertions(+) > > > > > > diff --git a/tests/xfs/444 b/tests/xfs/444 > > > index 141be52..fe80502 100755 > > > --- a/tests/xfs/444 > > > +++ b/tests/xfs/444 > > > @@ -58,6 +58,22 @@ _require_test_program "punch-alternating" > > > # This is only a v5 filesystem problem > > > _require_scratch_xfs_crc > > > > > > +check_xfs_db_write_bno_array() { > > > > _require_xfs_db_write_array(), and move to common/xfs. > > > > > + _scratch_xfs_set_metadata_field "bno[32]" "78" "agfl 0" >> $seqres.full > > > + > > > + # Before xfsprogs commit 0ebbf1d58898 ("db: limit AGFL bno > > > + # arrayi printing), When asking for a single agfl entry, > > > + # the result outputs the entire remainder of the array > > > + # starting at the given index. > > > + # It is difficult to extract single entry values. > > > + # So filter them. > > > + bno2=$(_scratch_xfs_get_metadata_field "bno[32]" \ > > > + "agfl 0" |sed -e 's/ .*$//g' | sed -e 's/^.*://g') > > > + echo "bno[32] set 78 get $bno2" >> $seqres.full > > > + > > > + [ "${bno2}" != "78" ] && _fail "xfs_db write can't support bno array" > > > > test should _notrun is db doesn't have therequired support, not > > _fail. > > If xfsprogs commit 4222d000ed ("db: write via array indexing doesn't > work") introduced a new feature, I agreed that we need the _require rule > and _notrun the test. _require* rules are not for "new features" - they are for defining the support needed to run the test correctly. This is no different to detecting the mkfs binary version we are running to determine if a certain fix is present or not: # Skip the test if all calls passed - mkfs accepts invalid input _require_xfs_mkfs_validation() { _xfs_mkfs_validation_check if [ "$?" -eq 0 ]; then _notrun "Requires newer mkfs with stricter input checks: the oldest supported version of xfsprogs is 4.7." fi } This is exactly the same sort of situation, because.... > But this issue looks like a bug in xfsprogs to me, not a missing feature > in xfsprogs on RHEL7, so I tend to fail the test instead of adding a new > _require rule & _notrun the test. And in this case, IMHO, I don't think > it's necessary to do any update to the test, just leave the test as it > is and file a new bug in Red Hat bugzilla. ... this isn't a RHEL specific issue - it's an xfsprogs version issue. i.e. any older distro that has a binary with a broken "write array" command will fail this test. None of them are going to get updated xfsprogs packages, so like having an old mkfs.xfs binary, this test should run conditionally on having a version of xfs_db that actually works correctly.... Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] xfs/444: add check for xfs_db write bno array 2018-04-12 23:00 ` Dave Chinner @ 2018-04-13 4:36 ` Eryu Guan 2018-04-13 13:42 ` Dave Chinner 0 siblings, 1 reply; 13+ messages in thread From: Eryu Guan @ 2018-04-13 4:36 UTC (permalink / raw) To: Dave Chinner; +Cc: yang xu, fstests On Fri, Apr 13, 2018 at 09:00:35AM +1000, Dave Chinner wrote: > On Thu, Apr 12, 2018 at 09:00:38PM +0800, Eryu Guan wrote: > > On Thu, Apr 12, 2018 at 10:09:52PM +1000, Dave Chinner wrote: > > > On Thu, Apr 12, 2018 at 06:00:30PM +0800, yang xu wrote: > > > > --- > > > > tests/xfs/444 | 20 ++++++++++++++++++++ > > > > 1 file changed, 20 insertions(+) > > > > > > > > diff --git a/tests/xfs/444 b/tests/xfs/444 > > > > index 141be52..fe80502 100755 > > > > --- a/tests/xfs/444 > > > > +++ b/tests/xfs/444 > > > > @@ -58,6 +58,22 @@ _require_test_program "punch-alternating" > > > > # This is only a v5 filesystem problem > > > > _require_scratch_xfs_crc > > > > > > > > +check_xfs_db_write_bno_array() { > > > > > > _require_xfs_db_write_array(), and move to common/xfs. > > > > > > > + _scratch_xfs_set_metadata_field "bno[32]" "78" "agfl 0" >> $seqres.full > > > > + > > > > + # Before xfsprogs commit 0ebbf1d58898 ("db: limit AGFL bno > > > > + # arrayi printing), When asking for a single agfl entry, > > > > + # the result outputs the entire remainder of the array > > > > + # starting at the given index. > > > > + # It is difficult to extract single entry values. > > > > + # So filter them. > > > > + bno2=$(_scratch_xfs_get_metadata_field "bno[32]" \ > > > > + "agfl 0" |sed -e 's/ .*$//g' | sed -e 's/^.*://g') > > > > + echo "bno[32] set 78 get $bno2" >> $seqres.full > > > > + > > > > + [ "${bno2}" != "78" ] && _fail "xfs_db write can't support bno array" > > > > > > test should _notrun is db doesn't have therequired support, not > > > _fail. > > > > If xfsprogs commit 4222d000ed ("db: write via array indexing doesn't > > work") introduced a new feature, I agreed that we need the _require rule > > and _notrun the test. > > _require* rules are not for "new features" - they are for defining > the support needed to run the test correctly. This is no different Agreed, I was not clear previously, by "new features" I mean changes like new features that change the behavior. > to detecting the mkfs binary version we are running to determine > if a certain fix is present or not: > > # Skip the test if all calls passed - mkfs accepts invalid input > _require_xfs_mkfs_validation() > { > _xfs_mkfs_validation_check > if [ "$?" -eq 0 ]; then > _notrun "Requires newer mkfs with stricter input checks: the oldest supported version of xfsprogs is 4.7." > fi > } Just like this _require rule, it's more like a behavior change that added stricter input checks. > > This is exactly the same sort of situation, because.... > > > But this issue looks like a bug in xfsprogs to me, not a missing feature > > in xfsprogs on RHEL7, so I tend to fail the test instead of adding a new > > _require rule & _notrun the test. And in this case, IMHO, I don't think > > it's necessary to do any update to the test, just leave the test as it > > is and file a new bug in Red Hat bugzilla. > > ... this isn't a RHEL specific issue - it's an xfsprogs version > issue. i.e. any older distro that has a binary with a broken > "write array" command will fail this test. None of them are going to > get updated xfsprogs packages, so like having an old mkfs.xfs > binary, this test should run conditionally on having a version of > xfs_db that actually works correctly.... But I still think it's a pure bug in xfsprogs, not xfsprogs version issue nor a behavior change in xfsprogs, as we did support "write via array indexing", just that it was broken in a certain case, and commit 4222d000ed3b fixed that bug. We should expose bugs by letting the test fail, not paper over it by _notrun the test. Thanks, Eryu ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] xfs/444: add check for xfs_db write bno array 2018-04-13 4:36 ` Eryu Guan @ 2018-04-13 13:42 ` Dave Chinner 2018-04-16 6:00 ` Eryu Guan 0 siblings, 1 reply; 13+ messages in thread From: Dave Chinner @ 2018-04-13 13:42 UTC (permalink / raw) To: Eryu Guan; +Cc: yang xu, fstests On Fri, Apr 13, 2018 at 12:36:12PM +0800, Eryu Guan wrote: > > > But this issue looks like a bug in xfsprogs to me, not a missing feature > > > in xfsprogs on RHEL7, so I tend to fail the test instead of adding a new > > > _require rule & _notrun the test. And in this case, IMHO, I don't think > > > it's necessary to do any update to the test, just leave the test as it > > > is and file a new bug in Red Hat bugzilla. > > > > ... this isn't a RHEL specific issue - it's an xfsprogs version > > issue. i.e. any older distro that has a binary with a broken > > "write array" command will fail this test. None of them are going to > > get updated xfsprogs packages, so like having an old mkfs.xfs > > binary, this test should run conditionally on having a version of > > xfs_db that actually works correctly.... > > But I still think it's a pure bug in xfsprogs, not xfsprogs version > issue nor a behavior change in xfsprogs, as we did support "write via > array indexing", just that it was broken in a certain case, and commit > 4222d000ed3b fixed that bug. We should expose bugs by letting the test > fail, not paper over it by _notrun the test. Yes, it's a bug in xfsprogs. But it's a bug in a diagnostic utility that is only used by test infrastructure and XFS developers. Yes, it's ialso fixed in recent version of xfsprogs, but you know very well that we test distros that have ancient xfsprogs and will never have this issue fixed in them. We use detectiona nd notrun to avoid tests they should not run all the time, and I don't see how this is any different. I really don't understand why you are pushing back on this - why should this specific infrastructure deficiency cause test failures, when all the existing infrastructure support checks cause tests to notrun rather than fail? Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] xfs/444: add check for xfs_db write bno array 2018-04-13 13:42 ` Dave Chinner @ 2018-04-16 6:00 ` Eryu Guan 2018-04-16 9:07 ` [PATCH v2] common/xfs: Add require_xfs_db_write_array function yang xu 0 siblings, 1 reply; 13+ messages in thread From: Eryu Guan @ 2018-04-16 6:00 UTC (permalink / raw) To: Dave Chinner; +Cc: yang xu, fstests On Fri, Apr 13, 2018 at 11:42:10PM +1000, Dave Chinner wrote: > On Fri, Apr 13, 2018 at 12:36:12PM +0800, Eryu Guan wrote: > > > > But this issue looks like a bug in xfsprogs to me, not a missing feature > > > > in xfsprogs on RHEL7, so I tend to fail the test instead of adding a new > > > > _require rule & _notrun the test. And in this case, IMHO, I don't think > > > > it's necessary to do any update to the test, just leave the test as it > > > > is and file a new bug in Red Hat bugzilla. > > > > > > ... this isn't a RHEL specific issue - it's an xfsprogs version > > > issue. i.e. any older distro that has a binary with a broken > > > "write array" command will fail this test. None of them are going to > > > get updated xfsprogs packages, so like having an old mkfs.xfs > > > binary, this test should run conditionally on having a version of > > > xfs_db that actually works correctly.... > > > > But I still think it's a pure bug in xfsprogs, not xfsprogs version > > issue nor a behavior change in xfsprogs, as we did support "write via > > array indexing", just that it was broken in a certain case, and commit > > 4222d000ed3b fixed that bug. We should expose bugs by letting the test > > fail, not paper over it by _notrun the test. > > Yes, it's a bug in xfsprogs. But it's a bug in a diagnostic > utility that is only used by test infrastructure and XFS > developers. OK, I got your point now, it's a bug in a diagnostic tool that is only used by XFS developers as a test infrastructure, so we can treat it as a infrastructure dependency as all other _require rules. I think that makes sense in this case. Thanks for the explanation! yang xu, would you mind sending a v2 patch as Dave suggested? Thanks! Eryu > > Yes, it's ialso fixed in recent version of xfsprogs, but you know > very well that we test distros that have ancient xfsprogs and will > never have this issue fixed in them. We use detectiona nd notrun to > avoid tests they should not run all the time, and I don't see how > this is any different. > > I really don't understand why you are pushing back on this - why > should this specific infrastructure deficiency cause test failures, > when all the existing infrastructure support checks cause tests to > notrun rather than fail? > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] common/xfs: Add require_xfs_db_write_array function 2018-04-16 6:00 ` Eryu Guan @ 2018-04-16 9:07 ` yang xu 2018-04-16 17:22 ` Darrick J. Wong 0 siblings, 1 reply; 13+ messages in thread From: yang xu @ 2018-04-16 9:07 UTC (permalink / raw) To: guaneryu; +Cc: david, fstests, yang xu xfsprogs commit 4222d000ed("db: write via array indexing doesn't work") fixes a bug that xfs_db write can't support array indexing. This function will check whether the bug is fixed on the current xfsprogs. xfs/444 applies the function, and skips if this bug exists. Signed-off-by: yang xu <xuyang.jy@cn.fujitsu.com> --- common/xfs | 18 ++++++++++++++++++ tests/xfs/444 | 1 + 2 files changed, 19 insertions(+) diff --git a/common/xfs b/common/xfs index 3169f87..c4611a1 100644 --- a/common/xfs +++ b/common/xfs @@ -701,3 +701,21 @@ _scratch_xfs_set_sb_field() { _scratch_xfs_set_metadata_field "$1" "$2" "sb 0" } + +#Before xfsprogs commit 4222d000ed("db: write via array indexing doesn't work"), +#xfs_db command to write a specific AGFL index doesn't work. +_require_xfs_db_write_array() +{ + _scratch_xfs_set_metadata_field "bno[32]" "78" "agfl 0" >/dev/null 2>&1 + + # Before xfsprogs commit 0ebbf1d58898 ("db: limit AGFL bno + # arrayi printing), When asking for a single agfl entry, + # the result outputs the entire remainder of the array + # starting at the given index. + # It is difficult to extract single entry values. + # So filter useless information. + bno_value=$(_scratch_xfs_get_metadata_field "bno[32]" \ + "agfl 0" |sed -e 's/ .*$//g' | sed -e 's/^.*://g') + + [ "${bno_value}" != "78" ] && _notrun "xfs_db write can't support array" +} diff --git a/tests/xfs/444 b/tests/xfs/444 index 141be52..9700422 100755 --- a/tests/xfs/444 +++ b/tests/xfs/444 @@ -54,6 +54,7 @@ _supported_os Linux _require_check_dmesg _require_scratch _require_test_program "punch-alternating" +_require_xfs_db_write_array # This is only a v5 filesystem problem _require_scratch_xfs_crc -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] common/xfs: Add require_xfs_db_write_array function 2018-04-16 9:07 ` [PATCH v2] common/xfs: Add require_xfs_db_write_array function yang xu @ 2018-04-16 17:22 ` Darrick J. Wong [not found] ` <5AD5721C.10709@cn.fujitsu.com> 0 siblings, 1 reply; 13+ messages in thread From: Darrick J. Wong @ 2018-04-16 17:22 UTC (permalink / raw) To: yang xu; +Cc: guaneryu, david, fstests On Mon, Apr 16, 2018 at 05:07:35PM +0800, yang xu wrote: > xfsprogs commit 4222d000ed("db: write via array indexing doesn't > work") fixes a bug that xfs_db write can't support array indexing. > This function will check whether the bug is fixed on the current > xfsprogs. > > xfs/444 applies the function, and skips if this bug exists. > > Signed-off-by: yang xu <xuyang.jy@cn.fujitsu.com> > --- > common/xfs | 18 ++++++++++++++++++ > tests/xfs/444 | 1 + > 2 files changed, 19 insertions(+) > > diff --git a/common/xfs b/common/xfs > index 3169f87..c4611a1 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -701,3 +701,21 @@ _scratch_xfs_set_sb_field() > { > _scratch_xfs_set_metadata_field "$1" "$2" "sb 0" > } > + > +#Before xfsprogs commit 4222d000ed("db: write via array indexing doesn't work"), > +#xfs_db command to write a specific AGFL index doesn't work. > +_require_xfs_db_write_array() > +{ > + _scratch_xfs_set_metadata_field "bno[32]" "78" "agfl 0" >/dev/null 2>&1 This is going to fail randomly unless the caller previously called _require_scratch to make sure that there's some kind of xfs fs image sitting on the scratch device (and that there even is a scratch device). Please document that requirement. Alternately, consider formatting a xfs image file in $TEST_DIR and poking that with xfs_db so you don't have to deal with formatting and cleaning up the scratch device. > + > + # Before xfsprogs commit 0ebbf1d58898 ("db: limit AGFL bno > + # arrayi printing), When asking for a single agfl entry, > + # the result outputs the entire remainder of the array > + # starting at the given index. > + # It is difficult to extract single entry values. > + # So filter useless information. > + bno_value=$(_scratch_xfs_get_metadata_field "bno[32]" \ > + "agfl 0" |sed -e 's/ .*$//g' | sed -e 's/^.*://g') > + > + [ "${bno_value}" != "78" ] && _notrun "xfs_db write can't support array" It seems like a bad idea to leave a potentially corrupt scratch fs lying around after this helper exits. --D > +} > diff --git a/tests/xfs/444 b/tests/xfs/444 > index 141be52..9700422 100755 > --- a/tests/xfs/444 > +++ b/tests/xfs/444 > @@ -54,6 +54,7 @@ _supported_os Linux > _require_check_dmesg > _require_scratch > _require_test_program "punch-alternating" > +_require_xfs_db_write_array > > # This is only a v5 filesystem problem > _require_scratch_xfs_crc > -- > 1.8.3.1 > > > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <5AD5721C.10709@cn.fujitsu.com>]
* Re: [PATCH v2] common/xfs: Add require_xfs_db_write_array function [not found] ` <5AD5721C.10709@cn.fujitsu.com> @ 2018-04-17 4:06 ` Darrick J. Wong 2018-04-17 6:11 ` [PATCH v3] " yang xu 0 siblings, 1 reply; 13+ messages in thread From: Darrick J. Wong @ 2018-04-17 4:06 UTC (permalink / raw) To: xuyang.jy; +Cc: guaneryu, david, fstests On Tue, Apr 17, 2018 at 12:03:40PM +0800, xuyang.jy wrote: > on 2018/4/17 1:22, Darrick J. Wong write: > > >On Mon, Apr 16, 2018 at 05:07:35PM +0800, yang xu wrote: > >>xfsprogs commit 4222d000ed("db: write via array indexing doesn't > >>work") fixes a bug that xfs_db write can't support array indexing. > >>This function will check whether the bug is fixed on the current > >>xfsprogs. > >> > >>xfs/444 applies the function, and skips if this bug exists. > >> > >>Signed-off-by: yang xu<xuyang.jy@cn.fujitsu.com> > >>--- > >> common/xfs | 18 ++++++++++++++++++ > >> tests/xfs/444 | 1 + > >> 2 files changed, 19 insertions(+) > >> > >>diff --git a/common/xfs b/common/xfs > >>index 3169f87..c4611a1 100644 > >>--- a/common/xfs > >>+++ b/common/xfs > >>@@ -701,3 +701,21 @@ _scratch_xfs_set_sb_field() > >> { > >> _scratch_xfs_set_metadata_field "$1" "$2" "sb 0" > >> } > >>+ > >>+#Before xfsprogs commit 4222d000ed("db: write via array indexing doesn't work"), > >>+#xfs_db command to write a specific AGFL index doesn't work. > >>+_require_xfs_db_write_array() > >>+{ > >>+ _scratch_xfs_set_metadata_field "bno[32]" "78" "agfl 0">/dev/null 2>&1 > >This is going to fail randomly unless the caller previously called > >_require_scratch to make sure that there's some kind of xfs fs image > >sitting on the scratch device (and that there even is a scratch device). > >Please document that requirement. Alternately, consider formatting a xfs > >image file in $TEST_DIR and poking that with xfs_db so you don't have to > >deal with formatting and cleaning up the scratch device. > Hi Darrick > > We want to create a xfs image file in /tmp/ and use it, as below: > > [root@RHEL7U5Alpha_CLIENT xfstests-dev]# git diff common/xfs > diff --git a/common/xfs b/common/xfs > index 3169f87..39702f7 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -701,3 +701,19 @@ _scratch_xfs_set_sb_field() > { _scratch_xfs_set_metadata_field "$1" "$2" "sb 0" > } > + > +#Before xfsprogs commit 4222d000ed("db: write via array indexing doesn't work"), > +##xfs_db command to write a specific AGFL index doesn't work. > +_require_xfs_db_write_array() > +{ > + local supported=0 > + > + touch /tmp/$$.img > + $MKFS_XFS_PROG -d file,name=/tmp/$$.img,size=512m>/dev/null 2>&1 > + $XFS_DB_PROG -x -c "agfl 0" -c "write bno[32] 78" /tmp/$$.img \ > +>/dev/null 2>&1 > + $XFS_DB_PROG -x -c "agfl 0" -c "print bno[32]" /tmp/$$.img \ > + | grep -q "bno\[32\] = 78"&& supported=1 > + rm -f /tmp/$$.img > + [ $supported -eq 0 ]&& _notrun "xfs_db write can't support array" > +} > [root@RHEL7U5Alpha_CLIENT xfstests-dev]# > > How about this modification? $TEST_DIR/$seqres.img, not /tmp/$$.img Don't assume anything's writable (and big enough for fs images) if it isn't under $TEST_DIR or $SCRATCH_MNT. --D > > >>+ > >>+ # Before xfsprogs commit 0ebbf1d58898 ("db: limit AGFL bno > >>+ # arrayi printing), When asking for a single agfl entry, > >>+ # the result outputs the entire remainder of the array > >>+ # starting at the given index. > >>+ # It is difficult to extract single entry values. > >>+ # So filter useless information. > >>+ bno_value=$(_scratch_xfs_get_metadata_field "bno[32]" \ > >>+ "agfl 0" |sed -e 's/ .*$//g' | sed -e 's/^.*://g') > >>+ > >>+ [ "${bno_value}" != "78" ]&& _notrun "xfs_db write can't support array" > >It seems like a bad idea to leave a potentially corrupt scratch fs lying > >around after this helper exits. > Agreed. I will use a temp xfs image file. > > Thanks > Yang Xu > > >--D > > > >>+} > >>diff --git a/tests/xfs/444 b/tests/xfs/444 > >>index 141be52..9700422 100755 > >>--- a/tests/xfs/444 > >>+++ b/tests/xfs/444 > >>@@ -54,6 +54,7 @@ _supported_os Linux > >> _require_check_dmesg > >> _require_scratch > >> _require_test_program "punch-alternating" > >>+_require_xfs_db_write_array > >> > >> # This is only a v5 filesystem problem > >> _require_scratch_xfs_crc > >>-- > >>1.8.3.1 > >> > >> > >> > >>-- > >>To unsubscribe from this list: send the line "unsubscribe fstests" in > >>the body of a message tomajordomo@vger.kernel.org > >>More majordomo info athttp://vger.kernel.org/majordomo-info.html > >. > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3] common/xfs: Add require_xfs_db_write_array function 2018-04-17 4:06 ` Darrick J. Wong @ 2018-04-17 6:11 ` yang xu 2018-04-17 17:59 ` Darrick J. Wong 0 siblings, 1 reply; 13+ messages in thread From: yang xu @ 2018-04-17 6:11 UTC (permalink / raw) To: darrick.wong; +Cc: guaneryu, david, fstests, yang xu xfsprogs commit 4222d00("db: write via array indexing doesn't work") fixes a bug that xfs_db write can't support array indexing. This function will check whether the bug is fixed on the current xfsprogs. xfs/444 applies the function, and skips if this bug exists. Signed-off-by: yang xu <xuyang.jy@cn.fujitsu.com> --- common/xfs | 17 +++++++++++++++++ tests/xfs/444 | 1 + 2 files changed, 18 insertions(+) diff --git a/common/xfs b/common/xfs index 3169f87..0b59994 100644 --- a/common/xfs +++ b/common/xfs @@ -701,3 +701,20 @@ _scratch_xfs_set_sb_field() { _scratch_xfs_set_metadata_field "$1" "$2" "sb 0" } + +#Before xfsprogs commit 4222d000ed("db: write via array indexing doesn't work"), +##xfs_db command to write a specific AGFL index doesn't work. +_require_xfs_db_write_array() +{ + local supported=0 + + _require_test + touch $TEST_DIR/$seq.img + $MKFS_XFS_PROG -d file,name=$TEST_DIR/$seq.img,size=512m >/dev/null 2>&1 + $XFS_DB_PROG -x -c "agfl 0" -c "write bno[32] 78" $TEST_DIR/$seq.img \ + >/dev/null 2>&1 + $XFS_DB_PROG -x -c "agfl 0" -c "print bno[32]" $TEST_DIR/$seq.img \ + | grep -q "bno\[32\] = 78" && supported=1 + rm -f $TEST_DIR/$seq.img + [ $supported -eq 0 ] && _notrun "xfs_db write can't support array" +} diff --git a/tests/xfs/444 b/tests/xfs/444 index 141be52..9700422 100755 --- a/tests/xfs/444 +++ b/tests/xfs/444 @@ -54,6 +54,7 @@ _supported_os Linux _require_check_dmesg _require_scratch _require_test_program "punch-alternating" +_require_xfs_db_write_array # This is only a v5 filesystem problem _require_scratch_xfs_crc -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3] common/xfs: Add require_xfs_db_write_array function 2018-04-17 6:11 ` [PATCH v3] " yang xu @ 2018-04-17 17:59 ` Darrick J. Wong 2018-04-18 4:29 ` Eryu Guan 0 siblings, 1 reply; 13+ messages in thread From: Darrick J. Wong @ 2018-04-17 17:59 UTC (permalink / raw) To: yang xu; +Cc: guaneryu, david, fstests On Tue, Apr 17, 2018 at 02:11:40PM +0800, yang xu wrote: > xfsprogs commit 4222d00("db: write via array indexing doesn't > work") fixes a bug that xfs_db write can't support array indexing. > This function will check whether the bug is fixed on the current > xfsprogs. > > xfs/444 applies the function, and skips if this bug exists. > > Signed-off-by: yang xu <xuyang.jy@cn.fujitsu.com> > --- > common/xfs | 17 +++++++++++++++++ > tests/xfs/444 | 1 + > 2 files changed, 18 insertions(+) > > diff --git a/common/xfs b/common/xfs > index 3169f87..0b59994 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -701,3 +701,20 @@ _scratch_xfs_set_sb_field() > { > _scratch_xfs_set_metadata_field "$1" "$2" "sb 0" > } > + > +#Before xfsprogs commit 4222d000ed("db: write via array indexing doesn't work"), > +##xfs_db command to write a specific AGFL index doesn't work. > +_require_xfs_db_write_array() > +{ > + local supported=0 > + > + _require_test > + touch $TEST_DIR/$seq.img > + $MKFS_XFS_PROG -d file,name=$TEST_DIR/$seq.img,size=512m >/dev/null 2>&1 > + $XFS_DB_PROG -x -c "agfl 0" -c "write bno[32] 78" $TEST_DIR/$seq.img \ > + >/dev/null 2>&1 > + $XFS_DB_PROG -x -c "agfl 0" -c "print bno[32]" $TEST_DIR/$seq.img \ > + | grep -q "bno\[32\] = 78" && supported=1 > + rm -f $TEST_DIR/$seq.img > + [ $supported -eq 0 ] && _notrun "xfs_db write can't support array" > +} > diff --git a/tests/xfs/444 b/tests/xfs/444 > index 141be52..9700422 100755 > --- a/tests/xfs/444 > +++ b/tests/xfs/444 > @@ -54,6 +54,7 @@ _supported_os Linux > _require_check_dmesg > _require_scratch > _require_test_program "punch-alternating" > +_require_xfs_db_write_array Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > > # This is only a v5 filesystem problem > _require_scratch_xfs_crc > -- > 1.8.3.1 > > > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] common/xfs: Add require_xfs_db_write_array function 2018-04-17 17:59 ` Darrick J. Wong @ 2018-04-18 4:29 ` Eryu Guan 0 siblings, 0 replies; 13+ messages in thread From: Eryu Guan @ 2018-04-18 4:29 UTC (permalink / raw) To: Darrick J. Wong, xuyang.jy; +Cc: david, fstests On Tue, Apr 17, 2018 at 10:59:17AM -0700, Darrick J. Wong wrote: > On Tue, Apr 17, 2018 at 02:11:40PM +0800, yang xu wrote: > > xfsprogs commit 4222d00("db: write via array indexing doesn't > > work") fixes a bug that xfs_db write can't support array indexing. > > This function will check whether the bug is fixed on the current > > xfsprogs. > > > > xfs/444 applies the function, and skips if this bug exists. > > > > Signed-off-by: yang xu <xuyang.jy@cn.fujitsu.com> > > --- > > common/xfs | 17 +++++++++++++++++ > > tests/xfs/444 | 1 + > > 2 files changed, 18 insertions(+) > > > > diff --git a/common/xfs b/common/xfs > > index 3169f87..0b59994 100644 > > --- a/common/xfs > > +++ b/common/xfs > > @@ -701,3 +701,20 @@ _scratch_xfs_set_sb_field() > > { > > _scratch_xfs_set_metadata_field "$1" "$2" "sb 0" > > } > > + > > +#Before xfsprogs commit 4222d000ed("db: write via array indexing doesn't work"), > > +##xfs_db command to write a specific AGFL index doesn't work. I'll add more comments here to explain why we require a bug fix instead of exposing the bug to users, e.g. "It's a bug in a diagnostic tool that is only used by XFS developers as a test infrastructure, so we can treat it as a infrastructure dependency as all other _require rules." > > +_require_xfs_db_write_array() > > +{ > > + local supported=0 > > + > > + _require_test > > + touch $TEST_DIR/$seq.img > > + $MKFS_XFS_PROG -d file,name=$TEST_DIR/$seq.img,size=512m >/dev/null 2>&1 > > + $XFS_DB_PROG -x -c "agfl 0" -c "write bno[32] 78" $TEST_DIR/$seq.img \ > > + >/dev/null 2>&1 > > + $XFS_DB_PROG -x -c "agfl 0" -c "print bno[32]" $TEST_DIR/$seq.img \ > > + | grep -q "bno\[32\] = 78" && supported=1 > > + rm -f $TEST_DIR/$seq.img > > + [ $supported -eq 0 ] && _notrun "xfs_db write can't support array" > > +} > > diff --git a/tests/xfs/444 b/tests/xfs/444 > > index 141be52..9700422 100755 > > --- a/tests/xfs/444 > > +++ b/tests/xfs/444 > > @@ -54,6 +54,7 @@ _supported_os Linux > > _require_check_dmesg > > _require_scratch > > _require_test_program "punch-alternating" > > +_require_xfs_db_write_array > > Looks ok, > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Thanks for the review! Eryu > > --D > > > > > # This is only a v5 filesystem problem > > _require_scratch_xfs_crc > > -- > > 1.8.3.1 > > > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe fstests" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-04-18 4:29 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-12 10:00 [PATCH] xfs/444: add check for xfs_db write bno array yang xu 2018-04-12 12:09 ` Dave Chinner 2018-04-12 13:00 ` Eryu Guan 2018-04-12 23:00 ` Dave Chinner 2018-04-13 4:36 ` Eryu Guan 2018-04-13 13:42 ` Dave Chinner 2018-04-16 6:00 ` Eryu Guan 2018-04-16 9:07 ` [PATCH v2] common/xfs: Add require_xfs_db_write_array function yang xu 2018-04-16 17:22 ` Darrick J. Wong [not found] ` <5AD5721C.10709@cn.fujitsu.com> 2018-04-17 4:06 ` Darrick J. Wong 2018-04-17 6:11 ` [PATCH v3] " yang xu 2018-04-17 17:59 ` Darrick J. Wong 2018-04-18 4:29 ` Eryu Guan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.