cluster-commits January 2010

cluster-commits@lists.stg.fedorahosted.org

7 participants
186 discussions

cluster: STABLE3 - fsck.gfs2: use gfs2_meta_inval vs. gfs2_inval_inode
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=7b… Commit: 7bd37e30f2dd4a54283ac57f0df003840fe010d7 Parent: de4850b3764331085a4657c57fd518b3586d5b6c Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 10:21:16 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:31 2010 -0600 fsck.gfs2: use gfs2_meta_inval vs. gfs2_inval_inode This patch removes the vestigial gfs2_journal_blk and reuses it for a new gfs2_inode_invalid. This allows the code to keep better track of invalid inodes rather than other invalid metadata. For invalid inodes, there may be cases where we want to invalidate its metadata list. We can't do that for other kinds of metadata like invalid eattrs. rhbz#455300 --- gfs2/fsck/pass1.c | 2 +- gfs2/fsck/pass1b.c | 33 ++++++++++++++++----------------- gfs2/fsck/pass2.c | 12 ++++++------ gfs2/fsck/pass5.c | 2 +- gfs2/libgfs2/libgfs2.h | 2 +- 5 files changed, 25 insertions(+), 26 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 4382ad9..b7ee6d5 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -766,7 +766,7 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, break; default: if (fsck_blockmap_set(ip, block, _("invalid mode"), - gfs2_meta_inval)) { + gfs2_inode_invalid)) { stack; fsck_inode_put(&ip); return -1; diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 16b5ee5..1180d98 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -166,7 +166,8 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, /* Setting the block to invalid means the inode is * cleared in pass2 */ fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, - _("inode with duplicate"), gfs2_meta_inval); + _("inode with duplicate"), + gfs2_inode_invalid); } return 0; } @@ -420,7 +421,7 @@ static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) is cleared in pass2 */ fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, _("inode with bad duplicate"), - gfs2_meta_inval); + gfs2_inode_invalid); fsck_inode_put(&ip); } else { log_warn( _("The bad inode was not cleared.")); @@ -465,7 +466,7 @@ static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) check_metatree(ip, &clear_dup_fxns); fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, - _("inode with duplicate"), gfs2_meta_inval); + _("bad"), gfs2_inode_invalid); fsck_inode_put(&ip); /* out, brelse, free */ dh.ref_inode_count--; if(dh.ref_inode_count == 1) @@ -512,20 +513,18 @@ int pass1b(struct gfs2_sbd *sbp) log_debug( _("Scanning block %" PRIu64 " (0x%" PRIx64 ") for inodes\n"), i, i); q = block_type(i); - if((q == gfs2_inode_dir) || - (q == gfs2_inode_file) || - (q == gfs2_inode_lnk) || - (q == gfs2_inode_blk) || - (q == gfs2_inode_chr) || - (q == gfs2_inode_fifo) || - (q == gfs2_inode_sock)) { - for (n = osi_first(&dup_blocks); n; n = osi_next(n)) { - b = (struct duptree *)n; - if(find_block_ref(sbp, i, b)) { - stack; - rc = FSCK_ERROR; - goto out; - } + + if (q < gfs2_inode_dir) + continue; + if (q > gfs2_inode_sock) + continue; + + for (n = osi_first(&dup_blocks); n; n = osi_next(n)) { + b = (struct duptree *)n; + if(find_block_ref(sbp, i, b)) { + stack; + rc = FSCK_ERROR; + goto out; } } if(q == gfs2_inode_dir) { diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index a0c5f88..ee29893 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -222,7 +222,7 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, } fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, _("corrupt directory entry"), - gfs2_meta_inval); + gfs2_inode_invalid); log_err( _("Bad directory entry deleted.\n")); return 1; } @@ -251,7 +251,7 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, q = block_type(entryblock); /* Get the status of the directory inode */ - if(q == gfs2_bad_block) { + if(q == gfs2_inode_invalid || q == gfs2_bad_block) { /* This entry's inode has bad blocks in it */ /* Handle bad blocks */ @@ -282,7 +282,7 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, (unsigned long long)entryblock, (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr, - q, q == gfs2_meta_inval ? + q, q == gfs2_inode_invalid ? _("previously marked invalid") : _("is not an inode")); @@ -549,7 +549,7 @@ static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname, } if (error > 0) fsck_blockmap_set(sysinode, iblock, dirname, - gfs2_meta_inval); + gfs2_inode_invalid); if(check_inode_eattr(sysinode, &pass2_fxns)) { stack; @@ -738,8 +738,8 @@ int pass2(struct gfs2_sbd *sbp) (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); + gfs2_blockmap_set(bl, dirblk, gfs2_inode_invalid); + check_n_fix_bitmap(sbp, dirblk, gfs2_inode_invalid); } ip = fsck_load_inode(sbp, dirblk); if(!ds.dotdir) { diff --git a/gfs2/fsck/pass5.c b/gfs2/fsck/pass5.c index ff6c181..07b0559 100644 --- a/gfs2/fsck/pass5.c +++ b/gfs2/fsck/pass5.c @@ -15,6 +15,7 @@ static int convert_mark(uint8_t q, uint32_t *count) switch(q) { case gfs2_meta_inval: + case gfs2_inode_invalid: /* Convert invalid metadata to free blocks */ case gfs2_block_free: count[0]++; @@ -36,7 +37,6 @@ static int convert_mark(uint8_t q, uint32_t *count) case gfs2_indir_blk: case gfs2_leaf_blk: - case gfs2_journal_blk: case gfs2_meta_other: case gfs2_meta_eattr: count[2]++; diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index 7091a37..6d1a89f 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -277,7 +277,7 @@ enum gfs2_mark_block { gfs2_inode_fifo = (0x8), gfs2_inode_sock = (0x9), - gfs2_journal_blk = (0xa), + gfs2_inode_invalid = (0xa), gfs2_meta_inval = (0xb), gfs2_leaf_blk = (0xc), gfs2_meta_other = (0xd),

1 0

cluster: STABLE3 - fsck.gfs2: Check for massive amounts of pointer corruption
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=de… Commit: de4850b3764331085a4657c57fd518b3586d5b6c Parent: fd0a7e2b1ba68ef3e96fa10b54b1049cf963202e Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 09:03:16 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:31 2010 -0600 fsck.gfs2: Check for massive amounts of pointer corruption Sometimes, due to faulty hardware or whatever, a whole bunch of random nonsense is written into a block. If that block happens to be a indirect list of pointers, pass1 may not find the corruption for a long time. This happens when the corruption starts, for example, at offset 0x200, or if the corruption just happens to look like valid pointers for a while, like low blocks that correspond to system inodes, rgrps, or journals. If pass1 marks a whole bunch of pointers as valid, then later decides the whole inode is corrupt, it becomes a major pain to undo what it has done. For example, if it had found one of the "bad" pointers to be the statfs file's dinode and marked that as a duplicate reference, it's a pain to undo that once it becomes apparent that there's too much damage to recover. This patch introduces a block range check function that pass1 can use to traverse the metadata tree initially, just checking for lots of damage to pointers. If there are a lot of damaged metadata pointers it's better to just mark the dinode as free space and let pass5 clean up any blocks that it referenced. If a bridge has too many damaged rungs to cross, it's better to find that out first rather than to cross half-way and have to tip-toe back to the start. rhbz#455300 --- gfs2/fsck/fsck.h | 2 + gfs2/fsck/pass1.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 122 insertions(+), 8 deletions(-) diff --git a/gfs2/fsck/fsck.h b/gfs2/fsck/fsck.h index 5948210..30eb223 100644 --- a/gfs2/fsck/fsck.h +++ b/gfs2/fsck/fsck.h @@ -23,6 +23,8 @@ #define FSCK_CANCELED 32 /* Aborted with a signal or ^C */ #define FSCK_LIBRARY 128 /* Shared library error */ +#define BAD_POINTER_TOLERANCE 10 /* How many bad pointers is too many? */ + struct inode_info { struct osi_node node; diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 04938db..4382ad9 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -554,6 +554,97 @@ static int clear_leaf(struct gfs2_inode *ip, uint64_t block, return 0; } +/** + * Check for massive amounts of pointer corruption. If the block has + * lots of out-of-range pointers, we can't trust any of the pointers. + * For example, a stray pointer with a value of 0x1d might be + * corruption/nonsense, and if so, we don't want to delete an + * important file (like master or the root directory) because of it. + * We need to check for a large number of bad pointers BEFORE we start + * messing with them because we don't want to mark a block as a + * duplicate (for example) until we know if the pointers in general can + * be trusted. Thus it needs to be in a separate loop. + */ +static int rangecheck_block(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, + const char *btype, void *private) +{ + long *bad_pointers = (long *)private; + uint8_t q; + + if (gfs2_check_range(ip->i_sbd, block) != 0) { + (*bad_pointers)++; + log_debug( _("Bad %s block pointer (out of range #%ld) " + "found in inode %lld (0x%llx).\n"), btype, + *bad_pointers, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + if ((*bad_pointers) <= BAD_POINTER_TOLERANCE) + return ENOENT; + else + return -ENOENT; /* Exits check_metatree quicker */ + } + /* See how many duplicate blocks it has */ + q = block_type(block); + if (q != gfs2_block_free) { + (*bad_pointers)++; + log_debug( _("Duplicated %s block pointer (violation #%ld) " + "found in inode %lld (0x%llx).\n"), btype, + *bad_pointers, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + if ((*bad_pointers) <= BAD_POINTER_TOLERANCE) + return ENOENT; + else + return -ENOENT; /* Exits check_metatree quicker */ + } + return 0; +} + +static int rangecheck_metadata(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, void *private) +{ + return rangecheck_block(ip, block, bh, _("metadata"), private); +} + +static int rangecheck_leaf(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head *bh, void *private) +{ + return rangecheck_block(ip, block, &bh, _("leaf"), private); +} + +static int rangecheck_data(struct gfs2_inode *ip, uint64_t block, + void *private) +{ + return rangecheck_block(ip, block, NULL, _("data"), private); +} + +static int rangecheck_eattr_indir(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, + struct gfs2_buffer_head **bh, void *private) +{ + return rangecheck_block(ip, block, NULL, + _("indirect extended attribute"), + private); +} + +static int rangecheck_eattr_leaf(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, struct gfs2_buffer_head **bh, + void *private) +{ + return rangecheck_block(ip, block, NULL, _("extended attribute"), + private); +} + +struct metawalk_fxns rangecheck_fxns = { + .private = NULL, + .check_metalist = rangecheck_metadata, + .check_data = rangecheck_data, + .check_leaf = rangecheck_leaf, + .check_eattr_indir = rangecheck_eattr_indir, + .check_eattr_leaf = rangecheck_eattr_leaf, +}; + static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, uint64_t block) { @@ -562,10 +653,16 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, int error; struct block_count bc = {0}; struct metawalk_fxns invalidate_metatree = {0}; + long bad_pointers; - invalidate_metatree.check_metalist = clear_metalist; - invalidate_metatree.check_data = clear_data; - invalidate_metatree.check_leaf = clear_leaf; + q = block_type(block); + if(q != gfs2_block_free) { + log_err( _("Found duplicate block referenced as an inode at " + "#%" PRIu64 " (0x%" PRIx64 ")\n"), block, block); + gfs2_dup_set(block); + fsck_inode_put(&ip); + return 0; + } ip = fsck_inode_get(sdp, bh); if (ip->i_di.di_num.no_addr != block) { @@ -584,11 +681,22 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, " (0x%" PRIx64 ") not fixed\n"), block, block); } - q = block_type(block); - if(q != gfs2_block_free) { - log_err( _("Found duplicate block referenced as an inode at " - "#%" PRIu64 " (0x%" PRIx64 ")\n"), block, block); - gfs2_dup_set(block); + bad_pointers = 0L; + + /* First, check the metadata for massive amounts of pointer corruption. + Such corruption can only lead us to ruin trying to clean it up, + so it's better to check it up front and delete the inode if + there is corruption. */ + rangecheck_fxns.private = &bad_pointers; + error = check_metatree(ip, &rangecheck_fxns); + if (bad_pointers > BAD_POINTER_TOLERANCE) { + log_err( _("Error: inode %llu (0x%llx) has more than " + "%d bad pointers.\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr, + BAD_POINTER_TOLERANCE); + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("badly corrupt"), gfs2_block_free); fsck_inode_put(&ip); return 0; } @@ -703,6 +811,10 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, "errors; invalidating.\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); + invalidate_metatree.check_metalist = clear_metalist; + invalidate_metatree.check_data = clear_data; + invalidate_metatree.check_leaf = clear_leaf; + /* FIXME: Must set all leaves invalid as well */ check_metatree(ip, &invalidate_metatree); fsck_blockmap_set(ip, ip->i_di.di_num.no_addr,

1 0

cluster: STABLE3 - Misc cleanups
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=fd… Commit: fd0a7e2b1ba68ef3e96fa10b54b1049cf963202e Parent: 9e52bf1d064e6e0fee60927b8cd5496e19917196 Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Fri Jan 22 16:51:44 2010 -0600 Committer: Bob Peterson <rpeterso(a)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 */

1 0

cluster: STABLE3 - fsck.gfs2: reprocess lost+found and other inode metadata when blocks are added
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=9e… Commit: 9e52bf1d064e6e0fee60927b8cd5496e19917196 Parent: b46c19c385f182f950c6df56bafd3db15459c48e Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Fri Jan 22 15:29:19 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:31 2010 -0600 fsck.gfs2: reprocess lost+found and other inode metadata when blocks are added On rare occasions, fsck.gfs2 needs to allocate blocks in the file system. For example, this can happen when new directory leafs are added to lost+found and when lost+found's hash table needs to be doubled. While libgfs2 did a stellar job of updating the rgrp bitmap information, those newly allocated blocks were unfortunately not being properly accounted for in the fsck.gfs2 blockmap. Therefore, pass5 would mistakenly mark them as free blocks. Further use of the file system would often allocate those blocks a second time, creating a duplicate block reference. That means very bad file system corruption. This patch fixes the problem by checking for block count changes in inodes at critical points, and subsequently fixing up the blockmap using new "alloc_*" helper functions. rhbz#455300 --- gfs2/fsck/lost_n_found.c | 8 ++++ gfs2/fsck/metawalk.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++ gfs2/fsck/metawalk.h | 1 + gfs2/fsck/pass2.c | 16 +++++++ gfs2/fsck/pass3.c | 10 +++++ 5 files changed, 133 insertions(+), 0 deletions(-) diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index 6d26096..5b9be67 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -27,6 +27,7 @@ int add_inode_to_lf(struct gfs2_inode *ip){ char tmp_name[256]; __be32 inode_type; + uint64_t lf_blocks; struct dir_info *di; if(!lf_dip) { @@ -81,6 +82,8 @@ int add_inode_to_lf(struct gfs2_inode *ip){ log_err( _("Trying to add lost+found to itself...skipping")); return 0; } + lf_blocks = lf_dip->i_di.di_blocks; + switch(ip->i_di.di_mode & S_IFMT){ case S_IFDIR: log_info( _("Adding .. entry pointing to lost+found for " @@ -163,6 +166,11 @@ int add_inode_to_lf(struct gfs2_inode *ip){ dir_add(lf_dip, tmp_name, strlen(tmp_name), &(ip->i_di.di_num), inode_type); + /* If the lf directory had new blocks added we have to mark them + properly in the bitmap so they're not freed. */ + if (lf_dip->i_di.di_blocks != lf_blocks) + reprocess_inode(lf_dip, "lost+found"); + /* This inode is linked from lost+found */ increment_link(ip->i_di.di_num.no_addr, lf_dip->i_di.di_num.no_addr, _("from lost+found")); diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 2ad47fe..3ccb0bc 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1430,3 +1430,101 @@ int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent, return delete_blocks(ip, block, NULL, _("extended attribute"), private); } + +static int alloc_metalist(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, void *private) +{ + uint8_t q; + const char *desc = (const char *)private; + + /* No need to range_check here--if it was added, it's in range. */ + /* We can't check the bitmap here because this function is called + after the bitmap has been set but before the blockmap has. */ + *bh = bread(ip->i_sbd, block); + q = block_type(block); + if (blockmap_to_bitmap(q) == GFS2_BLKST_FREE) { /* If not marked yet */ + log_debug(_("%s reference to new metadata block " + "%lld (0x%llx) is now marked as indirect.\n"), + desc, (unsigned long long)block, + (unsigned long long)block); + gfs2_blockmap_set(bl, block, gfs2_indir_blk); + } + return 0; +} + +static int alloc_data(struct gfs2_inode *ip, uint64_t block, void *private) +{ + uint8_t q; + const char *desc = (const char *)private; + + /* No need to range_check here--if it was added, it's in range. */ + /* We can't check the bitmap here because this function is called + after the bitmap has been set but before the blockmap has. */ + q = block_type(block); + if (blockmap_to_bitmap(q) == GFS2_BLKST_FREE) { /* If not marked yet */ + log_debug(_("%s reference to new data block " + "%lld (0x%llx) is now marked as data.\n"), + desc, (unsigned long long)block, + (unsigned long long)block); + gfs2_blockmap_set(bl, block, gfs2_block_used); + } + return 0; +} + +static int alloc_leaf(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head *bh, void *private) +{ + uint8_t q; + + /* No need to range_check here--if it was added, it's in range. */ + /* We can't check the bitmap here because this function is called + after the bitmap has been set but before the blockmap has. */ + q = block_type(block); + if (blockmap_to_bitmap(q) == GFS2_BLKST_FREE) /* If not marked yet */ + fsck_blockmap_set(ip, block, _("newly allocated leaf"), + gfs2_leaf_blk); + return 0; +} + +struct metawalk_fxns alloc_fxns = { + .private = NULL, + .check_leaf = alloc_leaf, + .check_metalist = alloc_metalist, + .check_data = alloc_data, + .check_eattr_indir = NULL, + .check_eattr_leaf = NULL, + .check_dentry = NULL, + .check_eattr_entry = NULL, + .check_eattr_extentry = NULL, + .finish_eattr_indir = NULL, +}; + +/* + * reprocess_inode - fixes the blockmap to match the bitmap due to an + * unexpected block allocation via libgfs2. + * + * The problem we're trying to overcome here is when a new block must be + * added to a dinode because of a write. This will happen when lost+found + * needs a new indirect block for its hash table. In that case, the write + * causes a new block to be assigned in the bitmap but that block is not yet + * accurately reflected in the fsck blockmap. We need to compensate here. + * + * We can't really use fsck_blockmap_set here because the new block + * was already allocated by libgfs2 and therefore it took care of + * the rgrp free space variable. fsck_blockmap_set adjusts the free space + * in the rgrp according to the change, which has already been done. + * So it's only our blockmap that now disagrees with the rgrp bitmap, so we + * need to fix only that. + */ +void reprocess_inode(struct gfs2_inode *ip, const char *desc) +{ + int error; + + alloc_fxns.private = (void *)desc; + log_info( _("%s had blocks added; reprocessing its metadata tree " + "at height=%d.\n"), desc, ip->i_di.di_height); + error = check_metatree(ip, &alloc_fxns); + if (error) + log_err( _("Error %d reprocessing the %s metadata tree.\n"), + error, desc); +} diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index 72c282a..653b512 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -32,6 +32,7 @@ extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock, const char *caller, int line); extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, enum gfs2_mark_block new_blockmap_state); +extern void reprocess_inode(struct gfs2_inode *ip, const char *desc); extern struct duptree *dupfind(uint64_t block); #define is_duplicate(dblock) ((dupfind(dblock)) ? 1 : 0) diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 0650966..583bc09 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -549,6 +549,8 @@ static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname, if(!ds.dotdir) { log_err( _("No '.' entry found for %s directory.\n"), dirname); if (query( _("Is it okay to add '.' entry? (y/n) "))) { + uint64_t cur_blks = sysinode->i_di.di_blocks; + sprintf(tmp_name, "."); filename_len = strlen(tmp_name); /* no trailing NULL */ if(!(filename = malloc(sizeof(char) * filename_len))) { @@ -566,6 +568,8 @@ static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname, log_warn( _("Adding '.' entry\n")); dir_add(sysinode, filename, filename_len, &(sysinode->i_di.di_num), DT_DIR); + if (cur_blks != sysinode->i_di.di_blocks) + reprocess_inode(sysinode, dirname); /* This system inode is linked to itself via '.' */ increment_link(sysinode->i_di.di_num.no_addr, sysinode->i_di.di_num.no_addr, @@ -731,6 +735,8 @@ int pass2(struct gfs2_sbd *sbp) dirblk, dirblk); if (query( _("Is it okay to add '.' entry? (y/n) "))) { + uint64_t cur_blks; + sprintf(tmp_name, "."); filename_len = strlen(tmp_name); /* no trailing NULL */ @@ -748,8 +754,18 @@ int pass2(struct gfs2_sbd *sbp) } memcpy(filename, tmp_name, filename_len); + cur_blks = ip->i_di.di_blocks; dir_add(ip, filename, filename_len, &(ip->i_di.di_num), DT_DIR); + if (cur_blks != ip->i_di.di_blocks) { + char dirname[80]; + + sprintf(dirname, _("Directory at %lld " + "(0x%llx)"), + (unsigned long long)dirblk, + (unsigned long long)dirblk); + reprocess_inode(ip, dirname); + } /* directory links to itself via '.' */ increment_link(ip->i_di.di_num.no_addr, ip->i_di.di_num.no_addr, diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c index 2ff4fbf..260f4fe 100644 --- a/gfs2/fsck/pass3.c +++ b/gfs2/fsck/pass3.c @@ -20,6 +20,7 @@ static int attach_dotdot_to(struct gfs2_sbd *sbp, uint64_t newdotdot, char *filename; int filename_len; struct gfs2_inode *ip, *pip; + uint64_t cur_blks; ip = fsck_load_inode(sbp, block); pip = fsck_load_inode(sbp, newdotdot); @@ -50,7 +51,16 @@ static int attach_dotdot_to(struct gfs2_sbd *sbp, uint64_t newdotdot, log_warn( _("Unable to remove \"..\" directory entry.\n")); else decrement_link(olddotdot, block, _("old \"..\"")); + cur_blks = ip->i_di.di_blocks; dir_add(ip, filename, filename_len, &pip->i_di.di_num, DT_DIR); + if (cur_blks != ip->i_di.di_blocks) { + char dirname[80]; + + sprintf(dirname, _("Directory at %lld (0x%llx)"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + reprocess_inode(ip, dirname); + } increment_link(newdotdot, block, _("new \"..\"")); bmodified(ip->i_bh); fsck_inode_put(&ip);

1 0

cluster: STABLE3 - lost+found link count and connections were not properly managed
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=b4… Commit: b46c19c385f182f950c6df56bafd3db15459c48e Parent: 5df69b1d9f020df9e97629240196596c330567fa Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Fri Jan 22 15:07:19 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:31 2010 -0600 lost+found link count and connections were not properly managed When the lost+found directory was created, function createi in libgfs2 incremented the di_nlink link count for the root directory, but fsck.gfs2 did not increment the nlink value in the hash table. As a result, pass4 doesn't find and fix the discrepancy. Subsequent runs of fsck.gfs2 detect the problem, though. This patch adjusts for the new link and keeps everything sane. In a similar fashion, if the dinode being moved to lost+found happens to be a directory, the links were not properly adjusted for that directory's ".." dentry. This patch also takes care of that so the links don't get off. rhbz#455300 --- gfs2/fsck/lost_n_found.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0 deletions(-) diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index ea7b370..6d26096 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -27,6 +27,7 @@ int add_inode_to_lf(struct gfs2_inode *ip){ char tmp_name[256]; __be32 inode_type; + struct dir_info *di; if(!lf_dip) { uint8_t q; @@ -35,6 +36,13 @@ int add_inode_to_lf(struct gfs2_inode *ip){ lf_dip = createi(ip->i_sbd->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); + q = block_type(lf_dip->i_di.di_num.no_addr); if(q != gfs2_inode_dir) { /* This is a new lost+found directory, so set its @@ -59,6 +67,15 @@ int add_inode_to_lf(struct gfs2_inode *ip){ ip->i_sbd->md.rooti->i_di.di_num.no_addr, "\"..\""); } + log_info( _("lost+found directory is dinode %lld (0x%llx)\n"), + (unsigned long long)lf_dip->i_di.di_num.no_addr, + (unsigned long long)lf_dip->i_di.di_num.no_addr); + di = dirtree_find(lf_dip->i_di.di_num.no_addr); + if (di) { + log_info( _("Marking lost+found inode connected\n")); + di->checked = 1; + di = NULL; + } } if(ip->i_di.di_num.no_addr == lf_dip->i_di.di_num.no_addr) { log_err( _("Trying to add lost+found to itself...skipping")); @@ -71,6 +88,33 @@ 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 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){ + struct gfs2_inode *dip; + + log_debug(_("Directory %lld (0x%llx) already had a " + "\"..\" link to %lld (0x%llx).\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)di->dotdot_parent, + (unsigned long long)di->dotdot_parent); + 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->i_di.di_nlink--; + log_debug(_("Decrementing its links to %d\n"), + dip->i_di.di_nlink); + bmodified(dip->i_bh); + fsck_inode_put(&dip); + di = NULL; + } else + log_debug(_("Couldn't find a valid \"..\" entry " + "for orphan directory %lld (0x%llx)\n"), + (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 " "\"..\" directory entry.\n"));

1 0

cluster: STABLE3 - fsck.gfs2: separate check_data function in check_metatree
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=5d… Commit: 5df69b1d9f020df9e97629240196596c330567fa Parent: 611c213c35e1af8dc36f3b15fccc5ee5c352eaa3 Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Fri Jan 22 14:38:05 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:31 2010 -0600 fsck.gfs2: separate check_data function in check_metatree Function check_metatree had a loop for checking data blocks. I broke this loop into its own function. That serves two purposes: First, it makes check_metatree smaller and easier to read (it's already big an unruly). Second, it avoids looping unnecessarily when the caller has no check_data() function of its own. Net result is faster code. rhbz#455300 --- gfs2/fsck/metawalk.c | 72 ++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 55 insertions(+), 17 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 13290e1..2ad47fe 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1157,6 +1157,49 @@ fail: } /** + * check_data - check all data pointers for a given buffer + * This does not include "data" blocks that are really + * hash table blocks for directories. + * + * @ip: + * + * returns: +ENOENT if there are too many bad pointers + * -1 if a more serious error occurred. + * 0 if no errors occurred + * 1 if errors were found and corrected + * 2 (ENOENT) is there were too many bad pointers + */ +static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, + uint64_t *ptr_start, char *ptr_end, + uint64_t *blks_checked) +{ + int error = 0, rc = 0; + uint64_t block, *ptr; + + /* If there isn't much pointer corruption check the pointers */ + for (ptr = ptr_start ; (char *)ptr < ptr_end && !fsck_abort; ptr++) { + if (!*ptr) + continue; + + if (skip_this_pass || fsck_abort) + return error; + block = be64_to_cpu(*ptr); + /* It's important that we don't call gfs2_check_range and + bypass calling check_data on invalid blocks because that + would defeat the rangecheck_block related functions in + pass1. Therefore the individual check_data functions + should do a range check. */ + rc = pass->check_data(ip, block, pass->private); + if (rc < 0) + return rc; + if (!error && rc) + error = rc; + (*blks_checked)++; + } + return error; +} + +/** * check_metatree * @ip: * @rgd: @@ -1167,11 +1210,10 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) osi_list_t metalist[GFS2_MAX_META_HEIGHT]; osi_list_t *list; struct gfs2_buffer_head *bh; - uint64_t block, *ptr; uint32_t height = ip->i_di.di_height; int i, head_size; uint64_t blks_checked = 0; - int error; + int error, rc; if (!height && !S_ISDIR(ip->i_di.di_mode)) return 0; @@ -1227,23 +1269,19 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) } head_size = sizeof(struct gfs2_dinode); } - ptr = (uint64_t *)(bh->b_data + head_size); - for ( ; (char *)ptr < (bh->b_data + ip->i_sbd->bsize); ptr++) { - if (!*ptr) - continue; - - block = be64_to_cpu(*ptr); + if (pass->check_data) + rc = check_data(ip, pass, (uint64_t *) + (bh->b_data + head_size), + (bh->b_data + ip->i_sbd->bsize), + &blks_checked); + else + rc = 0; - if(pass->check_data && - (pass->check_data(ip, block, pass->private) < 0)) { - stack; - return -1; - } - blks_checked++; - if (ip->i_di.di_blocks > COMFORTABLE_BLKS) - big_file_comfort(ip, blks_checked); - } + if (rc && (!error || rc < 0)) + error = rc; + if (ip->i_di.di_blocks > COMFORTABLE_BLKS) + big_file_comfort(ip, blks_checked); if (bh == ip->i_bh) osi_list_del(&bh->b_altlist); else

1 0

cluster: STABLE3 - fsck.gfs2: metawalk was not checking many directories
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=61… Commit: 611c213c35e1af8dc36f3b15fccc5ee5c352eaa3 Parent: 013d20413ffb8d960089836be487b9c16a9f12d6 Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Fri Jan 22 14:24:31 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:31 2010 -0600 fsck.gfs2: metawalk was not checking many directories The fsck.gfs2 program was not checking the directory hash tables of many subdirectories. It was checking the leaf blocks, but it wasn't checking the hash table block pointers if the height value was equal to 0, which means every directory with a small number of leaf blocks. That means before, you could have all kinds of directory corruption and never know it. rhbz#455300 --- gfs2/fsck/metawalk.c | 70 +++++++++++++++++++++++++++---------------------- 1 files changed, 39 insertions(+), 31 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index a0ca65b..13290e1 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1070,12 +1070,24 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, uint32_t height = ip->i_di.di_height; struct gfs2_buffer_head *bh, *nbh, *metabh = ip->i_bh; osi_list_t *prev_list, *cur_list, *tmp; - int i, head_size; + int i, head_size, iblk_type; uint64_t *ptr, block; - int err; + int error = 0, err; osi_list_add(&metabh->b_altlist, &mlp[0]); + /* Directories are special. Their 'data' is the hash table, which is + basically an indirect block list. Their height is not important + because it checks everything through the hash table using + "depth" field calculations. However, we still have to check the + indirect blocks, even if the height == 1. */ + if (S_ISDIR(ip->i_di.di_mode)) { + height++; + iblk_type = GFS2_METATYPE_JD; + } else { + iblk_type = GFS2_METATYPE_IN; + } + /* if(<there are no indirect blocks to check>) */ if (height < 2) return 0; @@ -1089,7 +1101,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, if (i > 1) { /* if this isn't really a block list skip it */ - if (gfs2_check_meta(bh, GFS2_METATYPE_IN)) + if (gfs2_check_meta(bh, iblk_type)) continue; head_size = sizeof(struct gfs2_meta_header); } else { @@ -1114,9 +1126,12 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, it gets with "bread". */ if(err < 0) { stack; + error = err; goto fail; } if(err > 0) { + if (!error) + error = err; log_debug( _("Skipping block %" PRIu64 " (0x%" PRIx64 ")\n"), block, block); @@ -1135,10 +1150,10 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, } /* for all data on the indirect block */ } /* for blocks at that height */ } /* for height */ - return 0; + return error; fail: free_metalist(ip, mlp); - return -1; + return error; } /** @@ -1156,26 +1171,32 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) uint32_t height = ip->i_di.di_height; int i, head_size; uint64_t blks_checked = 0; - int error = 0; + int error; - if (!height) - goto end; + if (!height && !S_ISDIR(ip->i_di.di_mode)) + return 0; for (i = 0; i < GFS2_MAX_META_HEIGHT; i++) osi_list_init(&metalist[i]); - /* create metalist for each level */ - if (build_and_check_metalist(ip, &metalist[0], pass)){ + /* create and check the metadata list for each height */ + error = build_and_check_metalist(ip, &metalist[0], pass); + if (error) { stack; - return -1; + return error; } - /* We don't need to record directory blocks - they will be - * recorded later...i think... */ - if (S_ISDIR(ip->i_di.di_mode)) - log_debug( _("Directory with height > 0 at %llu (0x%llx)\n"), - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); + /* For directories, we've already checked the "data" blocks which + * comprise the directory hash table, so we perform the directory + * checks and exit. */ + if (S_ISDIR(ip->i_di.di_mode)) { + free_metalist(ip, &metalist[0]); + if (!(ip->i_di.di_flags & GFS2_DIF_EXHASH)) + return 0; + /* check validity of leaf blocks and leaf chains */ + error = check_leaf_blks(ip, pass); + return error; + } /* check data blocks */ list = &metalist[height - 1]; @@ -1236,20 +1257,7 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) (unsigned long long)ip->i_di.di_num.no_addr); fflush(stdout); } - -end: - if (S_ISDIR(ip->i_di.di_mode)) { - /* check validity of leaf blocks and leaf chains */ - if (ip->i_di.di_flags & GFS2_DIF_EXHASH) { - error = check_leaf_blks(ip, pass); - if(error < 0) - return -1; - if(error > 0) - return 1; - } - } - - return 0; + return error; } /* Checks stuffed inode directories */

