May 2013 - cluster-commits - Fedora mailing-lists

gfs2-utils: master - fsck.gfs2: don't invalidate files with duplicate data block refs
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=5c93311761… Commit: 5c93311761c14819ed20087d23fe23f9a9ab2c21 Parent: df909ed72b1c31663c75a72fd1080b2f10e98a13 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Apr 3 09:28:09 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: don't invalidate files with duplicate data block refs Before this patch, whenever pass1 encountered a duplicated data block pointer, it would mark the file as invalid. But if reason the block was duplicated was due to a different bad inode, the inode with the valid data block reference was still punished and deleted. This patch adds an additional check to see if the previous reference to the data block was as a _valid_ metadata block. If the previous reference was as metadata, and the metadata checked out okay, then it can't possibly be a data block for the second reference. In that case, we know for a fact that the second reference is invalid. But if the previous reference was also as data, the inode might be okay and duplicate resolving in pass1b might sort it out and leave this inode as the only valid reference. In that case, we should treat the inode as valid, not invalid. So this patch basically treats duplicate data block references as "innocent until proven guilty" rather than just the opposite. --- gfs2/fsck/pass1.c | 20 +++++++++++++++++--- 1 files changed, 17 insertions(+), 3 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 3c4dc89..df10089 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -425,18 +425,32 @@ static int check_data(struct gfs2_inode *ip, uint64_t metablock, log_err(_("from metadata block %llu (0x%llx)\n"), (unsigned long long)metablock, (unsigned long long)metablock); - + + if (q >= gfs2_indir_blk && q <= gfs2_jdata) { + log_info(_("The block was processed earlier as valid " + "metadata, so it can't possibly be " + "data.\n")); + /* We still need to add a duplicate record here because + when check_metatree tries to delete the inode, we + can't have the "undo" functions freeing the block + out from other the original referencing inode. */ + add_duplicate_ref(ip, block, ref_as_data, 0, + INODE_VALID); + return 1; + } if (q != gfs2_meta_inval) { log_info( _("Seems to be a normal duplicate; I'll " "sort it out in pass1b.\n")); add_duplicate_ref(ip, block, ref_as_data, 0, INODE_VALID); - return 1; + /* This inode references the block as data. So if this + all is validated, we want to keep this count. */ + return 0; } log_info( _("The block was invalid as metadata but might be " "okay as data. I'll sort it out in pass1b.\n")); add_duplicate_ref(ip, block, ref_as_data, 0, INODE_VALID); - return 1; + return 0; } /* In gfs1, rgrp indirect blocks are marked in the bitmap as "meta". In gfs2, "meta" is only for dinodes. So here we dummy up the

1 0

gfs2-utils: master - fsck.gfs2: standardize check_metatree return codes
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=df909ed72b… Commit: df909ed72b1c31663c75a72fd1080b2f10e98a13 Parent: 88a7654d148bb81e2c8da36d5fc882ba4575e10a Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Apr 3 08:55:00 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: standardize check_metatree return codes This patch aims to not change functionality at all. What it does is adds a standard set of three return codes with the following meanings: meta_is_good - all is well, keep processing metadata normally meta_skip_further - an non-fatal error occurred, so further metadata processing for this inode should be skipped. meta_error - a fatal error occurred in this metadata, so we need to abort processing. --- gfs2/fsck/metawalk.c | 14 +++++++------- gfs2/fsck/metawalk.h | 6 ++++++ gfs2/fsck/pass1.c | 28 ++++++++++++++-------------- gfs2/fsck/pass1b.c | 6 +++--- gfs2/fsck/util.c | 12 ++++++------ 5 files changed, 36 insertions(+), 30 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 135d8fa..dc585ae 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -996,9 +996,9 @@ int free_block_if_notdup(struct gfs2_inode *ip, uint64_t block, { if (!find_remove_dup(ip, block, btype)) { /* not a dup */ fsck_blockmap_set(ip, block, btype, gfs2_block_free); - return 1; + return meta_skip_further; } - return 0; + return meta_is_good; } /** @@ -1015,7 +1015,7 @@ static int delete_block_if_notdup(struct gfs2_inode *ip, uint64_t block, uint8_t q; if (!valid_block(ip->i_sbd, block)) - return -EFAULT; + return meta_error; q = block_type(block); if (q == gfs2_block_free) { @@ -1025,7 +1025,7 @@ static int delete_block_if_notdup(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); - return 0; + return meta_is_good; } return free_block_if_notdup(ip, block, btype); } @@ -1255,12 +1255,12 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, pass->private); /* check_metalist should hold any buffers it gets with "bread". */ - if (err < 0) { + if (err == meta_error) { stack; error = err; return error; } - if (err > 0) { + if (err == meta_skip_further) { if (!error) error = err; log_debug( _("Skipping block %llu (0x%llx)\n"), @@ -1659,7 +1659,7 @@ static int alloc_metalist(struct gfs2_inode *ip, uint64_t block, (unsigned long long)block); gfs2_blockmap_set(bl, block, gfs2_indir_blk); } - return 0; + return meta_is_good; } static int alloc_data(struct gfs2_inode *ip, uint64_t metablock, diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index 2ba0d72..49217cc 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -56,6 +56,12 @@ extern int free_block_if_notdup(struct gfs2_inode *ip, uint64_t block, #define fsck_blockmap_set(ip, b, bt, m) _fsck_blockmap_set(ip, b, bt, m, \ __FUNCTION__, __LINE__) +enum meta_check_rc { + meta_error = -1, + meta_is_good = 0, + meta_skip_further = 1, +}; + /* 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 a6fe9a7..3c4dc89 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -139,14 +139,14 @@ static int resuscitate_metalist(struct gfs2_inode *ip, uint64_t block, "range) found in system 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; + return meta_skip_further; } if (fsck_system_inode(ip->i_sbd, block)) fsck_blockmap_set(ip, block, _("system file"), gfs2_indir_blk); else check_n_fix_bitmap(ip->i_sbd, block, gfs2_indir_blk); bc->indir_count++; - return 0; + return meta_is_good; } /* @@ -263,7 +263,7 @@ static int check_metalist(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); - return 1; + return meta_skip_further; } if (is_dir(&ip->i_di, ip->i_sbd->gfs1) && h == ip->i_di.di_height) { iblk_type = GFS2_METATYPE_JD; @@ -300,7 +300,7 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, gfs2_meta_inval); brelse(nbh); nbh = NULL; - return 1; + return meta_skip_further; } brelse(nbh); nbh = NULL; @@ -314,12 +314,12 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, nbh = NULL; *bh = NULL; } - return 1; /* don't process the metadata again */ + return meta_skip_further; /* don't process the metadata again */ } else fsck_blockmap_set(ip, block, _("indirect"), gfs2_indir_blk); - return 0; + return meta_is_good; } /* undo_reference - undo previously processed data or metadata @@ -825,7 +825,7 @@ static int mark_block_invalid(struct gfs2_inode *ip, uint64_t block, * and as a result, they'll be freed when this dinode is deleted, * despite being used by another dinode as a valid block. */ if (!valid_block(ip->i_sbd, block)) - return 0; + return meta_is_good; q = block_type(block); if (q != gfs2_block_free) { @@ -837,10 +837,10 @@ static int mark_block_invalid(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); - return 0; + return meta_is_good; } fsck_blockmap_set(ip, block, btype, gfs2_meta_inval); - return 0; + return meta_is_good; } static int invalidate_metadata(struct gfs2_inode *ip, uint64_t block, @@ -910,9 +910,9 @@ static int rangecheck_block(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); if ((*bad_pointers) <= BAD_POINTER_TOLERANCE) - return ENOENT; + return meta_skip_further; else - return -ENOENT; /* Exits check_metatree quicker */ + return meta_error; /* Exits check_metatree quicker */ } /* See how many duplicate blocks it has */ q = block_type(block); @@ -925,11 +925,11 @@ static int rangecheck_block(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); if ((*bad_pointers) <= BAD_POINTER_TOLERANCE) - return ENOENT; + return meta_skip_further; else - return -ENOENT; /* Exits check_metatree quicker */ + return meta_error; /* Exits check_metatree quicker */ } - return 0; + return meta_is_good; } static int rangecheck_metadata(struct gfs2_inode *ip, uint64_t block, diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index b2532fd..b5da200 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -215,7 +215,7 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, struct duptree *dt; if (!valid_block(ip->i_sbd, block)) - return 0; + return meta_is_good; /* This gets tricky. We're traversing a metadata tree trying to delete an inode based on it having a duplicate block reference @@ -231,7 +231,7 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, if (!dt) { fsck_blockmap_set(ip, block, _("no longer valid"), gfs2_block_free); - return 0; + return meta_is_good; } /* This block, having failed the above test, is duplicated somewhere */ if (block == dh->dt->block) { @@ -254,7 +254,7 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, be mistakenly freed as "no longer valid" (in this function above) even though it's valid metadata for a different inode. Returning 1 ensures that the metadata isn't processed again. */ - return 1; + return meta_skip_further; } static int clear_dup_data(struct gfs2_inode *ip, uint64_t metablock, diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index c11768f..078d5f6 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -316,19 +316,19 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, struct duptree *dt; if (!valid_block(ip->i_sbd, block)) - return 0; + return meta_is_good; /* If this is not the first reference (i.e. all calls from pass1) we need to create the duplicate reference. If this is pass1b, we want to ignore references that aren't found. */ dt = gfs2_dup_set(block, !first); if (!dt) /* If this isn't a duplicate */ - return 0; + return meta_is_good; /* If we found the duplicate reference but we've already discovered the first reference (in pass1b) and the other references in pass1, we don't need to count it, so just return. */ if (dt->first_ref_found) - return 0; + return meta_is_good; /* The first time this is called from pass1 is actually the second reference. When we go back in pass1b looking for the original @@ -350,12 +350,12 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, if (!(id = malloc(sizeof(*id)))) { log_crit( _("Unable to allocate " "inode_with_dups structure\n")); - return -1; + return meta_error; } if (!(memset(id, 0, sizeof(*id)))) { log_crit( _("Unable to zero inode_with_dups " "structure\n")); - return -1; + return meta_error; } id->block_no = ip->i_di.di_num.no_addr; q = block_type(ip->i_di.di_num.no_addr); @@ -389,7 +389,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, else log_info( _("This brings the total to: %d duplicate " "references\n"), dt->refs); - return 0; + return meta_is_good; } struct dir_info *dirtree_insert(struct gfs2_inum inum)

1 0

gfs2-utils: master - fsck.gfs2: pass2: check leaf blocks when fixing hash table
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=88a7654d14… Commit: 88a7654d148bb81e2c8da36d5fc882ba4575e10a Parent: 5ec85b9f58354868eef23e15ade1be74e751eca0 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Apr 3 07:25:00 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: pass2: check leaf blocks when fixing hash table Before this patch, pass2 would attempt to fix the hash table without first checking the basic integrity of the leaf blocks it was checking. A misplaced leaf might have its entries relocated as a matter of course. But if that leaf block had a problem, it could cause all kinds of errors, including segfaults. This patch gives the hash table repair function the ability to do basic integrity checks on the leaf block, and perform repairs if necessary. --- gfs2/fsck/pass2.c | 100 +++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 79 insertions(+), 21 deletions(-) diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 2336aa5..77718be 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -1050,6 +1050,66 @@ static int lost_leaf(struct gfs2_inode *ip, uint64_t *tbl, uint64_t leafno, return 1; } +static int basic_check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, + struct gfs2_dirent *prev_de, + struct gfs2_buffer_head *bh, char *filename, + uint32_t *count, int lindex, void *priv) +{ + uint8_t q = 0; + char tmp_name[MAX_FILENAME]; + struct gfs2_inum entry; + struct dir_status *ds = (struct dir_status *) priv; + struct gfs2_dirent dentry, *de; + int error; + + memset(&dentry, 0, sizeof(struct gfs2_dirent)); + gfs2_dirent_in(&dentry, (char *)dent); + de = &dentry; + + entry.no_addr = de->de_inum.no_addr; + entry.no_formal_ino = de->de_inum.no_formal_ino; + + /* Start of checks */ + memset(tmp_name, 0, MAX_FILENAME); + if (de->de_name_len < MAX_FILENAME) + strncpy(tmp_name, filename, de->de_name_len); + else + strncpy(tmp_name, filename, MAX_FILENAME - 1); + + error = basic_dentry_checks(ip, dent, &entry, tmp_name, count, de, + ds, &q, bh); + if (error) { + dirent2_del(ip, bh, prev_de, dent); + log_err( _("Bad directory entry '%s' cleared.\n"), tmp_name); + return 1; + } else { + return 0; + } +} + +static int pass2_repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, + int lindex, int ref_count, const char *msg, + void *private) +{ + return repair_leaf(ip, leaf_no, lindex, ref_count, msg); +} + +/* The purpose of leafck_fxns is to provide a means for function fix_hashtable + * to do basic sanity checks on leaf blocks before manipulating them, for + * example, splitting them. If they're corrupt, splitting them or trying to + * move their contents can cause a segfault. We can't really use the standard + * pass2_fxns because that will do things we don't want. For example, it will + * find '.' and '..' and increment the directory link count, which would be + * done a second time when the dirent is really checked in pass2_fxns. + * We don't want it to do the "wrong leaf" thing, or set_parent_dir either. + * We just want a basic sanity check on pointers and lengths. + */ +struct metawalk_fxns leafck_fxns = { + .check_leaf_depth = check_leaf_depth, + .check_dentry = basic_check_dentry, + .repair_leaf = pass2_repair_leaf, +}; + /* fix_hashtable - fix a corrupt hash table * * The main intent of this function is to sort out hash table problems. @@ -1079,10 +1139,11 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize, int len, int *proper_len, int factor) { struct gfs2_buffer_head *lbh; - struct gfs2_leaf *leaf; + struct gfs2_leaf leaf; struct gfs2_dirent dentry, *de; int changes = 0, error, i, extras, hash_index; uint64_t new_leaf_blk; + uint64_t leaf_no; uint32_t leaf_proper_start; *proper_len = len; @@ -1096,14 +1157,20 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize, return 0; } + memset(&leaf, 0, sizeof(leaf)); + leaf_no = leafblk; + error = check_leaf(ip, lindex, &leafck_fxns, &leaf_no, &leaf, &len); + if (error) { + log_debug("Leaf repaired while fixing the hash table.\n"); + error = 0; + } lbh = bread(ip->i_sbd, leafblk); - leaf = (struct gfs2_leaf *)lbh->b_data; /* If the leaf's depth is out of range for this dinode, it's obviously attached to the wrong dinode. Move the dirents to lost+found. */ - if (be16_to_cpu(leaf->lf_depth) > ip->i_di.di_depth) { + if (leaf.lf_depth > ip->i_di.di_depth) { log_err(_("This leaf block's depth (%d) is too big for this " "dinode's depth (%d)\n"), - be16_to_cpu(leaf->lf_depth), ip->i_di.di_depth); + leaf.lf_depth, ip->i_di.di_depth); error = lost_leaf(ip, tbl, leafblk, len, lindex, lbh); brelse(lbh); return error; @@ -1129,7 +1196,7 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize, } /* Calculate the proper number of pointers based on the leaf depth. */ - *proper_len = 1 << (ip->i_di.di_depth - be16_to_cpu(leaf->lf_depth)); + *proper_len = 1 << (ip->i_di.di_depth - leaf.lf_depth); /* Look at the first dirent and check its hash value to see if it's at the proper starting offset. */ @@ -1162,7 +1229,7 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize, already at its maximum depth. */ if ((leaf_proper_start < proper_start) || ((*proper_len > len || lindex > leaf_proper_start) && - be16_to_cpu(leaf->lf_depth) == ip->i_di.di_depth)) { + leaf.lf_depth == ip->i_di.di_depth)) { log_err(_("Leaf block should start at 0x%x, but it appears at " "0x%x in the hash table.\n"), leaf_proper_start, proper_start); @@ -1177,24 +1244,22 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize, later than they should, we can split the leaf to give it a smaller footprint in the hash table. */ if ((*proper_len > len || lindex > leaf_proper_start) && - ip->i_di.di_depth > be16_to_cpu(leaf->lf_depth)) { + ip->i_di.di_depth > leaf.lf_depth) { log_err(_("For depth %d, length %d, the proper start is: " "0x%x.\n"), factor, len, proper_start); changes++; new_leaf_blk = find_free_blk(ip->i_sbd); dir_split_leaf(ip, lindex, leafblk, lbh); /* re-read the leaf to pick up dir_split_leaf's changes */ - gfs2_leaf_in(leaf, lbh); - *proper_len = 1 << (ip->i_di.di_depth - - be16_to_cpu(leaf->lf_depth)); + gfs2_leaf_in(&leaf, lbh); + *proper_len = 1 << (ip->i_di.di_depth - leaf.lf_depth); log_err(_("Leaf block %llu (0x%llx) was split from length " "%d to %d\n"), (unsigned long long)leafblk, (unsigned long long)leafblk, len, *proper_len); if (*proper_len < 0) { log_err(_("Programming error: proper_len=%d, " "di_depth = %d, lf_depth = %d.\n"), - *proper_len, ip->i_di.di_depth, - be16_to_cpu(leaf->lf_depth)); + *proper_len, ip->i_di.di_depth, leaf.lf_depth); exit(FSCK_ERROR); } log_err(_("New split-off leaf block was allocated at %lld " @@ -1219,8 +1284,8 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize, if (*proper_len < len) { log_err(_("There are %d pointers, but leaf 0x%llx's " "depth, %d, only allows %d\n"), - len, (unsigned long long)leafblk, - be16_to_cpu(leaf->lf_depth), *proper_len); + len, (unsigned long long)leafblk, leaf.lf_depth, + *proper_len); } brelse(lbh); /* At this point, lindex should be at the proper end of the pointers. @@ -1422,13 +1487,6 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, return error; } -static int pass2_repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, - int lindex, int ref_count, const char *msg, - void *private) -{ - return repair_leaf(ip, leaf_no, lindex, ref_count, msg); -} - struct metawalk_fxns pass2_fxns = { .private = NULL, .check_leaf_depth = check_leaf_depth,

1 0

gfs2-utils: master - fsck.gfs2: externalize check_leaf
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=5ec85b9f58… Commit: 5ec85b9f58354868eef23e15ade1be74e751eca0 Parent: 5befaedc6b7f8cea80989994ec1934953f4db404 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Apr 3 07:13:52 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: externalize check_leaf This patch makes metawalk function check_leaf external so that it may be called in a future patch for fixing hash tables. --- gfs2/fsck/metawalk.c | 5 ++--- gfs2/fsck/metawalk.h | 3 +++ gfs2/fsck/pass1.c | 6 +++--- gfs2/fsck/pass1b.c | 6 +++--- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 3baa60b..135d8fa 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -483,9 +483,8 @@ static int check_entries(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, * Reads in the leaf block * Leaves the buffer around for further analysis (caller must brelse) */ -static int check_leaf(struct gfs2_inode *ip, int lindex, - struct metawalk_fxns *pass, - uint64_t *leaf_no, struct gfs2_leaf *leaf, int *ref_count) +int check_leaf(struct gfs2_inode *ip, int lindex, struct metawalk_fxns *pass, + uint64_t *leaf_no, struct gfs2_leaf *leaf, int *ref_count) { int error = 0, fix; struct gfs2_buffer_head *lbh = NULL; diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index 05b0e7a..2ba0d72 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -15,6 +15,9 @@ extern int check_dir(struct gfs2_sbd *sdp, uint64_t block, struct metawalk_fxns *pass); extern int check_linear_dir(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, struct metawalk_fxns *pass); +extern int check_leaf(struct gfs2_inode *ip, int lindex, + struct metawalk_fxns *pass, uint64_t *leaf_no, + struct gfs2_leaf *leaf, int *ref_count); extern int remove_dentry_from_dir(struct gfs2_sbd *sdp, uint64_t dir, uint64_t dentryblock); extern int delete_block(struct gfs2_inode *ip, uint64_t block, diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 004ca78..a6fe9a7 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -35,7 +35,7 @@ struct block_count { uint64_t ea_count; }; -static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private); +static int p1check_leaf(struct gfs2_inode *ip, uint64_t block, void *private); static int check_metalist(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, void *private); static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block, @@ -93,7 +93,7 @@ static int pass1_repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, struct metawalk_fxns pass1_fxns = { .private = NULL, - .check_leaf = check_leaf, + .check_leaf = p1check_leaf, .check_metalist = check_metalist, .check_data = check_data, .check_eattr_indir = check_eattr_indir, @@ -211,7 +211,7 @@ struct metawalk_fxns sysdir_fxns = { .check_dentry = resuscitate_dentry, }; -static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private) +static int p1check_leaf(struct gfs2_inode *ip, uint64_t block, void *private) { struct block_count *bc = (struct block_count *) private; uint8_t q; diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 6114ba3..b2532fd 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -28,7 +28,7 @@ struct dup_handler { int ref_count; }; -static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private); +static int check_leaf_refs(struct gfs2_inode *ip, uint64_t block, void *private); static int check_metalist(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, void *private); static int check_data(struct gfs2_inode *ip, uint64_t metablock, @@ -56,7 +56,7 @@ static int find_dentry(struct gfs2_inode *ip, struct gfs2_dirent *de, struct metawalk_fxns find_refs = { .private = NULL, - .check_leaf = check_leaf, + .check_leaf = check_leaf_refs, .check_metalist = check_metalist, .check_data = check_data, .check_eattr_indir = check_eattr_indir, @@ -78,7 +78,7 @@ struct metawalk_fxns find_dirents = { .check_eattr_extentry = NULL, }; -static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private) +static int check_leaf_refs(struct gfs2_inode *ip, uint64_t block, void *private) { return add_duplicate_ref(ip, block, ref_as_meta, 1, INODE_VALID); }

1 0

gfs2-utils: master - fsck.gfs2: Don't flag GFS1 non-dinode blocks as duplicates
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=5befaedc6b… Commit: 5befaedc6b7f8cea80989994ec1934953f4db404 Parent: bbb17b6bc32e8006afbf199f98555283c453ce05 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue Apr 2 13:19:35 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: Don't flag GFS1 non-dinode blocks as duplicates Before this patch, fsck.gfs2 could get into problems when processing a GFS1 file system. The issue goes back to the fact that all GFS1 metadata is marked as "Meta" in the bitmap, whereas that bitmap designation is reserved for dinodes in GFS2. For example, take a GFS1 file of height 2, which looks like this: Block ------ 0x1234 dinode 0x1235 |----> indirect meta 0x1236 |---->data at offset 0 of the file Before this patch, fsck.gfs2 would: 1. Encounter the dinode at 0x1234 and mark it as "dinode" in the blockmap. 2. Process its metadata, see block 0x1235, mark it as "indirect meta" in the blockmap. 3. Process the metadata's data, see block 0x1236, mark it as "data". 4. When it's done with the dinode, it moves on to the next dinode. But since GFS1 doesn't distinguish dinodes from other metadata, the next block in the bitmap that has that designation is block 0x1235. 5. Since block 0x1235 was previously marked "indirect meta" pass1 gets confused and thinks the block is a duplicate reference, and it's invalid as a dinode. This is a non-problem that's treated as a problem, and it makes bad decisions based on it, deleting what it perceives to be corruption. This patch adds special checks for this problem and assumes the block is just normal GFS1 non-dinode metadata. --- gfs2/fsck/pass1.c | 69 ++++++++++++++++++++++++++++++++++------------------ 1 files changed, 45 insertions(+), 24 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 0f3adfe..004ca78 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -1084,22 +1084,11 @@ bad_dinode: */ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh) { - uint8_t q; int error = 0; uint64_t block = bh->b_blocknr; struct gfs2_inode *ip; ip = fsck_inode_get(sdp, bh); - q = block_type(block); - if (q != gfs2_block_free) { - log_err( _("Found a duplicate inode block at #%llu" - " (0x%llx) previously marked as a %s\n"), - (unsigned long long)block, - (unsigned long long)block, block_type_string(q)); - add_duplicate_ref(ip, block, ref_as_meta, 0, INODE_VALID); - fsck_inode_put(&ip); - return 0; - } if (ip->i_di.di_num.no_addr != block) { log_err( _("Inode #%llu (0x%llx): Bad inode address found: %llu " @@ -1359,8 +1348,13 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin struct gfs2_buffer_head *bh; unsigned i; uint64_t block; + struct gfs2_inode *ip; + uint8_t q; for (i = 0; i < n; i++) { + int is_inode; + uint32_t check_magic; + block = ibuf[i]; /* skip gfs1 rindex indirect blocks */ @@ -1389,9 +1383,47 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin (unsigned long long)block); continue; } + bh = bread(sdp, block); - if (gfs2_check_meta(bh, GFS2_METATYPE_DI)) { + is_inode = 0; + if (gfs2_check_meta(bh, GFS2_METATYPE_DI) == 0) + is_inode = 1; + + check_magic = ((struct gfs2_meta_header *) + (bh->b_data))->mh_magic; + + q = block_type(block); + if (q != gfs2_block_free) { + if (be32_to_cpu(check_magic) == GFS2_MAGIC && + sdp->gfs1 && !is_inode) { + log_debug("Block 0x%llx assumed to be " + "previously processed GFS1 " + "non-dinode metadata.\n", + (unsigned long long)block); + brelse(bh); + continue; + } + log_err( _("Found a duplicate inode block at #%llu " + "(0x%llx) previously marked as a %s\n"), + (unsigned long long)block, + (unsigned long long)block, + block_type_string(q)); + ip = fsck_inode_get(sdp, bh); + if (is_inode && ip->i_di.di_num.no_addr == block) + add_duplicate_ref(ip, block, ref_is_inode, 0, + INODE_VALID); + else + log_info(_("dinum.no_addr is wrong, so I " + "assume the bitmap is just " + "wrong.\n")); + fsck_inode_put(&ip); + brelse(bh); + continue; + } + + if (!is_inode) { + if (be32_to_cpu(check_magic) == GFS2_MAGIC) { /* In gfs2, a bitmap mark of 2 means an inode, but in gfs1 it means any metadata. So if this is gfs1 and not an inode, it may be @@ -1399,12 +1431,7 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin be referenced by an inode, so we need to skip it here and it will be sorted out when the referencing inode is checked. */ - if (sdp->gfs1) { - uint32_t check_magic; - - check_magic = ((struct gfs2_meta_header *) - (bh->b_data))->mh_magic; - if (be32_to_cpu(check_magic) == GFS2_MAGIC) { + if (sdp->gfs1) { log_debug( _("Deferring GFS1 " "metadata block #" "%" PRIu64" (0x%" @@ -1418,12 +1445,6 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin "%llu (0x%llx)\n"), (unsigned long long)block, (unsigned long long)block); - if (gfs2_blockmap_set(bl, block, gfs2_block_free)) { - stack; - brelse(bh); - gfs2_special_free(&gfs1_rindex_blks); - return FSCK_ERROR; - } check_n_fix_bitmap(sdp, block, gfs2_block_free); } else if (handle_di(sdp, bh) < 0) { stack;

1 0

gfs2-utils: master - fsck.gfs2: don't remove buffers from the list when errors are found
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=bbb17b6bc3… Commit: bbb17b6bc32e8006afbf199f98555283c453ce05 Parent: 03a5a25fd54d33b211ffad9af8f6b5e2490ddb74 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue Apr 2 13:03:15 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: don't remove buffers from the list when errors are found Before this patch, if an error was encountered while marking the data blocks, the blocks would be removed from the linked list. Now that we've got "undo" functions, we need to be able to undo the designations of those blocks, which means we need to keep those buffers on the linked list so they're found later. If we don't, the undo data block function won't process them, and therefore they'll be marked as "data" blocks in the bitmap, but no files will reference the blocks (because the error causes the inode to be deleted). With this patch, the metadata that points to the faulty data is kept on the linked list, and after the error is found, the undo function will therefore find it and mark its blocks as "free". --- gfs2/fsck/metawalk.c | 15 ++++----------- 1 files changed, 4 insertions(+), 11 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 8d97c17..3baa60b 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1383,7 +1383,7 @@ static int hdr_size(struct gfs2_buffer_head *bh, int height) int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) { osi_list_t metalist[GFS2_MAX_META_HEIGHT]; - osi_list_t *list; + osi_list_t *list, *tmp; struct gfs2_buffer_head *bh; uint32_t height = ip->i_di.di_height; int i, head_size; @@ -1423,22 +1423,15 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) if (ip->i_di.di_blocks > COMFORTABLE_BLKS) last_reported_fblock = -10000000; - while (!error && !osi_list_empty(list)) { + for (tmp = list->next; !error && tmp != list; tmp = tmp->next) { if (fsck_abort) { free_metalist(ip, &metalist[0]); return 0; } - bh = osi_list_entry(list->next, struct gfs2_buffer_head, - b_altlist); - + bh = osi_list_entry(tmp, struct gfs2_buffer_head, b_altlist); head_size = hdr_size(bh, height); - if (!head_size) { - if (bh == ip->i_bh) - osi_list_del(&bh->b_altlist); - else - brelse(bh); + if (!head_size) continue; - } if (pass->check_data) error = check_data(ip, pass, bh, head_size,

1 0

gfs2-utils: master - fsck.gfs2: Log when unrecoverable data block errors are encountered
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=03a5a25fd5… Commit: 03a5a25fd54d33b211ffad9af8f6b5e2490ddb74 Parent: 5e7c67118f2599c0016adc7d900b3832b5dfbb21 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue Apr 2 12:59:16 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: Log when unrecoverable data block errors are encountered This patch adds a log message whenever unrecoverable data block errors are found. Otherwise the output doesn't say why it stopped processing data, and which block had the problem. --- gfs2/fsck/metawalk.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 594d4e0..8d97c17 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1322,10 +1322,15 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, pass1. Therefore the individual check_data functions should do a range check. */ rc = pass->check_data(ip, metablock, block, pass->private); + if (!error && rc) { + error = rc; + log_info(_("\nUnrecoverable data block error %d on " + "block %llu (0x%llx).\n"), rc, + (unsigned long long)block, + (unsigned long long)block); + } if (rc < 0) return rc; - if (!error && rc) - error = rc; (*blks_checked)++; } return error;

1 0

gfs2-utils: master - fsck.gfs2: Do not invalidate metablocks of dinodes with invalid mode
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=5e7c67118f… Commit: 5e7c67118f2599c0016adc7d900b3832b5dfbb21 Parent: 8f388da439c05aa4876fbafe9b4a69634d78896d Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue Apr 2 12:24:15 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: Do not invalidate metablocks of dinodes with invalid mode Before this patch, when fsck.gfs2 encountered a dinode with an invalid mode, it would take steps to invalidate its metadata. That's wrong because if the mode is invalid, you don't know how to treat it. It's especially wrong if its metadata references the same blocks that other valid dinodes reference, because then we could end up deleting blocks belonging to valid files and directories. --- gfs2/fsck/pass1.c | 25 ++++++++----------------- 1 files changed, 8 insertions(+), 17 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 964e60b..0f3adfe 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -1005,23 +1005,14 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip) error = set_ip_blockmap(ip, 1); if (error == -EINVAL) { - /* We found a dinode that has an invalid mode, so we can't - tell if it's a data file, directory or a socket. - Regardless, we have to invalidate its metadata in case there - are duplicate blocks referenced. If we don't call - check_metatree, the blocks it references will be deleted - wholesale by pass2, and if any of those blocks are - duplicates--referenced by another dinode for some reason-- - we will mark it free, even though it's in use. In other - words, we would introduce file system corruption. So we - need to keep track of the fact that it's invalid and - skip parts that we can't be sure of based on dinode type. */ - log_debug("Invalid mode dinode found at block %lld (0x%llx): " - "Invalidating all its metadata.\n", - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); - check_metatree(ip, &invalidate_fxns); - check_inode_eattr(ip, &invalidate_fxns); + /* We found a dinode that has an invalid mode. At this point + set_ip_blockmap returned an error, which means it never + got inserted into the inode tree. Since we haven't even + processed its metadata with pass1_fxns, none of its + metadata will be flagged as metadata or data blocks yet. + Therefore, we don't need to invalidate anything. */ + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("invalid mode"), gfs2_block_free); return 0; } else if (error) goto bad_dinode;

1 0

gfs2-utils: master - fsck.gfs2: print block count values when fixing them
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=8f388da439… Commit: 8f388da439c05aa4876fbafe9b4a69634d78896d Parent: 273d844cac9ff64a6f6ee7fd7130d9a0e775f911 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue Apr 2 11:50:31 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: print block count values when fixing them Before this patch, block counts were fixed, but it didn't log what the new value was changed to. That made it very difficult to track down block count problems. This patch changes the logging so that it prints the new block count, and a breakdown of how many blocks were counted for metadata, data, extended attributes, etc. --- gfs2/fsck/pass1.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 3a47184..964e60b 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -613,7 +613,11 @@ static int finish_eattr_indir(struct gfs2_inode *ip, int leaf_pointers, ip->i_di.di_blocks = 1 + bc->indir_count + bc->data_count + bc->ea_count; bmodified(ip->i_bh); - log_err( _("Block count fixed.\n")); + log_err(_("Block count fixed: 1+%lld+%lld+%lld = %lld.\n"), + (unsigned long long)bc->indir_count, + (unsigned long long)bc->data_count, + (unsigned long long)bc->ea_count, + (unsigned long long)ip->i_di.di_blocks); return 1; } log_err( _("Block count not fixed.\n"));

1 0

gfs2-utils: master - fsck.gfs2: print metadata block reference on data errors
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=273d844cac… Commit: 273d844cac9ff64a6f6ee7fd7130d9a0e775f911 Parent: a9af1133710ab33e992264b8889645fb88b1aeaf Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue Apr 2 11:45:09 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: print metadata block reference on data errors Before this patch, fsck.gfs2 would cite data block errors, but it wouldn't tell you which metadata block referenced the bad data block. That's fine, but it makes it very difficult to backtrack problems. This patch prints out which metadata block referenced the bad data so it may be backtracked easier. --- gfs2/fsck/metawalk.c | 17 ++++++++++------- gfs2/fsck/metawalk.h | 7 ++++--- gfs2/fsck/pass1.c | 37 ++++++++++++++++++++++++++----------- gfs2/fsck/pass1b.c | 9 ++++++--- 4 files changed, 46 insertions(+), 24 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 7fe5a88..594d4e0 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1299,11 +1299,14 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, * 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, + struct gfs2_buffer_head *bh, int head_size, uint64_t *blks_checked) { int error = 0, rc = 0; uint64_t block, *ptr; + uint64_t *ptr_start = (uint64_t *)(bh->b_data + head_size); + char *ptr_end = (bh->b_data + ip->i_sbd->bsize); + uint64_t metablock = bh->b_blocknr; /* If there isn't much pointer corruption check the pointers */ for (ptr = ptr_start ; (char *)ptr < ptr_end && !fsck_abort; ptr++) { @@ -1318,7 +1321,7 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, 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); + rc = pass->check_data(ip, metablock, block, pass->private); if (rc < 0) return rc; if (!error && rc) @@ -1433,9 +1436,7 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) } if (pass->check_data) - error = check_data(ip, pass, (uint64_t *) - (bh->b_data + head_size), - (bh->b_data + ip->i_sbd->bsize), + error = check_data(ip, pass, bh, head_size, &blks_checked); if (pass->big_file_msg && ip->i_di.di_blocks > COMFORTABLE_BLKS) pass->big_file_msg(ip, blks_checked); @@ -1602,7 +1603,8 @@ int delete_leaf(struct gfs2_inode *ip, uint64_t block, void *private) return delete_block_if_notdup(ip, block, NULL, _("leaf"), private); } -int delete_data(struct gfs2_inode *ip, uint64_t block, void *private) +int delete_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private) { return delete_block_if_notdup(ip, block, NULL, _("data"), private); } @@ -1663,7 +1665,8 @@ static int alloc_metalist(struct gfs2_inode *ip, uint64_t block, return 0; } -static int alloc_data(struct gfs2_inode *ip, uint64_t block, void *private) +static int alloc_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private) { uint8_t q; const char *desc = (const char *)private; diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index f5e71e1..05b0e7a 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -23,7 +23,8 @@ extern int delete_block(struct gfs2_inode *ip, uint64_t block, extern int delete_metadata(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, void *private); extern int delete_leaf(struct gfs2_inode *ip, uint64_t block, void *private); -extern int delete_data(struct gfs2_inode *ip, uint64_t block, void *private); +extern int delete_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private); extern int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private); extern int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent, @@ -76,8 +77,8 @@ struct metawalk_fxns { int (*check_metalist) (struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, void *private); - int (*check_data) (struct gfs2_inode *ip, uint64_t block, - void *private); + int (*check_data) (struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private); int (*check_eattr_indir) (struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private); diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 27bb5d4..3a47184 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -40,7 +40,8 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, void *private); static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block, int h, void *private); -static int check_data(struct gfs2_inode *ip, uint64_t block, void *private); +static int check_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private); static int undo_check_data(struct gfs2_inode *ip, uint64_t block, void *private); static int check_eattr_indir(struct gfs2_inode *ip, uint64_t indirect, @@ -66,8 +67,8 @@ static int invalidate_metadata(struct gfs2_inode *ip, uint64_t block, void *private); static int invalidate_leaf(struct gfs2_inode *ip, uint64_t block, void *private); -static int invalidate_data(struct gfs2_inode *ip, uint64_t block, - void *private); +static int invalidate_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private); static int invalidate_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, @@ -382,17 +383,24 @@ static int undo_check_data(struct gfs2_inode *ip, uint64_t block, return undo_reference(ip, block, 0, private); } -static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) +static int check_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private) { uint8_t q; struct block_count *bc = (struct block_count *) private; if (!valid_block(ip->i_sbd, block)) { log_err( _("inode %lld (0x%llx) has a bad data block pointer " - "%lld (invalid or out of range)\n"), + "%lld (0x%llx) (invalid or out of range) "), (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, (unsigned long long)block); + if (metablock == ip->i_di.di_num.no_addr) + log_err("\n"); + else + log_err(_("from metadata block %llu (0x%llx)\n"), + (unsigned long long)metablock, + (unsigned long long)metablock); /* Mark the owner of this block with the bad_block * designator so we know to check it for out of range * blocks later */ @@ -405,12 +413,19 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) q = block_type(block); if (q != gfs2_block_free) { log_err( _("Found duplicate %s block %llu (0x%llx) " - "referenced as data by dinode %llu (0x%llx)\n"), + "referenced as data by dinode %llu (0x%llx) "), block_type_string(q), (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); + if (metablock == ip->i_di.di_num.no_addr) + log_err("\n"); + else + log_err(_("from metadata block %llu (0x%llx)\n"), + (unsigned long long)metablock, + (unsigned long long)metablock); + if (q != gfs2_meta_inval) { log_info( _("Seems to be a normal duplicate; I'll " "sort it out in pass1b.\n")); @@ -837,8 +852,8 @@ static int invalidate_leaf(struct gfs2_inode *ip, uint64_t block, return mark_block_invalid(ip, block, ref_as_meta, _("leaf")); } -static int invalidate_data(struct gfs2_inode *ip, uint64_t block, - void *private) +static int invalidate_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private) { return mark_block_invalid(ip, block, ref_as_data, _("data")); } @@ -926,8 +941,8 @@ static int rangecheck_leaf(struct gfs2_inode *ip, uint64_t block, return rangecheck_block(ip, block, NULL, btype_leaf, private); } -static int rangecheck_data(struct gfs2_inode *ip, uint64_t block, - void *private) +static int rangecheck_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private) { return rangecheck_block(ip, block, NULL, btype_data, private); } diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 54c8649..6114ba3 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -31,7 +31,8 @@ struct dup_handler { static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private); static int check_metalist(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, void *private); -static int check_data(struct gfs2_inode *ip, uint64_t block, void *private); +static int check_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private); static int check_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private); @@ -88,7 +89,8 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, return add_duplicate_ref(ip, block, ref_as_meta, 1, INODE_VALID); } -static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) +static int check_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private) { return add_duplicate_ref(ip, block, ref_as_data, 1, INODE_VALID); } @@ -255,7 +257,8 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, return 1; } -static int clear_dup_data(struct gfs2_inode *ip, uint64_t block, void *private) +static int clear_dup_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private) { return clear_dup_metalist(ip, block, NULL, 0, private); }

1 0

gfs2-utils: master - fsck.gfs2: Keep proper counts when duplicates are found
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=a9af113371… Commit: a9af1133710ab33e992264b8889645fb88b1aeaf Parent: 107a5e95546c8fd975f3137fd4d324b77f921229 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue Apr 2 11:14:55 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: Keep proper counts when duplicates are found When fsck.gfs2 discovered a duplicate reference to the same block, it was not properly incrementing the block counters for data and metadata. Therefore, when the duplicate situation is resolved, the resulting dinode is likely to have the wrong block count. This patch makes it increment the counters, regardless of whether the block is a duplicate reference. --- gfs2/fsck/pass1.c | 9 ++------- 1 files changed, 2 insertions(+), 7 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index a88895f..27bb5d4 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -218,6 +218,7 @@ static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private) /* Note if we've gotten this far, the block has already passed the check in metawalk: gfs2_check_meta(lbh, GFS2_METATYPE_LF). So we know it's a leaf block. */ + bc->indir_count++; q = block_type(block); if (q != gfs2_block_free) { log_err( _("Found duplicate block #%llu (0x%llx) referenced " @@ -235,7 +236,6 @@ static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private) return -EEXIST; } fsck_blockmap_set(ip, block, _("directory leaf"), gfs2_leaf_blk); - bc->indir_count++; return 0; } @@ -401,6 +401,7 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) gfs2_bad_block); return 1; } + bc->data_count++; /* keep the count sane anyway */ q = block_type(block); if (q != gfs2_block_free) { log_err( _("Found duplicate %s block %llu (0x%llx) " @@ -415,16 +416,11 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) "sort it out in pass1b.\n")); add_duplicate_ref(ip, block, ref_as_data, 0, INODE_VALID); - /* If the prev ref was as data, this is likely a data - block, so keep the block count for both refs. */ - if (q == gfs2_block_used) - bc->data_count++; return 1; } log_info( _("The block was invalid as metadata but might be " "okay as data. I'll sort it out in pass1b.\n")); add_duplicate_ref(ip, block, ref_as_data, 0, INODE_VALID); - bc->data_count++; return 1; } /* In gfs1, rgrp indirect blocks are marked in the bitmap as "meta". @@ -443,7 +439,6 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) fsck_blockmap_set(ip, block, _("jdata"), gfs2_jdata); } else fsck_blockmap_set(ip, block, _("data"), gfs2_block_used); - bc->data_count++; return 0; }

1 0

gfs2-utils: master - fsck.gfs2: Consistent naming of struct duptree variables
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=107a5e9554… Commit: 107a5e95546c8fd975f3137fd4d324b77f921229 Parent: 9c52a36321f730c9859fb006326936530fe4da8f Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue Apr 2 10:39:26 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: Consistent naming of struct duptree variables There were several places in the fsck.gfs2 code that referenced variables of type struct duptree, but sometimes they were called dt, d, b or even data. This patch achieves a level of consistency and calls them all dt. This helps readability: when you see a variable dt, you know it's a struct duptree. --- gfs2/fsck/fsck.h | 2 +- gfs2/fsck/metawalk.c | 24 +++++----- gfs2/fsck/pass1b.c | 113 +++++++++++++++++++++++++------------------------- gfs2/fsck/util.c | 42 +++++++++--------- 4 files changed, 91 insertions(+), 90 deletions(-) diff --git a/gfs2/fsck/fsck.h b/gfs2/fsck/fsck.h index 5313bb3..b21a670 100644 --- a/gfs2/fsck/fsck.h +++ b/gfs2/fsck/fsck.h @@ -117,7 +117,7 @@ extern int fsck_query(const char *format, ...) __attribute__((format(printf,1,2))); extern struct dir_info *dirtree_find(uint64_t block); extern void dup_listent_delete(struct inode_with_dups *id); -extern void dup_delete(struct duptree *b); +extern void dup_delete(struct duptree *dt); extern void dirtree_delete(struct dir_info *b); /* FIXME: Hack to get this going for pass2 - this should be pulled out diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 9e43556..7fe5a88 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -179,14 +179,14 @@ struct duptree *dupfind(uint64_t block) struct osi_node *node = dup_blocks.osi_node; while (node) { - struct duptree *data = (struct duptree *)node; + struct duptree *dt = (struct duptree *)node; - if (block < data->block) + if (block < dt->block) node = node->osi_left; - else if (block > data->block) + else if (block > dt->block) node = node->osi_right; else - return data; + return dt; } return NULL; } @@ -955,15 +955,15 @@ int delete_block(struct gfs2_inode *ip, uint64_t block, */ int find_remove_dup(struct gfs2_inode *ip, uint64_t block, const char *btype) { - struct duptree *d; + struct duptree *dt; struct inode_with_dups *id; - d = dupfind(block); - if (!d) + dt = dupfind(block); + if (!dt) return 0; /* remove the inode reference id structure for this reference. */ - id = find_dup_ref_inode(d, ip); + id = find_dup_ref_inode(dt, ip); if (!id) return 0; @@ -973,14 +973,14 @@ int find_remove_dup(struct gfs2_inode *ip, uint64_t block, const char *btype) (unsigned long long)block, (unsigned long long)block, btype, (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); - d->refs--; /* one less reference */ - if (d->refs == 1) { + dt->refs--; /* one less reference */ + if (dt->refs == 1) { log_info( _("This leaves only one reference: it's " "no longer a duplicate.\n")); - dup_delete(d); /* not duplicate now */ + dup_delete(dt); /* not duplicate now */ } else log_info( _("%d block reference(s) remain.\n"), - d->refs); + dt->refs); return 1; /* but the original ref still exists so do not free it. */ } diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 7108bb4..54c8649 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -22,7 +22,7 @@ struct fxn_info { }; struct dup_handler { - struct duptree *b; + struct duptree *dt; struct inode_with_dups *id; int ref_inode_count; int ref_count; @@ -179,21 +179,21 @@ static int find_dentry(struct gfs2_inode *ip, struct gfs2_dirent *de, { struct osi_node *n, *next = NULL; osi_list_t *tmp2; - struct duptree *b; + struct duptree *dt; int found; for (n = osi_first(&dup_blocks); n; n = next) { next = osi_next(n); - b = (struct duptree *)n; + dt = (struct duptree *)n; found = 0; - osi_list_foreach(tmp2, &b->ref_invinode_list) { + osi_list_foreach(tmp2, &dt->ref_invinode_list) { if (check_dir_dup_ref(ip, de, tmp2, filename)) { found = 1; break; } } if (!found) { - osi_list_foreach(tmp2, &b->ref_inode_list) { + osi_list_foreach(tmp2, &dt->ref_inode_list) { if (check_dir_dup_ref(ip, de, tmp2, filename)) break; } @@ -210,7 +210,7 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, void *private) { struct dup_handler *dh = (struct dup_handler *) private; - struct duptree *d; + struct duptree *dt; if (!valid_block(ip->i_sbd, block)) return 0; @@ -225,14 +225,14 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, to delete it altogether. If the block is a duplicate referenced block, we need to keep its type intact and let the caller sort it out once we're down to a single reference. */ - d = dupfind(block); - if (!d) { + dt = dupfind(block); + if (!dt) { fsck_blockmap_set(ip, block, _("no longer valid"), gfs2_block_free); return 0; } /* This block, having failed the above test, is duplicated somewhere */ - if (block == dh->b->block) { + if (block == dh->dt->block) { log_err( _("Not clearing duplicate reference in inode \"%s\" " "at block #%llu (0x%llx) to block #%llu (0x%llx) " "because it's valid for another inode.\n"), @@ -400,7 +400,7 @@ static enum dup_ref_type get_ref_type(struct inode_with_dups *id) return ref_types; } -static void log_inode_reference(struct duptree *b, osi_list_t *tmp, int inval) +static void log_inode_reference(struct duptree *dt, osi_list_t *tmp, int inval) { char reftypestring[32]; struct inode_with_dups *id; @@ -420,8 +420,8 @@ static void log_inode_reference(struct duptree *b, osi_list_t *tmp, int inval) "block %llu (0x%llx) (%s)\n"), id->name, (unsigned long long)id->block_no, (unsigned long long)id->block_no, id->dup_count, - (unsigned long long)b->block, - (unsigned long long)b->block, reftypestring); + (unsigned long long)dt->block, + (unsigned long long)dt->block, reftypestring); } /* * resolve_dup_references - resolve all but the last dinode that has a @@ -436,7 +436,7 @@ static void log_inode_reference(struct duptree *b, osi_list_t *tmp, int inval) * acceptable_ref - Delete dinodes that reference the given block as anything * _but_ this type. Try to save references as this type. */ -static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *b, +static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt, osi_list_t *ref_list, struct dup_handler *dh, int inval, int acceptable_ref) { @@ -463,7 +463,7 @@ static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *b, return FSCK_OK; id = osi_list_entry(tmp, struct inode_with_dups, list); - dh->b = b; + dh->dt = dt; dh->id = id; if (dh->ref_inode_count == 1) /* down to the last reference */ @@ -494,8 +494,8 @@ static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *b, id->name, (unsigned long long)id->block_no, (unsigned long long)id->block_no, - (unsigned long long)b->block, - (unsigned long long)b->block, + (unsigned long long)dt->block, + (unsigned long long)dt->block, reftypes[this_ref]); continue; /* don't delete the dinode */ } @@ -513,8 +513,8 @@ static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *b, "really %s.\n"), id->name, (unsigned long long)id->block_no, (unsigned long long)id->block_no, - (unsigned long long)b->block, - (unsigned long long)b->block, + (unsigned long long)dt->block, + (unsigned long long)dt->block, reftypes[this_ref], reftypes[acceptable_ref]); if (!(query( _("Okay to delete %s inode %lld (0x%llx)? " "(y/n) "), @@ -564,7 +564,7 @@ static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *b, return 0; } -static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b) +static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *dt) { struct gfs2_inode *ip; osi_list_t *tmp; @@ -576,12 +576,12 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b) enum dup_ref_type acceptable_ref; /* Count the duplicate references, both valid and invalid */ - osi_list_foreach(tmp, &b->ref_invinode_list) { + osi_list_foreach(tmp, &dt->ref_invinode_list) { id = osi_list_entry(tmp, struct inode_with_dups, list); dh.ref_inode_count++; dh.ref_count += id->dup_count; } - osi_list_foreach(tmp, &b->ref_inode_list) { + osi_list_foreach(tmp, &dt->ref_inode_list) { id = osi_list_entry(tmp, struct inode_with_dups, list); dh.ref_inode_count++; dh.ref_count += id->dup_count; @@ -590,13 +590,14 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b) /* Log the duplicate references */ log_notice( _("Block %llu (0x%llx) has %d inodes referencing it" " for a total of %d duplicate references:\n"), - (unsigned long long)b->block, (unsigned long long)b->block, - dh.ref_inode_count, dh.ref_count); + (unsigned long long)dt->block, + (unsigned long long)dt->block, + dh.ref_inode_count, dh.ref_count); - osi_list_foreach(tmp, &b->ref_invinode_list) - log_inode_reference(b, tmp, 1); - osi_list_foreach(tmp, &b->ref_inode_list) - log_inode_reference(b, tmp, 0); + osi_list_foreach(tmp, &dt->ref_invinode_list) + log_inode_reference(dt, tmp, 1); + osi_list_foreach(tmp, &dt->ref_inode_list) + log_inode_reference(dt, tmp, 0); /* Figure out the block type to see if we can eliminate references to a different type. In other words, if the duplicate block looks @@ -605,7 +606,7 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b) references to it as metadata. Dinodes with such references are clearly corrupt and need to be deleted. And if we're left with a single reference, problem solved. */ - bh = bread(sdp, b->block); + bh = bread(sdp, dt->block); cmagic = ((struct gfs2_meta_header *)(bh->b_data))->mh_magic; ctype = ((struct gfs2_meta_header *)(bh->b_data))->mh_type; brelse(bh); @@ -650,10 +651,10 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b) "Step 1: Eliminate references to block %llu " "(0x%llx) that were previously marked " "invalid.\n"), - (unsigned long long)b->block, - (unsigned long long)b->block); - last_reference = resolve_dup_references(sdp, b, - &b->ref_invinode_list, + (unsigned long long)dt->block, + (unsigned long long)dt->block); + last_reference = resolve_dup_references(sdp, dt, + &dt->ref_invinode_list, &dh, 1, ref_types); } /* Step 2 - eliminate reference from inodes that reference it as the @@ -665,10 +666,10 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b) log_debug( _("----------------------------------------------\n" "Step 2: Eliminate references to block %llu " "(0x%llx) that need the wrong block type.\n"), - (unsigned long long)b->block, - (unsigned long long)b->block); - last_reference = resolve_dup_references(sdp, b, - &b->ref_inode_list, + (unsigned long long)dt->block, + (unsigned long long)dt->block); + last_reference = resolve_dup_references(sdp, dt, + &dt->ref_inode_list, &dh, 0, acceptable_ref); } @@ -680,20 +681,20 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b) log_debug( _("----------------------------------------------\n" "Step 3: Choose one reference to block %llu " "(0x%llx) to keep.\n"), - (unsigned long long)b->block, - (unsigned long long)b->block); - last_reference = resolve_dup_references(sdp, b, - &b->ref_inode_list, + (unsigned long long)dt->block, + (unsigned long long)dt->block); + last_reference = resolve_dup_references(sdp, dt, + &dt->ref_inode_list, &dh, 0, ref_types); } /* Now fix the block type of the block in question. */ - if (osi_list_empty(&b->ref_inode_list)) { + if (osi_list_empty(&dt->ref_inode_list)) { log_notice( _("Block %llu (0x%llx) has no more references; " "Marking as 'free'.\n"), - (unsigned long long)b->block, - (unsigned long long)b->block); - gfs2_blockmap_set(bl, b->block, gfs2_block_free); - check_n_fix_bitmap(sdp, b->block, gfs2_block_free); + (unsigned long long)dt->block, + (unsigned long long)dt->block); + gfs2_blockmap_set(bl, dt->block, gfs2_block_free); + check_n_fix_bitmap(sdp, dt->block, gfs2_block_free); return 0; } if (last_reference) { @@ -701,14 +702,14 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b) log_notice( _("Block %llu (0x%llx) has only one remaining " "reference.\n"), - (unsigned long long)b->block, - (unsigned long long)b->block); + (unsigned long long)dt->block, + (unsigned long long)dt->block); /* If we're down to a single reference (and not all references deleted, which may be the case of an inode that has only itself and a reference), we need to reset the block type from invalid to data or metadata. Start at the first one in the list, not the structure's place holder. */ - tmp = (&b->ref_inode_list)->next; + tmp = (&dt->ref_inode_list)->next; id = osi_list_entry(tmp, struct inode_with_dups, list); log_debug( _("----------------------------------------------\n" "Step 4. Set block type based on the remaining " @@ -724,27 +725,27 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b) "the block as free.\n"), (unsigned long long)id->block_no, (unsigned long long)id->block_no); - fsck_blockmap_set(ip, b->block, + fsck_blockmap_set(ip, dt->block, _("reference-repaired leaf"), gfs2_block_free); } else if (id->reftypecount[ref_is_inode]) { set_ip_blockmap(ip, 0); /* 0=do not add to dirtree */ } else if (id->reftypecount[ref_as_data]) { - fsck_blockmap_set(ip, b->block, + fsck_blockmap_set(ip, dt->block, _("reference-repaired data"), gfs2_block_used); } else if (id->reftypecount[ref_as_meta]) { if (is_dir(&ip->i_di, sdp->gfs1)) - fsck_blockmap_set(ip, b->block, + fsck_blockmap_set(ip, dt->block, _("reference-repaired leaf"), gfs2_leaf_blk); else - fsck_blockmap_set(ip, b->block, + fsck_blockmap_set(ip, dt->block, _("reference-repaired " "indirect"), gfs2_indir_blk); } else - fsck_blockmap_set(ip, b->block, + fsck_blockmap_set(ip, dt->block, _("reference-repaired extended " "attribute"), gfs2_meta_eattr); @@ -761,7 +762,7 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b) * use in pass2 */ int pass1b(struct gfs2_sbd *sdp) { - struct duptree *b; + struct duptree *dt; uint64_t i; uint8_t q; struct osi_node *n, *next = NULL; @@ -817,9 +818,9 @@ int pass1b(struct gfs2_sbd *sdp) out: for (n = osi_first(&dup_blocks); n; n = next) { next = osi_next(n); - b = (struct duptree *)n; + dt = (struct duptree *)n; if (!skip_this_pass && !rc) /* no error & not asked to skip the rest */ - handle_dup_blk(sdp, b); + handle_dup_blk(sdp, dt); /* Do not attempt to free the dup_blocks list or its parts here because any func that calls check_metatree needs to check duplicate status based on this linked list. diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index 5be260c..c11768f 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -234,7 +234,7 @@ int fsck_query(const char *format, ...) static struct duptree *gfs2_dup_set(uint64_t dblock, int create) { struct osi_node **newn = &dup_blocks.osi_node, *parent = NULL; - struct duptree *data; + struct duptree *dt; /* Figure out where to put new node */ while (*newn) { @@ -251,24 +251,24 @@ static struct duptree *gfs2_dup_set(uint64_t dblock, int create) if (!create) return NULL; - data = malloc(sizeof(struct duptree)); - if (data == NULL) { + dt = malloc(sizeof(struct duptree)); + if (dt == NULL) { log_crit( _("Unable to allocate duptree structure\n")); return NULL; } dups_found++; - memset(data, 0, sizeof(struct duptree)); + memset(dt, 0, sizeof(struct duptree)); /* Add new node and rebalance tree. */ - data->block = dblock; - data->refs = 1; /* reference 1 is actually the reference we need to - discover in pass1b. */ - data->first_ref_found = 0; - osi_list_init(&data->ref_inode_list); - osi_list_init(&data->ref_invinode_list); - osi_link_node(&data->node, parent, newn); - osi_insert_color(&data->node, &dup_blocks); - - return data; + dt->block = dblock; + dt->refs = 1; /* reference 1 is actually the reference we need to + discover in pass1b. */ + dt->first_ref_found = 0; + osi_list_init(&dt->ref_inode_list); + osi_list_init(&dt->ref_invinode_list); + osi_link_node(&dt->node, parent, newn); + osi_insert_color(&dt->node, &dup_blocks); + + return dt; } /** @@ -453,23 +453,23 @@ void dup_listent_delete(struct inode_with_dups *id) free(id); } -void dup_delete(struct duptree *b) +void dup_delete(struct duptree *dt) { struct inode_with_dups *id; osi_list_t *tmp; - while (!osi_list_empty(&b->ref_invinode_list)) { - tmp = (&b->ref_invinode_list)->next; + while (!osi_list_empty(&dt->ref_invinode_list)) { + tmp = (&dt->ref_invinode_list)->next; id = osi_list_entry(tmp, struct inode_with_dups, list); dup_listent_delete(id); } - while (!osi_list_empty(&b->ref_inode_list)) { - tmp = (&b->ref_inode_list)->next; + while (!osi_list_empty(&dt->ref_inode_list)) { + tmp = (&dt->ref_inode_list)->next; id = osi_list_entry(tmp, struct inode_with_dups, list); dup_listent_delete(id); } - osi_erase(&b->node, &dup_blocks); - free(b); + osi_erase(&dt->node, &dup_blocks); + free(dt); } void dirtree_delete(struct dir_info *b)

1 0

gfs2-utils: master - fsck.gfs2: Check for interrupt when resolving duplicates
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=9c52a36321… Commit: 9c52a36321f730c9859fb006326936530fe4da8f Parent: b4b33946e354f2591661ea7263b64e54fba61fb0 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Fri Mar 15 12:28:56 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: Check for interrupt when resolving duplicates This patch adds another check for interrupts while resolving duplicate block references in pass1b. --- gfs2/fsck/pass1b.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 56b77f5..7108bb4 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -459,6 +459,9 @@ static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *b, int found_good_ref = 0; osi_list_foreach_safe(tmp, ref_list, x) { + if (skip_this_pass || fsck_abort) + return FSCK_OK; + id = osi_list_entry(tmp, struct inode_with_dups, list); dh->b = b; dh->id = id;

1 0

gfs2-utils: master - fsck.gfs2: Rework the "undo" functions
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=b4b33946e3… Commit: b4b33946e354f2591661ea7263b64e54fba61fb0 Parent: 66e3ed1ab064d897075251c555869cd430458703 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue Mar 12 06:54:39 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: Rework the "undo" functions In pass1, it traverses the metadata free, processing each dinode and marking which blocks are used by that dinode. If a dinode is found to have unrecoverable errors, it does a bunch of work to "undo" the things it did. This is especially important for the processing of duplicate block references. Suppose dinode X references block 1234. Later in pass1, suppose a different dinode, Y, also references block 1234. This is flagged as a duplicate block reference. Still later, suppose pass1 determines dinode Y is bad. Now it has to undo the work it did. It needs to properly unmark the data and metadata blocks it marked as no longer "free" so that valid references that follow aren't flagged as duplicate references. At the same time, it needs to unflag block 1234 as a duplicate reference as well, so that dinode X's original reference is still considered valid. Before this patch, fsck.gfs2 was trying to traverse the entire metadata tree for the bad dinode, trying to "undo" the designations. That becomes a huge problem if the damage was discovered in the middle of the metadata, in which case it may never have flagged any of the data blocks as "in use as data" in its blockmap. The result of "undoing" the designations sometimes resulted in blocks improperly being marked as "free" when they were, in fact, referenced by other valid dinodes. For example, suppose corrupt dinode Y references metadata blocks 1234, 1235, and 1236. Now suppose a serious problem is found as part of its processing of block 1234, and so it stopped its metadata tree traversal there. Metadata blocks 1235 and 1236 are still listed as metadata for the bad dinode, but if we traverse the entire tree, those two blocks may be improperly processed. If another dinode actually uses blocks 1235 or 1236, the improper "undo" processing of those two blocks can screw up the valid references. This patch reworks the "undo" functions so that the "undo" functions don't get called on the entire metadata and data of the defective dinode. Instead, only the metadata and data blocks queued onto the metadata list are processed. This should ensure that the "undo" functions only operate on blocks that were processed in the first place. --- gfs2/fsck/metawalk.c | 120 +++++++++++++++++++++++------------ gfs2/fsck/metawalk.h | 4 + gfs2/fsck/pass1.c | 172 +++++++++++++++---------------------------------- 3 files changed, 137 insertions(+), 159 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 4fbe2c5..9e43556 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1259,7 +1259,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, if (err < 0) { stack; error = err; - goto fail; + return error; } if (err > 0) { if (!error) @@ -1278,14 +1278,11 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, } if (!nbh) nbh = bread(ip->i_sbd, block); - osi_list_add(&nbh->b_altlist, cur_list); + osi_list_add_prev(&nbh->b_altlist, cur_list); } /* for all data on the indirect block */ } /* for blocks at that height */ } /* for height */ - return error; -fail: - free_metalist(ip, mlp); - return error; + return 0; } /** @@ -1331,6 +1328,27 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, return error; } +static int undo_check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, + uint64_t *ptr_start, char *ptr_end) +{ + int 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 1; + block = be64_to_cpu(*ptr); + rc = pass->undo_check_data(ip, block, pass->private); + if (rc < 0) + return rc; + } + return 0; +} + static int hdr_size(struct gfs2_buffer_head *bh, int height) { if (height > 1) { @@ -1363,6 +1381,7 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) int i, head_size; uint64_t blks_checked = 0; int error, rc; + int metadata_clean = 0; if (!height && !is_dir(&ip->i_di, ip->i_sbd->gfs1)) return 0; @@ -1374,35 +1393,21 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) error = build_and_check_metalist(ip, &metalist[0], pass); if (error) { stack; - free_metalist(ip, &metalist[0]); - return error; + goto undo_metalist; } + metadata_clean = 1; /* For directories, we've already checked the "data" blocks which * comprise the directory hash table, so we perform the directory * checks and exit. */ if (is_dir(&ip->i_di, ip->i_sbd->gfs1)) { - free_metalist(ip, &metalist[0]); if (!(ip->i_di.di_flags & GFS2_DIF_EXHASH)) - return 0; + goto out; /* check validity of leaf blocks and leaf chains */ error = check_leaf_blks(ip, pass); - return error; - } - - /* Free the metalist buffers from heights we don't need to check. - For the rest we'll free as we check them to save time. - metalist[0] will only have the dinode bh, so we can skip it. */ - for (i = 1; i < height - 1; 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); - } + if (error) + goto undo_metalist; + goto out; } /* check data blocks */ @@ -1410,7 +1415,7 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) if (ip->i_di.di_blocks > COMFORTABLE_BLKS) last_reported_fblock = -10000000; - while (error >= 0 && !osi_list_empty(list)) { + while (!error && !osi_list_empty(list)) { if (fsck_abort) { free_metalist(ip, &metalist[0]); return 0; @@ -1428,21 +1433,12 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) } 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 (rc && (!error || rc < 0)) - error = rc; + error = check_data(ip, pass, (uint64_t *) + (bh->b_data + head_size), + (bh->b_data + ip->i_sbd->bsize), + &blks_checked); if (pass->big_file_msg && ip->i_di.di_blocks > COMFORTABLE_BLKS) pass->big_file_msg(ip, blks_checked); - if (bh == ip->i_bh) - osi_list_del(&bh->b_altlist); - else - brelse(bh); } if (pass->big_file_msg && ip->i_di.di_blocks > COMFORTABLE_BLKS) { log_notice( _("\rLarge file at %lld (0x%llx) - 100 percent " @@ -1452,6 +1448,50 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) (unsigned long long)ip->i_di.di_num.no_addr); fflush(stdout); } +undo_metalist: + if (!error) + goto out; + log_err( _("Error: inode %llu (0x%llx) had unrecoverable errors.\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + if (!query( _("Remove the invalid inode? (y/n) "))) { + free_metalist(ip, &metalist[0]); + log_err(_("Invalid inode not deleted.\n")); + return error; + } + for (i = 0; pass->undo_check_meta && i < height; i++) { + while (!osi_list_empty(&metalist[i])) { + list = &metalist[i]; + bh = osi_list_entry(list->next, + struct gfs2_buffer_head, + b_altlist); + log_err(_("Undoing metadata work for block %llu " + "(0x%llx)\n"), + (unsigned long long)bh->b_blocknr, + (unsigned long long)bh->b_blocknr); + if (i) + rc = pass->undo_check_meta(ip, bh->b_blocknr, + i, pass->private); + else + rc = 0; + if (metadata_clean && rc == 0 && i == height - 1) { + head_size = hdr_size(bh, height); + if (head_size) + undo_check_data(ip, pass, (uint64_t *) + (bh->b_data + head_size), + (bh->b_data + ip->i_sbd->bsize)); + } + if (bh == ip->i_bh) + osi_list_del(&bh->b_altlist); + else + brelse(bh); + } + } + /* Set the dinode as "bad" so it gets deleted */ + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("corrupt"), gfs2_block_free); + log_err(_("The corrupt inode was invalidated.\n")); +out: free_metalist(ip, &metalist[0]); return error; } diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index 486c6eb..f5e71e1 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -108,6 +108,10 @@ struct metawalk_fxns { int (*repair_leaf) (struct gfs2_inode *ip, uint64_t *leaf_no, int lindex, int ref_count, const char *msg, void *private); + int (*undo_check_meta) (struct gfs2_inode *ip, uint64_t block, + int h, void *private); + int (*undo_check_data) (struct gfs2_inode *ip, uint64_t block, + void *private); }; #endif /* _METAWALK_H */ diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 04e5289..a88895f 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -39,8 +39,7 @@ static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private); static int check_metalist(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, void *private); static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - void *private); + int h, void *private); static int check_data(struct gfs2_inode *ip, uint64_t block, void *private); static int undo_check_data(struct gfs2_inode *ip, uint64_t block, void *private); @@ -104,12 +103,8 @@ struct metawalk_fxns pass1_fxns = { .finish_eattr_indir = finish_eattr_indir, .big_file_msg = big_file_comfort, .repair_leaf = pass1_repair_leaf, -}; - -struct metawalk_fxns undo_fxns = { - .private = NULL, - .check_metalist = undo_check_metalist, - .check_data = undo_check_data, + .undo_check_meta = undo_check_metalist, + .undo_check_data = undo_check_data, }; struct metawalk_fxns invalidate_fxns = { @@ -326,53 +321,67 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, return 0; } -static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - void *private) +/* undo_reference - undo previously processed data or metadata + * We've treated the metadata for this dinode as good so far, but not we + * realize it's bad. So we need to undo what we've done. + * + * Returns: 0 - We need to process the block as metadata. In other words, + * we need to undo any blocks it refers to. + * 1 - We can't process the block as metadata. + */ + +static int undo_reference(struct gfs2_inode *ip, uint64_t block, int meta, + void *private) { - int found_dup = 0, iblk_type; - struct gfs2_buffer_head *nbh; struct block_count *bc = (struct block_count *)private; - - *bh = NULL; + struct duptree *dt; + struct inode_with_dups *id; if (!valid_block(ip->i_sbd, block)) { /* blk outside of FS */ fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, _("bad block referencing"), gfs2_block_free); return 1; } - if (is_dir(&ip->i_di, ip->i_sbd->gfs1) && h == ip->i_di.di_height) - iblk_type = GFS2_METATYPE_JD; - else - iblk_type = GFS2_METATYPE_IN; - found_dup = find_remove_dup(ip, block, _("Metadata")); - nbh = bread(ip->i_sbd, block); + if (meta) + bc->indir_count--; + dt = dupfind(block); + if (dt) { + /* remove all duplicate reference structures from this inode */ + do { + id = find_dup_ref_inode(dt, ip); + if (!id) + break; - if (gfs2_check_meta(nbh, iblk_type)) { - if (!found_dup) { - fsck_blockmap_set(ip, block, _("bad indirect"), - gfs2_block_free); - brelse(nbh); + dup_listent_delete(id); + } while (id); + + if (dt->refs) { + log_err(_("Block %llu (0x%llx) is still referenced " + "from another inode; not freeing.\n"), + (unsigned long long)block, + (unsigned long long)block); return 1; } - brelse(nbh); - nbh = NULL; - } else /* blk check ok */ - *bh = nbh; - - bc->indir_count--; - if (found_dup) { - if (nbh) - brelse(nbh); - *bh = NULL; - return 1; /* don't process the metadata again */ - } else - fsck_blockmap_set(ip, block, _("bad indirect"), - gfs2_block_free); + } + fsck_blockmap_set(ip, block, + meta ? _("bad indirect") : _("referenced data"), + gfs2_block_free); return 0; } +static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block, + int h, void *private) +{ + return undo_reference(ip, block, 1, private); +} + +static int undo_check_data(struct gfs2_inode *ip, uint64_t block, + void *private) +{ + return undo_reference(ip, block, 0, private); +} + static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) { uint8_t q; @@ -438,71 +447,9 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) return 0; } -static int undo_check_data(struct gfs2_inode *ip, uint64_t block, - void *private) -{ - struct block_count *bc = (struct block_count *) private; - - if (!valid_block(ip->i_sbd, block)) { - /* Mark the owner of this block with the bad_block - * designator so we know to check it for out of range - * blocks later */ - fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, - _("bad (invalid or out of range) data"), - gfs2_block_free); - return 1; - } - bc->data_count--; - return free_block_if_notdup(ip, block, _("data")); -} - static int remove_inode_eattr(struct gfs2_inode *ip, struct block_count *bc) { - struct duptree *dt; - struct inode_with_dups *id; - osi_list_t *ref; - int moved = 0; - - /* If it's a duplicate reference to the block, we need to check - if the reference is on the valid or invalid inodes list. - If it's on the valid inode's list, move it to the invalid - inodes list. The reason is simple: This inode, although - valid, has an now-invalid reference, so we should not give - this reference preferential treatment over others. */ - dt = dupfind(ip->i_di.di_eattr); - if (dt) { - osi_list_foreach(ref, &dt->ref_inode_list) { - id = osi_list_entry(ref, struct inode_with_dups, list); - if (id->block_no == ip->i_di.di_num.no_addr) { - log_debug( _("Moving inode %lld (0x%llx)'s " - "duplicate reference to %lld " - "(0x%llx) from the valid to the " - "invalid reference list.\n"), - (unsigned long long) - ip->i_di.di_num.no_addr, - (unsigned long long) - ip->i_di.di_num.no_addr, - (unsigned long long) - ip->i_di.di_eattr, - (unsigned long long) - ip->i_di.di_eattr); - /* Move from the normal to the invalid list */ - osi_list_del(&id->list); - osi_list_add_prev(&id->list, - &dt->ref_invinode_list); - moved = 1; - break; - } - } - if (!moved) - log_debug( _("Duplicate reference to %lld " - "(0x%llx) not moved.\n"), - (unsigned long long)ip->i_di.di_eattr, - (unsigned long long)ip->i_di.di_eattr); - } else { - delete_block(ip, ip->i_di.di_eattr, NULL, - "extended attribute", NULL); - } + undo_reference(ip, ip->i_di.di_eattr, 0, bc); ip->i_di.di_eattr = 0; bc->ea_count = 0; ip->i_di.di_blocks = 1 + bc->indir_count + bc->data_count; @@ -1080,23 +1027,10 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip) if (lf_dip && lf_dip->i_di.di_blocks != lf_blks) reprocess_inode(lf_dip, "lost+found"); - if (fsck_abort || error < 0) + /* We there was an error, we return 0 because we want fsck to continue + and analyze the other dinodes as well. */ + if (fsck_abort || error != 0) return 0; - if (error > 0) { - log_err( _("Error: inode %llu (0x%llx) has unrecoverable " - "errors; invalidating.\n"), - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); - undo_fxns.private = &bc; - check_metatree(ip, &undo_fxns); - /* If we undo the metadata accounting, including metadatas - duplicate block status, we need to make sure later passes - don't try to free up the metadata referenced by this inode. - Therefore we mark the inode as free space. */ - fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, - _("corrupt"), gfs2_block_free); - return 0; - } error = check_inode_eattr(ip, &pass1_fxns);

1 0

gfs2-utils: master - fsck.gfs2: separate function to calculate metadata block header size
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=66e3ed1ab0… Commit: 66e3ed1ab064d897075251c555869cd430458703 Parent: 3aa5d7f7ffe9b135f62f39524183efcecbb059a1 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue Apr 2 12:49:50 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: separate function to calculate metadata block header size This patch creates a new function hdr_size that calculates the size of a GFS2 metadata header depending on the height and type of block. --- gfs2/fsck/metawalk.c | 44 +++++++++++++++++++++++--------------------- 1 files changed, 23 insertions(+), 21 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 2f574bf..4fbe2c5 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1331,6 +1331,23 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, return error; } +static int hdr_size(struct gfs2_buffer_head *bh, int height) +{ + if (height > 1) { + if (gfs2_check_meta(bh, GFS2_METATYPE_IN)) + return 0; + if (bh->sdp->gfs1) + return sizeof(struct gfs_indirect); + else + return sizeof(struct gfs2_meta_header); + } + /* if this isn't really a dinode, skip it */ + if (gfs2_check_meta(bh, GFS2_METATYPE_DI)) + return 0; + + return sizeof(struct gfs2_dinode); +} + /** * check_metatree * @ip: inode structure in memory @@ -1401,28 +1418,13 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) bh = osi_list_entry(list->next, struct gfs2_buffer_head, b_altlist); - if (height > 1) { - if (gfs2_check_meta(bh, GFS2_METATYPE_IN)) { - if (bh == ip->i_bh) - osi_list_del(&bh->b_altlist); - else - brelse(bh); - continue; - } - if (ip->i_sbd->gfs1) - head_size = sizeof(struct gfs_indirect); + head_size = hdr_size(bh, height); + if (!head_size) { + if (bh == ip->i_bh) + osi_list_del(&bh->b_altlist); else - head_size = sizeof(struct gfs2_meta_header); - } else { - /* if this isn't really a dinode, skip it */ - if (gfs2_check_meta(bh, GFS2_METATYPE_DI)) { - if (bh == ip->i_bh) - osi_list_del(&bh->b_altlist); - else - brelse(bh); - continue; - } - head_size = sizeof(struct gfs2_dinode); + brelse(bh); + continue; } if (pass->check_data)

1 0

gfs2-utils: master - fsck.gfs2: Add clarifying message to duplicate processing
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=3aa5d7f7ff… Commit: 3aa5d7f7ffe9b135f62f39524183efcecbb059a1 Parent: ef2da09e89f15e5e302571d4ef9025887d2d6900 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Mon Mar 11 07:06:36 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: Add clarifying message to duplicate processing This patch adds a message to the fsck output that indicates which block reference is acceptable. That helps to determine if fsck made the right decision when a duplicate is resolved. --- gfs2/fsck/pass1b.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index bd60d84..56b77f5 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -485,6 +485,15 @@ static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *b, q = block_type(id->block_no); if (q != gfs2_inode_invalid) { found_good_ref = 1; + log_warn( _("Inode %s (%lld/0x%llx)'s " + "reference to block %llu (0x%llx) " + "as '%s' is acceptable.\n"), + id->name, + (unsigned long long)id->block_no, + (unsigned long long)id->block_no, + (unsigned long long)b->block, + (unsigned long long)b->block, + reftypes[this_ref]); continue; /* don't delete the dinode */ } }

1 0

gfs2-utils: master - fsck.gfs2: link dinodes that only have extended attribute problems
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=ef2da09e89… Commit: ef2da09e89f15e5e302571d4ef9025887d2d6900 Parent: 7d603456b564da34b7e22680e05f781012998d53 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Fri Mar 8 11:50:56 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: link dinodes that only have extended attribute problems The job of pass1b is to resolve duplicate references to the same block. Eventually it does a fair job of determining the rightful owner of the block, and then it has to deal with the other dinode(s) that referenced the block improperly. If another dinode improperly referenced the block as data or metadata, it's obvious file corruption and the dinode should be deleted. However, if the other dinode improperly referenced the block as an extended attribute, it can fix the situation by removing the extended attributes from the dinode. Prior to this patch, there was a check in the code for this situation so that the dinode was only deleted if the bad block reference was as data or metadata. However, regardless of the situation, the code removed the inode from the inode rbtree. That resulted in the dinode being considered unlinked, so it would get improperly tossed into lost+found and left in a indeterminate state. Subsequent runs of fsck.gfs2 could find the discrepancy and flag it as unlinked again. This patch adds another check so that the inode is not removed from the inode rbtree, so it is linked properly during pass2. --- gfs2/fsck/pass1b.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index f3f90ef..bd60d84 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -523,9 +523,12 @@ static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *b, (unsigned long long)id->block_no); ip = fsck_load_inode(sdp, id->block_no); - ii = inodetree_find(ip->i_di.di_num.no_addr); - if (ii) - inodetree_delete(ii); + if (id->reftypecount[ref_as_data] || + id->reftypecount[ref_as_meta]) { + ii = inodetree_find(ip->i_di.di_num.no_addr); + if (ii) + inodetree_delete(ii); + } clear_dup_fxns.private = (void *) dh; /* Clear the EAs for the inode first */ check_inode_eattr(ip, &clear_dup_fxns);

1 0

gfs2-utils: master - fsck.gfs2: Remove redundant leaf depth check
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=7d603456b5… Commit: 7d603456b564da34b7e22680e05f781012998d53 Parent: 782609bd24ee9be771952b77a3fbdf5097820eb1 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Fri Mar 8 10:16:14 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: Remove redundant leaf depth check A previous patch changed the way we check leaf block depth. This patch removes the redundant check from pass1. --- gfs2/fsck/pass1.c | 16 ---------------- 1 files changed, 0 insertions(+), 16 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 5137559..04e5289 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -1021,7 +1021,6 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip) int error; struct block_count bc = {0}; long bad_pointers; - uint64_t block = ip->i_bh->b_blocknr; uint64_t lf_blks = 0; bad_pointers = 0L; @@ -1069,21 +1068,6 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip) if (set_di_nlink(ip)) goto bad_dinode; - if (is_dir(&ip->i_di, sdp->gfs1) && (ip->i_di.di_flags & GFS2_DIF_EXHASH)) { - if (((1 << ip->i_di.di_depth) * sizeof(uint64_t)) != ip->i_di.di_size){ - log_warn( _("Directory dinode block #%llu (0x%llx" - ") has bad depth. Found %u, Expected %u\n"), - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr, - ip->i_di.di_depth, - (1 >> (ip->i_di.di_size/sizeof(uint64_t)))); - if (fsck_blockmap_set(ip, block, _("bad depth"), - gfs2_block_free)) - goto bad_dinode; - return 0; - } - } - if (lf_dip) lf_blks = lf_dip->i_di.di_blocks;

