| Message ID | 1656340203-2322-1-git-send-email-xuyang2018.jy@fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] xfs/270: only check new_ro_compat value when mkfs.xfs supports nrext64 feature | expand |
Hi Zorro
What do you think this patch or should I just add a 'x' string when
comparing get_value/new_value, then just remind user they may hit a old
xfsprogs bug
f4afdcb0a ("xfs_db: clean up the salvage read callsites in set_cur()").
Best Regards
Yang Xu
> Currently, this case fails on old xfsprogs as below:
> +/var/lib/xfstests/tests/xfs/270: line 51: [: !=: unary operator expected
>
> Getting ro_compat value will report the following error after setting new ro_compat
> value:
> +cache_purge: shake on cache 0x56033fde4920 left 1 nodes!?
> +cache_purge: shake on cache 0x56033fde4920 left 1 nodes!?
> +cache_zero_check: refcount is 1, not zero (node=0x56033fdf9110
>
> Old xfsprogs miss a bugfix
> f4afdcb0ad ("xfs_db: clean up the salvage read callsites in set_cur()").
>
> Here we skip the get step of new ro_comap value when nrext64 feature is supported.
> Also will add a new test to cover this xfsprog bug.
>
> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
> ---
> tests/xfs/270 | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/tests/xfs/270 b/tests/xfs/270
> index b740c379..5ff83ead 100755
> --- a/tests/xfs/270
> +++ b/tests/xfs/270
> @@ -22,6 +22,10 @@ _require_scratch_nocheck
> # Only V5 XFS disallow rw mount/remount with unknown ro-compat features
> _require_scratch_xfs_crc
>
> +nrext64_supported=0
> +_scratch_mkfs_xfs_supported -m crc=1 -i nrext64> /dev/null 2>&1&& \
> + nrext64_supported=1
> +
> _scratch_mkfs_xfs>>$seqres.full 2>&1
>
> # set the highest bit of features_ro_compat, use it as an unknown
> @@ -43,13 +47,18 @@ ro_compat=$(echo $ro_compat | \
> _scratch_xfs_set_metadata_field "features_ro_compat" "$ro_compat" "sb 0" \
> > $seqres.full 2>&1
>
> -# read the newly set ro compat filed for verification
> -new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" "sb 0" \
> - 2>/dev/null)
> -
> -# verify the new ro_compat field is correct.
> -if [ $new_ro_compat != $ro_compat ]; then
> - echo "Unable to set new features_ro_compat. Wanted $ro_compat, got $new_ro_compat"
> +# Indeed, xfsprogs has a bug here and fixed by commit f4afdcb
> +# ("xfs_db: clean up the salvage read callsites in set_cur()")
> +# Here, we use nrext64 feature as a proxy.
> +if [ $nrext64_supported -eq 1 ]; then
> + # read the newly set ro compat filed for verification
> + new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" \
> + "sb 0" 2>/dev/null)
> + # verify the new ro_compat field is correct.
> + if [ $new_ro_compat != $ro_compat ]; then
> + echo "Unable to set new features_ro_compat. Wanted $ro_compat, \
> + got $new_ro_compat"
> + fi
> fi
>
> # rw mount with unknown ro-compat feature should fail
On Mon, Jun 27, 2022 at 10:30:02PM +0800, Yang Xu wrote: > Currently, this case fails on old xfsprogs as below: > +/var/lib/xfstests/tests/xfs/270: line 51: [: !=: unary operator expected > > Getting ro_compat value will report the following error after setting new ro_compat > value: > +cache_purge: shake on cache 0x56033fde4920 left 1 nodes!? > +cache_purge: shake on cache 0x56033fde4920 left 1 nodes!? > +cache_zero_check: refcount is 1, not zero (node=0x56033fdf9110 > > Old xfsprogs miss a bugfix > f4afdcb0ad ("xfs_db: clean up the salvage read callsites in set_cur()"). > > Here we skip the get step of new ro_comap value when nrext64 feature is supported. > Also will add a new test to cover this xfsprog bug. > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- > tests/xfs/270 | 23 ++++++++++++++++------- > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/tests/xfs/270 b/tests/xfs/270 > index b740c379..5ff83ead 100755 > --- a/tests/xfs/270 > +++ b/tests/xfs/270 > @@ -22,6 +22,10 @@ _require_scratch_nocheck > # Only V5 XFS disallow rw mount/remount with unknown ro-compat features > _require_scratch_xfs_crc > > +nrext64_supported=0 > +_scratch_mkfs_xfs_supported -m crc=1 -i nrext64 > /dev/null 2>&1 && \ > + nrext64_supported=1 > + > _scratch_mkfs_xfs >>$seqres.full 2>&1 > > # set the highest bit of features_ro_compat, use it as an unknown > @@ -43,13 +47,18 @@ ro_compat=$(echo $ro_compat | \ > _scratch_xfs_set_metadata_field "features_ro_compat" "$ro_compat" "sb 0" \ > > $seqres.full 2>&1 > > -# read the newly set ro compat filed for verification > -new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" "sb 0" \ > - 2>/dev/null) > - > -# verify the new ro_compat field is correct. > -if [ $new_ro_compat != $ro_compat ]; then My personal opinion is -- change above line as: if [ "$new_ro_compat" != "$ro_compat" ] to avoid the bash syntax error. Then the failure (on old xfsprogs) correspond to a known xfsprogs bug. That's good enough. Thanks, Zorro > - echo "Unable to set new features_ro_compat. Wanted $ro_compat, got $new_ro_compat" > +# Indeed, xfsprogs has a bug here and fixed by commit f4afdcb > +# ("xfs_db: clean up the salvage read callsites in set_cur()") > +# Here, we use nrext64 feature as a proxy. > +if [ $nrext64_supported -eq 1 ]; then > + # read the newly set ro compat filed for verification > + new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" \ > + "sb 0" 2>/dev/null) > + # verify the new ro_compat field is correct. > + if [ $new_ro_compat != $ro_compat ]; then > + echo "Unable to set new features_ro_compat. Wanted $ro_compat, \ > + got $new_ro_compat" > + fi > fi > > # rw mount with unknown ro-compat feature should fail > -- > 2.27.0 >
on 2022/07/04 23:42, Zorro Lang wrote: > On Mon, Jun 27, 2022 at 10:30:02PM +0800, Yang Xu wrote: >> Currently, this case fails on old xfsprogs as below: >> +/var/lib/xfstests/tests/xfs/270: line 51: [: !=: unary operator expected >> >> Getting ro_compat value will report the following error after setting new ro_compat >> value: >> +cache_purge: shake on cache 0x56033fde4920 left 1 nodes!? >> +cache_purge: shake on cache 0x56033fde4920 left 1 nodes!? >> +cache_zero_check: refcount is 1, not zero (node=0x56033fdf9110 >> >> Old xfsprogs miss a bugfix >> f4afdcb0ad ("xfs_db: clean up the salvage read callsites in set_cur()"). >> >> Here we skip the get step of new ro_comap value when nrext64 feature is supported. >> Also will add a new test to cover this xfsprog bug. >> >> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com> >> --- >> tests/xfs/270 | 23 ++++++++++++++++------- >> 1 file changed, 16 insertions(+), 7 deletions(-) >> >> diff --git a/tests/xfs/270 b/tests/xfs/270 >> index b740c379..5ff83ead 100755 >> --- a/tests/xfs/270 >> +++ b/tests/xfs/270 >> @@ -22,6 +22,10 @@ _require_scratch_nocheck >> # Only V5 XFS disallow rw mount/remount with unknown ro-compat features >> _require_scratch_xfs_crc >> >> +nrext64_supported=0 >> +_scratch_mkfs_xfs_supported -m crc=1 -i nrext64> /dev/null 2>&1&& \ >> + nrext64_supported=1 >> + >> _scratch_mkfs_xfs>>$seqres.full 2>&1 >> >> # set the highest bit of features_ro_compat, use it as an unknown >> @@ -43,13 +47,18 @@ ro_compat=$(echo $ro_compat | \ >> _scratch_xfs_set_metadata_field "features_ro_compat" "$ro_compat" "sb 0" \ >> > $seqres.full 2>&1 >> >> -# read the newly set ro compat filed for verification >> -new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" "sb 0" \ >> - 2>/dev/null) >> - >> -# verify the new ro_compat field is correct. >> -if [ $new_ro_compat != $ro_compat ]; then > > My personal opinion is -- change above line as: > if [ "$new_ro_compat" != "$ro_compat" ] > to avoid the bash syntax error. Then the failure (on old xfsprogs) correspond > to a known xfsprogs bug. That's good enough. Ok, will do it on v2. Thanks. Best Regards Yang Xu > > Thanks, > Zorro > >> - echo "Unable to set new features_ro_compat. Wanted $ro_compat, got $new_ro_compat" >> +# Indeed, xfsprogs has a bug here and fixed by commit f4afdcb >> +# ("xfs_db: clean up the salvage read callsites in set_cur()") >> +# Here, we use nrext64 feature as a proxy. >> +if [ $nrext64_supported -eq 1 ]; then >> + # read the newly set ro compat filed for verification >> + new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" \ >> + "sb 0" 2>/dev/null) >> + # verify the new ro_compat field is correct. >> + if [ $new_ro_compat != $ro_compat ]; then >> + echo "Unable to set new features_ro_compat. Wanted $ro_compat, \ >> + got $new_ro_compat" >> + fi >> fi >> >> # rw mount with unknown ro-compat feature should fail >> -- >> 2.27.0 >> >
diff --git a/tests/xfs/270 b/tests/xfs/270 index b740c379..5ff83ead 100755 --- a/tests/xfs/270 +++ b/tests/xfs/270 @@ -22,6 +22,10 @@ _require_scratch_nocheck # Only V5 XFS disallow rw mount/remount with unknown ro-compat features _require_scratch_xfs_crc +nrext64_supported=0 +_scratch_mkfs_xfs_supported -m crc=1 -i nrext64 > /dev/null 2>&1 && \ + nrext64_supported=1 + _scratch_mkfs_xfs >>$seqres.full 2>&1 # set the highest bit of features_ro_compat, use it as an unknown @@ -43,13 +47,18 @@ ro_compat=$(echo $ro_compat | \ _scratch_xfs_set_metadata_field "features_ro_compat" "$ro_compat" "sb 0" \ > $seqres.full 2>&1 -# read the newly set ro compat filed for verification -new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" "sb 0" \ - 2>/dev/null) - -# verify the new ro_compat field is correct. -if [ $new_ro_compat != $ro_compat ]; then - echo "Unable to set new features_ro_compat. Wanted $ro_compat, got $new_ro_compat" +# Indeed, xfsprogs has a bug here and fixed by commit f4afdcb +# ("xfs_db: clean up the salvage read callsites in set_cur()") +# Here, we use nrext64 feature as a proxy. +if [ $nrext64_supported -eq 1 ]; then + # read the newly set ro compat filed for verification + new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" \ + "sb 0" 2>/dev/null) + # verify the new ro_compat field is correct. + if [ $new_ro_compat != $ro_compat ]; then + echo "Unable to set new features_ro_compat. Wanted $ro_compat, \ + got $new_ro_compat" + fi fi # rw mount with unknown ro-compat feature should fail
Currently, this case fails on old xfsprogs as below: +/var/lib/xfstests/tests/xfs/270: line 51: [: !=: unary operator expected Getting ro_compat value will report the following error after setting new ro_compat value: +cache_purge: shake on cache 0x56033fde4920 left 1 nodes!? +cache_purge: shake on cache 0x56033fde4920 left 1 nodes!? +cache_zero_check: refcount is 1, not zero (node=0x56033fdf9110 Old xfsprogs miss a bugfix f4afdcb0ad ("xfs_db: clean up the salvage read callsites in set_cur()"). Here we skip the get step of new ro_comap value when nrext64 feature is supported. Also will add a new test to cover this xfsprog bug. Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> --- tests/xfs/270 | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)