1 0

cluster: STABLE3 - fsck.gfs2: metawalk needs to check for no valid leaf blocks
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=01… Commit: 013d20413ffb8d960089836be487b9c16a9f12d6 Parent: 3da2b5547b562a9f332f4875738f131d8f6f0a2e Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Fri Jan 22 13:55:51 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:30 2010 -0600 fsck.gfs2: metawalk needs to check for no valid leaf blocks The code that validates directory leaf blocks was failing in rare cases where the directory had no valid leaf blocks at all. It would give a good return code and pretend everything was okay when, in fact, it left corruption out there. rhbz#455300 --- gfs2/fsck/metawalk.c | 20 +++++++++++++------- 1 files changed, 13 insertions(+), 7 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 0a25160..a0ca65b 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -532,7 +532,7 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) int error; struct gfs2_leaf leaf, oldleaf; uint64_t leaf_no, old_leaf, bad_leaf = -1; - uint64_t first_leaf_ptr = -1, first_ok_leaf = -1; + uint64_t first_ok_leaf; struct gfs2_buffer_head *lbh; int lindex; struct gfs2_sbd *sbp = ip->i_sbd; @@ -542,21 +542,27 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) /* Find the first valid leaf pointer in range and use it as our "old" leaf. That way, bad blocks at the beginning will be overwritten with the first valid leaf. */ - first_ok_leaf = -1; + first_ok_leaf = leaf_no = -1; for(lindex = 0; lindex < (1 << ip->i_di.di_depth); lindex++) { - gfs2_get_leaf_nr(ip, lindex, &first_ok_leaf); - if (first_leaf_ptr == -1) - first_leaf_ptr = first_ok_leaf; - if(gfs2_check_range(ip->i_sbd, first_ok_leaf) == 0) { - lbh = bread(sbp, first_ok_leaf); + gfs2_get_leaf_nr(ip, lindex, &leaf_no); + if (gfs2_check_range(ip->i_sbd, leaf_no) == 0) { + lbh = bread(sbp, leaf_no); /* Make sure it's really a valid leaf block. */ if (gfs2_check_meta(lbh, GFS2_METATYPE_LF) == 0) { brelse(lbh); + first_ok_leaf = leaf_no; break; } brelse(lbh); } } + if (first_ok_leaf == -1) { /* no valid leaf found */ + log_err( _("Directory #%llu (0x%llx) has no valid leaf " + "blocks\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + return 1; + } old_leaf = -1; memset(&oldleaf, 0, sizeof(oldleaf)); for(lindex = 0; lindex < (1 << ip->i_di.di_depth); lindex++) {