1 0

gfs2-utils: master - fsck.gfs2: reprocess inodes when blocks are added
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=782609bd24… Commit: 782609bd24ee9be771952b77a3fbdf5097820eb1 Parent: 39c9d28e4989ffd16cb8c5248c90424d34599c4b Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Fri Mar 8 10:12:16 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: reprocess inodes when blocks are added This patch adds several calls to reprocess_inode when functions may have potentially added blocks to a dinode. This happens, for example, when leaf blocks are split or new leaf blocks are added. The purpose of reprocessing the inode is to properly mark the new blocks in the fsck blockmap. If we don't, the new blocks may be flagged as wrong in the bitmap, and set free in pass5. --- gfs2/fsck/metawalk.c | 6 ++++++ gfs2/fsck/pass1.c | 11 +++++++++++ gfs2/fsck/pass2.c | 15 +++++++++------ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index cb1ffa4..2f574bf 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1474,9 +1474,12 @@ int check_dir(struct gfs2_sbd *sdp, uint64_t block, struct metawalk_fxns *pass) { struct gfs2_inode *ip; int error = 0; + uint64_t cur_blks; ip = fsck_load_inode(sdp, block); + cur_blks = ip->i_di.di_blocks; + if (ip->i_di.di_flags & GFS2_DIF_EXHASH) error = check_leaf_blks(ip, pass); else @@ -1485,6 +1488,9 @@ int check_dir(struct gfs2_sbd *sdp, uint64_t block, struct metawalk_fxns *pass) if (error < 0) stack; + if (ip->i_di.di_blocks != cur_blks) + reprocess_inode(ip, _("Current")); + fsck_inode_put(&ip); /* does a brelse */ return error; } diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index e827a55..5137559 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -1022,6 +1022,7 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip) struct block_count bc = {0}; long bad_pointers; uint64_t block = ip->i_bh->b_blocknr; + uint64_t lf_blks = 0; bad_pointers = 0L; @@ -1083,8 +1084,18 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip) } } + if (lf_dip) + lf_blks = lf_dip->i_di.di_blocks; + pass1_fxns.private = &bc; error = check_metatree(ip, &pass1_fxns); + + /* Pass1 may have added some blocks to lost+found by virtue of leafs + that were misplaced. If it did, we need to reprocess lost+found + to correctly account for its blocks. */ + if (lf_dip && lf_dip->i_di.di_blocks != lf_blks) + reprocess_inode(lf_dip, "lost+found"); + if (fsck_abort || error < 0) return 0; if (error > 0) { diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 0b714d6..2336aa5 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -1448,7 +1448,7 @@ struct metawalk_fxns pass2_fxns = { static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname, int builder(struct gfs2_sbd *sdp)) { - uint64_t iblock = 0; + uint64_t iblock = 0, cur_blks; struct dir_status ds = {0}; char *filename; int filename_len; @@ -1468,12 +1468,15 @@ static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname, pass2_fxns.private = (void *) &ds; if (ds.q == gfs2_bad_block) { + cur_blks = sysinode->i_di.di_blocks; /* First check that the directory's metatree is valid */ error = check_metatree(sysinode, &pass2_fxns); if (error < 0) { stack; return error; } + if (sysinode->i_di.di_blocks != cur_blks) + reprocess_inode(sysinode, _("System inode")); } error = check_dir(sysinode->i_sbd, iblock, &pass2_fxns); if (skip_this_pass || fsck_abort) /* if asked to skip the rest */ @@ -1493,8 +1496,7 @@ 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; - + 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))) { @@ -1585,7 +1587,7 @@ static inline int is_system_dir(struct gfs2_sbd *sdp, uint64_t block) */ int pass2(struct gfs2_sbd *sdp) { - uint64_t dirblk; + uint64_t dirblk, cur_blks; uint8_t q; struct dir_status ds = {0}; struct gfs2_inode *ip; @@ -1647,12 +1649,15 @@ int pass2(struct gfs2_sbd *sdp) /* First check that the directory's metatree * is valid */ ip = fsck_load_inode(sdp, dirblk); + cur_blks = ip->i_di.di_blocks; error = check_metatree(ip, &pass2_fxns); fsck_inode_put(&ip); if (error < 0) { stack; return error; } + if (ip->i_di.di_blocks != cur_blks) + reprocess_inode(ip, "current"); } error = check_dir(sdp, dirblk, &pass2_fxns); if (skip_this_pass || fsck_abort) /* if asked to skip the rest */ @@ -1711,8 +1716,6 @@ int pass2(struct gfs2_sbd *sdp) (unsigned long long)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 */

1 0

gfs2-utils: master - fsck.gfs2: small cleanups
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=39c9d28e49… Commit: 39c9d28e4989ffd16cb8c5248c90424d34599c4b Parent: 829f490e5f8fd9a1a708667783514bca5fd95b05 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Fri Mar 8 09:49:31 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: small cleanups This patch just fixes some messages that were wrong, adds comments, and so forth. --- gfs2/fsck/metawalk.c | 24 +++++++++++++----------- gfs2/fsck/pass2.c | 9 +++------ gfs2/fsck/util.h | 2 +- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 5373bd4..cb1ffa4 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -356,14 +356,12 @@ static int check_entries(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, if (type == DIR_LINEAR) { dent = (struct gfs2_dirent *)(bh->b_data + sizeof(struct gfs2_dinode)); - } - else if (type == DIR_EXHASH) { + } else if (type == DIR_EXHASH) { dent = (struct gfs2_dirent *)(bh->b_data + sizeof(struct gfs2_leaf)); - log_debug( _("Checking leaf %llu (0x%llu)\n"), + log_debug( _("Checking leaf %llu (0x%llx)\n"), (unsigned long long)bh->b_blocknr, (unsigned long long)bh->b_blocknr); - } - else { + } else { log_err( _("Invalid directory type %d specified\n"), type); return -1; } @@ -498,11 +496,12 @@ static int check_leaf(struct gfs2_inode *ip, int lindex, /* Make sure the block number is in range. */ if (!valid_block(ip->i_sbd, *leaf_no)) { log_err( _("Leaf block #%llu (0x%llx) is out of range for " - "directory #%llu (0x%llx).\n"), + "directory #%llu (0x%llx) at index %d (0x%x).\n"), (unsigned long long)*leaf_no, (unsigned long long)*leaf_no, (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); + (unsigned long long)ip->i_di.di_num.no_addr, + lindex, lindex); msg = _("that is out of range"); goto bad_leaf; } @@ -1334,8 +1333,8 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, /** * check_metatree - * @ip: - * @rgd: + * @ip: inode structure in memory + * @pass: structure passed in from caller to determine the sub-functions * */ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) @@ -1684,8 +1683,11 @@ 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); + log_info( _("%s inode %llu (0x%llx) had blocks added; reprocessing " + "its metadata tree at height=%d.\n"), desc, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr, + ip->i_di.di_height); error = check_metatree(ip, &alloc_fxns); if (error) log_err( _("Error %d reprocessing the %s metadata tree.\n"), diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 91bc398..0b714d6 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -46,9 +46,9 @@ static int set_parent_dir(struct gfs2_sbd *sdp, struct gfs2_inum child, if (di->dinode.no_addr == child.no_addr && di->dinode.no_formal_ino == child.no_formal_ino) { if (di->treewalk_parent) { - log_err( _("Another directory at block %llx (0x%llx) " - "already contains this child %lld (%llx) - " - "checking parent %llx (0x%llx)\n"), + log_err( _("Another directory at block %lld (0x%llx) " + "already contains this child %lld (0x%llx)" + " - checking parent %lld (0x%llx)\n"), (unsigned long long)di->treewalk_parent, (unsigned long long)di->treewalk_parent, (unsigned long long)child.no_addr, @@ -1784,6 +1784,3 @@ int pass2(struct gfs2_sbd *sdp) gfs2_dup_free(); return FSCK_OK; } - - - diff --git a/gfs2/fsck/util.h b/gfs2/fsck/util.h index 7b587d4..00c2239 100644 --- a/gfs2/fsck/util.h +++ b/gfs2/fsck/util.h @@ -65,7 +65,7 @@ static const inline char *block_type_string(uint8_t q) const char *blktyp[] = { "free", "data", - "indirect data", + "indirect meta", "directory", "file",

1 0

gfs2-utils: master - fsck.gfs2: check leaf depth when validating leaf blocks
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=829f490e5f… Commit: 829f490e5f8fd9a1a708667783514bca5fd95b05 Parent: 4d4cf4c98159b6059f435fa465defbde6a4dd221 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Fri Mar 8 09:34:32 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: check leaf depth when validating leaf blocks Now that fsck.gfs2 can validate the hash table is relatively sane without actually reading the leaf blocks, when it does need to read in those leaf blocks, we need to check the leaf depth is appropriate for the (now sane) number of pointers we encountered in the hash table. This patch adds a call to check the leaf depth from pass2. --- gfs2/fsck/metawalk.c | 3 +++ gfs2/fsck/metawalk.h | 2 ++ gfs2/fsck/pass2.c | 1 + 3 files changed, 6 insertions(+), 0 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index fac91a5..5373bd4 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -514,6 +514,9 @@ static int check_leaf(struct gfs2_inode *ip, int lindex, msg = _("that is not really a leaf"); goto bad_leaf; } + if (pass->check_leaf_depth) + error = pass->check_leaf_depth(ip, *leaf_no, *ref_count, lbh); + if (pass->check_leaf) { error = pass->check_leaf(ip, *leaf_no, pass->private); if (error) { diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index e11b5e0..486c6eb 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -69,6 +69,8 @@ extern int free_block_if_notdup(struct gfs2_inode *ip, uint64_t block, */ struct metawalk_fxns { void *private; + int (*check_leaf_depth) (struct gfs2_inode *ip, uint64_t leaf_no, + int ref_count, struct gfs2_buffer_head *lbh); int (*check_leaf) (struct gfs2_inode *ip, uint64_t block, void *private); int (*check_metalist) (struct gfs2_inode *ip, uint64_t block, diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 1810040..91bc398 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -1431,6 +1431,7 @@ static int pass2_repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, struct metawalk_fxns pass2_fxns = { .private = NULL, + .check_leaf_depth = check_leaf_depth, .check_leaf = NULL, .check_metalist = NULL, .check_data = NULL,

1 0

gfs2-utils: master - fsck.gfs2: fix leaf blocks, don't try to patch the hash table
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=4d4cf4c981… Commit: 4d4cf4c98159b6059f435fa465defbde6a4dd221 Parent: 46cce9dc2161fc83d3f5a35b6c12a6a0a0dc6b3f Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Fri Mar 8 09:22:11 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: fix leaf blocks, don't try to patch the hash table Before this patch, when we detected a bad leaf block, fsck.gfs2 would try to patch the hash table. That's very wrong, because the hash table needs to be on nice power-of-two boundaries. This patch changes the code so that the hash table is actually repaired. --- gfs2/fsck/metawalk.c | 135 ++++++++++++++++++-------------------------------- gfs2/fsck/metawalk.h | 3 +- gfs2/fsck/pass1.c | 14 +++++ gfs2/fsck/pass2.c | 9 +++- 4 files changed, 72 insertions(+), 89 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 2e998df..fac91a5 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -480,52 +480,14 @@ static int check_entries(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, return 0; } -/* warn_and_patch - Warn the user of an error and ask permission to fix it - * Process a bad leaf pointer and ask to repair the first time. - * The repair process involves extending the previous leaf's entries - * so that they replace the bad ones. We have to hack up the old - * leaf a bit, but it's better than deleting the whole directory, - * which is what used to happen before. */ -static int warn_and_patch(struct gfs2_inode *ip, uint64_t *leaf_no, - uint64_t *bad_leaf, uint64_t old_leaf, - uint64_t first_ok_leaf, int pindex, const char *msg) -{ - int okay_to_fix = 0; - - if (*bad_leaf != *leaf_no) { - log_err( _("Directory Inode %llu (0x%llx) points to leaf %llu" - " (0x%llx) %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)*leaf_no, - (unsigned long long)*leaf_no, msg); - } - if (*leaf_no == *bad_leaf || - (okay_to_fix = query( _("Attempt to patch around it? (y/n) ")))) { - if (valid_block(ip->i_sbd, old_leaf)) - gfs2_put_leaf_nr(ip, pindex, old_leaf); - else - gfs2_put_leaf_nr(ip, pindex, first_ok_leaf); - log_err( _("Directory Inode %llu (0x%llx) repaired.\n"), - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); - } else - log_err( _("Bad leaf left in place.\n")); - *bad_leaf = *leaf_no; - *leaf_no = old_leaf; - return okay_to_fix; -} - /** * check_leaf - check a leaf block for errors * Reads in the leaf block * Leaves the buffer around for further analysis (caller must brelse) */ static int check_leaf(struct gfs2_inode *ip, int lindex, - struct metawalk_fxns *pass, int *ref_count, - uint64_t *leaf_no, uint64_t old_leaf, uint64_t *bad_leaf, - uint64_t first_ok_leaf, struct gfs2_leaf *leaf, - struct gfs2_leaf *oldleaf) + struct metawalk_fxns *pass, + uint64_t *leaf_no, struct gfs2_leaf *leaf, int *ref_count) { int error = 0, fix; struct gfs2_buffer_head *lbh = NULL; @@ -533,7 +495,6 @@ static int check_leaf(struct gfs2_inode *ip, int lindex, struct gfs2_sbd *sdp = ip->i_sbd; const char *msg; - *ref_count = 1; /* Make sure the block number is in range. */ if (!valid_block(ip->i_sbd, *leaf_no)) { log_err( _("Leaf block #%llu (0x%llx) is out of range for " @@ -543,7 +504,7 @@ static int check_leaf(struct gfs2_inode *ip, int lindex, (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); msg = _("that is out of range"); - goto out_copy_old_leaf; + goto bad_leaf; } /* Try to read in the leaf block. */ @@ -551,7 +512,7 @@ static int check_leaf(struct gfs2_inode *ip, int lindex, /* Make sure it's really a valid leaf block. */ if (gfs2_check_meta(lbh, GFS2_METATYPE_LF)) { msg = _("that is not really a leaf"); - goto out_copy_old_leaf; + goto bad_leaf; } if (pass->check_leaf) { error = pass->check_leaf(ip, *leaf_no, pass->private); @@ -587,7 +548,7 @@ static int check_leaf(struct gfs2_inode *ip, int lindex, (unsigned long long)*leaf_no, (unsigned long long)*leaf_no); msg = _("that is not a leaf"); - goto out_copy_old_leaf; + goto bad_leaf; } if (pass->check_dentry && is_dir(&ip->i_di, sdp->gfs1)) { @@ -599,16 +560,18 @@ static int check_leaf(struct gfs2_inode *ip, int lindex, if (error < 0) { stack; - goto out; + goto out; /* This seems wrong: needs investigation */ } - if (count != leaf->lf_entries) { - /* release and re-read the leaf in case check_entries - changed it. */ - brelse(lbh); - lbh = bread(sdp, *leaf_no); - gfs2_leaf_in(leaf, lbh); + if (count == leaf->lf_entries) + goto out; + /* release and re-read the leaf in case check_entries + changed it. */ + brelse(lbh); + lbh = bread(sdp, *leaf_no); + gfs2_leaf_in(leaf, lbh); + if (count != leaf->lf_entries) { log_err( _("Leaf %llu (0x%llx) entry count in " "directory %llu (0x%llx) does not match " "number of entries found - is %u, found %u\n"), @@ -630,17 +593,16 @@ out: brelse(lbh); return 0; -out_copy_old_leaf: - /* The leaf we read in is bad. So we'll copy the old leaf into the - * new one. However, that will make us shift our ref count. */ - fix = warn_and_patch(ip, leaf_no, bad_leaf, old_leaf, - first_ok_leaf, lindex, msg); - (*ref_count)++; - memcpy(leaf, oldleaf, sizeof(struct gfs2_leaf)); - if (lbh) { - if (fix) - bmodified(lbh); +bad_leaf: + if (lbh) brelse(lbh); + if (pass->repair_leaf) { + /* The leaf we read in is bad so we need to repair it. */ + fix = pass->repair_leaf(ip, leaf_no, lindex, *ref_count, msg, + pass->private); + if (fix < 0) + return fix; + } return 1; } @@ -679,17 +641,17 @@ static void dir_leaf_reada(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize) /* Checks exhash directory entries */ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) { - int error; - struct gfs2_leaf leaf, oldleaf; + int error = 0; + struct gfs2_leaf leaf; unsigned hsize = (1 << ip->i_di.di_depth); - uint64_t leaf_no, old_leaf, bad_leaf = -1; + uint64_t leaf_no, leaf_next; uint64_t first_ok_leaf, orig_di_blocks; struct gfs2_buffer_head *lbh; int lindex; struct gfs2_sbd *sdp = ip->i_sbd; - int ref_count = 0, orig_ref_count, orig_di_depth, orig_di_height, old_was_dup; + int ref_count, orig_ref_count, orig_di_depth, orig_di_height; uint64_t *tbl; - int tbl_valid; + int chained_leaf, tbl_valid; tbl = get_dir_hash(ip); if (tbl == NULL) { @@ -751,10 +713,11 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) posix_fadvise(sdp->device_fd, 0, 0, POSIX_FADV_NORMAL); return 1; } - old_leaf = -1; - memset(&oldleaf, 0, sizeof(oldleaf)); - old_was_dup = 0; - for (lindex = 0; lindex < hsize; lindex++) { + lindex = 0; + leaf_next = -1; + while (lindex < hsize) { + int l; + if (fsck_abort) break; @@ -774,38 +737,36 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) } leaf_no = be64_to_cpu(tbl[lindex]); - /* GFS has multiple indirect pointers to the same leaf - * until those extra pointers are needed, so skip the dups */ - if (leaf_no == bad_leaf) { - tbl[lindex] = cpu_to_be64(old_leaf); - gfs2_put_leaf_nr(ip, lindex, old_leaf); - ref_count++; - continue; - } else if (old_leaf == leaf_no) { + /* count the number of block pointers to this leaf. We don't + need to count the current lindex, because we already know + it's a reference */ + ref_count = 1; + + for (l = lindex + 1; l < hsize; l++) { + leaf_next = be64_to_cpu(tbl[l]); + if (leaf_next != leaf_no) + break; ref_count++; - continue; } orig_ref_count = ref_count; + chained_leaf = 0; do { if (fsck_abort) { free(tbl); posix_fadvise(sdp->device_fd, 0, 0, POSIX_FADV_NORMAL); return 0; } - error = check_leaf(ip, lindex, pass, &ref_count, - &leaf_no, old_leaf, &bad_leaf, - first_ok_leaf, &leaf, &oldleaf); + error = check_leaf(ip, lindex, pass, &leaf_no, &leaf, + &ref_count); if (ref_count != orig_ref_count) tbl_valid = 0; - old_was_dup = (error == -EEXIST); - old_leaf = leaf_no; - memcpy(&oldleaf, &leaf, sizeof(oldleaf)); if (!leaf.lf_next || error) break; leaf_no = leaf.lf_next; - log_debug( _("Leaf chain (0x%llx) detected.\n"), - (unsigned long long)leaf_no); + chained_leaf++; + log_debug( _("Leaf chain #%d (0x%llx) detected.\n"), + chained_leaf, (unsigned long long)leaf_no); } while (1); /* while we have chained leaf blocks */ if (orig_di_depth != ip->i_di.di_depth) { log_debug(_("Depth of 0x%llx changed from %d to %d\n"), diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index bef99ae..e11b5e0 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -104,7 +104,8 @@ struct metawalk_fxns { int (*check_hash_tbl) (struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize, void *private); int (*repair_leaf) (struct gfs2_inode *ip, uint64_t *leaf_no, - int lindex, int ref_count, const char *msg); + int lindex, int ref_count, const char *msg, + void *private); }; #endif /* _METAWALK_H */ diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index dd6b958..e827a55 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -78,6 +78,19 @@ static int invalidate_eattr_leaf(struct gfs2_inode *ip, uint64_t block, void *private); static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip); +static int pass1_repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, + int lindex, int ref_count, const char *msg, + void *private) +{ + struct block_count *bc = (struct block_count *)private; + int new_leaf_blks; + + new_leaf_blks = repair_leaf(ip, leaf_no, lindex, ref_count, msg); + bc->indir_count += new_leaf_blks; + + return new_leaf_blks; +} + struct metawalk_fxns pass1_fxns = { .private = NULL, .check_leaf = check_leaf, @@ -90,6 +103,7 @@ struct metawalk_fxns pass1_fxns = { .check_eattr_extentry = check_extended_leaf_eattr, .finish_eattr_indir = finish_eattr_indir, .big_file_msg = big_file_comfort, + .repair_leaf = pass1_repair_leaf, }; struct metawalk_fxns undo_fxns = { diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index babac15..1810040 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -1422,6 +1422,13 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, return error; } +static int pass2_repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, + int lindex, int ref_count, const char *msg, + void *private) +{ + return repair_leaf(ip, leaf_no, lindex, ref_count, msg); +} + struct metawalk_fxns pass2_fxns = { .private = NULL, .check_leaf = NULL, @@ -1432,7 +1439,7 @@ struct metawalk_fxns pass2_fxns = { .check_dentry = check_dentry, .check_eattr_entry = NULL, .check_hash_tbl = check_hash_tbl, - .repair_leaf = repair_leaf, + .repair_leaf = pass2_repair_leaf, }; /* Check system directory inode */

1 0

gfs2-utils: master - fsck.gfs2: re-read hash table if directory height or depth changes
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=46cce9dc21… Commit: 46cce9dc2161fc83d3f5a35b6c12a6a0a0dc6b3f Parent: 5d6bf12f59b07e9eb6e926408b21b5ba49132522 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Mar 7 13:58:18 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: re-read hash table if directory height or depth changes There may be times when fsck.gfs2 wants to move things around. For example, if it finds dirents entries on the wrong leaf block, it may want to move them to a different leaf. If it does, it may need to split the leaf, which means we're adding another block. We may in fact have doubled our exhash table, so the table in cache is no longer valid. In this case, we need to discard the old one and read it in again. This patch checks for these things and re-reads the hash table as appropriate. --- gfs2/fsck/metawalk.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 47 insertions(+), 2 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 5bcac77..2e998df 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -683,18 +683,23 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) struct gfs2_leaf leaf, oldleaf; unsigned hsize = (1 << ip->i_di.di_depth); uint64_t leaf_no, old_leaf, bad_leaf = -1; - uint64_t first_ok_leaf; + uint64_t first_ok_leaf, orig_di_blocks; struct gfs2_buffer_head *lbh; int lindex; struct gfs2_sbd *sdp = ip->i_sbd; - int ref_count = 0, old_was_dup; + int ref_count = 0, orig_ref_count, orig_di_depth, orig_di_height, old_was_dup; uint64_t *tbl; + int tbl_valid; tbl = get_dir_hash(ip); if (tbl == NULL) { perror("get_dir_hash"); return -1; } + tbl_valid = 1; + orig_di_depth = ip->i_di.di_depth; + orig_di_height = ip->i_di.di_height; + orig_di_blocks = ip->i_di.di_blocks; /* Turn off system readahead */ posix_fadvise(sdp->device_fd, 0, 0, POSIX_FADV_RANDOM); @@ -752,6 +757,21 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) for (lindex = 0; lindex < hsize; lindex++) { if (fsck_abort) break; + + if (!tbl_valid) { + free(tbl); + log_debug(_("Re-reading 0x%llx hash table.\n"), + (unsigned long long)ip->i_di.di_num.no_addr); + tbl = get_dir_hash(ip); + if (tbl == NULL) { + perror("get_dir_hash"); + return -1; + } + tbl_valid = 1; + orig_di_depth = ip->i_di.di_depth; + orig_di_height = ip->i_di.di_height; + orig_di_blocks = ip->i_di.di_blocks; + } leaf_no = be64_to_cpu(tbl[lindex]); /* GFS has multiple indirect pointers to the same leaf @@ -765,6 +785,7 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) ref_count++; continue; } + orig_ref_count = ref_count; do { if (fsck_abort) { @@ -775,6 +796,8 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) error = check_leaf(ip, lindex, pass, &ref_count, &leaf_no, old_leaf, &bad_leaf, first_ok_leaf, &leaf, &oldleaf); + if (ref_count != orig_ref_count) + tbl_valid = 0; old_was_dup = (error == -EEXIST); old_leaf = leaf_no; memcpy(&oldleaf, &leaf, sizeof(oldleaf)); @@ -784,6 +807,28 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) log_debug( _("Leaf chain (0x%llx) detected.\n"), (unsigned long long)leaf_no); } while (1); /* while we have chained leaf blocks */ + if (orig_di_depth != ip->i_di.di_depth) { + log_debug(_("Depth of 0x%llx changed from %d to %d\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + orig_di_depth, ip->i_di.di_depth); + tbl_valid = 0; + } + if (orig_di_height != ip->i_di.di_height) { + log_debug(_("Height of 0x%llx changed from %d to " + "%d\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + orig_di_height, ip->i_di.di_height); + tbl_valid = 0; + } + if (orig_di_blocks != ip->i_di.di_blocks) { + log_debug(_("Block count of 0x%llx changed from %llu " + "to %llu\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)orig_di_blocks, + (unsigned long long)ip->i_di.di_blocks); + tbl_valid = 0; + } + lindex += ref_count; } /* for every leaf block */ free(tbl); posix_fadvise(sdp->device_fd, 0, 0, POSIX_FADV_NORMAL);

1 0

gfs2-utils: master - fsck.gfs2: Verify dirent hash values correspond to proper leaf block
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=5d6bf12f59… Commit: 5d6bf12f59b07e9eb6e926408b21b5ba49132522 Parent: f52d1afdd01ec941804b6a26141e5628450521c3 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Mon Feb 25 13:28:00 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:46 2013 -0500 fsck.gfs2: Verify dirent hash values correspond to proper leaf block This patch checks to make sure all the dirents on a leaf block have hash values that are actually appropriate to the leaf block. With extended hashing, the file name is used to generate a hash value. Today's fsck checks that the hash value is proper to the file name, but that's not enough. The hash value is also shifted by a certain amount (determined by i_depth) to produce an index into the hash table. For example, suppose di_depth == 8. Valid indexes into the hash table go from 0 to 1<<di_depth-1 which is 1<<8-1 which is 256-1 or 255. Now suppose we have four actual leaf blocks, and each leaf block is repeated 64 (0x40) times in the index. So the hash table, indexed by file name would be something like: entries 00-3f = first leaf block entries 40-7f = second leaf block entries 80-bf = third leaf block entries c0-ff = fourth leaf block So ht index = name->hash >> (32 - ip->i_depth). In our example, i_depth is 8, so: ht index == hash >> (32 - 8) == hash >> 24 In this case, the hash value is shifted by a certain amount to get the index into the table. For example, file name "Solar" has hash value 0x59f4dde1. So the hr index == 0x59f4dde1 >> 24 == 0x59. Therefore, a file with the name "Solar" better appear on the second leaf, which covers index values from 0x40 to 0x7f. What this patch does is verify that all the dirents on the first leaf block have a hash value starting with 0x00 to 0x3f, and all the dirents on the second leaf block have a hash value starting with 0x40 to 0x7f, and so forth. If they appear on the wrong leaf block, they need to be relocated to the proper leaf block. --- gfs2/fsck/metawalk.c | 12 ++- gfs2/fsck/metawalk.h | 3 +- gfs2/fsck/pass1.c | 2 +- gfs2/fsck/pass1b.c | 5 +- gfs2/fsck/pass2.c | 190 ++++++++++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 197 insertions(+), 15 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 05678dd..5bcac77 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -342,7 +342,8 @@ static void dirblk_truncate(struct gfs2_inode *ip, struct gfs2_dirent *fixb, * -1 - error occurred */ static int check_entries(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, - int type, uint32_t *count, struct metawalk_fxns *pass) + int type, uint32_t *count, int lindex, + struct metawalk_fxns *pass) { struct gfs2_dirent *dent; struct gfs2_dirent de, *prev; @@ -450,6 +451,7 @@ static int check_entries(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, } else { error = pass->check_dentry(ip, dent, prev, bh, filename, count, + lindex, pass->private); if (error < 0) { stack; @@ -589,7 +591,8 @@ static int check_leaf(struct gfs2_inode *ip, int lindex, } if (pass->check_dentry && is_dir(&ip->i_di, sdp->gfs1)) { - error = check_entries(ip, lbh, DIR_EXHASH, &count, pass); + error = check_entries(ip, lbh, DIR_EXHASH, &count, lindex, + pass); if (skip_this_pass || fsck_abort) goto out; @@ -1450,7 +1453,7 @@ int check_linear_dir(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, int error = 0; uint32_t count = 0; - error = check_entries(ip, bh, DIR_LINEAR, &count, pass); + error = check_entries(ip, bh, DIR_LINEAR, &count, 0, pass); if (error < 0) { stack; return -1; @@ -1481,7 +1484,8 @@ int check_dir(struct gfs2_sbd *sdp, uint64_t block, struct metawalk_fxns *pass) static int remove_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, struct gfs2_dirent *prev_de, struct gfs2_buffer_head *bh, - char *filename, uint32_t *count, void *private) + char *filename, uint32_t *count, int lindex, + void *private) { /* the metawalk_fxn's private field must be set to the dentry * block we want to clear */ diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index c43baf0..bef99ae 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -85,7 +85,8 @@ struct metawalk_fxns { int (*check_dentry) (struct gfs2_inode *ip, struct gfs2_dirent *de, struct gfs2_dirent *prev, struct gfs2_buffer_head *bh, - char *filename, uint32_t *count, void *private); + char *filename, uint32_t *count, + int lindex, void *private); int (*check_eattr_entry) (struct gfs2_inode *ip, struct gfs2_buffer_head *leaf_bh, struct gfs2_ea_header *ea_hdr, diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 0d4da5d..dd6b958 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -148,7 +148,7 @@ static int resuscitate_metalist(struct gfs2_inode *ip, uint64_t block, static int resuscitate_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, struct gfs2_dirent *prev_de, struct gfs2_buffer_head *bh, char *filename, - uint32_t *count, void *priv) + uint32_t *count, int lindex, void *priv) { struct gfs2_sbd *sdp = ip->i_sbd; struct gfs2_dirent dentry, *de; diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index e8c39be..f3f90ef 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -50,7 +50,8 @@ static int check_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, void *private); static int find_dentry(struct gfs2_inode *ip, struct gfs2_dirent *de, struct gfs2_dirent *prev, struct gfs2_buffer_head *bh, - char *filename, uint32_t *count, void *priv); + char *filename, uint32_t *count, int lindex, + void *priv); struct metawalk_fxns find_refs = { .private = NULL, @@ -174,7 +175,7 @@ static int check_dir_dup_ref(struct gfs2_inode *ip, struct gfs2_dirent *de, static int find_dentry(struct gfs2_inode *ip, struct gfs2_dirent *de, struct gfs2_dirent *prev, struct gfs2_buffer_head *bh, char *filename, - uint32_t *count, void *priv) + uint32_t *count, int lindex, void *priv) { struct osi_node *n, *next = NULL; osi_list_t *tmp2; diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 00e226c..babac15 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -301,6 +301,166 @@ static int hash_table_index(uint32_t hash, struct gfs2_inode *ip) return hash >> (32 - ip->i_di.di_depth); } +static int hash_table_max(int lindex, struct gfs2_inode *ip, + struct gfs2_buffer_head *bh) +{ + struct gfs2_leaf *leaf = (struct gfs2_leaf *)bh->b_data; + return (1 << (ip->i_di.di_depth - be16_to_cpu(leaf->lf_depth))) + + lindex - 1; +} + +static int check_leaf_depth(struct gfs2_inode *ip, uint64_t leaf_no, + int ref_count, struct gfs2_buffer_head *lbh) +{ + struct gfs2_leaf *leaf = (struct gfs2_leaf *)lbh->b_data; + int cur_depth = be16_to_cpu(leaf->lf_depth); + int exp_count = 1 << (ip->i_di.di_depth - cur_depth); + int divisor; + int factor, correct_depth; + + if (exp_count == ref_count) + return 0; + + factor = 0; + divisor = ref_count; + while (divisor > 1) { + factor++; + divisor >>= 1; + } + correct_depth = ip->i_di.di_depth - factor; + if (cur_depth == correct_depth) + return 0; + + log_err(_("Leaf block %llu (0x%llx) in dinode %llu (0x%llx) has the " + "wrong depth: is %d (length %d), should be %d (length " + "%d).\n"), + (unsigned long long)leaf_no, (unsigned long long)leaf_no, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr, + cur_depth, ref_count, correct_depth, exp_count); + if (!query( _("Fix the leaf block? (y/n)"))) { + log_err( _("The leaf block was not fixed.\n")); + return 0; + } + + leaf->lf_depth = cpu_to_be16(correct_depth); + bmodified(lbh); + log_err( _("The leaf block depth was fixed.\n")); + return 1; +} + +/* wrong_leaf: Deal with a dirent discovered to be on the wrong leaf block + * + * Returns: 1 if the dirent is to be removed, 0 if it needs to be kept, + * or -1 on error + */ +static int wrong_leaf(struct gfs2_inode *ip, struct gfs2_inum *entry, + const char *tmp_name, int lindex, int lindex_max, + int hash_index, struct gfs2_buffer_head *bh, + struct dir_status *ds, struct gfs2_dirent *dent, + struct gfs2_dirent *de, struct gfs2_dirent *prev_de, + uint32_t *count, uint8_t q) +{ + struct gfs2_sbd *sdp = ip->i_sbd; + struct gfs2_buffer_head *dest_lbh; + uint64_t planned_leaf, real_leaf; + int li, dest_ref, error; + uint64_t *tbl; + + log_err(_("Directory entry '%s' at block %lld (0x%llx) is on the " + "wrong leaf block.\n"), tmp_name, + (unsigned long long)entry->no_addr, + (unsigned long long)entry->no_addr); + log_err(_("Leaf index is: 0x%x. The range for this leaf block is " + "0x%x - 0x%x\n"), hash_index, lindex, lindex_max); + if (!query( _("Move the misplaced directory entry to " + "a valid leaf block? (y/n) "))) { + log_err( _("Misplaced directory entry not moved.\n")); + return 0; + } + + /* check the destination leaf block's depth */ + tbl = get_dir_hash(ip); + if (tbl == NULL) { + perror("get_dir_hash"); + return -1; + } + planned_leaf = be64_to_cpu(tbl[hash_index]); + log_err(_("Moving it from leaf %llu (0x%llx) to %llu (0x%llx)\n"), + (unsigned long long)be64_to_cpu(tbl[lindex]), + (unsigned long long)be64_to_cpu(tbl[lindex]), + (unsigned long long)planned_leaf, + (unsigned long long)planned_leaf); + /* Can't trust lf_depth; we have to count */ + dest_ref = 0; + for (li = 0; li < (1 << ip->i_di.di_depth); li++) { + if (be64_to_cpu(tbl[li]) == planned_leaf) + dest_ref++; + else if (dest_ref) + break; + } + dest_lbh = bread(sdp, planned_leaf); + check_leaf_depth(ip, planned_leaf, dest_ref, dest_lbh); + brelse(dest_lbh); + free(tbl); + + /* check if it's already on the correct leaf block */ + error = dir_search(ip, tmp_name, de->de_name_len, NULL, &de->de_inum); + if (!error) { + log_err(_("The misplaced directory entry already appears on " + "the correct leaf block.\n")); + log_err( _("The bad duplicate directory entry " + "'%s' was cleared.\n"), tmp_name); + return 1; /* nuke the dent upon return */ + } + + if (dir_add(ip, tmp_name, de->de_name_len, &de->de_inum, + de->de_type) == 0) { + log_err(_("The misplaced directory entry was moved to a " + "valid leaf block.\n")); + gfs2_get_leaf_nr(ip, hash_index, &real_leaf); + if (real_leaf != planned_leaf) { + log_err(_("The planned leaf was split. The new leaf " + "is: %llu (0x%llx)"), + (unsigned long long)real_leaf, + (unsigned long long)real_leaf); + fsck_blockmap_set(ip, real_leaf, _("split leaf"), + gfs2_indir_blk); + } + /* If the misplaced dirent was supposed to be earlier in the + hash table, we need to adjust our counts for the blocks + that have already been processed. If it's supposed to + appear later, we'll count it has part of our normal + processing when we get to that leaf block later on in the + hash table. */ + if (hash_index > lindex) { + log_err(_("Accounting deferred.\n")); + return 1; /* nuke the dent upon return */ + } + /* If we get here, it's because we moved a dent to another + leaf, but that leaf has already been processed. So we have + to nuke the dent from this leaf when we return, but we + still need to do the "good dent" accounting. */ + error = incr_link_count(*entry, ip, _("valid reference")); + if (error > 0 && + bad_formal_ino(ip, dent, *entry, tmp_name, q, de, bh) == 1) + return 1; /* nuke it */ + + /* You cannot do this: + (*count)++; + The reason is: *count is the count of dentries on the leaf, + and we moved the dentry to a previous leaf within the same + directory dinode. So the directory counts still get + incremented, but not leaf entries. When we called dir_add + above, it should have fixed that prev leaf's lf_entries. */ + ds->entry_count++; + return 1; + } else { + log_err(_("Error moving directory entry.\n")); + return 1; /* nuke it */ + } +} + /* basic_dentry_checks - fundamental checks for directory entries * * @ip: pointer to the incode inode structure @@ -522,9 +682,9 @@ static int basic_dentry_checks(struct gfs2_inode *ip, struct gfs2_dirent *dent, /* FIXME: should maybe refactor this a bit - but need to deal with * FIXMEs internally first */ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, - struct gfs2_dirent *prev_de, - struct gfs2_buffer_head *bh, char *filename, - uint32_t *count, void *priv) + struct gfs2_dirent *prev_de, + struct gfs2_buffer_head *bh, char *filename, + uint32_t *count, int lindex, void *priv) { struct gfs2_sbd *sdp = ip->i_sbd; uint8_t q = 0; @@ -534,6 +694,8 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, int error; struct gfs2_inode *entry_ip = NULL; struct gfs2_dirent dentry, *de; + int hash_index; /* index into the hash table based on the hash */ + int lindex_max; /* largest acceptable hash table index for hash */ memset(&dentry, 0, sizeof(struct gfs2_dirent)); gfs2_dirent_in(&dentry, (char *)dent); @@ -674,6 +836,21 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, ds->dotdotdir = 1; goto dentry_is_valid; } + /* If this is an exhash directory, make sure the dentries in the leaf + block have a hash table index that fits */ + if (ip->i_di.di_flags & GFS2_DIF_EXHASH) { + hash_index = hash_table_index(de->de_hash, ip); + lindex_max = hash_table_max(lindex, ip, bh); + if (hash_index < lindex || hash_index > lindex_max) { + int nuke_dent; + + nuke_dent = wrong_leaf(ip, &entry, tmp_name, lindex, + lindex_max, hash_index, bh, ds, + dent, de, prev_de, count, q); + if (nuke_dent) + goto nuke_dentry; + } + } /* After this point we're only concerned with directories */ if (q != gfs2_inode_dir) { @@ -705,10 +882,9 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, dentry_is_valid: /* This directory inode links to this inode via this dentry */ error = incr_link_count(entry, ip, _("valid reference")); - if (error > 0) { - if (bad_formal_ino(ip, dent, entry, tmp_name, q, de, bh) == 1) - goto nuke_dentry; - } + if (error > 0 && + bad_formal_ino(ip, dent, entry, tmp_name, q, de, bh) == 1) + goto nuke_dentry; (*count)++; ds->entry_count++;

1 0

gfs2-utils: master - fsck.gfs2: Misc cleanups
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=f52d1afdd0… Commit: f52d1afdd01ec941804b6a26141e5628450521c3 Parent: 1bc68e624966a4d73c98500452d69290cd15caa8 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Fri Feb 22 06:31:22 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:45 2013 -0500 fsck.gfs2: Misc cleanups This patch contains some trivial cleanups. --- gfs2/fsck/metawalk.c | 6 ++++++ gfs2/fsck/pass1.c | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 32afc46..05678dd 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -335,6 +335,7 @@ static void dirblk_truncate(struct gfs2_inode *ip, struct gfs2_dirent *fixb, * bh - buffer for the leaf block * type - type of block this is (linear or exhash) * @count - set to the count entries + * @lindex - the last inde * @pass - structure pointing to pass-specific functions * * returns: 0 - good block or it was repaired to be good @@ -515,6 +516,8 @@ static int warn_and_patch(struct gfs2_inode *ip, uint64_t *leaf_no, /** * check_leaf - check a leaf block for errors + * Reads in the leaf block + * Leaves the buffer around for further analysis (caller must brelse) */ static int check_leaf(struct gfs2_inode *ip, int lindex, struct metawalk_fxns *pass, int *ref_count, @@ -1170,6 +1173,9 @@ static void free_metalist(struct gfs2_inode *ip, osi_list_t *mlp) * This includes hash table blocks for directories * which are technically "data" in the bitmap. * + * Returns: 0 - all is well, process the blocks this metadata references + * 1 - something went wrong, but process the sub-blocks anyway + * -1 - something went wrong, so don't process the sub-blocks * @ip: * @mlp: */ diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index cc69e84..0d4da5d 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -211,7 +211,7 @@ static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private) So we know it's a leaf block. */ q = block_type(block); if (q != gfs2_block_free) { - log_err( _("Found duplicate block %llu (0x%llx) referenced " + log_err( _("Found duplicate block #%llu (0x%llx) referenced " "as a directory leaf in dinode " "%llu (0x%llx) - was marked %d (%s)\n"), (unsigned long long)block, @@ -264,7 +264,7 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, } q = block_type(block); if (q != gfs2_block_free) { - log_err( _("Found duplicate block %llu (0x%llx) referenced " + 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,

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

cluster-commits May 2013