gfs2-utils: Fix two logic errors
PATCH 1/3 gfs2-utils: Fix two logic errors
Two uses of lgfs2_get_leaf_ptr() were still expecting it to return 0 on failure. Remove the negations.
--- gfs2/edit/savemeta.c | 2 +- gfs2/fsck/pass2.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
gfs2/edit/savemeta.c +1 -1
@@ -542,7 +542,7 @@ static void save_inode_data(struct metafd *mfd)int li;for (li =0 ; li < (1 << inode->i_di.di_depth); li++) { -if (!lgfs2_get_leaf_ptr (inode, li, &leaf_no)) { +if (lgfs2_get_leaf_ptr (inode, li, &leaf_no)) {fprintf (stderr,"Could not read leaf index %d in dinode %" PRIu64"\n" , li, (uint64_t )inode->i_di.di_num.no_addr);exit (-1 );
gfs2/fsck/pass2.c +1 -1
@@ -370,7 +370,7 @@ static int wrong_leaf(struct gfs2_inode *ip, struct gfs2_inum *entry, de->de_type) == 0) {log_err (_ ("The misplaced directory entry was moved to a " "valid leaf block.\n" )); -if (!lgfs2_get_leaf_ptr (ip, hash_index, &real_leaf)) { +if (lgfs2_get_leaf_ptr (ip, hash_index, &real_leaf)) {log_err (_ ("Could not read leaf %d in dinode %" PRIu64": %s\n" ), hash_index, (uint64_t )ip->i_di.di_num.no_addr,strerror (errno)); }
Signature · 1 line
1.9.3
PATCH 2/3 gfs2_edit: Ensure all leaf blocks in per_node are saved
block_is_in_per_node() only checks the first leaf block when the per_node directory has been split (when the block size is small and/or the number of journals is large). This caused savemeta to treat the files in per_node as user data and so didn't save their metadata. Add an outer loop to iterate over the indirect blocks.
--- gfs2/edit/savemeta.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
gfs2/edit/savemeta.c +7 -4
@@ -75,7 +75,7 @@ static int block_is_a_journal(void)static int block_is_in_per_node (void ) { -int d; +int i;struct gfs2_inode *per_node_di;if (sbd.gfs1)
@@ -90,9 +90,12 @@ static int block_is_in_per_node(void)do_dinode_extended (&per_node_di->i_di, per_node_di->i_bh);inode_put (&per_node_di); -for (d =0 ; d < indirect->ii[0 ].dirents; d++) { -if (block == indirect->ii[0 ].dirent[d].block) -return TRUE; +for (i =0 ; i < indirect_blocks; i++) { +int d; +for (d =0 ; d < indirect->ii[i].dirents; d++) { +if (block == indirect->ii[i].dirent[d].block) +return TRUE; + } }return FALSE; }
Signature · 1 line
1.9.3
PATCH 2/3 gfs2_edit: Ensure all leaf blocks in per_node are saved
Hi,
Quoted 5 lines · Andrew Price
block_is_in_per_node()only checks the first leaf block when theper_nodedirectory has been split (when the block size is small and/or the number of journals is large). This caused savemeta to treat the files inper_nodeas user data and so didn't save their metadata. Add an outer loop to iterate over the indirect blocks.
Why does it matter whether the block in the per_node subdir or not? Are we doing something special compared with other directories?
Steve.
Quoted 34 lines
Signed-off-by Andrew Price <[email protected]>--- gfs2/edit/savemeta.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)gfs2/edit/savemeta.c +7
-4@@ -75,7 +75,7 @@ static int block_is_a_journal(void)static int block_is_in_per_node (void ) { -int d; +int i;struct gfs2_inode *per_node_di;if (sbd.gfs1)@@ -90,9 +90,12 @@ static int block_is_in_per_node(void)do_dinode_extended (&per_node_di->i_di, per_node_di->i_bh);inode_put (&per_node_di); -for (d =0 ; d < indirect->ii[0 ].dirents; d++) { -if (block == indirect->ii[0 ].dirent[d].block) -return TRUE; +for (i =0 ; i < indirect_blocks; i++) { +int d; +for (d =0 ; d < indirect->ii[i].dirents; d++) { +if (block == indirect->ii[i].dirent[d].block) +return TRUE; + } }return FALSE; }
PATCH 2/3 gfs2_edit: Ensure all leaf blocks in per_node are saved
Quoted 10 lines · Steven Whitehouse
Hi,
Quoted 5 lines · Andrew Price
block_is_in_per_node()only checks the first leaf block when theper_nodedirectory has been split (when the block size is small and/or the number of journals is large). This caused savemeta to treat the files inper_nodeas user data and so didn't save their metadata. Add an outer loop to iterate over the indirect blocks.Why does it matter whether the block in the
per_nodesubdir or not? Are we doing something special compared with other directories?
Yes, sorry I didn't make it very clear in the commit log: gfs2_edit savemeta saves less metadata when a block isn't part of a set of "system" structures to avoid saving user data while gathering enough metadata to aid debugging.
static int block_is_systemfile(void)
{
return block_is_jindex() || block_is_inum_file() ||
block_is_statfs_file() || block_is_quota_file() ||
block_is_rindex() || block_is_a_journal() ||
block_is_per_node() || block_is_in_per_node();
}save_inode_data() uses this function to decide the height at which to stop saving metadata.
https://git.fedorahosted.org/cgit/gfs2-utils.git/tree/gfs2/edit/savemeta.c#n499
Andy
Quoted 35 lines
Steve.
Quoted 31 lines
Signed-off-by Andrew Price <[email protected]>--- gfs2/edit/savemeta.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)gfs2/edit/savemeta.c +7
-4@@ -75,7 +75,7 @@ static int block_is_a_journal(void)static int block_is_in_per_node (void ) { -int d; +int i;struct gfs2_inode *per_node_di;if (sbd.gfs1)@@ -90,9 +90,12 @@ static int block_is_in_per_node(void)do_dinode_extended (&per_node_di->i_di, per_node_di->i_bh);inode_put (&per_node_di); -for (d =0 ; d < indirect->ii[0 ].dirents; d++) { -if (block == indirect->ii[0 ].dirent[d].block) -return TRUE; +for (i =0 ; i < indirect_blocks; i++) { +int d; +for (d =0 ; d < indirect->ii[i].dirents; d++) { +if (block == indirect->ii[i].dirent[d].block) +return TRUE; + } }return FALSE; }
PATCH 3/3 gfs2-utils tests: Add small-block savemeta tests
In reply to Andrew PriceAdd some savemeta/restoremeta test scenarios which were used to discover some recently fixed bugs, to make sure they stay fixed. They mainly cover scenarios relating to small block sizes.
--- tests/edit.at | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)
tests/edit.at +30 -2
@@ -1,7 +1,7 @@ AT_TESTED([gfs2_edit]) AT_BANNER([gfs2_edit tests]) -AT_SETUP([Savemeta/restoremeta, defaults]) +AT_SETUP([Save/restoremeta, defaults]) GFS_TGT_REGEN AT_CHECK([$GFS_MKFS -p lock_nolock $GFS_TGT $(($(gfs_max_blocks 4096)/2))], 0, [ignore], [ignore]) AT_CHECK([gfs2_edit savemeta $GFS_TGT test.meta > savemeta.log], 0, [ignore], [ignore])
@@ -13,10 +13,38 @@ AT_CHECK([gfs2_edit restoremeta test.meta $GFS_TGT], 0, [ignore], [ignore]) AT_CHECK([fsck.gfs2 -n $GFS_TGT], 0, [ignore], [ignore]) AT_CLEANUP -AT_SETUP([Savemeta/restoremeta, no compression]) +AT_SETUP([Save/restoremeta, no compression]) GFS_TGT_REGEN AT_CHECK([$GFS_MKFS -p lock_nolock $GFS_TGT $(($(gfs_max_blocks 4096)/2))], 0, [ignore], [ignore]) AT_CHECK([gfs2_edit savemeta -z0 $GFS_TGT test.meta], 0, [ignore], [ignore]) +GFS_TGT_REGEN +AT_CHECK([gfs2_edit restoremeta test.meta $GFS_TGT], 0, [ignore], [ignore]) +AT_CHECK([fsck.gfs2 -n $GFS_TGT], 0, [ignore], [ignore]) +AT_CLEANUP + +AT_SETUP([Save/restoremeta, min. block size]) +GFS_TGT_REGEN +AT_CHECK([$GFS_MKFS -p lock_nolock -b512 $GFS_TGT], 0, [ignore], [ignore]) +AT_CHECK([gfs2_edit savemeta -z0 $GFS_TGT test.meta], 0, [ignore], [ignore]) +GFS_TGT_REGEN +AT_CHECK([gfs2_edit restoremeta test.meta $GFS_TGT], 0, [ignore], [ignore]) +AT_CHECK([fsck.gfs2 -n $GFS_TGT], 0, [ignore], [ignore]) +AT_CLEANUP + +AT_SETUP([Save/restoremeta, 4 journals]) +GFS_TGT_REGEN +AT_CHECK([$GFS_MKFS -p lock_nolock -j4 -J8 $GFS_TGT], 0, [ignore], [ignore]) +AT_CHECK([gfs2_edit savemeta -z0 $GFS_TGT test.meta], 0, [ignore], [ignore]) +GFS_TGT_REGEN +AT_CHECK([gfs2_edit restoremeta test.meta $GFS_TGT], 0, [ignore], [ignore]) +AT_CHECK([fsck.gfs2 -n $GFS_TGT], 0, [ignore], [ignore]) +AT_CLEANUP + +AT_SETUP([Save/restoremeta, min. block size, 4 journals]) +GFS_TGT_REGEN +AT_CHECK([$GFS_MKFS -p lock_nolock -b512 -j4 -J8 $GFS_TGT], 0, [ignore], [ignore]) +AT_CHECK([gfs2_edit savemeta -z0 $GFS_TGT test.meta], 0, [ignore], [ignore]) +GFS_TGT_REGEN AT_CHECK([gfs2_edit restoremeta test.meta $GFS_TGT], 0, [ignore], [ignore]) AT_CHECK([fsck.gfs2 -n $GFS_TGT], 0, [ignore], [ignore]) AT_CLEANUP
Signature · 1 line
1.9.3