Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=fd0... Commit: fd0a7e2b1ba68ef3e96fa10b54b1049cf963202e Parent: 9e52bf1d064e6e0fee60927b8cd5496e19917196 Author: Bob Peterson bob@ganesha.peterson AuthorDate: Fri Jan 22 16:51:44 2010 -0600 Committer: Bob Peterson rpeterso@redhat.com CommitterDate: Tue Jan 26 14:39:31 2010 -0600

Misc cleanups

This patch contains mostly formatting, cleanups, spelling corrections and code refactoring / restructuring in order for the code to be more readable. There are a few small things that might make a difference, but hopefully only a positive one. For example I fixed an uninitialized variable for lost+found inode pointer, and when checking directory entries, I make sure the file names are all printable characters.

rhbz#455300 --- gfs2/fsck/lost_n_found.c | 24 ++++++----- gfs2/fsck/main.c | 2 +- gfs2/fsck/metawalk.c | 5 ++ gfs2/fsck/pass1.c | 38 +++++++++++++---- gfs2/fsck/pass2.c | 103 ++++++++++++++++++++++++++-------------------- gfs2/fsck/pass3.c | 1 - gfs2/fsck/pass4.c | 3 +- gfs2/fsck/pass5.c | 19 ++------ gfs2/fsck/rgrepair.c | 10 ++-- gfs2/libgfs2/libgfs2.h | 6 +- 10 files changed, 121 insertions(+), 90 deletions(-)

diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index 5b9be67..c6a8a6e 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -28,21 +28,22 @@ int add_inode_to_lf(struct gfs2_inode *ip){ char tmp_name[256]; __be32 inode_type; uint64_t lf_blocks; + struct gfs2_sbd *sdp = ip->i_sbd; struct dir_info *di;

if(!lf_dip) { uint8_t q;

- log_info( _("Locating/Creating lost and found directory\n")); + log_info( _("Locating/Creating lost+found directory\n"));

- lf_dip = createi(ip->i_sbd->md.rooti, "lost+found", + lf_dip = createi(sdp->md.rooti, "lost+found", S_IFDIR | 0700, 0); /* createi will have incremented the di_nlink link count for the root directory. We must increment the nlink value in the hash table to keep them in sync so that pass4 can - detect and fix any discrepancies. */ - set_link_count(ip->i_sbd->sd_sb.sb_root_dir.no_addr, - ip->i_sbd->md.rooti->i_di.di_nlink); + detect and fix any descrepancies. */ + set_link_count(sdp->sd_sb.sb_root_dir.no_addr, + sdp->md.rooti->i_di.di_nlink);

q = block_type(lf_dip->i_di.di_num.no_addr); if(q != gfs2_inode_dir) { @@ -58,14 +59,14 @@ int add_inode_to_lf(struct gfs2_inode *ip){ _("lost+found dinode"), gfs2_inode_dir); /* root inode links to lost+found */ - increment_link(ip->i_sbd->md.rooti->i_di.di_num.no_addr, + increment_link(sdp->md.rooti->i_di.di_num.no_addr, lf_dip->i_di.di_num.no_addr, _("root")); /* lost+found link for '.' from itself */ increment_link(lf_dip->i_di.di_num.no_addr, lf_dip->i_di.di_num.no_addr, "".""); /* lost+found link for '..' back to root */ increment_link(lf_dip->i_di.di_num.no_addr, - ip->i_sbd->md.rooti->i_di.di_num.no_addr, + sdp->md.rooti->i_di.di_num.no_addr, ""..""); } log_info( _("lost+found directory is dinode %lld (0x%llx)\n"), @@ -94,7 +95,7 @@ int add_inode_to_lf(struct gfs2_inode *ip){ /* If there's a pre-existing .. directory entry, we have to back out the links. */ di = dirtree_find(ip->i_di.di_num.no_addr); - if (di && gfs2_check_range(ip->i_sbd, di->dotdot_parent) == 0){ + if (di && gfs2_check_range(sdp, di->dotdot_parent) == 0) { struct gfs2_inode *dip;

log_debug(_("Directory %lld (0x%llx) already had a " @@ -106,7 +107,7 @@ int add_inode_to_lf(struct gfs2_inode *ip){ decrement_link(di->dotdot_parent, ip->i_di.di_num.no_addr, _(".. unlinked, moving to lost+found")); - dip = fsck_load_inode(ip->i_sbd, di->dotdot_parent); + dip = fsck_load_inode(sdp, di->dotdot_parent); dip->i_di.di_nlink--; log_debug(_("Decrementing its links to %d\n"), dip->i_di.di_nlink); @@ -119,7 +120,7 @@ int add_inode_to_lf(struct gfs2_inode *ip){ (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); if(gfs2_dirent_del(ip, "..", 2)) - log_warn( _("add_inode_to_lf: Unable to remove " + log_warn( _("add_inode_to_lf: Unable to remove " "".." directory entry.\n"));

dir_add(ip, "..", 2, &(lf_dip->i_di.di_num), DT_DIR); @@ -179,7 +180,8 @@ int add_inode_to_lf(struct gfs2_inode *ip){ increment_link(lf_dip->i_di.di_num.no_addr, ip->i_di.di_mode, _("to lost+found"));

- log_notice( _("Added inode #%llu to lost+found dir\n"), + log_notice( _("Added inode #%llu (0x%llx) to lost+found dir\n"), + (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); return 0; } diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c index b026cd9..43aa77a 100644 --- a/gfs2/fsck/main.c +++ b/gfs2/fsck/main.c @@ -19,7 +19,7 @@ #include "util.h"

struct gfs2_options opts = {0}; -struct gfs2_inode *lf_dip; /* Lost and found directory inode */ +struct gfs2_inode *lf_dip = NULL; /* Lost and found directory inode */ struct gfs2_bmap *bl = NULL; uint64_t last_fs_block, last_reported_block = -1; int64_t last_reported_fblock = -1000000; diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 3ccb0bc..609aa4b 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -249,6 +249,7 @@ static int dirent_repair(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, /* first, figure out a probable name length */ p = (char *)dent + sizeof(struct gfs2_dirent); while (*p && /* while there's a non-zero char and */ + isprint(*p) && /* a printable character and */ p < bh_end) { /* not past end of buffer */ calc_de_name_len++; p++; @@ -1114,6 +1115,8 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, for (ptr = (uint64_t *)(bh->b_data + head_size); (char *)ptr < (bh->b_data + ip->i_sbd->bsize); ptr++) { + if (skip_this_pass || fsck_abort) + return FSCK_OK; nbh = NULL;

if (!*ptr) @@ -1246,6 +1249,8 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) last_reported_fblock = -10000000;

while (error >= 0 && !osi_list_empty(list)) { + if (fsck_abort) + return 0; bh = osi_list_entry(list->next, struct gfs2_buffer_head, b_altlist);

diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 09c6a99..04938db 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -83,31 +83,53 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, void *private) { uint8_t q; - int found_dup = 0; + int found_dup = 0, iblk_type; struct gfs2_buffer_head *nbh; struct block_count *bc = (struct block_count *)private; + const char *blktypedesc;

*bh = NULL;

if (gfs2_check_range(ip->i_sbd, block)){ /* blk outside of FS */ fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, _("itself"), gfs2_bad_block); - log_debug( _("Bad indirect block pointer (out of range).\n")); + log_debug( _("Bad indirect block pointer (out of range) " + "found in inode %lld (0x%llx).\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr);

return 1; } + if (S_ISDIR(ip->i_di.di_mode)) { + iblk_type = GFS2_METATYPE_JD; + blktypedesc = _("a directory hash table block"); + } else { + iblk_type = GFS2_METATYPE_IN; + blktypedesc = _("a journaled data block"); + } q = block_type(block); if(q != gfs2_block_free) { - log_err( _("Found duplicate block referenced as metadata in " - "indirect block - was marked %d\n"), q); + log_err( _("Found duplicate block %llu (0x%llx) referenced " + "as metadata in indirect block for dinode " + "%llu (0x%llx) - was marked %d (%s)\n"), + (unsigned long long)block, + (unsigned long long)block, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr, q, + block_type_string(q)); gfs2_dup_set(block); found_dup = 1; } nbh = bread(ip->i_sbd, block);

- if (gfs2_check_meta(nbh, GFS2_METATYPE_IN)){ - log_debug( _("Bad indirect block pointer (points to " - "something that is not an indirect block).\n")); + if (gfs2_check_meta(nbh, iblk_type)){ + log_debug( _("Inode %lld (0x%llx) has a bad indirect block " + "pointer %lld (0x%llx) (points to something " + "that is not %s).\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)block, + (unsigned long long)block, blktypedesc); if(!found_dup) { fsck_blockmap_set(ip, block, _("bad indirect"), gfs2_meta_inval); @@ -378,7 +400,6 @@ static int check_leaf_block(struct gfs2_inode *ip, uint64_t block, int btype, clear_eas(ip, bc, block, 0, _("Extended Attribute block removed due to " "previous errors.\n")); - bmodified(leaf_bh); brelse(leaf_bh); return 1; } @@ -557,7 +578,6 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, PRIu64 " (0x%" PRIx64 ")? (y/n) "), block, block)) { ip->i_di.di_num.no_addr = ip->i_di.di_num.no_formal_ino = block; - gfs2_dinode_out(&ip->i_di, ip->i_bh); bmodified(ip->i_bh); } else log_err( _("Address in inode at block #%" PRIu64 diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 583bc09..a0c5f88 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -24,24 +24,32 @@ static int set_parent_dir(struct gfs2_sbd *sbp, uint64_t childblock, struct dir_info *di;

di = dirtree_find(childblock); - if (di) { - if(di->dinode == childblock) { - if (di->treewalk_parent) { - log_err( _("Another directory at block %" PRIu64 - " (0x%" PRIx64 ") already contains" - " this child - checking %" PRIu64 " (0x%" PRIx64 ")\n"), - di->treewalk_parent, di->treewalk_parent, - parentblock, parentblock); - return 1; - } - di->treewalk_parent = parentblock; - } - } else { + if(!di) { log_err( _("Unable to find block %"PRIu64" (0x%" PRIx64 ") in dir_info list\n"), childblock, childblock); return -1; }

+ if(di->dinode == childblock) { + if (di->treewalk_parent) { + log_err( _("Another directory at block %" PRIu64 + " (0x%" PRIx64 ") already contains this " + "child %lld (%llx) - checking parent %" + PRIu64 " (0x%" PRIx64 ")\n"), + di->treewalk_parent, di->treewalk_parent, + (unsigned long long)childblock, + (unsigned long long)childblock, + parentblock, parentblock); + return 1; + } + log_debug( _("Child %lld (0x%llx) has parent %lld (0x%llx)\n"), + (unsigned long long)childblock, + (unsigned long long)childblock, + (unsigned long long)parentblock, + (unsigned long long)parentblock); + di->treewalk_parent = parentblock; + } + return 0; }

@@ -247,7 +255,7 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, /* This entry's inode has bad blocks in it */

/* Handle bad blocks */ - log_err( _("Found a bad directory entry: %s\n"), filename); + log_err( _("Found a bad directory entry: %s\n"), tmp_name);

if(!query( _("Delete inode containing bad blocks? (y/n)"))) { log_warn( _("Entry to inode containing bad blocks remains\n")); @@ -528,9 +536,10 @@ static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname, pass2_fxns.private = (void *) &ds; if(ds.q == gfs2_bad_block) { /* First check that the directory's metatree is valid */ - if(check_metatree(sysinode, &pass2_fxns)) { + error = check_metatree(sysinode, &pass2_fxns); + if (error < 0) { stack; - return -1; + return error; } } error = check_dir(sysinode->i_sbd, iblock, &pass2_fxns); @@ -681,10 +690,11 @@ int pass2(struct gfs2_sbd *sbp) /* First check that the directory's metatree * is valid */ ip = fsck_load_inode(sbp, dirblk); - if(check_metatree(ip, &pass2_fxns)) { - fsck_inode_put(&ip); + error = check_metatree(ip, &pass2_fxns); + fsck_inode_put(&ip); + if (error < 0) { stack; - return FSCK_ERROR; + return error; } } error = check_dir(sbp, dirblk, &pass2_fxns); @@ -693,40 +703,43 @@ int pass2(struct gfs2_sbd *sbp) return FSCK_ERROR; } if (error > 0) { - struct dir_info *di = NULL; + struct dir_info *di;

di = dirtree_find(dirblk); if(!di) { stack; return FSCK_ERROR; } - if(error == 0) { - /* FIXME: factor */ - if(query( _("Remove directory entry for bad" - " inode %"PRIu64" (0x%" PRIx64 ") in %"PRIu64 - " (0x%" PRIx64 ")? (y/n)"), - dirblk, dirblk, di->treewalk_parent, - di->treewalk_parent)) { - error = remove_dentry_from_dir(sbp, - di->treewalk_parent, - dirblk); - if(error < 0) { - stack; - return FSCK_ERROR; - } - if(error > 0) { - log_warn( _("Unable to find dentry for %" - PRIu64 " (0x%" PRIx64 ") in %" PRIu64 - " (0x%" PRIx64 ")\n"), dirblk, dirblk, - di->treewalk_parent, di->treewalk_parent); - } - log_warn( _("Directory entry removed\n")); - } else - log_err( _("Directory entry to invalid inode remains.\n")); - } + if(query( _("Remove directory entry for bad" + " inode %"PRIu64" (0x%" PRIx64 ") in %"PRIu64 + " (0x%" PRIx64 ")? (y/n)"), dirblk, + dirblk, di->treewalk_parent, + di->treewalk_parent)) { + error = remove_dentry_from_dir(sbp, di->treewalk_parent, + dirblk); + if(error < 0) { + stack; + return FSCK_ERROR; + } + if(error > 0) { + log_warn( _("Unable to find dentry for %" + PRIu64 " (0x%" PRIx64 ") in %" PRIu64 + " (0x%" PRIx64 ")\n"), + dirblk, dirblk, + di->treewalk_parent, + di->treewalk_parent); + } + log_warn( _("Directory entry removed\n")); + } else + log_err( _("Directory entry to invalid inode remains.\n")); + log_debug( _("Directory block %lld (0x%llx) " + "is now marked as 'invalid'\n"), + (unsigned long long)dirblk, + (unsigned long long)dirblk); /* Can't use fsck_blockmap_set here because we don't have an inode in memory. */ gfs2_blockmap_set(bl, dirblk, gfs2_meta_inval); + check_n_fix_bitmap(sbp, dirblk, gfs2_meta_inval); } ip = fsck_load_inode(sbp, dirblk); if(!ds.dotdir) { @@ -778,7 +791,7 @@ int pass2(struct gfs2_sbd *sbp) } }

- if(ip->i_di.di_entries != ds.entry_count) { + if(!fsck_abort && ip->i_di.di_entries != ds.entry_count) { log_err( _("Entries is %d - should be %d for inode " "block %llu (0x%llx)\n"), ip->i_di.di_entries, ds.entry_count, diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c index 260f4fe..c10e0bb 100644 --- a/gfs2/fsck/pass3.c +++ b/gfs2/fsck/pass3.c @@ -62,7 +62,6 @@ static int attach_dotdot_to(struct gfs2_sbd *sbp, uint64_t newdotdot, reprocess_inode(ip, dirname); } increment_link(newdotdot, block, _("new ".."")); - bmodified(ip->i_bh); fsck_inode_put(&ip); fsck_inode_put(&pip); free(filename); diff --git a/gfs2/fsck/pass4.c b/gfs2/fsck/pass4.c index e41d995..d79f8e5 100644 --- a/gfs2/fsck/pass4.c +++ b/gfs2/fsck/pass4.c @@ -114,7 +114,8 @@ static int scan_inode_list(struct gfs2_sbd *sbp) { * them. */ if(!ip->i_di.di_size && !ip->i_di.di_eattr){ log_err( _("Unlinked inode has zero size\n")); - if(query( _("Clear zero-size unlinked inode? (y/n) "))) { + if(query(_("Clear zero-size unlinked inode? " + "(y/n) "))) { fsck_blockmap_set(ip, ii->inode, _("unlinked zero-length"), gfs2_block_free); diff --git a/gfs2/fsck/pass5.c b/gfs2/fsck/pass5.c index 41da203..ff6c181 100644 --- a/gfs2/fsck/pass5.c +++ b/gfs2/fsck/pass5.c @@ -57,7 +57,6 @@ static int check_block_status(struct gfs2_sbd *sbp, char *buffer, unsigned int b unsigned char rg_status, block_status; uint8_t q; uint64_t block; - static int free_unlinked = -1;

/* FIXME verify cast */ byte = (unsigned char *) buffer; @@ -81,19 +80,11 @@ static int check_block_status(struct gfs2_sbd *sbp, char *buffer, unsigned int b So we ignore it. */ if (rg_status == GFS2_BLKST_UNLINKED && block_status == GFS2_BLKST_FREE) { - if (free_unlinked == -1) { - log_err( _("Unlinked inode block found at " - "block %llu (0x%llx).\n"), - (unsigned long long)block, - (unsigned long long)block); - if(query( _("Do you want me to fix the " - "bitmap for all unlinked " - "blocks? (y/n) "))) - free_unlinked = 1; - else - free_unlinked = 0; - } - if (free_unlinked) { + log_err( _("Unlinked inode block found at " + "block %llu (0x%llx).\n"), + (unsigned long long)block, + (unsigned long long)block); + if(query(_("Do you want to fix the bitmap? (y/n) "))) { if(gfs2_set_bitmap(sbp, block, block_status)) log_err(_("Unlinked block %llu " "(0x%llx) bitmap not fixed." diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c index 2dace5d..0b5c576 100644 --- a/gfs2/fsck/rgrepair.c +++ b/gfs2/fsck/rgrepair.c @@ -416,7 +416,7 @@ static int rewrite_rg_block(struct gfs2_sbd *sdp, struct rgrp_list *rg, */ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count) { - int error, descrepencies; + int error, discrepancies; osi_list_t expected_rglist; int calc_rg_count = 0, rgcount_from_index, rg; osi_list_t *exp, *act; /* expected, actual */ @@ -468,7 +468,7 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count) /* we have a large number that are completely wrong, we should */ /* abandon this method of recovery and try a better one. */ /* ------------------------------------------------------------- */ - descrepencies = 0; + discrepancies = 0; for (rg = 0, act = sdp->rglist.next, exp = expected_rglist.next; act != &sdp->rglist && exp != &expected_rglist; act = act->next, exp = exp->next, rg++) { @@ -481,14 +481,14 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count) !ri_equal(actual->ri, expected->ri, ri_data0) || !ri_equal(actual->ri, expected->ri, ri_data) || !ri_equal(actual->ri, expected->ri, ri_bitbytes)) { - descrepencies++; + discrepancies++; } } - if (trust_lvl < distrust && descrepencies > (trust_lvl * 8)) { + if (trust_lvl < distrust && discrepancies > (trust_lvl * 8)) { log_warn( _("Level %d didn't work. Too many descepencies.\n"), trust_lvl + 1); log_warn( _("%d out of %d RGs did not match what was expected.\n"), - descrepencies, rg); + discrepancies, rg); gfs2_rgrp_free(&expected_rglist); gfs2_rgrp_free(&sdp->rglist); return -1; diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index 951966a..7091a37 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -257,9 +257,9 @@ struct metapath {

/* bitmap.c */ struct gfs2_bmap { - uint64_t size; - uint64_t mapsize; - unsigned char *map; + uint64_t size; + uint64_t mapsize; + unsigned char *map; };

/* block_list.c */