1 0

cluster: STABLE3 - fsck.gfs2: enforce consistency between bitmap and blockmap
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=3d… Commit: 3da2b5547b562a9f332f4875738f131d8f6f0a2e Parent: 7393e8776b33ff5e569a81b1871d3e8ad33fb686 Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Fri Jan 22 13:41:44 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:30 2010 -0600 fsck.gfs2: enforce consistency between bitmap and blockmap One of the big problems with fsck.gfs2 was that there was nothing to enforce that the incore bitmap from the rgrps corresponded one-to-one with the incore blockmaps. This patch enforces that by making a central function that sets values into the blockmap and makes sure the bitmap has the correct corresponding bits. It also tracks all block references with debug messages so block map problems may be easily tracked down by running fsck.gfs2 with -vv. I needed to add some support functions and modified how the metadata buffers are managed. rhbz#455300 --- gfs2/fsck/eattr.c | 11 ++- gfs2/fsck/lost_n_found.c | 4 +- gfs2/fsck/main.c | 14 ++- gfs2/fsck/metawalk.c | 216 ++++++++++++++++++++++++++++++++++++++-------- gfs2/fsck/metawalk.h | 8 ++ gfs2/fsck/pass1.c | 196 ++++++++++++++---------------------------- gfs2/fsck/pass1b.c | 23 +++-- gfs2/fsck/pass2.c | 11 ++- gfs2/fsck/pass3.c | 40 +++++++-- gfs2/fsck/pass4.c | 14 ++-- gfs2/libgfs2/libgfs2.h | 28 ++++++ 11 files changed, 360 insertions(+), 205 deletions(-) diff --git a/gfs2/fsck/eattr.c b/gfs2/fsck/eattr.c index 199bc46..2f31c38 100644 --- a/gfs2/fsck/eattr.c +++ b/gfs2/fsck/eattr.c @@ -8,14 +8,15 @@ #include "eattr.h" #include "metawalk.h" -static int clear_blk_nodup(struct gfs2_sbd *sbp, uint64_t block) +static int clear_blk_nodup(struct gfs2_inode *ip, uint64_t block) { if(is_duplicate(block)) { log_debug( _("Not clearing block with marked as a duplicate\n")); return 1; } - gfs2_blockmap_set(bl, block, gfs2_block_free); + fsck_blockmap_set(ip, block, _("cleared eattr block"), + gfs2_block_free); return 0; @@ -25,14 +26,14 @@ int clear_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private) { - return clear_blk_nodup(ip->i_sbd, block); + return clear_blk_nodup(ip, block); } int clear_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private) { - return clear_blk_nodup(ip->i_sbd, block); + return clear_blk_nodup(ip, block); } int clear_eattr_entry (struct gfs2_inode *ip, @@ -81,7 +82,7 @@ int clear_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, { uint64_t block = be64_to_cpu(*ea_data_ptr); - return clear_blk_nodup(ip->i_sbd, block); + return clear_blk_nodup(ip, block); } diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index 2eee341..ea7b370 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -12,6 +12,7 @@ #include "libgfs2.h" #include "lost_n_found.h" #include "link.h" +#include "metawalk.h" #include "util.h" /* add_inode_to_lf - Add dir entry to lost+found for the inode @@ -44,7 +45,8 @@ int add_inode_to_lf(struct gfs2_inode *ip){ * directory or just found an old one, and we * used that instead of the block_type to run * this */ - gfs2_blockmap_set(bl, lf_dip->i_di.di_num.no_addr, + fsck_blockmap_set(ip, lf_dip->i_di.di_num.no_addr, + _("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, diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c index 25c36ad..b026cd9 100644 --- a/gfs2/fsck/main.c +++ b/gfs2/fsck/main.c @@ -15,6 +15,7 @@ #include "libgfs2.h" #include "fsck.h" #include "osi_list.h" +#include "metawalk.h" #include "util.h" struct gfs2_options opts = {0}; @@ -168,9 +169,11 @@ static int check_system_inode(struct gfs2_inode *sysinode, const char *filename, /* bitmap. In that case, don't rebuild the inode. */ /* Just reuse the inode and fix the bitmap. */ if (ds.q == gfs2_block_free) { - log_info( _("The inode exists but the block is not marked 'in use'; fixing it.\n")); - gfs2_blockmap_set(bl,sysinode->i_di.di_num.no_addr, - mark); + log_info( _("The inode exists but the block is not " + "marked 'in use'; fixing it.\n")); + fsck_blockmap_set(sysinode, + sysinode->i_di.di_num.no_addr, + filename, mark); ds.q = mark; if (mark == gfs2_inode_dir) dirtree_insert(sysinode->i_di.di_num.no_addr); @@ -186,8 +189,9 @@ static int check_system_inode(struct gfs2_inode *sysinode, const char *filename, log_err( _("Invalid or missing %s system inode.\n"), filename); if (query(_("Create new %s system inode? (y/n) "), filename)) { builder(sysinode->i_sbd); - gfs2_blockmap_set(bl, sysinode->i_di.di_num.no_addr, - mark); + fsck_blockmap_set(sysinode, + sysinode->i_di.di_num.no_addr, + filename, mark); ds.q = mark; if (mark == gfs2_inode_dir) dirtree_insert(sysinode->i_di.di_num.no_addr); diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 8b75311..0a25160 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -6,16 +6,155 @@ #include <sys/stat.h> #include <unistd.h> #include <libintl.h> +#include <ctype.h> #define _(String) gettext(String) #include "libgfs2.h" +#include "osi_tree.h" #include "fsck.h" #include "util.h" #include "metawalk.h" #include "hash.h" +#include "inode_hash.h" #define COMFORTABLE_BLKS 5242880 /* 20GB in 4K blocks */ +/* There are two bitmaps: (1) The "blockmap" that fsck uses to keep track of + what block type has been discovered, and (2) The rgrp bitmap. Function + gfs2_blockmap_set is used to set the former and gfs2_set_bitmap + is used to set the latter. The two must be kept in sync, otherwise + you'll get bitmap mismatches. This function checks the status of the + bitmap whenever the blockmap changes, and fixes it accordingly. */ +int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, + enum gfs2_mark_block new_blockmap_state) +{ + int old_bitmap_state, new_bitmap_state; + struct rgrp_list *rgd; + + rgd = gfs2_blk2rgrpd(sdp, blk); + + old_bitmap_state = gfs2_get_bitmap(sdp, blk, rgd); + if (old_bitmap_state < 0) { + log_err( _("Block %lld (0x%llx) is not represented in the" + "system bitmap; part of an rgrp or superblock.\n"), + (unsigned long long)blk, (unsigned long long)blk); + return -1; + } + new_bitmap_state = blockmap_to_bitmap(new_blockmap_state); + if (old_bitmap_state != new_bitmap_state) { + const char *allocdesc[] = {"free space", "data", "unlinked", + "inode", "reserved"}; + + log_err( _("Block %llu (0x%llx) seems to be %s, but is " + "marked as %s in the bitmap.\n"), + (unsigned long long)blk, (unsigned long long)blk, + allocdesc[new_bitmap_state], + allocdesc[old_bitmap_state]); + if(query( _("Okay to fix the bitmap? (y/n)"))) { + /* If the new bitmap state is free (and therefore the + old state was not) we have to add to the free + space in the rgrp. If the old bitmap state was + free (and therefore it no longer is) we have to + subtract to the free space. If the type changed + from dinode to data or data to dinode, no change in + free space. */ + gfs2_set_bitmap(sdp, blk, new_bitmap_state); + if (new_bitmap_state == GFS2_BLKST_FREE) { + /* If we're freeing a dinode, get rid of + the hash table entries for it. */ + if (old_bitmap_state == GFS2_BLKST_DINODE) { + struct dir_info *dt; + struct inode_info *ii; + + dt = dirtree_find(blk); + if (dt) + dirtree_delete(dt); + ii = inodetree_find(blk); + if (ii) + inodetree_delete(ii); + } + rgd->rg.rg_free++; + gfs2_rgrp_out(&rgd->rg, rgd->bh[0]); + } else if (old_bitmap_state == GFS2_BLKST_FREE) { + rgd->rg.rg_free--; + gfs2_rgrp_out(&rgd->rg, rgd->bh[0]); + } + log_err( _("The bitmap was fixed.\n")); + } else { + log_err( _("The bitmap inconsistency was ignored.\n")); + } + } + return 0; +} + +/* + * _fsck_blockmap_set - Mark a block in the 4-bit blockmap and the 2-bit + * bitmap, and adjust free space accordingly. + */ +int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock, + const char *btype, enum gfs2_mark_block mark, + const char *caller, int fline) +{ + int error; + + if (print_level >= MSG_DEBUG) { + const char *p; + + p = strrchr(caller, '/'); + if (p) + p++; + else + p = caller; + /* I'm circumventing the log levels here on purpose to make the + output easier to debug. */ + if (ip->i_di.di_num.no_addr == bblock) { + print_fsck_log(MSG_DEBUG, p, fline, + _("%s inode found at block %lld " + "(0x%llx): marking as '%s'\n"), + btype, (unsigned long long) + ip->i_di.di_num.no_addr, + (unsigned long long) + ip->i_di.di_num.no_addr, + block_type_string(mark)); + } else if (mark == gfs2_bad_block || mark == gfs2_meta_inval) { + print_fsck_log(MSG_DEBUG, p, fline, + _("inode %lld (0x%llx) references " + "%s block %lld (0x%llx): " + "marking as '%s'\n"), + (unsigned long long) + ip->i_di.di_num.no_addr, + (unsigned long long) + ip->i_di.di_num.no_addr, + btype, (unsigned long long)bblock, + (unsigned long long)bblock, + block_type_string(mark)); + } else { + print_fsck_log(MSG_DEBUG, p, fline, + _("inode %lld (0x%llx) references " + "%s block %lld (0x%llx): " + "marking as '%s'\n"), + (unsigned long long) + ip->i_di.di_num.no_addr, + (unsigned long long) + ip->i_di.di_num.no_addr, btype, + (unsigned long long)bblock, + (unsigned long long)bblock, + block_type_string(mark)); + } + } + + /* First, check the rgrp bitmap against what we think it should be. + If that fails, it's an invalid block--part of an rgrp. */ + error = check_n_fix_bitmap(ip->i_sbd, bblock, mark); + if (error) { + log_err( _("This block is not represented in the bitmap.\n")); + return error; + } + + error = gfs2_blockmap_set(bl, bblock, mark); + return error; +} + struct duptree *dupfind(uint64_t block) { struct osi_node *node = dup_blocks.osi_node; @@ -692,9 +831,10 @@ static int check_eattr_entries(struct gfs2_inode *ip, /* Endianness doesn't matter in this case because it's a single byte. */ - gfs2_blockmap_set(bl, - ip->i_di.di_eattr, - gfs2_meta_eattr); + fsck_blockmap_set(ip, + ip->i_di.di_eattr, + _("extended attribute"), + gfs2_meta_eattr); log_err( _("The EA was " "fixed.\n")); } else { @@ -843,10 +983,9 @@ static int check_indirect_eattr(struct gfs2_inode *ip, uint64_t indirect, pass->private); } if (leaf_pointer_errors == leaf_pointers) { - if (indirect_buf->b_modified) - gfs2_set_bitmap(sdp, indirect, - GFS2_BLKST_FREE); - gfs2_blockmap_set(bl, indirect, + fsck_blockmap_set(ip, indirect, + _("indirect extended " + "attribute"), gfs2_block_free); error = 1; } @@ -889,6 +1028,29 @@ int check_inode_eattr(struct gfs2_inode *ip, struct metawalk_fxns *pass) } /** + * free_metalist - free all metadata on a multi-level metadata list + */ +static void free_metalist(struct gfs2_inode *ip, osi_list_t *mlp) +{ + int i; + struct gfs2_buffer_head *nbh; + + for (i = 0; i < GFS2_MAX_META_HEIGHT; i++) { + osi_list_t *list; + + list = &mlp[i]; + while (!osi_list_empty(list)) { + nbh = osi_list_entry(list->next, + struct gfs2_buffer_head, b_altlist); + if (nbh == ip->i_bh) + osi_list_del(&nbh->b_altlist); + else + brelse(nbh); + } + } +} + +/** * build_and_check_metalist - check a bunch of indirect blocks * This includes hash table blocks for directories * which are technically "data" in the bitmap. @@ -969,16 +1131,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, } /* for height */ return 0; fail: - for (i = 0; i < GFS2_MAX_META_HEIGHT; i++) { - osi_list_t *list; - list = &mlp[i]; - while (!osi_list_empty(list)) { - nbh = osi_list_entry(list->next, - struct gfs2_buffer_head, b_altlist); - brelse(nbh); - osi_list_del(&nbh->b_altlist); - } - } + free_metalist(ip, mlp); return -1; } @@ -991,7 +1144,7 @@ fail: int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) { osi_list_t metalist[GFS2_MAX_META_HEIGHT]; - osi_list_t *list, *tmp; + osi_list_t *list; struct gfs2_buffer_head *bh; uint64_t block, *ptr; uint32_t height = ip->i_di.di_height; @@ -1023,8 +1176,9 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) if (ip->i_di.di_blocks > COMFORTABLE_BLKS) last_reported_fblock = -10000000; - for (tmp = list->next; tmp != list; tmp = tmp->next) { - bh = osi_list_entry(tmp, struct gfs2_buffer_head, b_altlist); + while (error >= 0 && !osi_list_empty(list)) { + bh = osi_list_entry(list->next, struct gfs2_buffer_head, + b_altlist); if (height > 1) { if (gfs2_check_meta(bh, GFS2_METATYPE_IN)) { @@ -1063,6 +1217,10 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) if (ip->i_di.di_blocks > COMFORTABLE_BLKS) big_file_comfort(ip, blks_checked); } + if (bh == ip->i_bh) + osi_list_del(&bh->b_altlist); + else + brelse(bh); } if (ip->i_di.di_blocks > COMFORTABLE_BLKS) { log_notice( _("\rLarge file at %lld (0x%llx) - 100 percent " @@ -1073,22 +1231,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) fflush(stdout); } - /* free metalists */ - for (i = 0; i < GFS2_MAX_META_HEIGHT; i++) - { - list = &metalist[i]; - while (!osi_list_empty(list)) - { - bh = osi_list_entry(list->next, - struct gfs2_buffer_head, b_altlist); - if (bh == ip->i_bh) - osi_list_del(&bh->b_altlist); - else - brelse(bh); - osi_list_del(&bh->b_altlist); - } - } - end: if (S_ISDIR(ip->i_di.di_mode)) { /* check validity of leaf blocks and leaf chains */ @@ -1205,7 +1347,7 @@ int delete_blocks(struct gfs2_inode *ip, uint64_t 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); - gfs2_blockmap_set(bl, block, gfs2_block_free); + fsck_blockmap_set(ip, block, btype, gfs2_block_free); gfs2_free_block(ip->i_sbd, block); } } diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index 9710b01..72c282a 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -27,10 +27,18 @@ extern int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, extern int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private); +extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock, + const char *btype, enum gfs2_mark_block mark, + const char *caller, int line); +extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, + enum gfs2_mark_block new_blockmap_state); extern struct duptree *dupfind(uint64_t block); #define is_duplicate(dblock) ((dupfind(dblock)) ? 1 : 0) +#define fsck_blockmap_set(ip, b, bt, m) _fsck_blockmap_set(ip, b, bt, m, \ + __FILE__, __LINE__) + /* metawalk_fxns: function pointers to check various parts of the fs * * The functions should return -1 on fatal errors, 1 if the block diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 7563d60..09c6a99 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -74,12 +74,7 @@ static int leaf(struct gfs2_inode *ip, uint64_t block, { struct block_count *bc = (struct block_count *) private; - log_debug( _("\tDinode %lld (0x%llx) references leaf block #%lld " - "(0x%llx)\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); - gfs2_blockmap_set(bl, block, gfs2_leaf_blk); + fsck_blockmap_set(ip, block, _("directory leaf"), gfs2_leaf_blk); bc->indir_count++; return 0; } @@ -95,8 +90,8 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, *bh = NULL; if (gfs2_check_range(ip->i_sbd, block)){ /* blk outside of FS */ - gfs2_blockmap_set(bl, ip->i_di.di_num.no_addr, - gfs2_bad_block); + 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")); return 1; @@ -114,7 +109,8 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, log_debug( _("Bad indirect block pointer (points to " "something that is not an indirect block).\n")); if(!found_dup) { - gfs2_blockmap_set(bl, block, gfs2_meta_inval); + fsck_blockmap_set(ip, block, _("bad indirect"), + gfs2_meta_inval); brelse(nbh); return 1; } @@ -122,11 +118,9 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, } else /* blk check ok */ *bh = nbh; - if (!found_dup) { - log_debug( _("Setting %" PRIu64 " (0x%" PRIx64 ") to indirect " - "block.\n"), block, block); - gfs2_blockmap_set(bl, block, gfs2_indir_blk); - } + if (!found_dup) + fsck_blockmap_set(ip, block, _("indirect"), + gfs2_indir_blk); bc->indir_count++; return 0; @@ -136,7 +130,7 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) { uint8_t q; struct block_count *bc = (struct block_count *) private; - int error = 0, btype; + int error = 0; if (gfs2_check_range(ip->i_sbd, block)) { log_err( _("inode %lld (0x%llx) has a bad data block pointer " @@ -147,8 +141,9 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) /* Mark the owner of this block with the bad_block * designator so we know to check it for out of range * blocks later */ - gfs2_blockmap_set(bl, ip->i_di.di_num.no_addr, - gfs2_bad_block); + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("bad (out of range) data"), + gfs2_bad_block); return 1; } q = block_type(block); @@ -176,41 +171,7 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) gfs2_block_unmark(ip->i_sbd, bl, block, gfs2_meta_inval); gfs2_dup_set(block); } - log_debug( _("Marking block %llu (0x%llx) as data block\n"), - (unsigned long long)block, (unsigned long long)block); - gfs2_blockmap_set(bl, block, gfs2_block_used); - - /* This is also confusing, so I'll clarify. There are two bitmaps: - (1) The gfs2_bmap that fsck uses to keep track of what block - type has been discovered, and (2) The rgrp bitmap. Function - gfs2_blockmap_set is used to set the former and gfs2_set_bitmap - is used to set the latter. In this function we need to set both - because we found a "data" block that could be "meta" in the rgrp - bitmap. If we don't we could run into the data block again as - metadata when we're traversing the metadata with gfs2_next_rg_meta - in func pass1(). If that happens, it will look at the block, - say "hey this isn't metadata" and mark it incorrectly as an - invalid metadata block and free it. Ordinarily, fsck will wait - until pass5 to sync (2) so that it agrees with (1). However, in - this case, it's better to do it upfront. The duplicate solving - code in pass1b.c is better at resolving metadata referencing a - data block than it is at resolving a data block referencing a - metadata block. */ - btype = gfs2_get_bitmap(ip->i_sbd, block, NULL); - if (btype != GFS2_BLKST_USED) { - const char *allocdesc[] = {"free space", "data", "unlinked", - "metadata", "reserved"}; - - log_err( _("Block %llu (0x%llx) seems to be data, but is " - "marked as %s.\n"), (unsigned long long)block, - (unsigned long long)block, allocdesc[btype]); - if(query( _("Okay to mark it as 'data'? (y/n)"))) { - gfs2_set_bitmap(ip->i_sbd, block, GFS2_BLKST_USED); - log_err( _("The block was reassigned as data.\n")); - } else { - log_err( _("The invalid block was ignored.\n")); - } - } + fsck_blockmap_set(ip, block, _("data"), gfs2_block_used); bc->data_count++; return error; } @@ -218,11 +179,9 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) static int remove_inode_eattr(struct gfs2_inode *ip, struct block_count *bc, int duplicate) { - if (!duplicate) { - gfs2_set_bitmap(ip->i_sbd, ip->i_di.di_eattr, - GFS2_BLKST_FREE); - gfs2_blockmap_set(bl, ip->i_di.di_eattr, gfs2_block_free); - } + if (!duplicate) + fsck_blockmap_set(ip, ip->i_di.di_eattr, _("deleted eattr"), + gfs2_block_free); ip->i_di.di_eattr = 0; bc->ea_count = 0; ip->i_di.di_blocks = 1 + bc->indir_count + bc->data_count; @@ -262,8 +221,6 @@ static int ask_remove_inode_eattr(struct gfs2_inode *ip, static int clear_eas(struct gfs2_inode *ip, struct block_count *bc, uint64_t block, int duplicate, const char *emsg) { - struct gfs2_sbd *sdp = ip->i_sbd; - log_err( _("Inode #%llu (0x%llx): %s"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr, emsg); @@ -275,8 +232,8 @@ static int clear_eas(struct gfs2_inode *ip, struct block_count *bc, log_err( _("The bad extended attribute was " "removed.\n")); } else if (!duplicate) { - gfs2_blockmap_set(bl, block, gfs2_block_free); - gfs2_set_bitmap(sdp, block, GFS2_BLKST_FREE); + fsck_blockmap_set(ip, block, _("removed eattr"), + gfs2_block_free); log_err( _("The bad Extended Attribute was " "removed.\n")); } @@ -341,10 +298,9 @@ static int check_eattr_indir(struct gfs2_inode *ip, uint64_t indirect, bc->ea_count++; ret = 1; } else { - log_debug( _("Setting #%" PRIu64 " (0x%" PRIx64 - ") to indirect Extended Attribute block\n"), - indirect, indirect); - gfs2_blockmap_set(bl, indirect, gfs2_indir_blk); + fsck_blockmap_set(ip, indirect, + _("indirect Extended Attribute"), + gfs2_indir_blk); bc->ea_count++; } return ret; @@ -426,12 +382,10 @@ static int check_leaf_block(struct gfs2_inode *ip, uint64_t block, int btype, brelse(leaf_bh); return 1; } - log_debug( _("Setting block #%lld (0x%llx) to eattr block\n"), - (unsigned long long)block, (unsigned long long)block); /* Point of confusion: We've got to set the ea block itself to gfs2_meta_eattr here. Elsewhere we mark the inode with gfs2_eattr_block meaning it contains an eattr for pass1c. */ - gfs2_blockmap_set(bl, block, gfs2_meta_eattr); + fsck_blockmap_set(ip, block, _("Extended Attribute"), gfs2_meta_eattr); bc->ea_count++; *bh = leaf_bh; return 0; @@ -469,7 +423,9 @@ static int check_extended_leaf_eattr(struct gfs2_inode *ip, uint64_t *data_ptr, (unsigned long long)ip->i_di.di_eattr, (unsigned long long)el_blk, (unsigned long long)el_blk); - gfs2_blockmap_set(bl, ip->i_di.di_eattr, gfs2_bad_block); + fsck_blockmap_set(ip, ip->i_di.di_eattr, + _("bad (out of range) Extended Attribute "), + gfs2_bad_block); return 1; } error = check_leaf_block(ip, el_blk, GFS2_METATYPE_ED, &bh, private); @@ -500,7 +456,9 @@ static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block, (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); - gfs2_blockmap_set(bl, ip->i_di.di_eattr, gfs2_bad_block); + fsck_blockmap_set(ip, ip->i_di.di_eattr, + _("bad (out of range) Extended " + "Attribute leaf"), gfs2_bad_block); return 1; } return check_leaf_block(ip, block, GFS2_METATYPE_EA, bh, private); @@ -552,38 +510,26 @@ static int clear_metalist(struct gfs2_inode *ip, uint64_t block, { *bh = NULL; - if(!is_duplicate(block)) { - gfs2_blockmap_set(bl, block, gfs2_block_free); - return 0; - } + if(!is_duplicate(block)) + fsck_blockmap_set(ip, block, _("cleared metadata"), + gfs2_block_free); return 0; } static int clear_data(struct gfs2_inode *ip, uint64_t block, void *private) { - if(!is_duplicate(block)) { - gfs2_blockmap_set(bl, block, gfs2_block_free); - return 0; - } + if(!is_duplicate(block)) + fsck_blockmap_set(ip, block, _("cleared data"), + gfs2_block_free); return 0; - } static int clear_leaf(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head *bh, void *private) { - log_crit( _("Clearing leaf #%" PRIu64 " (0x%" PRIx64 ")\n"), - block, block); - - if(!is_duplicate(block)) { - log_crit( _("Setting leaf #%" PRIu64 " (0x%" PRIx64 ") invalid\n"), - block, block); - if(gfs2_blockmap_set(bl, block, gfs2_block_free)) { - stack; - return -1; - } - return 0; - } + if(!is_duplicate(block)) + fsck_blockmap_set(ip, block, _("cleared directory leaf"), + gfs2_block_free); return 0; } @@ -630,9 +576,8 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, switch(ip->i_di.di_mode & S_IFMT) { case S_IFDIR: - log_debug( _("Setting %" PRIu64 " (0x%" PRIx64 ") to directory inode.\n"), - block, block); - if(gfs2_blockmap_set(bl, block, gfs2_inode_dir)) { + if (fsck_blockmap_set(ip, block, _("directory"), + gfs2_inode_dir)) { stack; fsck_inode_put(&ip); return -1; @@ -644,68 +589,60 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, } break; case S_IFREG: - log_debug( _("Setting %" PRIu64 " (0x%" PRIx64 ") to file inode.\n"), - block, block); - if(gfs2_blockmap_set(bl, block, gfs2_inode_file)) { + if (fsck_blockmap_set(ip, block, _("file"), + gfs2_inode_file)) { stack; fsck_inode_put(&ip); return -1; } break; case S_IFLNK: - log_debug( _("Setting %" PRIu64 " (0x%" PRIx64 ") to symlink inode.\n"), - block, block); - if(gfs2_blockmap_set(bl, block, gfs2_inode_lnk)) { + if (fsck_blockmap_set(ip, block, _("symlink"), + gfs2_inode_lnk)) { stack; fsck_inode_put(&ip); return -1; } break; case S_IFBLK: - log_debug( _("Setting %" PRIu64 " (0x%" PRIx64 ") to block dev inode.\n"), - block, block); - if(gfs2_blockmap_set(bl, block, gfs2_inode_blk)) { + if (fsck_blockmap_set(ip, block, _("block device"), + gfs2_inode_blk)) { stack; fsck_inode_put(&ip); return -1; } break; case S_IFCHR: - log_debug( _("Setting %" PRIu64 " (0x%" PRIx64 ") to char dev inode.\n"), - block, block); - if(gfs2_blockmap_set(bl, block, gfs2_inode_chr)) { + if (fsck_blockmap_set(ip, block, _("character device"), + gfs2_inode_chr)) { stack; fsck_inode_put(&ip); return -1; } break; case S_IFIFO: - log_debug( _("Setting %" PRIu64 " (0x%" PRIx64 ") to fifo inode.\n"), - block, block); - if(gfs2_blockmap_set(bl, block, gfs2_inode_fifo)) { + if (fsck_blockmap_set(ip, block, _("fifo"), + gfs2_inode_fifo)) { stack; fsck_inode_put(&ip); return -1; } break; case S_IFSOCK: - log_debug( _("Setting %" PRIu64 " (0x%" PRIx64 ") to socket inode.\n"), - block, block); - if(gfs2_blockmap_set(bl, block, gfs2_inode_sock)) { + if (fsck_blockmap_set(ip, block, _("socket"), + gfs2_inode_sock)) { stack; fsck_inode_put(&ip); return -1; } break; default: - log_debug( _("Setting %" PRIu64 " (0x%" PRIx64 ") to invalid.\n"), - block, block); - if(gfs2_blockmap_set(bl, block, gfs2_meta_inval)) { + if (fsck_blockmap_set(ip, block, _("invalid mode"), + gfs2_meta_inval)) { stack; fsck_inode_put(&ip); return -1; } - gfs2_set_bitmap(sdp, block, GFS2_BLKST_FREE); fsck_inode_put(&ip); return 0; } @@ -724,14 +661,12 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, (unsigned long long)ip->i_di.di_num.no_addr, ip->i_di.di_depth, (1 >> (ip->i_di.di_size/sizeof(uint64_t)))); - /* once implemented, remove continue statement */ - log_warn( _("Marking inode invalid\n")); - if(gfs2_blockmap_set(bl, block, gfs2_meta_inval)) { + if(fsck_blockmap_set(ip, block, _("bad depth"), + gfs2_meta_inval)) { stack; fsck_inode_put(&ip); return -1; } - gfs2_set_bitmap(sdp, block, GFS2_BLKST_FREE); fsck_inode_put(&ip); return 0; } @@ -750,9 +685,8 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, (unsigned long long)ip->i_di.di_num.no_addr); /* FIXME: Must set all leaves invalid as well */ check_metatree(ip, &invalidate_metatree); - gfs2_blockmap_set(bl, ip->i_di.di_num.no_addr, - gfs2_meta_inval); - gfs2_set_bitmap(sdp, ip->i_di.di_num.no_addr, GFS2_BLKST_FREE); + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("corrupt"), gfs2_meta_inval); fsck_inode_put(&ip); return 0; } @@ -878,16 +812,14 @@ int pass1(struct gfs2_sbd *sbp) "%llu (0x%llx)\n"), (unsigned long long)block, (unsigned long long)block); - if(query( _("Okay to free the invalid block? " - "(y/n)"))) { - gfs2_set_bitmap(sbp, block, - GFS2_BLKST_FREE); - log_err( _("The invalid block was " - "freed.\n")); - } else { - log_err( _("The invalid block was " - "ignored.\n")); + if (gfs2_blockmap_set(bl, block, + gfs2_block_free)) { + stack; + brelse(bh); + return FSCK_ERROR; } + check_n_fix_bitmap(sbp, block, + gfs2_block_free); } else if (handle_di(sbp, bh, block) < 0) { stack; brelse(bh); diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 728ec8b..16b5ee5 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -165,8 +165,8 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, dh->id->parent); /* Setting the block to invalid means the inode is * cleared in pass2 */ - gfs2_blockmap_set(bl, ip->i_di.di_num.no_addr, - gfs2_meta_inval); + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("inode with duplicate"), gfs2_meta_inval); } return 0; } @@ -197,7 +197,8 @@ static int clear_dup_eattr_indir(struct gfs2_inode *ip, uint64_t block, log_err( _("Inode %s is in directory %" PRIu64 " (0x%" PRIx64 ")\n"), dh->id->name ? dh->id->name : "", dh->id->parent, dh->id->parent); - gfs2_blockmap_set(bl, ip->i_di.di_eattr, gfs2_meta_inval); + fsck_blockmap_set(ip, ip->i_di.di_eattr, + _("indirect eattr"), gfs2_meta_inval); } return 0; @@ -223,7 +224,8 @@ static int clear_dup_eattr_leaf(struct gfs2_inode *ip, uint64_t block, dh->id->name ? dh->id->name : "", dh->id->parent, dh->id->parent); /* mark the main eattr block invalid */ - gfs2_blockmap_set(bl, ip->i_di.di_eattr, gfs2_meta_inval); + fsck_blockmap_set(ip, ip->i_di.di_eattr, + _("indirect eattr leaf"), gfs2_meta_inval); } return 0; @@ -293,7 +295,8 @@ static int clear_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, dh->id->name ? dh->id->name : "", dh->id->parent, dh->id->parent); /* mark the main eattr block invalid */ - gfs2_blockmap_set(bl, ip->i_di.di_eattr, gfs2_meta_inval); + fsck_blockmap_set(ip, ip->i_di.di_eattr, + _("extended eattr leaf"), gfs2_meta_inval); } return 0; @@ -415,9 +418,9 @@ static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) inodetree_delete(ii); /* Setting the block to invalid means the inode is cleared in pass2 */ - gfs2_blockmap_set(bl, ip->i_di.di_num.no_addr, - gfs2_meta_inval); - bmodified(ip->i_bh); + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("inode with bad duplicate"), + gfs2_meta_inval); fsck_inode_put(&ip); } else { log_warn( _("The bad inode was not cleared.")); @@ -461,8 +464,8 @@ static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) if(!id->ea_only) check_metatree(ip, &clear_dup_fxns); - gfs2_blockmap_set(bl, ip->i_di.di_num.no_addr, - gfs2_meta_inval); + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("inode with duplicate"), gfs2_meta_inval); fsck_inode_put(&ip); /* out, brelse, free */ dh.ref_inode_count--; if(dh.ref_inode_count == 1) diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index b1be8a8..0650966 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -212,7 +212,8 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, log_err( _("Directory entry not fixed.\n")); goto dentry_is_valid; } - gfs2_blockmap_set(bl, ip->i_di.di_num.no_addr, + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("corrupt directory entry"), gfs2_meta_inval); log_err( _("Bad directory entry deleted.\n")); return 1; @@ -261,7 +262,8 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, check_metatree(entry_ip, &pass2_fxns_delete); if (entry_ip != ip) fsck_inode_put(&entry_ip); - gfs2_blockmap_set(bl, entryblock, gfs2_block_free); + fsck_blockmap_set(ip, entryblock, + _("bad directory entry"), gfs2_block_free); goto nuke_dentry; } if(q < gfs2_inode_dir || q > gfs2_inode_sock) { @@ -537,7 +539,8 @@ static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname, return -1; } if (error > 0) - gfs2_blockmap_set(bl, iblock, gfs2_meta_inval); + fsck_blockmap_set(sysinode, iblock, dirname, + gfs2_meta_inval); if(check_inode_eattr(sysinode, &pass2_fxns)) { stack; @@ -717,6 +720,8 @@ int pass2(struct gfs2_sbd *sbp) } else log_err( _("Directory entry to invalid inode remains.\n")); } + /* Can't use fsck_blockmap_set here because we don't + have an inode in memory. */ gfs2_blockmap_set(bl, dirblk, gfs2_meta_inval); } ip = fsck_load_inode(sbp, dirblk); diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c index bef6d00..2ff4fbf 100644 --- a/gfs2/fsck/pass3.c +++ b/gfs2/fsck/pass3.c @@ -201,9 +201,20 @@ int pass3(struct gfs2_sbd *sbp) log_err( _("Found unlinked directory containing bad block\n")); if(query( _("Clear unlinked directory with bad blocks? (y/n) "))) { + log_warn( _("inode %lld (0x%llx) is " + "now marked as free\n"), + (unsigned long long) + di->dinode, + (unsigned long long) + di->dinode); + /* Can't use fsck_blockmap_set + because we don't have ip */ gfs2_blockmap_set(bl, - di->dinode, - gfs2_block_free); + di->dinode, + gfs2_block_free); + check_n_fix_bitmap(sbp, + di->dinode, + gfs2_block_free); break; } else log_err( _("Unlinked directory with bad block remains\n")); @@ -216,9 +227,25 @@ int pass3(struct gfs2_sbd *sbp) q != gfs2_inode_fifo && q != gfs2_inode_sock) { log_err( _("Unlinked block marked as inode not an inode\n")); + if(!query(_("Clear the unlinked block?" + " (y/n) "))) { + log_err( _("The block was not " + "cleared\n")); + break; + } + log_warn( _("inode %lld (0x%llx) is now " + "marked as free\n"), + (unsigned long long) + di->dinode, + (unsigned long long) + di->dinode); + /* Can't use fsck_blockmap_set + because we don't have ip */ gfs2_blockmap_set(bl, di->dinode, gfs2_block_free); - log_err( _("Cleared\n")); + check_n_fix_bitmap(sbp, di->dinode, + gfs2_block_free); + log_err( _("The block was cleared\n")); break; } @@ -230,9 +257,10 @@ int pass3(struct gfs2_sbd *sbp) if(!ip->i_di.di_size && !ip->i_di.di_eattr){ log_err( _("Unlinked directory has zero size.\n")); if(query( _("Remove zero-size unlinked directory? (y/n) "))) { - gfs2_blockmap_set(bl, - di->dinode, - gfs2_block_free); + fsck_blockmap_set(ip, + di->dinode, + _("zero-sized unlinked inode"), + gfs2_block_free); fsck_inode_put(&ip); break; } else { diff --git a/gfs2/fsck/pass4.c b/gfs2/fsck/pass4.c index d275ecf..e41d995 100644 --- a/gfs2/fsck/pass4.c +++ b/gfs2/fsck/pass4.c @@ -70,10 +70,10 @@ static int scan_inode_list(struct gfs2_sbd *sbp) { check_inode_eattr(ip, &pass4_fxns_delete); check_metatree(ip, &pass4_fxns_delete); - bmodified(ip->i_bh); - fsck_inode_put(&ip); - gfs2_blockmap_set(bl, ii->inode, + fsck_blockmap_set(ip, ii->inode, + _("bad unlinked"), gfs2_block_free); + fsck_inode_put(&ip); continue; } else log_err( _("Unlinked inode with bad blocks not cleared\n")); @@ -95,9 +95,10 @@ static int scan_inode_list(struct gfs2_sbd *sbp) { check_inode_eattr(ip, &pass4_fxns_delete); check_metatree(ip, &pass4_fxns_delete); - bmodified(ip->i_bh); - gfs2_blockmap_set(bl, ii->inode, + fsck_blockmap_set(ip, ii->inode, + _("invalid unlinked"), gfs2_block_free); + fsck_inode_put(&ip); log_err( _("The inode was deleted\n")); } else { log_err( _("The inode was not " @@ -114,7 +115,8 @@ static int scan_inode_list(struct gfs2_sbd *sbp) { 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) "))) { - gfs2_blockmap_set(bl, ii->inode, + fsck_blockmap_set(ip, ii->inode, + _("unlinked zero-length"), gfs2_block_free); fsck_inode_put(&ip); continue; diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index 3a6c61e..951966a 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -313,6 +313,34 @@ static const inline char *block_type_string(uint8_t q) return blktyp[15]; } +/* Must be kept in sync with gfs2_mark_block enum above. Blocks marked as + invalid or bad are considered metadata until actually freed. */ +static inline int blockmap_to_bitmap(enum gfs2_mark_block m) +{ + static int bitmap_states[16] = { + GFS2_BLKST_FREE, + GFS2_BLKST_USED, + GFS2_BLKST_USED, + GFS2_BLKST_DINODE, + GFS2_BLKST_DINODE, + + GFS2_BLKST_DINODE, + GFS2_BLKST_DINODE, + GFS2_BLKST_DINODE, + GFS2_BLKST_DINODE, + GFS2_BLKST_DINODE, + + GFS2_BLKST_FREE, + GFS2_BLKST_FREE, + GFS2_BLKST_USED, + GFS2_BLKST_USED, + GFS2_BLKST_USED, + + GFS2_BLKST_USED + }; + return bitmap_states[m]; +} + extern struct gfs2_bmap *gfs2_bmap_create(struct gfs2_sbd *sdp, uint64_t size, uint64_t *addl_mem_needed); extern struct special_blocks *blockfind(struct special_blocks *blist, uint64_t num);

