| Message ID | 20220928210337.417054-1-shiina.hironori@fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] xfs: test for fixing wrong root inode number | expand |
On Wed, Sep 28, 2022 at 05:03:37PM -0400, Hironori Shiina wrote: > Test '-x' option of xfsrestore. With this option, a wrong root inode > number is corrected. A root inode number can be wrong in a dump created > by problematic xfsdump (v3.1.7 - v3.1.9) with blukstat misuse. This > patch adds a dump with a wrong inode number created by xfsdump 3.1.8. > > Link: https://lore.kernel.org/linux-xfs/20201113125127.966243-1-hsiangkao@redhat.com/ > Signed-off-by: Hironori Shiina <shiina.hironori@fujitsu.com> > --- > common/dump | 2 +- > src/root-inode-broken-dumpfile | Bin 0 -> 21648 bytes > tests/xfs/554 | 37 +++++++++++++++++++++ > tests/xfs/554.out | 57 +++++++++++++++++++++++++++++++++ > 4 files changed, 95 insertions(+), 1 deletion(-) > create mode 100644 src/root-inode-broken-dumpfile > create mode 100644 tests/xfs/554 > create mode 100644 tests/xfs/554.out > > diff --git a/common/dump b/common/dump > index 8e0446d9..50b2ba03 100644 > --- a/common/dump > +++ b/common/dump > @@ -1003,7 +1003,7 @@ _parse_restore_args() > --no-check-quota) > do_quota_check=false > ;; > - -K|-R) > + -K|-R|-x) > restore_args="$restore_args $1" > ;; > *) > diff --git a/src/root-inode-broken-dumpfile b/src/root-inode-broken-dumpfile > new file mode 100644 > index 0000000000000000000000000000000000000000..9a42e65d8047497be31f3abbaf4223ae384fd14d > GIT binary patch > literal 21648 > zcmeI)K}ge49KiAC_J<CEBr!0AwBf~zbCHB}5DyxL6eU8Jnqylvayj;2VuG+d)TN6; > z5J8vlAaw9hXi(UousT$Sin@BRJfwHM)O+*2*njz_zc+h*ckuV#@BMuKe;;JbgTL{< > z!SwZ9zC#ERe%reLe5&cz2e}qM<!i2dXQHt^{^`d1Qx{)MMzW#$%dR@J={0`IRsGx4 > z(yn@Oi-nBqCOVIG?&{kpwnv~&w_>6_ozY1^fph=w8(=^oTg&wOe=(WQByyQ_Hfd|4 > z^o76<0!zn#v>k3blbU)nzx=KF^}-G%R;OaQYsHwGDkO`kD^@q^(_Ac_8H<gKj^>a0 > z6p*%BK>q#b>2EE%^!@f?Z`-p+tI@M}qmMm@zMJk>K1U^=d~G^NUG?X4vkvKtOf-3Y > zpVM9YgM9Y7{*P0ApJNUh^uk1wCnA6V0tg_000IagfB*srAb<b@2q1s}0tg_000Iag > zfB*srAb<b@2q1s}0tg_000IagfB*srAb<b@2q1s}0tg_000IagfB*srAb`MM1p>_h > z2-R=Q9ooK1{l9<Dy8IIMUVXr9BW9uI1x7};j+kij|5kLI^32no;n|PvqD74ZOlJ#n > z0-~CKSlx#liMS>jt232#==00xPqwp;gsZrjc?`PPxaCE~>3(^o5-%+Nj=HegJFK2b > z=l5zT4IDgO=sJ1zp=jyrALvcQJK|j;sN2_jUmobjN<!S6m1{G<LZ^+JP;T!|3{9)w > eGf&ioo}iw|li2&4IyG;z<}vrl)Mic2`t2{52##q0 > > literal 0 > HcmV?d00001 Please don't try to add a binary file to fstests directly. > > diff --git a/tests/xfs/554 b/tests/xfs/554 > new file mode 100644 > index 00000000..13bc62c7 > --- /dev/null > +++ b/tests/xfs/554 > @@ -0,0 +1,37 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2022 Fujitsu Limited. All Rights Reserved. > +# > +# FS QA Test No. 554 > +# > +# Test restoring a dumpfile with a wrong root inode number created by > +# xfsdump 3.1.8. > +# This test restores the checked-in broken dump with '-x' flag. > +# > + > +. ./common/preamble > +_begin_fstest auto quick dump > + > +# Import common functions. > +. ./common/dump > + > +# real QA test starts here > +_supported_fs xfs > +_require_scratch The -x option is a new feature for xfsdump, not all system support that. So we need to _notrun if test on a system doesn't support it. A separated _require_* helper would be better if there'll be more testing about this new feature. Or a local detection in this case is fine too (can be moved as a common helper later). > +_scratch_mkfs_xfs >>$seqres.full || _fail "mkfs failed" > +_scratch_mount > + > +# Create dumpdir for comparing with restoredir > +rm -rf $dump_dir > +mkdir $dump_dir || _fail "failed to mkdir $restore_dir" ^^ ^^ Are you trying to create a dump dir or restore dir? > +touch $dump_dir/FILE_1019 > + > +_do_restore_toc -x -f $here/src/root-inode-broken-dumpfile Why I didn't see how you generate this broken dumpfile in this case? Oh... I see, you want to store a dumpfile in fstests source code directly. I thought you submited that file accidentally... No, we don't do things like this way, please try to generate the dumpfile in this test case at first, then restore it. For example using xfs_db to break a xfs, or using some tricky method (likes xfs/545). > + > +_do_restore_file -x -f $here/src/root-inode-broken-dumpfile -L stress_545 > +_diff_compare_sub > +_ls_nodate_compare_sub > + > +# success, all done > +status=0 > +exit > diff --git a/tests/xfs/554.out b/tests/xfs/554.out > new file mode 100644 > index 00000000..40a3f3a4 > --- /dev/null > +++ b/tests/xfs/554.out > @@ -0,0 +1,57 @@ > +QA output created by 554 > +Contents of dump ... > +xfsrestore -x -f DUMP_FILE -t > +xfsrestore: using file dump (drive_simple) strategy > +xfsrestore: searching media for dump > +xfsrestore: examining media file 0 > +xfsrestore: dump description: > +xfsrestore: hostname: xfsdump > +xfsrestore: mount point: SCRATCH_MNT > +xfsrestore: volume: SCRATCH_DEV > +xfsrestore: session time: TIME > +xfsrestore: level: 0 > +xfsrestore: session label: "stress_545" > +xfsrestore: media label: "stress_tape_media" > +xfsrestore: file system ID: ID > +xfsrestore: session id: ID > +xfsrestore: media ID: ID > +xfsrestore: searching media for directory dump > +xfsrestore: reading directories > +xfsrestore: found fake rootino #128, will fix. > +xfsrestore: fix root # to 1024 (bind mount?) > +xfsrestore: 2 directories and 2 entries processed > +xfsrestore: directory post-processing > +xfsrestore: reading non-directory files > +xfsrestore: table of contents display complete: SECS seconds elapsed > +xfsrestore: Restore Status: SUCCESS > + > +dumpdir/FILE_1019 > +Restoring from file... > +xfsrestore -x -f DUMP_FILE -L stress_545 RESTORE_DIR > +xfsrestore: using file dump (drive_simple) strategy > +xfsrestore: searching media for dump > +xfsrestore: examining media file 0 > +xfsrestore: found dump matching specified label: > +xfsrestore: hostname: xfsdump > +xfsrestore: mount point: SCRATCH_MNT > +xfsrestore: volume: SCRATCH_DEV > +xfsrestore: session time: TIME > +xfsrestore: level: 0 > +xfsrestore: session label: "stress_545" > +xfsrestore: media label: "stress_tape_media" > +xfsrestore: file system ID: ID > +xfsrestore: session id: ID > +xfsrestore: media ID: ID > +xfsrestore: searching media for directory dump > +xfsrestore: reading directories > +xfsrestore: found fake rootino #128, will fix. > +xfsrestore: fix root # to 1024 (bind mount?) > +xfsrestore: 2 directories and 2 entries processed > +xfsrestore: directory post-processing > +xfsrestore: restoring non-directory files > +xfsrestore: restore complete: SECS seconds elapsed > +xfsrestore: Restore Status: SUCCESS > +Comparing dump directory with restore directory > +Files DUMP_DIR/FILE_1019 and RESTORE_DIR/DUMP_SUBDIR/FILE_1019 are identical > +Comparing listing of dump directory with restore directory > +Files TMP.dump_dir and TMP.restore_dir are identical > -- > 2.37.3 >
On 9/28/22 21:49, Zorro Lang wrote: > On Wed, Sep 28, 2022 at 05:03:37PM -0400, Hironori Shiina wrote: >> Test '-x' option of xfsrestore. With this option, a wrong root inode >> number is corrected. A root inode number can be wrong in a dump created >> by problematic xfsdump (v3.1.7 - v3.1.9) with blukstat misuse. This >> patch adds a dump with a wrong inode number created by xfsdump 3.1.8. >> >> Link: https://lore.kernel.org/linux-xfs/20201113125127.966243-1-hsiangkao@redhat.com/ >> Signed-off-by: Hironori Shiina <shiina.hironori@fujitsu.com> >> --- >> common/dump | 2 +- >> src/root-inode-broken-dumpfile | Bin 0 -> 21648 bytes >> tests/xfs/554 | 37 +++++++++++++++++++++ >> tests/xfs/554.out | 57 +++++++++++++++++++++++++++++++++ >> 4 files changed, 95 insertions(+), 1 deletion(-) >> create mode 100644 src/root-inode-broken-dumpfile >> create mode 100644 tests/xfs/554 >> create mode 100644 tests/xfs/554.out >> >> diff --git a/common/dump b/common/dump >> index 8e0446d9..50b2ba03 100644 >> --- a/common/dump >> +++ b/common/dump >> @@ -1003,7 +1003,7 @@ _parse_restore_args() >> --no-check-quota) >> do_quota_check=false >> ;; >> - -K|-R) >> + -K|-R|-x) >> restore_args="$restore_args $1" >> ;; >> *) >> diff --git a/src/root-inode-broken-dumpfile b/src/root-inode-broken-dumpfile >> new file mode 100644 >> index 0000000000000000000000000000000000000000..9a42e65d8047497be31f3abbaf4223ae384fd14d >> GIT binary patch >> literal 21648 >> zcmeI)K}ge49KiAC_J<CEBr!0AwBf~zbCHB}5DyxL6eU8Jnqylvayj;2VuG+d)TN6; >> z5J8vlAaw9hXi(UousT$Sin@BRJfwHM)O+*2*njz_zc+h*ckuV#@BMuKe;;JbgTL{< >> z!SwZ9zC#ERe%reLe5&cz2e}qM<!i2dXQHt^{^`d1Qx{)MMzW#$%dR@J={0`IRsGx4 >> z(yn@Oi-nBqCOVIG?&{kpwnv~&w_>6_ozY1^fph=w8(=^oTg&wOe=(WQByyQ_Hfd|4 >> z^o76<0!zn#v>k3blbU)nzx=KF^}-G%R;OaQYsHwGDkO`kD^@q^(_Ac_8H<gKj^>a0 >> z6p*%BK>q#b>2EE%^!@f?Z`-p+tI@M}qmMm@zMJk>K1U^=d~G^NUG?X4vkvKtOf-3Y >> zpVM9YgM9Y7{*P0ApJNUh^uk1wCnA6V0tg_000IagfB*srAb<b@2q1s}0tg_000Iag >> zfB*srAb<b@2q1s}0tg_000IagfB*srAb<b@2q1s}0tg_000IagfB*srAb`MM1p>_h >> z2-R=Q9ooK1{l9<Dy8IIMUVXr9BW9uI1x7};j+kij|5kLI^32no;n|PvqD74ZOlJ#n >> z0-~CKSlx#liMS>jt232#==00xPqwp;gsZrjc?`PPxaCE~>3(^o5-%+Nj=HegJFK2b >> z=l5zT4IDgO=sJ1zp=jyrALvcQJK|j;sN2_jUmobjN<!S6m1{G<LZ^+JP;T!|3{9)w >> eGf&ioo}iw|li2&4IyG;z<}vrl)Mic2`t2{52##q0 >> >> literal 0 >> HcmV?d00001 > > Please don't try to add a binary file to fstests directly. > >> >> diff --git a/tests/xfs/554 b/tests/xfs/554 >> new file mode 100644 >> index 00000000..13bc62c7 >> --- /dev/null >> +++ b/tests/xfs/554 >> @@ -0,0 +1,37 @@ >> +#! /bin/bash >> +# SPDX-License-Identifier: GPL-2.0 >> +# Copyright (c) 2022 Fujitsu Limited. All Rights Reserved. >> +# >> +# FS QA Test No. 554 >> +# >> +# Test restoring a dumpfile with a wrong root inode number created by >> +# xfsdump 3.1.8. >> +# This test restores the checked-in broken dump with '-x' flag. >> +# >> + >> +. ./common/preamble >> +_begin_fstest auto quick dump >> + >> +# Import common functions. >> +. ./common/dump >> + >> +# real QA test starts here >> +_supported_fs xfs >> +_require_scratch > > The -x option is a new feature for xfsdump, not all system support that. So > we need to _notrun if test on a system doesn't support it. A separated > _require_* helper would be better if there'll be more testing about this > new feature. Or a local detection in this case is fine too (can be moved as > a common helper later). > >> +_scratch_mkfs_xfs >>$seqres.full || _fail "mkfs failed" >> +_scratch_mount >> + >> +# Create dumpdir for comparing with restoredir >> +rm -rf $dump_dir >> +mkdir $dump_dir || _fail "failed to mkdir $restore_dir" > ^^ ^^ > > Are you trying to create a dump dir or restore dir? > >> +touch $dump_dir/FILE_1019 >> + >> +_do_restore_toc -x -f $here/src/root-inode-broken-dumpfile > > Why I didn't see how you generate this broken dumpfile in this case? > > Oh... I see, you want to store a dumpfile in fstests source code directly. > I thought you submited that file accidentally... > > No, we don't do things like this way, please try to generate the dumpfile > in this test case at first, then restore it. For example using xfs_db to > break a xfs, or using some tricky method (likes xfs/545). > Thank you for the comments. I will try another approach. I'm having trouble creating a dumpfile for this test. Because xfsdump was already fixed, xfsdump no longer generates a corrupted dumpfile even if there is a lower inode number than a root inode. >> + >> +_do_restore_file -x -f $here/src/root-inode-broken-dumpfile -L stress_545 >> +_diff_compare_sub >> +_ls_nodate_compare_sub >> + >> +# success, all done >> +status=0 >> +exit >> diff --git a/tests/xfs/554.out b/tests/xfs/554.out >> new file mode 100644 >> index 00000000..40a3f3a4 >> --- /dev/null >> +++ b/tests/xfs/554.out >> @@ -0,0 +1,57 @@ >> +QA output created by 554 >> +Contents of dump ... >> +xfsrestore -x -f DUMP_FILE -t >> +xfsrestore: using file dump (drive_simple) strategy >> +xfsrestore: searching media for dump >> +xfsrestore: examining media file 0 >> +xfsrestore: dump description: >> +xfsrestore: hostname: xfsdump >> +xfsrestore: mount point: SCRATCH_MNT >> +xfsrestore: volume: SCRATCH_DEV >> +xfsrestore: session time: TIME >> +xfsrestore: level: 0 >> +xfsrestore: session label: "stress_545" >> +xfsrestore: media label: "stress_tape_media" >> +xfsrestore: file system ID: ID >> +xfsrestore: session id: ID >> +xfsrestore: media ID: ID >> +xfsrestore: searching media for directory dump >> +xfsrestore: reading directories >> +xfsrestore: found fake rootino #128, will fix. >> +xfsrestore: fix root # to 1024 (bind mount?) >> +xfsrestore: 2 directories and 2 entries processed >> +xfsrestore: directory post-processing >> +xfsrestore: reading non-directory files >> +xfsrestore: table of contents display complete: SECS seconds elapsed >> +xfsrestore: Restore Status: SUCCESS >> + >> +dumpdir/FILE_1019 >> +Restoring from file... >> +xfsrestore -x -f DUMP_FILE -L stress_545 RESTORE_DIR >> +xfsrestore: using file dump (drive_simple) strategy >> +xfsrestore: searching media for dump >> +xfsrestore: examining media file 0 >> +xfsrestore: found dump matching specified label: >> +xfsrestore: hostname: xfsdump >> +xfsrestore: mount point: SCRATCH_MNT >> +xfsrestore: volume: SCRATCH_DEV >> +xfsrestore: session time: TIME >> +xfsrestore: level: 0 >> +xfsrestore: session label: "stress_545" >> +xfsrestore: media label: "stress_tape_media" >> +xfsrestore: file system ID: ID >> +xfsrestore: session id: ID >> +xfsrestore: media ID: ID >> +xfsrestore: searching media for directory dump >> +xfsrestore: reading directories >> +xfsrestore: found fake rootino #128, will fix. >> +xfsrestore: fix root # to 1024 (bind mount?) >> +xfsrestore: 2 directories and 2 entries processed >> +xfsrestore: directory post-processing >> +xfsrestore: restoring non-directory files >> +xfsrestore: restore complete: SECS seconds elapsed >> +xfsrestore: Restore Status: SUCCESS >> +Comparing dump directory with restore directory >> +Files DUMP_DIR/FILE_1019 and RESTORE_DIR/DUMP_SUBDIR/FILE_1019 are identical >> +Comparing listing of dump directory with restore directory >> +Files TMP.dump_dir and TMP.restore_dir are identical >> -- >> 2.37.3 >> >
On Fri, Sep 30, 2022 at 11:01:51AM -0400, Hironori Shiina wrote: > > > On 9/28/22 21:49, Zorro Lang wrote: > > On Wed, Sep 28, 2022 at 05:03:37PM -0400, Hironori Shiina wrote: > >> Test '-x' option of xfsrestore. With this option, a wrong root inode > >> number is corrected. A root inode number can be wrong in a dump created > >> by problematic xfsdump (v3.1.7 - v3.1.9) with blukstat misuse. This > >> patch adds a dump with a wrong inode number created by xfsdump 3.1.8. > >> > >> Link: https://lore.kernel.org/linux-xfs/20201113125127.966243-1-hsiangkao@redhat.com/ > >> Signed-off-by: Hironori Shiina <shiina.hironori@fujitsu.com> > >> --- > >> common/dump | 2 +- > >> src/root-inode-broken-dumpfile | Bin 0 -> 21648 bytes > >> tests/xfs/554 | 37 +++++++++++++++++++++ > >> tests/xfs/554.out | 57 +++++++++++++++++++++++++++++++++ > >> 4 files changed, 95 insertions(+), 1 deletion(-) > >> create mode 100644 src/root-inode-broken-dumpfile > >> create mode 100644 tests/xfs/554 > >> create mode 100644 tests/xfs/554.out > >> > >> diff --git a/common/dump b/common/dump > >> index 8e0446d9..50b2ba03 100644 > >> --- a/common/dump > >> +++ b/common/dump > >> @@ -1003,7 +1003,7 @@ _parse_restore_args() > >> --no-check-quota) > >> do_quota_check=false > >> ;; > >> - -K|-R) > >> + -K|-R|-x) > >> restore_args="$restore_args $1" > >> ;; > >> *) > >> diff --git a/src/root-inode-broken-dumpfile b/src/root-inode-broken-dumpfile > >> new file mode 100644 > >> index 0000000000000000000000000000000000000000..9a42e65d8047497be31f3abbaf4223ae384fd14d > >> GIT binary patch > >> literal 21648 > >> zcmeI)K}ge49KiAC_J<CEBr!0AwBf~zbCHB}5DyxL6eU8Jnqylvayj;2VuG+d)TN6; > >> z5J8vlAaw9hXi(UousT$Sin@BRJfwHM)O+*2*njz_zc+h*ckuV#@BMuKe;;JbgTL{< > >> z!SwZ9zC#ERe%reLe5&cz2e}qM<!i2dXQHt^{^`d1Qx{)MMzW#$%dR@J={0`IRsGx4 > >> z(yn@Oi-nBqCOVIG?&{kpwnv~&w_>6_ozY1^fph=w8(=^oTg&wOe=(WQByyQ_Hfd|4 > >> z^o76<0!zn#v>k3blbU)nzx=KF^}-G%R;OaQYsHwGDkO`kD^@q^(_Ac_8H<gKj^>a0 > >> z6p*%BK>q#b>2EE%^!@f?Z`-p+tI@M}qmMm@zMJk>K1U^=d~G^NUG?X4vkvKtOf-3Y > >> zpVM9YgM9Y7{*P0ApJNUh^uk1wCnA6V0tg_000IagfB*srAb<b@2q1s}0tg_000Iag > >> zfB*srAb<b@2q1s}0tg_000IagfB*srAb<b@2q1s}0tg_000IagfB*srAb`MM1p>_h > >> z2-R=Q9ooK1{l9<Dy8IIMUVXr9BW9uI1x7};j+kij|5kLI^32no;n|PvqD74ZOlJ#n > >> z0-~CKSlx#liMS>jt232#==00xPqwp;gsZrjc?`PPxaCE~>3(^o5-%+Nj=HegJFK2b > >> z=l5zT4IDgO=sJ1zp=jyrALvcQJK|j;sN2_jUmobjN<!S6m1{G<LZ^+JP;T!|3{9)w > >> eGf&ioo}iw|li2&4IyG;z<}vrl)Mic2`t2{52##q0 > >> > >> literal 0 > >> HcmV?d00001 > > > > Please don't try to add a binary file to fstests directly. > > > >> > >> diff --git a/tests/xfs/554 b/tests/xfs/554 > >> new file mode 100644 > >> index 00000000..13bc62c7 > >> --- /dev/null > >> +++ b/tests/xfs/554 > >> @@ -0,0 +1,37 @@ > >> +#! /bin/bash > >> +# SPDX-License-Identifier: GPL-2.0 > >> +# Copyright (c) 2022 Fujitsu Limited. All Rights Reserved. > >> +# > >> +# FS QA Test No. 554 > >> +# > >> +# Test restoring a dumpfile with a wrong root inode number created by > >> +# xfsdump 3.1.8. > >> +# This test restores the checked-in broken dump with '-x' flag. > >> +# > >> + > >> +. ./common/preamble > >> +_begin_fstest auto quick dump > >> + > >> +# Import common functions. > >> +. ./common/dump > >> + > >> +# real QA test starts here > >> +_supported_fs xfs > >> +_require_scratch > > > > The -x option is a new feature for xfsdump, not all system support that. So > > we need to _notrun if test on a system doesn't support it. A separated > > _require_* helper would be better if there'll be more testing about this > > new feature. Or a local detection in this case is fine too (can be moved as > > a common helper later). > > > >> +_scratch_mkfs_xfs >>$seqres.full || _fail "mkfs failed" > >> +_scratch_mount > >> + > >> +# Create dumpdir for comparing with restoredir > >> +rm -rf $dump_dir > >> +mkdir $dump_dir || _fail "failed to mkdir $restore_dir" > > ^^ ^^ > > > > Are you trying to create a dump dir or restore dir? > > > >> +touch $dump_dir/FILE_1019 > >> + > >> +_do_restore_toc -x -f $here/src/root-inode-broken-dumpfile > > > > Why I didn't see how you generate this broken dumpfile in this case? > > > > Oh... I see, you want to store a dumpfile in fstests source code directly. > > I thought you submited that file accidentally... > > > > No, we don't do things like this way, please try to generate the dumpfile > > in this test case at first, then restore it. For example using xfs_db to > > break a xfs, or using some tricky method (likes xfs/545). > > > > Thank you for the comments. I will try another approach. I'm having > trouble creating a dumpfile for this test. Because xfsdump was already > fixed, xfsdump no longer generates a corrupted dumpfile even if there is > a lower inode number than a root inode. Oh, I see. You can try, but I think it's hard. It maybe not suitable to be a fstests case, if it has to depend on a binary fs dump file. If so, We can cover it on other place, with this existed "bad" dump file. Thanks, Zorro > > >> + > >> +_do_restore_file -x -f $here/src/root-inode-broken-dumpfile -L stress_545 > >> +_diff_compare_sub > >> +_ls_nodate_compare_sub > >> + > >> +# success, all done > >> +status=0 > >> +exit > >> diff --git a/tests/xfs/554.out b/tests/xfs/554.out > >> new file mode 100644 > >> index 00000000..40a3f3a4 > >> --- /dev/null > >> +++ b/tests/xfs/554.out > >> @@ -0,0 +1,57 @@ > >> +QA output created by 554 > >> +Contents of dump ... > >> +xfsrestore -x -f DUMP_FILE -t > >> +xfsrestore: using file dump (drive_simple) strategy > >> +xfsrestore: searching media for dump > >> +xfsrestore: examining media file 0 > >> +xfsrestore: dump description: > >> +xfsrestore: hostname: xfsdump > >> +xfsrestore: mount point: SCRATCH_MNT > >> +xfsrestore: volume: SCRATCH_DEV > >> +xfsrestore: session time: TIME > >> +xfsrestore: level: 0 > >> +xfsrestore: session label: "stress_545" > >> +xfsrestore: media label: "stress_tape_media" > >> +xfsrestore: file system ID: ID > >> +xfsrestore: session id: ID > >> +xfsrestore: media ID: ID > >> +xfsrestore: searching media for directory dump > >> +xfsrestore: reading directories > >> +xfsrestore: found fake rootino #128, will fix. > >> +xfsrestore: fix root # to 1024 (bind mount?) > >> +xfsrestore: 2 directories and 2 entries processed > >> +xfsrestore: directory post-processing > >> +xfsrestore: reading non-directory files > >> +xfsrestore: table of contents display complete: SECS seconds elapsed > >> +xfsrestore: Restore Status: SUCCESS > >> + > >> +dumpdir/FILE_1019 > >> +Restoring from file... > >> +xfsrestore -x -f DUMP_FILE -L stress_545 RESTORE_DIR > >> +xfsrestore: using file dump (drive_simple) strategy > >> +xfsrestore: searching media for dump > >> +xfsrestore: examining media file 0 > >> +xfsrestore: found dump matching specified label: > >> +xfsrestore: hostname: xfsdump > >> +xfsrestore: mount point: SCRATCH_MNT > >> +xfsrestore: volume: SCRATCH_DEV > >> +xfsrestore: session time: TIME > >> +xfsrestore: level: 0 > >> +xfsrestore: session label: "stress_545" > >> +xfsrestore: media label: "stress_tape_media" > >> +xfsrestore: file system ID: ID > >> +xfsrestore: session id: ID > >> +xfsrestore: media ID: ID > >> +xfsrestore: searching media for directory dump > >> +xfsrestore: reading directories > >> +xfsrestore: found fake rootino #128, will fix. > >> +xfsrestore: fix root # to 1024 (bind mount?) > >> +xfsrestore: 2 directories and 2 entries processed > >> +xfsrestore: directory post-processing > >> +xfsrestore: restoring non-directory files > >> +xfsrestore: restore complete: SECS seconds elapsed > >> +xfsrestore: Restore Status: SUCCESS > >> +Comparing dump directory with restore directory > >> +Files DUMP_DIR/FILE_1019 and RESTORE_DIR/DUMP_SUBDIR/FILE_1019 are identical > >> +Comparing listing of dump directory with restore directory > >> +Files TMP.dump_dir and TMP.restore_dir are identical > >> -- > >> 2.37.3 > >> > > >
On Sun, Oct 02, 2022 at 12:27:08PM +0800, Zorro Lang wrote: > On Fri, Sep 30, 2022 at 11:01:51AM -0400, Hironori Shiina wrote: > > > > > > On 9/28/22 21:49, Zorro Lang wrote: > > > On Wed, Sep 28, 2022 at 05:03:37PM -0400, Hironori Shiina wrote: > > >> Test '-x' option of xfsrestore. With this option, a wrong root inode > > >> number is corrected. A root inode number can be wrong in a dump created > > >> by problematic xfsdump (v3.1.7 - v3.1.9) with blukstat misuse. This > > >> patch adds a dump with a wrong inode number created by xfsdump 3.1.8. > > >> > > >> Link: https://lore.kernel.org/linux-xfs/20201113125127.966243-1-hsiangkao@redhat.com/ > > >> Signed-off-by: Hironori Shiina <shiina.hironori@fujitsu.com> > > >> --- > > >> common/dump | 2 +- > > >> src/root-inode-broken-dumpfile | Bin 0 -> 21648 bytes > > >> tests/xfs/554 | 37 +++++++++++++++++++++ > > >> tests/xfs/554.out | 57 +++++++++++++++++++++++++++++++++ > > >> 4 files changed, 95 insertions(+), 1 deletion(-) > > >> create mode 100644 src/root-inode-broken-dumpfile > > >> create mode 100644 tests/xfs/554 > > >> create mode 100644 tests/xfs/554.out > > >> > > >> diff --git a/common/dump b/common/dump > > >> index 8e0446d9..50b2ba03 100644 > > >> --- a/common/dump > > >> +++ b/common/dump > > >> @@ -1003,7 +1003,7 @@ _parse_restore_args() > > >> --no-check-quota) > > >> do_quota_check=false > > >> ;; > > >> - -K|-R) > > >> + -K|-R|-x) > > >> restore_args="$restore_args $1" > > >> ;; > > >> *) > > >> diff --git a/src/root-inode-broken-dumpfile b/src/root-inode-broken-dumpfile > > >> new file mode 100644 > > >> index 0000000000000000000000000000000000000000..9a42e65d8047497be31f3abbaf4223ae384fd14d > > >> GIT binary patch > > >> literal 21648 > > >> zcmeI)K}ge49KiAC_J<CEBr!0AwBf~zbCHB}5DyxL6eU8Jnqylvayj;2VuG+d)TN6; > > >> z5J8vlAaw9hXi(UousT$Sin@BRJfwHM)O+*2*njz_zc+h*ckuV#@BMuKe;;JbgTL{< > > >> z!SwZ9zC#ERe%reLe5&cz2e}qM<!i2dXQHt^{^`d1Qx{)MMzW#$%dR@J={0`IRsGx4 > > >> z(yn@Oi-nBqCOVIG?&{kpwnv~&w_>6_ozY1^fph=w8(=^oTg&wOe=(WQByyQ_Hfd|4 > > >> z^o76<0!zn#v>k3blbU)nzx=KF^}-G%R;OaQYsHwGDkO`kD^@q^(_Ac_8H<gKj^>a0 > > >> z6p*%BK>q#b>2EE%^!@f?Z`-p+tI@M}qmMm@zMJk>K1U^=d~G^NUG?X4vkvKtOf-3Y > > >> zpVM9YgM9Y7{*P0ApJNUh^uk1wCnA6V0tg_000IagfB*srAb<b@2q1s}0tg_000Iag > > >> zfB*srAb<b@2q1s}0tg_000IagfB*srAb<b@2q1s}0tg_000IagfB*srAb`MM1p>_h > > >> z2-R=Q9ooK1{l9<Dy8IIMUVXr9BW9uI1x7};j+kij|5kLI^32no;n|PvqD74ZOlJ#n > > >> z0-~CKSlx#liMS>jt232#==00xPqwp;gsZrjc?`PPxaCE~>3(^o5-%+Nj=HegJFK2b > > >> z=l5zT4IDgO=sJ1zp=jyrALvcQJK|j;sN2_jUmobjN<!S6m1{G<LZ^+JP;T!|3{9)w > > >> eGf&ioo}iw|li2&4IyG;z<}vrl)Mic2`t2{52##q0 > > >> > > >> literal 0 > > >> HcmV?d00001 > > > > > > Please don't try to add a binary file to fstests directly. > > > > > >> > > >> diff --git a/tests/xfs/554 b/tests/xfs/554 > > >> new file mode 100644 > > >> index 00000000..13bc62c7 > > >> --- /dev/null > > >> +++ b/tests/xfs/554 > > >> @@ -0,0 +1,37 @@ > > >> +#! /bin/bash > > >> +# SPDX-License-Identifier: GPL-2.0 > > >> +# Copyright (c) 2022 Fujitsu Limited. All Rights Reserved. > > >> +# > > >> +# FS QA Test No. 554 > > >> +# > > >> +# Test restoring a dumpfile with a wrong root inode number created by > > >> +# xfsdump 3.1.8. > > >> +# This test restores the checked-in broken dump with '-x' flag. > > >> +# > > >> + > > >> +. ./common/preamble > > >> +_begin_fstest auto quick dump > > >> + > > >> +# Import common functions. > > >> +. ./common/dump > > >> + > > >> +# real QA test starts here > > >> +_supported_fs xfs > > >> +_require_scratch > > > > > > The -x option is a new feature for xfsdump, not all system support that. So > > > we need to _notrun if test on a system doesn't support it. A separated > > > _require_* helper would be better if there'll be more testing about this > > > new feature. Or a local detection in this case is fine too (can be moved as > > > a common helper later). > > > > > >> +_scratch_mkfs_xfs >>$seqres.full || _fail "mkfs failed" > > >> +_scratch_mount > > >> + > > >> +# Create dumpdir for comparing with restoredir > > >> +rm -rf $dump_dir > > >> +mkdir $dump_dir || _fail "failed to mkdir $restore_dir" > > > ^^ ^^ > > > > > > Are you trying to create a dump dir or restore dir? > > > > > >> +touch $dump_dir/FILE_1019 > > >> + > > >> +_do_restore_toc -x -f $here/src/root-inode-broken-dumpfile > > > > > > Why I didn't see how you generate this broken dumpfile in this case? > > > > > > Oh... I see, you want to store a dumpfile in fstests source code directly. > > > I thought you submited that file accidentally... > > > > > > No, we don't do things like this way, please try to generate the dumpfile > > > in this test case at first, then restore it. For example using xfs_db to > > > break a xfs, or using some tricky method (likes xfs/545). > > > > > > > Thank you for the comments. I will try another approach. I'm having > > trouble creating a dumpfile for this test. Because xfsdump was already > > fixed, xfsdump no longer generates a corrupted dumpfile even if there is > > a lower inode number than a root inode. > > Oh, I see. You can try, but I think it's hard. It maybe not suitable to be a > fstests case, if it has to depend on a binary fs dump file. If so, We can cover > it on other place, with this existed "bad" dump file. How difficult is it to create a dumpfile with a broken root inode? Is it a simple matter of creating a good dump and patching a few bytes, or do we end up having to patch the whole file? (The reason I ask is that I've heard about this problem for ages but I don't actually know how to create a bad dump...) --D > Thanks, > Zorro > > > > > >> + > > >> +_do_restore_file -x -f $here/src/root-inode-broken-dumpfile -L stress_545 > > >> +_diff_compare_sub > > >> +_ls_nodate_compare_sub > > >> + > > >> +# success, all done > > >> +status=0 > > >> +exit > > >> diff --git a/tests/xfs/554.out b/tests/xfs/554.out > > >> new file mode 100644 > > >> index 00000000..40a3f3a4 > > >> --- /dev/null > > >> +++ b/tests/xfs/554.out > > >> @@ -0,0 +1,57 @@ > > >> +QA output created by 554 > > >> +Contents of dump ... > > >> +xfsrestore -x -f DUMP_FILE -t > > >> +xfsrestore: using file dump (drive_simple) strategy > > >> +xfsrestore: searching media for dump > > >> +xfsrestore: examining media file 0 > > >> +xfsrestore: dump description: > > >> +xfsrestore: hostname: xfsdump > > >> +xfsrestore: mount point: SCRATCH_MNT > > >> +xfsrestore: volume: SCRATCH_DEV > > >> +xfsrestore: session time: TIME > > >> +xfsrestore: level: 0 > > >> +xfsrestore: session label: "stress_545" > > >> +xfsrestore: media label: "stress_tape_media" > > >> +xfsrestore: file system ID: ID > > >> +xfsrestore: session id: ID > > >> +xfsrestore: media ID: ID > > >> +xfsrestore: searching media for directory dump > > >> +xfsrestore: reading directories > > >> +xfsrestore: found fake rootino #128, will fix. > > >> +xfsrestore: fix root # to 1024 (bind mount?) > > >> +xfsrestore: 2 directories and 2 entries processed > > >> +xfsrestore: directory post-processing > > >> +xfsrestore: reading non-directory files > > >> +xfsrestore: table of contents display complete: SECS seconds elapsed > > >> +xfsrestore: Restore Status: SUCCESS > > >> + > > >> +dumpdir/FILE_1019 > > >> +Restoring from file... > > >> +xfsrestore -x -f DUMP_FILE -L stress_545 RESTORE_DIR > > >> +xfsrestore: using file dump (drive_simple) strategy > > >> +xfsrestore: searching media for dump > > >> +xfsrestore: examining media file 0 > > >> +xfsrestore: found dump matching specified label: > > >> +xfsrestore: hostname: xfsdump > > >> +xfsrestore: mount point: SCRATCH_MNT > > >> +xfsrestore: volume: SCRATCH_DEV > > >> +xfsrestore: session time: TIME > > >> +xfsrestore: level: 0 > > >> +xfsrestore: session label: "stress_545" > > >> +xfsrestore: media label: "stress_tape_media" > > >> +xfsrestore: file system ID: ID > > >> +xfsrestore: session id: ID > > >> +xfsrestore: media ID: ID > > >> +xfsrestore: searching media for directory dump > > >> +xfsrestore: reading directories > > >> +xfsrestore: found fake rootino #128, will fix. > > >> +xfsrestore: fix root # to 1024 (bind mount?) > > >> +xfsrestore: 2 directories and 2 entries processed > > >> +xfsrestore: directory post-processing > > >> +xfsrestore: restoring non-directory files > > >> +xfsrestore: restore complete: SECS seconds elapsed > > >> +xfsrestore: Restore Status: SUCCESS > > >> +Comparing dump directory with restore directory > > >> +Files DUMP_DIR/FILE_1019 and RESTORE_DIR/DUMP_SUBDIR/FILE_1019 are identical > > >> +Comparing listing of dump directory with restore directory > > >> +Files TMP.dump_dir and TMP.restore_dir are identical > > >> -- > > >> 2.37.3 > > >> > > > > > >
On 10/3/22 18:04, Darrick J. Wong wrote: > On Sun, Oct 02, 2022 at 12:27:08PM +0800, Zorro Lang wrote: >> On Fri, Sep 30, 2022 at 11:01:51AM -0400, Hironori Shiina wrote: >>> >>> >>> On 9/28/22 21:49, Zorro Lang wrote: >>>> On Wed, Sep 28, 2022 at 05:03:37PM -0400, Hironori Shiina wrote: >>>>> Test '-x' option of xfsrestore. With this option, a wrong root inode >>>>> number is corrected. A root inode number can be wrong in a dump created >>>>> by problematic xfsdump (v3.1.7 - v3.1.9) with blukstat misuse. This >>>>> patch adds a dump with a wrong inode number created by xfsdump 3.1.8. >>>>> >>>>> Link: https://lore.kernel.org/linux-xfs/20201113125127.966243-1-hsiangkao@redhat.com/ >>>>> Signed-off-by: Hironori Shiina <shiina.hironori@fujitsu.com> >>>>> --- >>>>> common/dump | 2 +- >>>>> src/root-inode-broken-dumpfile | Bin 0 -> 21648 bytes >>>>> tests/xfs/554 | 37 +++++++++++++++++++++ >>>>> tests/xfs/554.out | 57 +++++++++++++++++++++++++++++++++ >>>>> 4 files changed, 95 insertions(+), 1 deletion(-) >>>>> create mode 100644 src/root-inode-broken-dumpfile >>>>> create mode 100644 tests/xfs/554 >>>>> create mode 100644 tests/xfs/554.out >>>>> >>>>> diff --git a/common/dump b/common/dump >>>>> index 8e0446d9..50b2ba03 100644 >>>>> --- a/common/dump >>>>> +++ b/common/dump >>>>> @@ -1003,7 +1003,7 @@ _parse_restore_args() >>>>> --no-check-quota) >>>>> do_quota_check=false >>>>> ;; >>>>> - -K|-R) >>>>> + -K|-R|-x) >>>>> restore_args="$restore_args $1" >>>>> ;; >>>>> *) >>>>> diff --git a/src/root-inode-broken-dumpfile b/src/root-inode-broken-dumpfile >>>>> new file mode 100644 >>>>> index 0000000000000000000000000000000000000000..9a42e65d8047497be31f3abbaf4223ae384fd14d >>>>> GIT binary patch >>>>> literal 21648 >>>>> zcmeI)K}ge49KiAC_J<CEBr!0AwBf~zbCHB}5DyxL6eU8Jnqylvayj;2VuG+d)TN6; >>>>> z5J8vlAaw9hXi(UousT$Sin@BRJfwHM)O+*2*njz_zc+h*ckuV#@BMuKe;;JbgTL{< >>>>> z!SwZ9zC#ERe%reLe5&cz2e}qM<!i2dXQHt^{^`d1Qx{)MMzW#$%dR@J={0`IRsGx4 >>>>> z(yn@Oi-nBqCOVIG?&{kpwnv~&w_>6_ozY1^fph=w8(=^oTg&wOe=(WQByyQ_Hfd|4 >>>>> z^o76<0!zn#v>k3blbU)nzx=KF^}-G%R;OaQYsHwGDkO`kD^@q^(_Ac_8H<gKj^>a0 >>>>> z6p*%BK>q#b>2EE%^!@f?Z`-p+tI@M}qmMm@zMJk>K1U^=d~G^NUG?X4vkvKtOf-3Y >>>>> zpVM9YgM9Y7{*P0ApJNUh^uk1wCnA6V0tg_000IagfB*srAb<b@2q1s}0tg_000Iag >>>>> zfB*srAb<b@2q1s}0tg_000IagfB*srAb<b@2q1s}0tg_000IagfB*srAb`MM1p>_h >>>>> z2-R=Q9ooK1{l9<Dy8IIMUVXr9BW9uI1x7};j+kij|5kLI^32no;n|PvqD74ZOlJ#n >>>>> z0-~CKSlx#liMS>jt232#==00xPqwp;gsZrjc?`PPxaCE~>3(^o5-%+Nj=HegJFK2b >>>>> z=l5zT4IDgO=sJ1zp=jyrALvcQJK|j;sN2_jUmobjN<!S6m1{G<LZ^+JP;T!|3{9)w >>>>> eGf&ioo}iw|li2&4IyG;z<}vrl)Mic2`t2{52##q0 >>>>> >>>>> literal 0 >>>>> HcmV?d00001 >>>> >>>> Please don't try to add a binary file to fstests directly. >>>> >>>>> >>>>> diff --git a/tests/xfs/554 b/tests/xfs/554 >>>>> new file mode 100644 >>>>> index 00000000..13bc62c7 >>>>> --- /dev/null >>>>> +++ b/tests/xfs/554 >>>>> @@ -0,0 +1,37 @@ >>>>> +#! /bin/bash >>>>> +# SPDX-License-Identifier: GPL-2.0 >>>>> +# Copyright (c) 2022 Fujitsu Limited. All Rights Reserved. >>>>> +# >>>>> +# FS QA Test No. 554 >>>>> +# >>>>> +# Test restoring a dumpfile with a wrong root inode number created by >>>>> +# xfsdump 3.1.8. >>>>> +# This test restores the checked-in broken dump with '-x' flag. >>>>> +# >>>>> + >>>>> +. ./common/preamble >>>>> +_begin_fstest auto quick dump >>>>> + >>>>> +# Import common functions. >>>>> +. ./common/dump >>>>> + >>>>> +# real QA test starts here >>>>> +_supported_fs xfs >>>>> +_require_scratch >>>> >>>> The -x option is a new feature for xfsdump, not all system support that. So >>>> we need to _notrun if test on a system doesn't support it. A separated >>>> _require_* helper would be better if there'll be more testing about this >>>> new feature. Or a local detection in this case is fine too (can be moved as >>>> a common helper later). >>>> >>>>> +_scratch_mkfs_xfs >>$seqres.full || _fail "mkfs failed" >>>>> +_scratch_mount >>>>> + >>>>> +# Create dumpdir for comparing with restoredir >>>>> +rm -rf $dump_dir >>>>> +mkdir $dump_dir || _fail "failed to mkdir $restore_dir" >>>> ^^ ^^ >>>> >>>> Are you trying to create a dump dir or restore dir? >>>> >>>>> +touch $dump_dir/FILE_1019 >>>>> + >>>>> +_do_restore_toc -x -f $here/src/root-inode-broken-dumpfile >>>> >>>> Why I didn't see how you generate this broken dumpfile in this case? >>>> >>>> Oh... I see, you want to store a dumpfile in fstests source code directly. >>>> I thought you submited that file accidentally... >>>> >>>> No, we don't do things like this way, please try to generate the dumpfile >>>> in this test case at first, then restore it. For example using xfs_db to >>>> break a xfs, or using some tricky method (likes xfs/545). >>>> >>> >>> Thank you for the comments. I will try another approach. I'm having >>> trouble creating a dumpfile for this test. Because xfsdump was already >>> fixed, xfsdump no longer generates a corrupted dumpfile even if there is >>> a lower inode number than a root inode. >> >> Oh, I see. You can try, but I think it's hard. It maybe not suitable to be a >> fstests case, if it has to depend on a binary fs dump file. If so, We can cover >> it on other place, with this existed "bad" dump file. > > How difficult is it to create a dumpfile with a broken root inode? > Is it a simple matter of creating a good dump and patching a few bytes, > or do we end up having to patch the whole file? > > (The reason I ask is that I've heard about this problem for ages but > I don't actually know how to create a bad dump...) > As I dug into the data structure, I succeeded in creating a dumpfile with a wrong root inode by modifying `content_inode_hdr_t.cih_rootino` and `global_hdr_t.gh_checksum`. After creating a inode with a lower number than the root inode with the method in generic/545, I modified a dumpfile as follows: --- _do_dump_file # Break dumpfile old_checksum=$(od -A n -j 12 -N 4 -i --endian=big $dump_file) gap=$(($root_inum - $inum)) new_checksum=$(($old_checksum + ($gap >> 32) + ($gap & 0x00000000ffffffff))) v=($(printf "%016x" $inum | awk -v FS='' '{for (i=1; i<=NF; i+=2) print $i$(i+1)}')) echo -en "\x${v[0]}\x${v[1]}\x${v[2]}\x${v[3]}\x${v[4]}\x${v[5]}\x${v[6]}\x${v[7]}" | dd of=$dump_file bs=1 seek=3928 conv=notrunc v=($(printf "%016x" $new_checksum | awk -v FS='' '{for (i=9; i<=NF; i+=2) print $i$(i+1)}')) echo -en "\x${v[0]}\x${v[1]}\x${v[2]}\x${v[3]}" | dd of=$dump_file bs=1 seek=12 conv=notrunc _do_restore_file -x --- (I'm afraid I'm not so familiar with editing a binary file.) The offset of `global_hdr_t.gh_checksum` is 12. The offset of `content_inode_hdr_t.cih_rootino` is: global_hdr_t.gh_upper (0x400) + drive_hdr_t.dh_upper (0x400) + media_hdr_t.mh_upper (0x400) + content_hdr_t.ch_specific (0x340) + content_inode_hdr_t.cih_rootino (0x18) = 0xf58 (3928) Thanks, Hiro > --D > >> Thanks, >> Zorro >> >>> >>>>> + >>>>> +_do_restore_file -x -f $here/src/root-inode-broken-dumpfile -L stress_545 >>>>> +_diff_compare_sub >>>>> +_ls_nodate_compare_sub >>>>> + >>>>> +# success, all done >>>>> +status=0 >>>>> +exit >>>>> diff --git a/tests/xfs/554.out b/tests/xfs/554.out >>>>> new file mode 100644 >>>>> index 00000000..40a3f3a4 >>>>> --- /dev/null >>>>> +++ b/tests/xfs/554.out >>>>> @@ -0,0 +1,57 @@ >>>>> +QA output created by 554 >>>>> +Contents of dump ... >>>>> +xfsrestore -x -f DUMP_FILE -t >>>>> +xfsrestore: using file dump (drive_simple) strategy >>>>> +xfsrestore: searching media for dump >>>>> +xfsrestore: examining media file 0 >>>>> +xfsrestore: dump description: >>>>> +xfsrestore: hostname: xfsdump >>>>> +xfsrestore: mount point: SCRATCH_MNT >>>>> +xfsrestore: volume: SCRATCH_DEV >>>>> +xfsrestore: session time: TIME >>>>> +xfsrestore: level: 0 >>>>> +xfsrestore: session label: "stress_545" >>>>> +xfsrestore: media label: "stress_tape_media" >>>>> +xfsrestore: file system ID: ID >>>>> +xfsrestore: session id: ID >>>>> +xfsrestore: media ID: ID >>>>> +xfsrestore: searching media for directory dump >>>>> +xfsrestore: reading directories >>>>> +xfsrestore: found fake rootino #128, will fix. >>>>> +xfsrestore: fix root # to 1024 (bind mount?) >>>>> +xfsrestore: 2 directories and 2 entries processed >>>>> +xfsrestore: directory post-processing >>>>> +xfsrestore: reading non-directory files >>>>> +xfsrestore: table of contents display complete: SECS seconds elapsed >>>>> +xfsrestore: Restore Status: SUCCESS >>>>> + >>>>> +dumpdir/FILE_1019 >>>>> +Restoring from file... >>>>> +xfsrestore -x -f DUMP_FILE -L stress_545 RESTORE_DIR >>>>> +xfsrestore: using file dump (drive_simple) strategy >>>>> +xfsrestore: searching media for dump >>>>> +xfsrestore: examining media file 0 >>>>> +xfsrestore: found dump matching specified label: >>>>> +xfsrestore: hostname: xfsdump >>>>> +xfsrestore: mount point: SCRATCH_MNT >>>>> +xfsrestore: volume: SCRATCH_DEV >>>>> +xfsrestore: session time: TIME >>>>> +xfsrestore: level: 0 >>>>> +xfsrestore: session label: "stress_545" >>>>> +xfsrestore: media label: "stress_tape_media" >>>>> +xfsrestore: file system ID: ID >>>>> +xfsrestore: session id: ID >>>>> +xfsrestore: media ID: ID >>>>> +xfsrestore: searching media for directory dump >>>>> +xfsrestore: reading directories >>>>> +xfsrestore: found fake rootino #128, will fix. >>>>> +xfsrestore: fix root # to 1024 (bind mount?) >>>>> +xfsrestore: 2 directories and 2 entries processed >>>>> +xfsrestore: directory post-processing >>>>> +xfsrestore: restoring non-directory files >>>>> +xfsrestore: restore complete: SECS seconds elapsed >>>>> +xfsrestore: Restore Status: SUCCESS >>>>> +Comparing dump directory with restore directory >>>>> +Files DUMP_DIR/FILE_1019 and RESTORE_DIR/DUMP_SUBDIR/FILE_1019 are identical >>>>> +Comparing listing of dump directory with restore directory >>>>> +Files TMP.dump_dir and TMP.restore_dir are identical >>>>> -- >>>>> 2.37.3 >>>>> >>>> >>> >>
On Fri, Oct 07, 2022 at 09:59:35PM -0400, Hironori Shiina wrote: > > > On 10/3/22 18:04, Darrick J. Wong wrote: > > On Sun, Oct 02, 2022 at 12:27:08PM +0800, Zorro Lang wrote: > >> On Fri, Sep 30, 2022 at 11:01:51AM -0400, Hironori Shiina wrote: > >>> > >>> > >>> On 9/28/22 21:49, Zorro Lang wrote: > >>>> On Wed, Sep 28, 2022 at 05:03:37PM -0400, Hironori Shiina wrote: > >>>>> Test '-x' option of xfsrestore. With this option, a wrong root inode > >>>>> number is corrected. A root inode number can be wrong in a dump created > >>>>> by problematic xfsdump (v3.1.7 - v3.1.9) with blukstat misuse. This > >>>>> patch adds a dump with a wrong inode number created by xfsdump 3.1.8. > >>>>> > >>>>> Link: https://lore.kernel.org/linux-xfs/20201113125127.966243-1-hsiangkao@redhat.com/ > >>>>> Signed-off-by: Hironori Shiina <shiina.hironori@fujitsu.com> > >>>>> --- > >>>>> common/dump | 2 +- > >>>>> src/root-inode-broken-dumpfile | Bin 0 -> 21648 bytes > >>>>> tests/xfs/554 | 37 +++++++++++++++++++++ > >>>>> tests/xfs/554.out | 57 +++++++++++++++++++++++++++++++++ > >>>>> 4 files changed, 95 insertions(+), 1 deletion(-) > >>>>> create mode 100644 src/root-inode-broken-dumpfile > >>>>> create mode 100644 tests/xfs/554 > >>>>> create mode 100644 tests/xfs/554.out > >>>>> > >>>>> diff --git a/common/dump b/common/dump > >>>>> index 8e0446d9..50b2ba03 100644 > >>>>> --- a/common/dump > >>>>> +++ b/common/dump > >>>>> @@ -1003,7 +1003,7 @@ _parse_restore_args() > >>>>> --no-check-quota) > >>>>> do_quota_check=false > >>>>> ;; > >>>>> - -K|-R) > >>>>> + -K|-R|-x) > >>>>> restore_args="$restore_args $1" > >>>>> ;; > >>>>> *) > >>>>> diff --git a/src/root-inode-broken-dumpfile b/src/root-inode-broken-dumpfile > >>>>> new file mode 100644 > >>>>> index 0000000000000000000000000000000000000000..9a42e65d8047497be31f3abbaf4223ae384fd14d > >>>>> GIT binary patch > >>>>> literal 21648 > >>>>> zcmeI)K}ge49KiAC_J<CEBr!0AwBf~zbCHB}5DyxL6eU8Jnqylvayj;2VuG+d)TN6; > >>>>> z5J8vlAaw9hXi(UousT$Sin@BRJfwHM)O+*2*njz_zc+h*ckuV#@BMuKe;;JbgTL{< > >>>>> z!SwZ9zC#ERe%reLe5&cz2e}qM<!i2dXQHt^{^`d1Qx{)MMzW#$%dR@J={0`IRsGx4 > >>>>> z(yn@Oi-nBqCOVIG?&{kpwnv~&w_>6_ozY1^fph=w8(=^oTg&wOe=(WQByyQ_Hfd|4 > >>>>> z^o76<0!zn#v>k3blbU)nzx=KF^}-G%R;OaQYsHwGDkO`kD^@q^(_Ac_8H<gKj^>a0 > >>>>> z6p*%BK>q#b>2EE%^!@f?Z`-p+tI@M}qmMm@zMJk>K1U^=d~G^NUG?X4vkvKtOf-3Y > >>>>> zpVM9YgM9Y7{*P0ApJNUh^uk1wCnA6V0tg_000IagfB*srAb<b@2q1s}0tg_000Iag > >>>>> zfB*srAb<b@2q1s}0tg_000IagfB*srAb<b@2q1s}0tg_000IagfB*srAb`MM1p>_h > >>>>> z2-R=Q9ooK1{l9<Dy8IIMUVXr9BW9uI1x7};j+kij|5kLI^32no;n|PvqD74ZOlJ#n > >>>>> z0-~CKSlx#liMS>jt232#==00xPqwp;gsZrjc?`PPxaCE~>3(^o5-%+Nj=HegJFK2b > >>>>> z=l5zT4IDgO=sJ1zp=jyrALvcQJK|j;sN2_jUmobjN<!S6m1{G<LZ^+JP;T!|3{9)w > >>>>> eGf&ioo}iw|li2&4IyG;z<}vrl)Mic2`t2{52##q0 > >>>>> > >>>>> literal 0 > >>>>> HcmV?d00001 > >>>> > >>>> Please don't try to add a binary file to fstests directly. > >>>> > >>>>> > >>>>> diff --git a/tests/xfs/554 b/tests/xfs/554 > >>>>> new file mode 100644 > >>>>> index 00000000..13bc62c7 > >>>>> --- /dev/null > >>>>> +++ b/tests/xfs/554 > >>>>> @@ -0,0 +1,37 @@ > >>>>> +#! /bin/bash > >>>>> +# SPDX-License-Identifier: GPL-2.0 > >>>>> +# Copyright (c) 2022 Fujitsu Limited. All Rights Reserved. > >>>>> +# > >>>>> +# FS QA Test No. 554 > >>>>> +# > >>>>> +# Test restoring a dumpfile with a wrong root inode number created by > >>>>> +# xfsdump 3.1.8. > >>>>> +# This test restores the checked-in broken dump with '-x' flag. > >>>>> +# > >>>>> + > >>>>> +. ./common/preamble > >>>>> +_begin_fstest auto quick dump > >>>>> + > >>>>> +# Import common functions. > >>>>> +. ./common/dump > >>>>> + > >>>>> +# real QA test starts here > >>>>> +_supported_fs xfs > >>>>> +_require_scratch > >>>> > >>>> The -x option is a new feature for xfsdump, not all system support that. So > >>>> we need to _notrun if test on a system doesn't support it. A separated > >>>> _require_* helper would be better if there'll be more testing about this > >>>> new feature. Or a local detection in this case is fine too (can be moved as > >>>> a common helper later). > >>>> > >>>>> +_scratch_mkfs_xfs >>$seqres.full || _fail "mkfs failed" > >>>>> +_scratch_mount > >>>>> + > >>>>> +# Create dumpdir for comparing with restoredir > >>>>> +rm -rf $dump_dir > >>>>> +mkdir $dump_dir || _fail "failed to mkdir $restore_dir" > >>>> ^^ ^^ > >>>> > >>>> Are you trying to create a dump dir or restore dir? > >>>> > >>>>> +touch $dump_dir/FILE_1019 > >>>>> + > >>>>> +_do_restore_toc -x -f $here/src/root-inode-broken-dumpfile > >>>> > >>>> Why I didn't see how you generate this broken dumpfile in this case? > >>>> > >>>> Oh... I see, you want to store a dumpfile in fstests source code directly. > >>>> I thought you submited that file accidentally... > >>>> > >>>> No, we don't do things like this way, please try to generate the dumpfile > >>>> in this test case at first, then restore it. For example using xfs_db to > >>>> break a xfs, or using some tricky method (likes xfs/545). > >>>> > >>> > >>> Thank you for the comments. I will try another approach. I'm having > >>> trouble creating a dumpfile for this test. Because xfsdump was already > >>> fixed, xfsdump no longer generates a corrupted dumpfile even if there is > >>> a lower inode number than a root inode. > >> > >> Oh, I see. You can try, but I think it's hard. It maybe not suitable to be a > >> fstests case, if it has to depend on a binary fs dump file. If so, We can cover > >> it on other place, with this existed "bad" dump file. > > > > How difficult is it to create a dumpfile with a broken root inode? > > Is it a simple matter of creating a good dump and patching a few bytes, > > or do we end up having to patch the whole file? > > > > (The reason I ask is that I've heard about this problem for ages but > > I don't actually know how to create a bad dump...) > > > > As I dug into the data structure, I succeeded in creating a dumpfile with a wrong > root inode by modifying `content_inode_hdr_t.cih_rootino` and > `global_hdr_t.gh_checksum`. > > After creating a inode with a lower number than the root inode with the method in > generic/545, I modified a dumpfile as follows: > --- > _do_dump_file > > # Break dumpfile > old_checksum=$(od -A n -j 12 -N 4 -i --endian=big $dump_file) > gap=$(($root_inum - $inum)) > new_checksum=$(($old_checksum + ($gap >> 32) + ($gap & 0x00000000ffffffff))) > v=($(printf "%016x" $inum | awk -v FS='' '{for (i=1; i<=NF; i+=2) print $i$(i+1)}')) > echo -en "\x${v[0]}\x${v[1]}\x${v[2]}\x${v[3]}\x${v[4]}\x${v[5]}\x${v[6]}\x${v[7]}" | dd of=$dump_file bs=1 seek=3928 conv=notrunc > v=($(printf "%016x" $new_checksum | awk -v FS='' '{for (i=9; i<=NF; i+=2) print $i$(i+1)}')) > echo -en "\x${v[0]}\x${v[1]}\x${v[2]}\x${v[3]}" | dd of=$dump_file bs=1 seek=12 conv=notrunc > > _do_restore_file -x > --- > (I'm afraid I'm not so familiar with editing a binary file.) That's the only way I know of to do this via shell script. :/ Perhaps it would be cleaner to submit a small C program to do the editing instead of relying on bash? > The offset of `global_hdr_t.gh_checksum` is 12. > The offset of `content_inode_hdr_t.cih_rootino` is: > global_hdr_t.gh_upper (0x400) + drive_hdr_t.dh_upper (0x400) + > media_hdr_t.mh_upper (0x400) + content_hdr_t.ch_specific (0x340) + > content_inode_hdr_t.cih_rootino (0x18) = 0xf58 (3928) Whatever you come up with, please include this comment with the program code so that it's more obvious what the byte editing is doing. --D > Thanks, > Hiro > > > --D > > > >> Thanks, > >> Zorro > >> > >>> > >>>>> + > >>>>> +_do_restore_file -x -f $here/src/root-inode-broken-dumpfile -L stress_545 > >>>>> +_diff_compare_sub > >>>>> +_ls_nodate_compare_sub > >>>>> + > >>>>> +# success, all done > >>>>> +status=0 > >>>>> +exit > >>>>> diff --git a/tests/xfs/554.out b/tests/xfs/554.out > >>>>> new file mode 100644 > >>>>> index 00000000..40a3f3a4 > >>>>> --- /dev/null > >>>>> +++ b/tests/xfs/554.out > >>>>> @@ -0,0 +1,57 @@ > >>>>> +QA output created by 554 > >>>>> +Contents of dump ... > >>>>> +xfsrestore -x -f DUMP_FILE -t > >>>>> +xfsrestore: using file dump (drive_simple) strategy > >>>>> +xfsrestore: searching media for dump > >>>>> +xfsrestore: examining media file 0 > >>>>> +xfsrestore: dump description: > >>>>> +xfsrestore: hostname: xfsdump > >>>>> +xfsrestore: mount point: SCRATCH_MNT > >>>>> +xfsrestore: volume: SCRATCH_DEV > >>>>> +xfsrestore: session time: TIME > >>>>> +xfsrestore: level: 0 > >>>>> +xfsrestore: session label: "stress_545" > >>>>> +xfsrestore: media label: "stress_tape_media" > >>>>> +xfsrestore: file system ID: ID > >>>>> +xfsrestore: session id: ID > >>>>> +xfsrestore: media ID: ID > >>>>> +xfsrestore: searching media for directory dump > >>>>> +xfsrestore: reading directories > >>>>> +xfsrestore: found fake rootino #128, will fix. > >>>>> +xfsrestore: fix root # to 1024 (bind mount?) > >>>>> +xfsrestore: 2 directories and 2 entries processed > >>>>> +xfsrestore: directory post-processing > >>>>> +xfsrestore: reading non-directory files > >>>>> +xfsrestore: table of contents display complete: SECS seconds elapsed > >>>>> +xfsrestore: Restore Status: SUCCESS > >>>>> + > >>>>> +dumpdir/FILE_1019 > >>>>> +Restoring from file... > >>>>> +xfsrestore -x -f DUMP_FILE -L stress_545 RESTORE_DIR > >>>>> +xfsrestore: using file dump (drive_simple) strategy > >>>>> +xfsrestore: searching media for dump > >>>>> +xfsrestore: examining media file 0 > >>>>> +xfsrestore: found dump matching specified label: > >>>>> +xfsrestore: hostname: xfsdump > >>>>> +xfsrestore: mount point: SCRATCH_MNT > >>>>> +xfsrestore: volume: SCRATCH_DEV > >>>>> +xfsrestore: session time: TIME > >>>>> +xfsrestore: level: 0 > >>>>> +xfsrestore: session label: "stress_545" > >>>>> +xfsrestore: media label: "stress_tape_media" > >>>>> +xfsrestore: file system ID: ID > >>>>> +xfsrestore: session id: ID > >>>>> +xfsrestore: media ID: ID > >>>>> +xfsrestore: searching media for directory dump > >>>>> +xfsrestore: reading directories > >>>>> +xfsrestore: found fake rootino #128, will fix. > >>>>> +xfsrestore: fix root # to 1024 (bind mount?) > >>>>> +xfsrestore: 2 directories and 2 entries processed > >>>>> +xfsrestore: directory post-processing > >>>>> +xfsrestore: restoring non-directory files > >>>>> +xfsrestore: restore complete: SECS seconds elapsed > >>>>> +xfsrestore: Restore Status: SUCCESS > >>>>> +Comparing dump directory with restore directory > >>>>> +Files DUMP_DIR/FILE_1019 and RESTORE_DIR/DUMP_SUBDIR/FILE_1019 are identical > >>>>> +Comparing listing of dump directory with restore directory > >>>>> +Files TMP.dump_dir and TMP.restore_dir are identical > >>>>> -- > >>>>> 2.37.3 > >>>>> > >>>> > >>> > >>
diff --git a/common/dump b/common/dump index 8e0446d9..50b2ba03 100644 --- a/common/dump +++ b/common/dump @@ -1003,7 +1003,7 @@ _parse_restore_args() --no-check-quota) do_quota_check=false ;; - -K|-R) + -K|-R|-x) restore_args="$restore_args $1" ;; *) diff --git a/src/root-inode-broken-dumpfile b/src/root-inode-broken-dumpfile new file mode 100644 index 0000000000000000000000000000000000000000..9a42e65d8047497be31f3abbaf4223ae384fd14d GIT binary patch literal 21648 zcmeI)K}ge49KiAC_J<CEBr!0AwBf~zbCHB}5DyxL6eU8Jnqylvayj;2VuG+d)TN6; z5J8vlAaw9hXi(UousT$Sin@BRJfwHM)O+*2*njz_zc+h*ckuV#@BMuKe;;JbgTL{< z!SwZ9zC#ERe%reLe5&cz2e}qM<!i2dXQHt^{^`d1Qx{)MMzW#$%dR@J={0`IRsGx4 z(yn@Oi-nBqCOVIG?&{kpwnv~&w_>6_ozY1^fph=w8(=^oTg&wOe=(WQByyQ_Hfd|4 z^o76<0!zn#v>k3blbU)nzx=KF^}-G%R;OaQYsHwGDkO`kD^@q^(_Ac_8H<gKj^>a0 z6p*%BK>q#b>2EE%^!@f?Z`-p+tI@M}qmMm@zMJk>K1U^=d~G^NUG?X4vkvKtOf-3Y zpVM9YgM9Y7{*P0ApJNUh^uk1wCnA6V0tg_000IagfB*srAb<b@2q1s}0tg_000Iag zfB*srAb<b@2q1s}0tg_000IagfB*srAb<b@2q1s}0tg_000IagfB*srAb`MM1p>_h z2-R=Q9ooK1{l9<Dy8IIMUVXr9BW9uI1x7};j+kij|5kLI^32no;n|PvqD74ZOlJ#n z0-~CKSlx#liMS>jt232#==00xPqwp;gsZrjc?`PPxaCE~>3(^o5-%+Nj=HegJFK2b z=l5zT4IDgO=sJ1zp=jyrALvcQJK|j;sN2_jUmobjN<!S6m1{G<LZ^+JP;T!|3{9)w eGf&ioo}iw|li2&4IyG;z<}vrl)Mic2`t2{52##q0 literal 0 HcmV?d00001 diff --git a/tests/xfs/554 b/tests/xfs/554 new file mode 100644 index 00000000..13bc62c7 --- /dev/null +++ b/tests/xfs/554 @@ -0,0 +1,37 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 Fujitsu Limited. All Rights Reserved. +# +# FS QA Test No. 554 +# +# Test restoring a dumpfile with a wrong root inode number created by +# xfsdump 3.1.8. +# This test restores the checked-in broken dump with '-x' flag. +# + +. ./common/preamble +_begin_fstest auto quick dump + +# Import common functions. +. ./common/dump + +# real QA test starts here +_supported_fs xfs +_require_scratch +_scratch_mkfs_xfs >>$seqres.full || _fail "mkfs failed" +_scratch_mount + +# Create dumpdir for comparing with restoredir +rm -rf $dump_dir +mkdir $dump_dir || _fail "failed to mkdir $restore_dir" +touch $dump_dir/FILE_1019 + +_do_restore_toc -x -f $here/src/root-inode-broken-dumpfile + +_do_restore_file -x -f $here/src/root-inode-broken-dumpfile -L stress_545 +_diff_compare_sub +_ls_nodate_compare_sub + +# success, all done +status=0 +exit diff --git a/tests/xfs/554.out b/tests/xfs/554.out new file mode 100644 index 00000000..40a3f3a4 --- /dev/null +++ b/tests/xfs/554.out @@ -0,0 +1,57 @@ +QA output created by 554 +Contents of dump ... +xfsrestore -x -f DUMP_FILE -t +xfsrestore: using file dump (drive_simple) strategy +xfsrestore: searching media for dump +xfsrestore: examining media file 0 +xfsrestore: dump description: +xfsrestore: hostname: xfsdump +xfsrestore: mount point: SCRATCH_MNT +xfsrestore: volume: SCRATCH_DEV +xfsrestore: session time: TIME +xfsrestore: level: 0 +xfsrestore: session label: "stress_545" +xfsrestore: media label: "stress_tape_media" +xfsrestore: file system ID: ID +xfsrestore: session id: ID +xfsrestore: media ID: ID +xfsrestore: searching media for directory dump +xfsrestore: reading directories +xfsrestore: found fake rootino #128, will fix. +xfsrestore: fix root # to 1024 (bind mount?) +xfsrestore: 2 directories and 2 entries processed +xfsrestore: directory post-processing +xfsrestore: reading non-directory files +xfsrestore: table of contents display complete: SECS seconds elapsed +xfsrestore: Restore Status: SUCCESS + +dumpdir/FILE_1019 +Restoring from file... +xfsrestore -x -f DUMP_FILE -L stress_545 RESTORE_DIR +xfsrestore: using file dump (drive_simple) strategy +xfsrestore: searching media for dump +xfsrestore: examining media file 0 +xfsrestore: found dump matching specified label: +xfsrestore: hostname: xfsdump +xfsrestore: mount point: SCRATCH_MNT +xfsrestore: volume: SCRATCH_DEV +xfsrestore: session time: TIME +xfsrestore: level: 0 +xfsrestore: session label: "stress_545" +xfsrestore: media label: "stress_tape_media" +xfsrestore: file system ID: ID +xfsrestore: session id: ID +xfsrestore: media ID: ID +xfsrestore: searching media for directory dump +xfsrestore: reading directories +xfsrestore: found fake rootino #128, will fix. +xfsrestore: fix root # to 1024 (bind mount?) +xfsrestore: 2 directories and 2 entries processed +xfsrestore: directory post-processing +xfsrestore: restoring non-directory files +xfsrestore: restore complete: SECS seconds elapsed +xfsrestore: Restore Status: SUCCESS +Comparing dump directory with restore directory +Files DUMP_DIR/FILE_1019 and RESTORE_DIR/DUMP_SUBDIR/FILE_1019 are identical +Comparing listing of dump directory with restore directory +Files TMP.dump_dir and TMP.restore_dir are identical
Test '-x' option of xfsrestore. With this option, a wrong root inode number is corrected. A root inode number can be wrong in a dump created by problematic xfsdump (v3.1.7 - v3.1.9) with blukstat misuse. This patch adds a dump with a wrong inode number created by xfsdump 3.1.8. Link: https://lore.kernel.org/linux-xfs/20201113125127.966243-1-hsiangkao@redhat.com/ Signed-off-by: Hironori Shiina <shiina.hironori@fujitsu.com> --- common/dump | 2 +- src/root-inode-broken-dumpfile | Bin 0 -> 21648 bytes tests/xfs/554 | 37 +++++++++++++++++++++ tests/xfs/554.out | 57 +++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 src/root-inode-broken-dumpfile create mode 100644 tests/xfs/554 create mode 100644 tests/xfs/554.out