1 0

cluster: STABLE3 - fsck.gfs2: Enforce consistent behavior in directory processing
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=73… Commit: 7393e8776b33ff5e569a81b1871d3e8ad33fb686 Parent: f8a4338b1c2978b935cb4fe9289663cce642a6ec Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Fri Jan 22 09:18:48 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:30 2010 -0600 fsck.gfs2: Enforce consistent behavior in directory processing In pass2, all directory entries are checked and validated by a huge, confusing, hard-to-debug function called check_dentry. However, when problems were found, there was inconsistent behavior regarding the keeping and accounting for valid directories or deleting invalid directories. But either a directory entry is good or it's not. If it is invalid, no matter what the problem is, it should be treated the same as all the other bad entries. If the directory is good, or if fsck.gfs2 is instructed to ignore the problem, it should be treated the same as all other good entries. To that end, this patch introduces two centralized labels (dentry_is_valid and nuke_dentry). Function check_dentry is simplified and its behavior made consistent by introducing several gotos (oh, the shame) to those labels. rhbz#455300 --- gfs2/fsck/pass2.c | 330 ++++++++++++++++++++++------------------------------ 1 files changed, 140 insertions(+), 190 deletions(-) diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index e04f73f..b1be8a8 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -180,41 +180,6 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, entryblock = de->de_inum.no_addr; /* Start of checks */ - if (de->de_rec_len < GFS2_DIRENT_SIZE(de->de_name_len)){ - log_err( _("Dir entry with bad record or name length\n" - "\tRecord length = %u\n" - "\tName length = %u\n"), - de->de_rec_len, - de->de_name_len); - gfs2_blockmap_set(bl, ip->i_di.di_num.no_addr, - gfs2_meta_inval); - return 1; - /* FIXME: should probably delete the entry here at the - * very least - maybe look at attempting to fix it */ - } - - calculated_hash = gfs2_disk_hash(filename, de->de_name_len); - if (de->de_hash != calculated_hash){ - log_err( _("Dir entry with bad hash or name length\n" - "\tHash found = %u (0x%x)\n" - "\tFilename = %s\n"), de->de_hash, de->de_hash, - filename); - log_err( _("\tName length found = %u\n" - "\tHash expected = %u (0x%x)\n"), - de->de_name_len, calculated_hash, calculated_hash); - if(query( _("Fix directory hash for %s? (y/n) "), - filename)) { - de->de_hash = calculated_hash; - gfs2_dirent_out(de, (char *)dent); - log_err( _("Directory entry hash for %s fixed.\n"), filename); - } - else { - log_err( _("Directory entry hash for %s not fixed.\n"), filename); - return 1; - } - } - /* FIXME: This should probably go to the top of the fxn, and - * references to filename should be replaced with tmp_name */ memset(tmp_name, 0, MAX_FILENAME); if(de->de_name_len < MAX_FILENAME) strncpy(tmp_name, filename, de->de_name_len); @@ -226,11 +191,9 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, "%lld (0x%llx) is out of range\n"), tmp_name, (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); - if(query( _("Clear directory entry tp out of range block? (y/n) "))) { - log_err( _("Clearing %s\n"), tmp_name); - dirent2_del(ip, bh, prev_de, dent); - bmodified(bh); - return 1; + if(query( _("Clear directory entry to out of range block? " + "(y/n) "))) { + goto nuke_dentry; } else { log_err( _("Directory entry to out of range block remains\n")); (*count)++; @@ -240,6 +203,43 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, return 0; } } + + if (de->de_rec_len < GFS2_DIRENT_SIZE(de->de_name_len)) { + log_err( _("Dir entry with bad record or name length\n" + "\tRecord length = %u\n\tName length = %u\n"), + de->de_rec_len, de->de_name_len); + if(!query( _("Clear the directory entry? (y/n) "))) { + log_err( _("Directory entry not fixed.\n")); + goto dentry_is_valid; + } + gfs2_blockmap_set(bl, ip->i_di.di_num.no_addr, + gfs2_meta_inval); + log_err( _("Bad directory entry deleted.\n")); + return 1; + } + + calculated_hash = gfs2_disk_hash(tmp_name, de->de_name_len); + if (de->de_hash != calculated_hash){ + log_err( _("Dir entry with bad hash or name length\n" + "\tHash found = %u (0x%x)\n" + "\tFilename = %s\n"), + de->de_hash, de->de_hash, tmp_name); + log_err( _("\tName length found = %u\n" + "\tHash expected = %u (0x%x)\n"), + de->de_name_len, calculated_hash, calculated_hash); + if(!query( _("Fix directory hash for %s? (y/n) "), + tmp_name)) { + log_err( _("Directory entry hash for %s not " + "fixed.\n"), tmp_name); + goto dentry_is_valid; + } + de->de_hash = calculated_hash; + gfs2_dirent_out(de, (char *)dent); + bmodified(bh); + log_err( _("Directory entry hash for %s fixed.\n"), + tmp_name); + } + q = block_type(entryblock); /* Get the status of the directory inode */ if(q == gfs2_bad_block) { @@ -248,25 +248,21 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, /* Handle bad blocks */ log_err( _("Found a bad directory entry: %s\n"), filename); - if(query( _("Delete inode containing bad blocks? (y/n)"))) { - entry_ip = fsck_load_inode(sbp, entryblock); - check_inode_eattr(entry_ip, &pass2_fxns_delete); - check_metatree(entry_ip, &pass2_fxns_delete); - bmodified(entry_ip->i_bh); - fsck_inode_put(&entry_ip); - dirent2_del(ip, bh, prev_de, dent); - gfs2_blockmap_set(bl, entryblock, gfs2_block_free); - bmodified(bh); - log_warn( _("The inode containing bad blocks was " - "deleted.\n")); - return 1; - } else { + if(!query( _("Delete inode containing bad blocks? (y/n)"))) { log_warn( _("Entry to inode containing bad blocks remains\n")); - (*count)++; - ds->entry_count++; - return 0; + goto dentry_is_valid; } + if (ip->i_di.di_num.no_addr == entryblock) + entry_ip = ip; + else + entry_ip = fsck_load_inode(sbp, entryblock); + check_inode_eattr(entry_ip, &pass2_fxns_delete); + check_metatree(entry_ip, &pass2_fxns_delete); + if (entry_ip != ip) + fsck_inode_put(&entry_ip); + gfs2_blockmap_set(bl, entryblock, gfs2_block_free); + goto nuke_dentry; } if(q < gfs2_inode_dir || q > gfs2_inode_sock) { log_err( _("Directory entry '%s' referencing inode %llu " @@ -280,41 +276,29 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, _("previously marked invalid") : _("is not an inode")); - if(query( _("Clear directory entry to non-inode block? " - "(y/n) "))) { - struct gfs2_buffer_head *bhi; - - dirent2_del(ip, bh, prev_de, dent); - bmodified(bh); - log_warn( _("Directory entry '%s' cleared\n"), tmp_name); - /* If it was previously marked invalid (i.e. known - to be bad, not just a free block, etc.) then - delete any metadata it holds. If not, return. */ - if (q != gfs2_meta_inval) - return 1; - - /* Now try to clear the dinode, if it is an dinode */ - bhi = bread(sbp, entryblock); - error = gfs2_check_meta(bhi, GFS2_METATYPE_DI); - bmodified(bhi); - brelse(bhi); - if (error) - return 1; /* not a dinode: nothing to delete */ - - entry_ip = fsck_load_inode(sbp, entryblock); - check_inode_eattr(entry_ip, &pass2_fxns_delete); - check_metatree(entry_ip, &pass2_fxns_delete); - bmodified(entry_ip->i_bh); - fsck_inode_put(&entry_ip); - gfs2_blockmap_set(bl, entryblock, gfs2_block_free); - - return 1; - } else { + if(!query( _("Clear directory entry to non-inode block? " + "(y/n) "))) { log_err( _("Directory entry to non-inode block remains\n")); - (*count)++; - ds->entry_count++; - return 0; + goto dentry_is_valid; } + + /* Don't decrement the link here: Here in pass2, we increment + only when we know it's okay. + decrement_link(ip->i_di.di_num.no_addr); */ + /* If it was previously marked invalid (i.e. known + to be bad, not just a free block, etc.) then the temptation + would be to delete any metadata it holds. The trouble is: + if it's invalid, we may or _may_not_ have traversed its + metadata tree, and therefore may or may not have marked the + blocks it points to as a metadata type, or as a duplicate. + If there is really a duplicate reference, but we didn't + process the metadata tree because it's invalid, some other + inode has a reference to the metadata block, in which case + freeing it would do more harm than good. IOW we cannot + count on "delete_block_if_notdup" knowing whether it's + really a duplicate block if we never traversed the metadata + tree for the invalid inode. */ + goto nuke_dentry; } error = check_file_type(de->de_type, q); @@ -338,22 +322,18 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, (unsigned long long)entryblock, (unsigned long long)entryblock, block_type_string(q)); - if(query( _("Clear stale directory entry? (y/n) "))) { + if(!query( _("Clear stale directory entry? (y/n) "))) { + log_err( _("Stale directory entry remains\n")); + goto dentry_is_valid; + } + if (ip->i_di.di_num.no_addr == entryblock) + entry_ip = ip; + else entry_ip = fsck_load_inode(sbp, entryblock); check_inode_eattr(entry_ip, &clear_eattrs); if (entry_ip != ip) fsck_inode_put(&entry_ip); - - dirent2_del(ip, bh, prev_de, dent); - bmodified(bh); - log_err( _("Stale directory entry deleted\n")); - return 1; - } else { - log_err( _("Stale directory entry remains\n")); - (*count)++; - ds->entry_count++; - return 0; - } + goto nuke_dentry; } if(!strcmp(".", tmp_name)) { @@ -364,26 +344,20 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, " (0x%llx)\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); - if(query( _("Clear duplicate '.' entry? (y/n) "))) { - entry_ip = fsck_load_inode(sbp, entryblock); - check_inode_eattr(entry_ip, &clear_eattrs); - fsck_inode_put(&entry_ip); - - dirent2_del(ip, bh, prev_de, dent); - bmodified(bh); - return 1; - } else { + if(!query( _("Clear duplicate '.' entry? (y/n) "))) { log_err( _("Duplicate '.' entry remains\n")); /* FIXME: Should we continue on here - * and check the rest of the '.' - * entry? */ - increment_link(entryblock, - ip->i_di.di_num.no_addr, - _("valid reference")); - (*count)++; - ds->entry_count++; - return 0; + * and check the rest of the '.' entry? */ + goto dentry_is_valid; } + if (ip->i_di.di_num.no_addr == entryblock) + entry_ip = ip; + else + entry_ip = fsck_load_inode(sbp, entryblock); + check_inode_eattr(entry_ip, &clear_eattrs); + if (entry_ip != ip) + fsck_inode_put(&entry_ip); + goto nuke_dentry; } /* GFS2 does not rely on '.' being in a certain @@ -401,35 +375,24 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, (unsigned long long)entryblock, (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); - if(query( _("Remove '.' reference? (y/n) "))) { - entry_ip = fsck_load_inode(sbp, entryblock); - check_inode_eattr(entry_ip, &clear_eattrs); - fsck_inode_put(&entry_ip); - - dirent2_del(ip, bh, prev_de, dent); - bmodified(bh); - return 1; - - } else { + if(!query( _("Remove '.' reference? (y/n) "))) { log_err( _("Invalid '.' reference remains\n")); /* Not setting ds->dotdir here since * this '.' entry is invalid */ - increment_link(entryblock, - ip->i_di.di_num.no_addr, - _("valid reference")); - (*count)++; - ds->entry_count++; - return 0; + goto dentry_is_valid; } + if (ip->i_di.di_num.no_addr == entryblock) + entry_ip = ip; + else + entry_ip = fsck_load_inode(sbp, entryblock); + check_inode_eattr(entry_ip, &clear_eattrs); + if (entry_ip != ip) + fsck_inode_put(&entry_ip); + goto nuke_dentry; } ds->dotdir = 1; - increment_link(entryblock, ip->i_di.di_num.no_addr, - _("valid reference")); - (*count)++; - ds->entry_count++; - - return 0; + goto dentry_is_valid; } if(!strcmp("..", tmp_name)) { log_debug( _("Found .. dentry\n")); @@ -438,27 +401,23 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, "(0x%llx)\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); - if(query( _("Clear duplicate '..' entry? (y/n) "))) { - - entry_ip = fsck_load_inode(sbp, entryblock); - check_inode_eattr(entry_ip, &clear_eattrs); - fsck_inode_put(&entry_ip); - - dirent2_del(ip, bh, prev_de, dent); - bmodified(bh); - return 1; - } else { + if(!query( _("Clear duplicate '..' entry? (y/n) "))) { log_err( _("Duplicate '..' entry remains\n")); /* FIXME: Should we continue on here * and check the rest of the '..' * entry? */ - increment_link(entryblock, - ip->i_di.di_num.no_addr, - _("valid reference")); - (*count)++; - ds->entry_count++; - return 0; + goto dentry_is_valid; } + + if (ip->i_di.di_num.no_addr == entryblock) + entry_ip = ip; + else + entry_ip = fsck_load_inode(sbp, entryblock); + check_inode_eattr(entry_ip, &clear_eattrs); + if (entry_ip != ip) + fsck_inode_put(&entry_ip); + + goto nuke_dentry; } if(q != gfs2_inode_dir) { @@ -466,24 +425,19 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, "pointing to something that's not a directory"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); - if(query( _("Clear bad '..' directory entry? (y/n) "))) { + if(!query( _("Clear bad '..' directory entry? (y/n) "))) { + log_err( _("Bad '..' directory entry remains\n")); + goto dentry_is_valid; + } + if (ip->i_di.di_num.no_addr == entryblock) + entry_ip = ip; + else entry_ip = fsck_load_inode(sbp, entryblock); check_inode_eattr(entry_ip, &clear_eattrs); if (entry_ip != ip) fsck_inode_put(&entry_ip); - dirent2_del(ip, bh, prev_de, dent); - bmodified(bh); - return 1; - } else { - log_err( _("Bad '..' directory entry remains\n")); - increment_link(entryblock, - ip->i_di.di_num.no_addr, - _("valid reference")); - (*count)++; - ds->entry_count++; - return 0; - } + goto nuke_dentry; } /* GFS2 does not rely on '..' being in a certain location */ @@ -496,21 +450,16 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, } ds->dotdotdir = 1; - increment_link(entryblock, ip->i_di.di_num.no_addr, - _("valid reference")); - (*count)++; - ds->entry_count++; - return 0; + goto dentry_is_valid; } /* After this point we're only concerned with directories */ if(q != gfs2_inode_dir) { - log_debug( _("Found non-dir inode dentry\n")); - increment_link(entryblock, ip->i_di.di_num.no_addr, - _("valid reference")); - (*count)++; - ds->entry_count++; - return 0; + log_debug( _("Found non-dir inode dentry pointing to %lld " + "(0x%llx)\n"), + (unsigned long long)entryblock, + (unsigned long long)entryblock); + goto dentry_is_valid; } /*log_debug( _("Found plain directory dentry\n"));*/ @@ -519,28 +468,29 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, log_err( _("%s: Hard link to block %" PRIu64" (0x%" PRIx64 ") detected.\n"), tmp_name, entryblock, entryblock); - if(query( _("Clear hard link to directory? (y/n) "))) { - bmodified(bh); - dirent2_del(ip, bh, prev_de, dent); - log_warn( _("Directory entry %s cleared\n"), filename); - - return 1; - } else { + if(query( _("Clear hard link to directory? (y/n) "))) + goto nuke_dentry; + else { log_err( _("Hard link to directory remains\n")); - (*count)++; - ds->entry_count++; - return 0; + goto dentry_is_valid; } } else if (error < 0) { stack; return -1; } +dentry_is_valid: + /* This directory inode links to this inode via this dentry */ increment_link(entryblock, ip->i_di.di_num.no_addr, _("valid reference")); (*count)++; ds->entry_count++; /* End of checks */ return 0; + +nuke_dentry: + dirent2_del(ip, bh, prev_de, dent); + log_err( _("Bad directory entry '%s' cleared.\n"), tmp_name); + return 1; }

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

cluster-commits January 2010