cluster-commits September 2015

cluster-commits@lists.stg.fedorahosted.org

2 participants
30 discussions

gfs2-utils: master - fsck.gfs2: Clone duplicate data block pointers
by Bob Peterson 29 Sep '15

29 Sep '15

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=0d899f6e8d… Commit: 0d899f6e8d0301b7fb017d374b3b7bf61b53260e Parent: 5263b863b9df015d2109f1faa1f6046fc7383c66 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Sep 24 15:40:24 2015 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Sep 29 13:23:55 2015 -0500 fsck.gfs2: Clone duplicate data block pointers Before this patch, pass1b would find duplicate data block pointers in two different inodes, then arbitrarily keep one of them and delete the other. That's harsh. This patch adds the ability to clone the data block so each inode has its own copy. --- gfs2/fsck/pass1b.c | 164 ++++++++++++++++++++++++++++++++++++++++------------ gfs2/fsck/util.c | 13 +++- 2 files changed, 136 insertions(+), 41 deletions(-) diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 4c41fef..16c0aec 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -33,6 +33,16 @@ struct clone_target { int first; }; +struct meta_blk_ref { + uint64_t block; /* block to locate */ + uint64_t metablock; /* returned metadata block addr containing ref */ + int off; /* offset to the reference within the buffer */ +}; + +static int clone_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private, + struct gfs2_buffer_head *bh, uint64_t *ptr); + static void log_inode_reference(struct duptree *dt, osi_list_t *tmp, int inval) { char reftypestring[32]; @@ -57,6 +67,108 @@ static void log_inode_reference(struct duptree *dt, osi_list_t *tmp, int inval) (unsigned long long)dt->block, reftypestring); } +static int findref_meta(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) +{ + *is_valid = 1; + *was_duplicate = 0; + return meta_is_good; +} + +static int findref_data(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private, + struct gfs2_buffer_head *bh, uint64_t *ptr) +{ + struct meta_blk_ref *mbr = (struct meta_blk_ref *)private; + + if (block == mbr->block) { + mbr->metablock = bh->b_blocknr; + mbr->off = (ptr - (uint64_t *)bh->b_data); + log_debug("Duplicate data reference located on metadata " + "block 0x%llx, offset 0x%x\n", + (unsigned long long)mbr->metablock, mbr->off); + } + return meta_is_good; +} + +static void clone_data_block(struct gfs2_sbd *sdp, struct duptree *dt, + struct inode_with_dups *id) +{ + struct meta_blk_ref metaref = { .block = dt->block, }; + struct metawalk_fxns find1ref_fxns = { + .private = &metaref, + .check_metalist = findref_meta, + .check_data = findref_data, + }; + struct clone_target clone = {.dup_block = dt->block,}; + struct gfs2_inode *ip; + struct gfs2_buffer_head *bh; + uint64_t *ptr; + + if (!(query(_("Okay to clone data block %lld (0x%llx) for inode " + "%lld (0x%llx)? (y/n) "), + (unsigned long long)dt->block, + (unsigned long long)dt->block, + (unsigned long long)id->block_no, + (unsigned long long)id->block_no))) { + log_warn(_("The duplicate reference was not cloned.\n")); + return; + } + ip = fsck_load_inode(sdp, id->block_no); + check_metatree(ip, &find1ref_fxns); + if (metaref.metablock == 0) { + log_err(_("Unable to clone data block.\n")); + } else { + if (metaref.metablock != id->block_no) + bh = bread(sdp, metaref.metablock); + else + bh = ip->i_bh; + ptr = (uint64_t *)bh->b_data + metaref.off; + clone_data(ip, 0, dt->block, &clone, bh, ptr); + if (metaref.metablock != id->block_no) + brelse(bh); + else + bmodified(ip->i_bh); + } + fsck_inode_put(&ip); /* out, brelse, free */ +} + +/* revise_dup_handler - get current information about a duplicate reference + * + * Function resolve_dup_references can delete dinodes that reference blocks + * which may have duplicate references. Therefore, the duplicate tree is + * constantly being changed. This function revises the duplicate handler so + * that it accurately matches what's in the duplicate tree regarding this block + */ +static void revise_dup_handler(uint64_t dup_blk, struct dup_handler *dh) +{ + osi_list_t *tmp; + struct duptree *dt; + struct inode_with_dups *id; + + dh->ref_inode_count = 0; + dh->ref_count = 0; + dh->dt = NULL; + + dt = dupfind(dup_blk); + if (!dt) + return; + + dh->dt = dt; + /* Count the duplicate references, both valid and invalid */ + 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, &dt->ref_inode_list) { + id = osi_list_entry(tmp, struct inode_with_dups, list); + dh->ref_inode_count++; + dh->ref_count += id->dup_count; + } +} + /* * resolve_dup_references - resolve all but the last dinode that has a * duplicate reference to a given block. @@ -164,6 +276,12 @@ static void resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt, dup_listent_delete(dt, id); continue; } + } else if (acceptable_ref == ref_types && + this_ref == ref_as_data) { + clone_data_block(sdp, dt, id); + dup_listent_delete(dt, id); + revise_dup_handler(dt->block, dh); + continue; } else if (!(query( _("Okay to delete %s inode %lld (0x%llx)? " "(y/n) "), (inval ? _("invalidated") : ""), @@ -215,9 +333,11 @@ static void resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt, ip->i_di.di_flags &= ~GFS2_DIF_EA_INDIRECT; bmodified(ip->i_bh); dup_listent_delete(dt, id); + (dh->ref_inode_count)--; } else { /* Clear the EAs for the inode first */ check_inode_eattr(ip, &pass1b_fxns_delete); + (dh->ref_inode_count)--; } /* If the reference was as metadata or data, we've got a corrupt dinode that will be deleted. */ @@ -261,41 +381,6 @@ static void resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt, return; } -/* revise_dup_handler - get current information about a duplicate reference - * - * Function resolve_dup_references can delete dinodes that reference blocks - * which may have duplicate references. Therefore, the duplicate tree is - * constantly being changed. This function revises the duplicate handler so - * that it accurately matches what's in the duplicate tree regarding this block - */ -static void revise_dup_handler(uint64_t dup_blk, struct dup_handler *dh) -{ - osi_list_t *tmp; - struct duptree *dt; - struct inode_with_dups *id; - - dh->ref_inode_count = 0; - dh->ref_count = 0; - dh->dt = NULL; - - dt = dupfind(dup_blk); - if (!dt) - return; - - dh->dt = dt; - /* Count the duplicate references, both valid and invalid */ - 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, &dt->ref_inode_list) { - id = osi_list_entry(tmp, struct inode_with_dups, list); - dh->ref_inode_count++; - dh->ref_count += id->dup_count; - } -} - static int clone_check_meta(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, int *is_valid, int *was_duplicate, void *private) @@ -333,8 +418,8 @@ static int clone_data(struct gfs2_inode *ip, uint64_t metablock, clonet->first = 0; return 0; } - log_err(_("Error: Inode %lld (0x%llx)'s subsequent reference to " - "block %lld (0x%llx) is an error.\n"), + log_err(_("Error: Inode %lld (0x%llx)'s reference to block %lld " + "(0x%llx) should be replaced with a clone.\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)block, (unsigned long long)block); @@ -398,6 +483,11 @@ static void clone_dup_ref_in_inode(struct gfs2_inode *ip, struct duptree *dt) .check_data = clone_data, }; + log_err(_("There are multiple references to block %lld (0x%llx) in " + "inode %lld (0x%llx)\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)dt->block, (unsigned long long)dt->block); error = check_metatree(ip, &pass1b_fxns_clone); if (error) { log_err(_("Error cloning duplicate reference(s) to block %lld " diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index 2745692..84402fc 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -18,7 +18,7 @@ #include "util.h" const char *reftypes[ref_types + 1] = {"data", "metadata", - "extended attribute", "itself", + "an extended attribute", "an inode", "unimportant"}; void big_file_comfort(struct gfs2_inode *ip, uint64_t blks_checked) @@ -401,9 +401,14 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, (unsigned long long)ip->i_di.di_num.no_addr); if (first) log_info( _("This is the original reference.\n")); - else - log_info( _("This brings the total to: %d references\n"), - dt->refs); + else { + /* Check for duplicate refs to the same block in one inode. */ + if (id->dup_count > 1) + dt->dup_flags |= DUPFLAG_REF1_FOUND; + log_info( _("This brings the total to: %d inode references, " + "%d from this inode.\n"), + dt->refs, id->dup_count); + } return meta_is_good; }

1 0

gfs2-utils: master - fsck.gfs2: Remove pass1c in favor of processing in pass1
by Bob Peterson 29 Sep '15

29 Sep '15

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=5263b863b9… Commit: 5263b863b9df015d2109f1faa1f6046fc7383c66 Parent: 40127ca5abc87bd93d748884a8b8b58d83afc234 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Sep 24 10:24:39 2015 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Sep 29 13:23:51 2015 -0500 fsck.gfs2: Remove pass1c in favor of processing in pass1 This patch eliminates pass1c and moves its checks to pass1. In pass1c, it used to delete bad extended attribute blocks, but in pass1, we have to go about things a little differently. Since pass1 is prior to resolving duplicate references (pass1b) it must "undo" the references rather than delete them. --- gfs2/fsck/Makefile.am | 1 - gfs2/fsck/main.c | 1 - gfs2/fsck/pass1.c | 98 +++++++++++------ gfs2/fsck/pass1c.c | 285 ------------------------------------------------ gfs2/fsck/util.c | 2 - gfs2/libgfs2/libgfs2.h | 2 - 6 files changed, 62 insertions(+), 327 deletions(-) diff --git a/gfs2/fsck/Makefile.am b/gfs2/fsck/Makefile.am index fb93110..73d957e 100644 --- a/gfs2/fsck/Makefile.am +++ b/gfs2/fsck/Makefile.am @@ -21,7 +21,6 @@ fsck_gfs2_SOURCES = \ metawalk.c \ pass1b.c \ pass1.c \ - pass1c.c \ pass2.c \ pass3.c \ pass4.c \ diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c index 658cd17..08343ca 100644 --- a/gfs2/fsck/main.c +++ b/gfs2/fsck/main.c @@ -237,7 +237,6 @@ struct fsck_pass { static const struct fsck_pass passes[] = { { .name = "pass1", .f = pass1 }, { .name = "pass1b", .f = pass1b }, - { .name = "pass1c", .f = pass1c }, { .name = "pass2", .f = pass2 }, { .name = "pass3", .f = pass3 }, { .name = "pass4", .f = pass4 }, diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 895d1d1..875f3d3 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -684,8 +684,6 @@ static int finish_eattr_indir(struct gfs2_inode *ip, int leaf_pointers, int leaf_pointer_errors, void *private) { struct block_count *bc = (struct block_count *) private; - osi_list_t *head; - struct special_blocks *b = NULL; if (leaf_pointer_errors == leaf_pointers) /* All eas were bad */ return ask_remove_inode_eattr(ip, bc); @@ -693,18 +691,6 @@ static int finish_eattr_indir(struct gfs2_inode *ip, int leaf_pointers, "attribute block\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); - /* Mark the inode as having an eattr in the block map - so pass1c can check it. We may have previously added this inode - to the eattr_blocks list and if we did, it would be the first - one on the list. So check that one only (to save time) and - if that one matches, no need to add it again. */ - if (!osi_list_empty(&ip->i_sbd->eattr_blocks.list)) { - head = &ip->i_sbd->eattr_blocks.list; - b = osi_list_entry(head->next, struct special_blocks, list); - } - if (!b || b->block != ip->i_di.di_num.no_addr) - gfs2_special_add(&ip->i_sbd->eattr_blocks, - ip->i_di.di_num.no_addr); if (!leaf_pointer_errors) return 0; log_err( _("Inode %lld (0x%llx) has recoverable indirect " @@ -774,7 +760,7 @@ static int check_ealeaf_block(struct gfs2_inode *ip, uint64_t block, int btype, } /* Point of confusion: We've got to set the ea block itself to GFS2_BLKST_USED here. Elsewhere we mark the inode with - gfs2_eattr_block meaning it contains an eattr for pass1c. */ + gfs2_eattr_block meaning it contains an eattr. */ fsck_blockmap_set(ip, block, _("Extended Attribute"), sdp->gfs1 ? GFS2_BLKST_DINODE : GFS2_BLKST_USED); bc->ea_count++; @@ -830,24 +816,7 @@ static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block, void *private) { struct gfs2_sbd *sdp = ip->i_sbd; - osi_list_t *head; - struct special_blocks *b = NULL; - /* This inode contains an eattr - it may be invalid, but the - * eattr attributes points to a non-zero block. - * Clarification: If we're here we're checking a leaf block, and the - * source dinode needs to be marked as having extended attributes. - * That instructs pass1c to check the contents of the ea blocks. */ - log_debug( _("Setting inode %lld (0x%llx) as having eattr " - "block(s) attached.\n"), - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); - if (!osi_list_empty(&ip->i_sbd->eattr_blocks.list)) { - head = &ip->i_sbd->eattr_blocks.list; - b = osi_list_entry(head->next, struct special_blocks, list); - } - if (!b || b->block != ip->i_di.di_num.no_addr) - gfs2_special_add(&sdp->eattr_blocks, ip->i_di.di_num.no_addr); if (!valid_block(sdp, block)) { log_warn( _("Inode #%llu (0x%llx): Extended Attribute leaf " "block #%llu (0x%llx) is invalid or out of " @@ -863,6 +832,41 @@ static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block, return check_ealeaf_block(ip, block, GFS2_METATYPE_EA, bh, private); } +static int ask_remove_eattr_entry(struct gfs2_sbd *sdp, + struct gfs2_buffer_head *leaf_bh, + struct gfs2_ea_header *curr, + struct gfs2_ea_header *prev, + int fix_curr, int fix_curr_len) +{ + if (!query( _("Remove the bad Extended Attribute entry? (y/n) "))) { + log_err( _("Bad Extended Attribute not removed.\n")); + return 0; + } + if (fix_curr) + curr->ea_flags |= GFS2_EAFLAG_LAST; + if (fix_curr_len) { + uint32_t max_size = sdp->sd_sb.sb_bsize; + uint32_t offset = (uint32_t)(((unsigned long)curr) - + ((unsigned long)leaf_bh->b_data)); + curr->ea_rec_len = cpu_to_be32(max_size - offset); + } + if (!prev) + curr->ea_type = GFS2_EATYPE_UNUSED; + else { + uint32_t tmp32 = be32_to_cpu(curr->ea_rec_len) + + be32_to_cpu(prev->ea_rec_len); + prev->ea_rec_len = cpu_to_be32(tmp32); + if (curr->ea_flags & GFS2_EAFLAG_LAST) + prev->ea_flags |= GFS2_EAFLAG_LAST; + } + log_err( _("Bad Extended Attribute at block #%llu" + " (0x%llx) removed.\n"), + (unsigned long long)leaf_bh->b_blocknr, + (unsigned long long)leaf_bh->b_blocknr); + bmodified(leaf_bh); + return 1; +} + static int check_eattr_entries(struct gfs2_inode *ip, struct gfs2_buffer_head *leaf_bh, struct gfs2_ea_header *ea_hdr, @@ -871,12 +875,32 @@ static int check_eattr_entries(struct gfs2_inode *ip, { struct gfs2_sbd *sdp = ip->i_sbd; char ea_name[256]; + uint32_t offset = (uint32_t)(((unsigned long)ea_hdr) - + ((unsigned long)leaf_bh->b_data)); + uint32_t max_size = sdp->sd_sb.sb_bsize; uint32_t avail_size; int max_ptrs; if (!ea_hdr->ea_name_len){ - /* Skip this entry for now */ - return 1; + log_err( _("EA has name length of zero\n")); + return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, + ea_hdr_prev, 1, 1); + } + if (offset + be32_to_cpu(ea_hdr->ea_rec_len) > max_size){ + log_err( _("EA rec length too long\n")); + return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, + ea_hdr_prev, 1, 1); + } + if (offset + be32_to_cpu(ea_hdr->ea_rec_len) == max_size && + (ea_hdr->ea_flags & GFS2_EAFLAG_LAST) == 0){ + log_err( _("last EA has no last entry flag\n")); + return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, + ea_hdr_prev, 0, 0); + } + if (!ea_hdr->ea_name_len){ + log_err( _("EA has name length of zero\n")); + return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, + ea_hdr_prev, 0, 0); } memset(ea_name, 0, sizeof(ea_name)); @@ -888,7 +912,8 @@ static int check_eattr_entries(struct gfs2_inode *ip, /* Skip invalid entry */ log_err(_("EA (%s) type is invalid (%d > %d).\n"), ea_name, ea_hdr->ea_type, GFS2_EATYPE_LAST); - return 1; + return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, + ea_hdr_prev, 0, 0); } if (!ea_hdr->ea_num_ptrs) @@ -902,7 +927,8 @@ static int check_eattr_entries(struct gfs2_inode *ip, ea_name); log_err(_(" Required: %d\n Reported: %d\n"), max_ptrs, ea_hdr->ea_num_ptrs); - return 1; + return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, + ea_hdr_prev, 0, 0); } else { log_debug( _(" Pointers Required: %d\n Pointers Reported: %d\n"), max_ptrs, ea_hdr->ea_num_ptrs); diff --git a/gfs2/fsck/pass1c.c b/gfs2/fsck/pass1c.c deleted file mode 100644 index aa905da..0000000 --- a/gfs2/fsck/pass1c.c +++ /dev/null @@ -1,285 +0,0 @@ -#include "clusterautoconfig.h" - -#include <inttypes.h> -#include <stdlib.h> -#include <string.h> -#include <unistd.h> -#include <libintl.h> -#define _(String) gettext(String) - -#include <logging.h> -#include "libgfs2.h" -#include "fsck.h" -#include "util.h" -#include "metawalk.h" - -struct metawalk_fxns pass1c_fxns_delete = { - .private = NULL, - .check_eattr_indir = delete_eattr_indir, - .check_eattr_leaf = delete_eattr_leaf, -}; - -static int remove_eattr_entry(struct gfs2_sbd *sdp, - struct gfs2_buffer_head *leaf_bh, - struct gfs2_ea_header *curr, - struct gfs2_ea_header *prev) -{ - if (!prev) - curr->ea_type = GFS2_EATYPE_UNUSED; - else { - uint32_t tmp32 = be32_to_cpu(curr->ea_rec_len) + - be32_to_cpu(prev->ea_rec_len); - prev->ea_rec_len = cpu_to_be32(tmp32); - if (curr->ea_flags & GFS2_EAFLAG_LAST) - prev->ea_flags |= GFS2_EAFLAG_LAST; - } - log_err( _("Bad Extended Attribute at block #%llu" - " (0x%llx) removed.\n"), - (unsigned long long)leaf_bh->b_blocknr, - (unsigned long long)leaf_bh->b_blocknr); - bmodified(leaf_bh); - return 0; -} - -static int ask_remove_eattr_entry(struct gfs2_sbd *sdp, - struct gfs2_buffer_head *leaf_bh, - struct gfs2_ea_header *curr, - struct gfs2_ea_header *prev, - int fix_curr, int fix_curr_len) -{ - if (query( _("Remove the bad Extended Attribute entry? (y/n) "))) { - if (fix_curr) - curr->ea_flags |= GFS2_EAFLAG_LAST; - if (fix_curr_len) { - uint32_t max_size = sdp->sd_sb.sb_bsize; - uint32_t offset = (uint32_t)(((unsigned long)curr) - - ((unsigned long)leaf_bh->b_data)); - curr->ea_rec_len = cpu_to_be32(max_size - offset); - } - if (remove_eattr_entry(sdp, leaf_bh, curr, prev)) { - stack; - return -1; - } - } else { - log_err( _("Bad Extended Attribute not removed.\n")); - } - return 1; -} - -static int ask_remove_eattr(struct gfs2_inode *ip) -{ - if (query( _("Remove the bad Extended Attribute? (y/n) "))) { - check_inode_eattr(ip, &pass1c_fxns_delete); - bmodified(ip->i_bh); - log_err( _("Bad Extended Attribute removed.\n")); - return 1; - } - log_err( _("Bad Extended Attribute not removed.\n")); - return 0; -} - -static int check_eattr_indir(struct gfs2_inode *ip, uint64_t block, - uint64_t parent, struct gfs2_buffer_head **bh, - void *private) -{ - struct gfs2_sbd *sdp = ip->i_sbd; - uint8_t q; - struct gfs2_buffer_head *indir_bh = NULL; - - if (!valid_block(sdp, block)) { - log_err( _("Extended attributes indirect block #%llu" - " (0x%llx) for inode #%llu" - " (0x%llx) is invalid...removing\n"), - (unsigned long long)block, - (unsigned long long)block, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); - return ask_remove_eattr(ip); - } - q = block_type(block); - if (q != (sdp->gfs1 ? GFS2_BLKST_DINODE : GFS2_BLKST_USED)) { - log_err( _("Extended attributes indirect block #%llu" - " (0x%llx) for inode #%llu" - " (0x%llx) is invalid.\n"), - (unsigned long long)block, - (unsigned long long)block, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); - return ask_remove_eattr(ip); - } - else - indir_bh = bread(sdp, block); - - *bh = indir_bh; - return 0; -} - -static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block, - uint64_t parent, struct gfs2_buffer_head **bh, - void *private) -{ - struct gfs2_sbd *sdp = ip->i_sbd; - uint8_t q; - - if (!valid_block(sdp, block)) { - log_err( _("Extended attributes block %lld (0x%llx) for " - "inode #%llu (0x%llx) is invalid.\n"), - (unsigned long long)block, (unsigned long long)block, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); - return ask_remove_eattr(ip); - } - q = block_type(block); - if (q != (sdp->gfs1 ? GFS2_BLKST_DINODE : GFS2_BLKST_USED)) { - log_err( _("Extended attributes block %lld (0x%llx) for " - "inode #%llu (0x%llx) invalid.\n"), - (unsigned long long)block, (unsigned long long)block, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); - return ask_remove_eattr(ip); - } - else - *bh = bread(sdp, block); - - return 0; -} - -static int check_eattr_entry(struct gfs2_inode *ip, - struct gfs2_buffer_head *leaf_bh, - struct gfs2_ea_header *ea_hdr, - struct gfs2_ea_header *ea_hdr_prev, - void *private) -{ - struct gfs2_sbd *sdp = ip->i_sbd; - char ea_name[256]; - uint32_t offset = (uint32_t)(((unsigned long)ea_hdr) - - ((unsigned long)leaf_bh->b_data)); - uint32_t max_size = sdp->sd_sb.sb_bsize; - - if (!ea_hdr->ea_name_len){ - log_err( _("EA has name length of zero\n")); - return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, - ea_hdr_prev, 1, 1); - } - if (offset + be32_to_cpu(ea_hdr->ea_rec_len) > max_size){ - log_err( _("EA rec length too long\n")); - return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, - ea_hdr_prev, 1, 1); - } - if (offset + be32_to_cpu(ea_hdr->ea_rec_len) == max_size && - (ea_hdr->ea_flags & GFS2_EAFLAG_LAST) == 0){ - log_err( _("last EA has no last entry flag\n")); - return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, - ea_hdr_prev, 0, 0); - } - if (!ea_hdr->ea_name_len){ - log_err( _("EA has name length of zero\n")); - return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, - ea_hdr_prev, 0, 0); - } - - memset(ea_name, 0, sizeof(ea_name)); - strncpy(ea_name, (char *)ea_hdr + sizeof(struct gfs2_ea_header), - ea_hdr->ea_name_len); - - if (!GFS2_EATYPE_VALID(ea_hdr->ea_type) && - ((ea_hdr_prev) || (!ea_hdr_prev && ea_hdr->ea_type))){ - log_err( _("EA (%s) type is invalid (%d > %d).\n"), - ea_name, ea_hdr->ea_type, GFS2_EATYPE_LAST); - return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, - ea_hdr_prev, 0, 0); - } - - if (ea_hdr->ea_num_ptrs){ - uint32_t avail_size; - int max_ptrs; - - avail_size = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_meta_header); - max_ptrs = (be32_to_cpu(ea_hdr->ea_data_len)+avail_size-1)/avail_size; - - if (max_ptrs > ea_hdr->ea_num_ptrs){ - log_err( _("EA (%s) has incorrect number of pointers.\n"), ea_name); - log_err( _(" Required: %d\n Reported: %d\n"), - max_ptrs, ea_hdr->ea_num_ptrs); - return ask_remove_eattr_entry(sdp, leaf_bh, ea_hdr, - ea_hdr_prev, 0, 0); - } else { - log_debug( _(" Pointers Required: %d\n Pointers Reported: %d\n"), - max_ptrs, ea_hdr->ea_num_ptrs); - } - } - return 0; -} - -static int check_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_ptr, - struct gfs2_buffer_head *leaf_bh, - struct gfs2_ea_header *ea_hdr, - struct gfs2_ea_header *ea_hdr_prev, void *private) -{ - uint8_t q; - struct gfs2_sbd *sdp = ip->i_sbd; - - q = block_type(be64_to_cpu(*ea_ptr)); - if (q != (sdp->gfs1 ? GFS2_BLKST_DINODE : GFS2_BLKST_USED)) { - if (remove_eattr_entry(sdp, leaf_bh, ea_hdr, ea_hdr_prev)){ - stack; - return -1; - } - return 1; - } - return 0; -} - -/* Go over all inodes with extended attributes and verify the EAs are - * valid */ -int pass1c(struct gfs2_sbd *sdp) -{ - uint64_t block_no = 0; - struct gfs2_buffer_head *bh; - struct gfs2_inode *ip = NULL; - struct metawalk_fxns pass1c_fxns = { 0 }; - int error = 0; - osi_list_t *tmp, *x; - struct special_blocks *ea_block; - - pass1c_fxns.check_eattr_indir = &check_eattr_indir; - pass1c_fxns.check_eattr_leaf = &check_eattr_leaf; - pass1c_fxns.check_eattr_entry = &check_eattr_entry; - pass1c_fxns.check_eattr_extentry = &check_eattr_extentry; - pass1c_fxns.private = NULL; - - log_info( _("Looking for inodes containing ea blocks...\n")); - osi_list_foreach_safe(tmp, &sdp->eattr_blocks.list, x) { - ea_block = osi_list_entry(tmp, struct special_blocks, list); - block_no = ea_block->block; - warm_fuzzy_stuff(block_no); - - if (skip_this_pass || fsck_abort) /* if asked to skip the rest */ - return FSCK_OK; - bh = bread(sdp, block_no); - if (!gfs2_check_meta(bh, GFS2_METATYPE_DI)) { /* if a dinode */ - log_info( _("EA in inode %llu (0x%llx)\n"), - (unsigned long long)block_no, - (unsigned long long)block_no); - gfs2_special_clear(&sdp->eattr_blocks, block_no); - ip = fsck_inode_get(sdp, bh); - ip->bh_owned = 1; - - log_debug( _("Found eattr at %llu (0x%llx)\n"), - (unsigned long long)ip->i_di.di_eattr, - (unsigned long long)ip->i_di.di_eattr); - /* FIXME: Handle walking the eattr here */ - error = check_inode_eattr(ip, &pass1c_fxns); - if (error < 0) { - stack; - brelse(bh); - return FSCK_ERROR; - } - - fsck_inode_put(&ip); /* dinode_out, brelse, free */ - } else { - brelse(bh); - } - } - return FSCK_OK; -} diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index b22abc6..2745692 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -556,7 +556,6 @@ struct gfs2_bmap *gfs2_bmap_create(struct gfs2_sbd *sdp, uint64_t size, free(il); il = NULL; } - osi_list_init(&sdp->eattr_blocks.list); return il; } @@ -584,7 +583,6 @@ void *gfs2_bmap_destroy(struct gfs2_sbd *sdp, struct gfs2_bmap *il) free(il); il = NULL; } - gfs2_special_free(&sdp->eattr_blocks); return il; } diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index ccae721..12d57c7 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -304,8 +304,6 @@ struct gfs2_sbd { struct gfs2_inode *master_dir; struct master_dir md; - struct special_blocks eattr_blocks; - uint64_t rg_one_length; uint64_t rg_length; int gfs1;

1 0

gfs2-utils: master - fsck.gfs2: Print debug message to dilineate metadata blocks
by Bob Peterson 29 Sep '15

29 Sep '15

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=40127ca5ab… Commit: 40127ca5abc87bd93d748884a8b8b58d83afc234 Parent: 7520428d1f78a538243b92226335ce9d1551633c Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Sep 24 13:42:03 2015 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Sep 29 13:23:48 2015 -0500 fsck.gfs2: Print debug message to dilineate metadata blocks Before this patch, when the data blocks were checked for a huge file, there was no way to distinguish which metadata block had the reference to the data block. This patch adds a new debug message to announce which metadata block is being processed for the next set of data blocks being analyzed. --- gfs2/fsck/metawalk.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index bb03d1a..8fcaea3 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1454,6 +1454,10 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, uint64_t metablock = bh->b_blocknr; /* If there isn't much pointer corruption check the pointers */ + log_debug(_("\nProcessing data blocks for inode 0x%llx, metadata " + "block 0x%llx.\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)bh->b_blocknr); for (ptr = ptr_start ; (char *)ptr < ptr_end && !fsck_abort; ptr++) { if (!*ptr) continue;

1 0

gfs2-utils: master - fsck.gfs2: Combine remove_inode_eattr with its only caller
by Bob Peterson 29 Sep '15

29 Sep '15

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=7520428d1f… Commit: 7520428d1f78a538243b92226335ce9d1551633c Parent: 6260b11af9222910550678f7c8933d4091870427 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Sep 24 10:15:46 2015 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Sep 29 13:23:44 2015 -0500 fsck.gfs2: Combine remove_inode_eattr with its only caller Since remove_inode_eattr() is now only called from one place, it is combined with its caller to make the code more readable and less confusing: function ask_remove_inode_eattr. --- gfs2/fsck/pass1.c | 23 +++++++---------------- 1 files changed, 7 insertions(+), 16 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index a515a62..895d1d1 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -553,17 +553,6 @@ static int check_data(struct gfs2_inode *ip, uint64_t metablock, return 0; } -static int remove_inode_eattr(struct gfs2_inode *ip, struct block_count *bc) -{ - 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; - ip->i_di.di_flags &= ~GFS2_DIF_EA_INDIRECT; - bmodified(ip->i_bh); - return 0; -} - static int ask_remove_inode_eattr(struct gfs2_inode *ip, struct block_count *bc) { @@ -573,11 +562,13 @@ static int ask_remove_inode_eattr(struct gfs2_inode *ip, "errors.\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); if (query( _("Clear all Extended Attributes from the inode? (y/n) "))){ - if (!remove_inode_eattr(ip, bc)) - log_err( _("Extended attributes were removed.\n")); - else - log_err( _("Unable to remove inode eattr pointer; " - "the error remains.\n")); + 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; + ip->i_di.di_flags &= ~GFS2_DIF_EA_INDIRECT; + bmodified(ip->i_bh); + log_err( _("Extended attributes were removed.\n")); } else { log_err( _("Extended attributes were not removed.\n")); }

1 0

gfs2-utils: master - fsck.gfs2: remove bad EAs at the end, not as-you-go
by Bob Peterson 29 Sep '15

29 Sep '15

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=6260b11af9… Commit: 6260b11af9222910550678f7c8933d4091870427 Parent: 9ef99edb80d10cb58ed3b44a3198b6c44544b8b2 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Sep 24 10:09:55 2015 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Sep 29 13:23:41 2015 -0500 fsck.gfs2: remove bad EAs at the end, not as-you-go Before this patch, pass1 would check extended attributes and if they were found to be bad, it would delete them as it found them. This is the wrong approach and only gets us into trouble. The proper approach is to traverse the tree and see if there are problems, and if so, delete them at the end. This patch: 1. Removes function clear_eas() which was delete-as-you-go. 2. Instead, the errors are complained about but the traversal goes on. 3. Adds a new tree traversal, eattr_undo_fxns, which is used to properly remove the corrupt extended attributes at the end of the previous tree traversal. --- gfs2/fsck/pass1.c | 107 ++++++++++++++++++++++++++++------------------------ 1 files changed, 58 insertions(+), 49 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index f5bf66b..a515a62 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -102,7 +102,6 @@ struct metawalk_fxns pass1_fxns = { .check_dentry = NULL, .check_eattr_entry = check_eattr_entries, .check_eattr_extentry = check_extended_leaf_eattr, - .finish_eattr_indir = finish_eattr_indir, .big_file_msg = big_file_comfort, .repair_leaf = pass1_repair_leaf, .undo_check_meta = undo_check_metalist, @@ -585,6 +584,36 @@ static int ask_remove_inode_eattr(struct gfs2_inode *ip, return 0; } +static int undo_eattr_indir_or_leaf(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, + struct gfs2_buffer_head **bh, + void *private) +{ + struct gfs2_sbd *sdp = ip->i_sbd; + uint8_t q; + int error; + struct block_count *bc = (struct block_count *) private; + + if (!valid_block(ip->i_sbd, block)) + return meta_error; + + /* Need to check block_type before undoing the reference, which can + set it to free, which would cause the test below to fail. */ + q = block_type(block); + + error = undo_reference(ip, block, 0, private); + if (error) + return error; + + bc->ea_count--; + + if (q != (sdp->gfs1 ? GFS2_BLKST_DINODE : GFS2_BLKST_USED)) + return 1; + + *bh = bread(sdp, block); + return 0; +} + /* complain_eas - complain about extended attribute errors for an inode * * @ip - in core inode pointer @@ -603,36 +632,6 @@ static void complain_eas(struct gfs2_inode *ip, uint64_t block, (unsigned long long)block, (unsigned long long)block); } -/* clear_eas - clear the extended attributes for an inode - * - * @ip - in core inode pointer - * @bc - pointer to a block count structure - * block - the block that had the problem - * duplicate - if this is a duplicate block, don't set it "free" - * emsg - what to tell the user about the eas being checked - * Returns: 1 if the EA is fixed, else 0 if it was not fixed. - */ -static int clear_eas(struct gfs2_inode *ip, struct block_count *bc, - uint64_t block, int duplicate, const char *emsg) -{ - complain_eas(ip, block, emsg); - if (query( _("Clear the bad Extended Attribute? (y/n) "))) { - if (block == ip->i_di.di_eattr) { - remove_inode_eattr(ip, bc); - log_err( _("The bad extended attribute was " - "removed.\n")); - } else if (!duplicate) { - delete_block(ip, block, NULL, - _("bad extended attribute"), NULL); - } - return 1; - } else { - log_err( _("The bad Extended Attribute was not fixed.\n")); - bc->ea_count++; - return 0; - } -} - static int check_eattr_indir(struct gfs2_inode *ip, uint64_t indirect, uint64_t parent, struct gfs2_buffer_head **bh, void *private) @@ -657,22 +656,22 @@ static int check_eattr_indir(struct gfs2_inode *ip, uint64_t indirect, count it as a duplicate. */ *bh = bread(sdp, indirect); if (gfs2_check_meta(*bh, GFS2_METATYPE_IN)) { + bc->ea_count++; if (q != GFS2_BLKST_FREE) { /* Duplicate? */ add_duplicate_ref(ip, indirect, ref_as_ea, 0, INODE_VALID); complain_eas(ip, indirect, _("Bad indirect Extended Attribute " "duplicate found")); - bc->ea_count++; /* Return 0 here because if all that's wrong is a duplicate block reference, we want pass1b to figure it out. We don't want to delete all the extended attributes as if they are in error. */ return 0; } - clear_eas(ip, bc, indirect, 0, - _("Extended Attribute indirect block has incorrect " - "type")); + complain_eas(ip, indirect, + _("Extended Attribute indirect block has " + "incorrect type")); return 1; } if (q != GFS2_BLKST_FREE) { /* Duplicate? */ @@ -765,8 +764,8 @@ static int check_ealeaf_block(struct gfs2_inode *ip, uint64_t block, int btype, attributes as if they are in error. */ return 0; } - clear_eas(ip, bc, block, 0, _("Extended Attribute leaf block " - "has incorrect type")); + complain_eas(ip, block, _("Extended Attribute leaf block has " + "incorrect type")); brelse(leaf_bh); return 1; } @@ -782,16 +781,6 @@ static int check_ealeaf_block(struct gfs2_inode *ip, uint64_t block, int btype, in error. */ return 0; } - if (ip->i_di.di_eattr == 0) { - /* Can only get in here if there were unrecoverable ea - errors that caused clear_eas to be called. What we - need to do here is remove the subsequent ea blocks. */ - clear_eas(ip, bc, block, 0, - _("Extended Attribute block removed due to " - "previous errors.\n")); - brelse(leaf_bh); - return 1; - } /* Point of confusion: We've got to set the ea block itself to GFS2_BLKST_USED here. Elsewhere we mark the inode with gfs2_eattr_block meaning it contains an eattr for pass1c. */ @@ -1129,6 +1118,13 @@ struct metawalk_fxns rangecheck_fxns = { .check_eattr_leaf = rangecheck_eattr_leaf, }; +struct metawalk_fxns eattr_undo_fxns = { + .private = NULL, + .check_eattr_indir = undo_eattr_indir_or_leaf, + .check_eattr_leaf = undo_eattr_indir_or_leaf, + .finish_eattr_indir = finish_eattr_indir, +}; + /* * handle_ip - process an incore structure representing a dinode. */ @@ -1195,9 +1191,22 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip) if (!error) { error = check_inode_eattr(ip, &pass1_fxns); - if (error && - !(ip->i_di.di_flags & GFS2_DIF_EA_INDIRECT)) + if (error) { + if (!query(_("Clear the bad Extended Attributes? " + "(y/n) "))) { + log_err( _("The bad Extended Attributes were " + "not fixed.\n")); + return 0; + } + log_err(_("Clearing the bad Extended Attributes in " + "inode %lld (0x%llx).\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + eattr_undo_fxns.private = &bc; + check_inode_eattr(ip, &eattr_undo_fxns); ask_remove_inode_eattr(ip, &bc); + return 1; + } } if (ip->i_di.di_blocks !=

1 0

gfs2-utils: master - fsck.gfs2: Refactor check_eattr_entries and add error messages
by Bob Peterson 29 Sep '15

29 Sep '15

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=9ef99edb80… Commit: 9ef99edb80d10cb58ed3b44a3198b6c44544b8b2 Parent: 647bf61ecc78ba5bcfbacdc1ab95488cfe787aeb Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Sep 24 08:52:47 2015 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Sep 29 13:23:38 2015 -0500 fsck.gfs2: Refactor check_eattr_entries and add error messages This patch adds a couple error messages to document when extended attributes have errors in function check_eattr_entries. It also refactors some of the code to make it more readable. --- gfs2/fsck/pass1.c | 28 +++++++++++++++++----------- 1 files changed, 17 insertions(+), 11 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index f21886b..f5bf66b 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -891,6 +891,8 @@ static int check_eattr_entries(struct gfs2_inode *ip, { struct gfs2_sbd *sdp = ip->i_sbd; char ea_name[256]; + uint32_t avail_size; + int max_ptrs; if (!ea_hdr->ea_name_len){ /* Skip this entry for now */ @@ -904,22 +906,26 @@ static int check_eattr_entries(struct gfs2_inode *ip, if (!GFS2_EATYPE_VALID(ea_hdr->ea_type) && ((ea_hdr_prev) || (!ea_hdr_prev && ea_hdr->ea_type))){ /* Skip invalid entry */ + log_err(_("EA (%s) type is invalid (%d > %d).\n"), + ea_name, ea_hdr->ea_type, GFS2_EATYPE_LAST); return 1; } - if (ea_hdr->ea_num_ptrs){ - uint32_t avail_size; - int max_ptrs; + if (!ea_hdr->ea_num_ptrs) + return 0; - avail_size = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_meta_header); - max_ptrs = (be32_to_cpu(ea_hdr->ea_data_len)+avail_size-1)/avail_size; + avail_size = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_meta_header); + max_ptrs = (be32_to_cpu(ea_hdr->ea_data_len)+avail_size-1)/avail_size; - if (max_ptrs > ea_hdr->ea_num_ptrs) { - return 1; - } else { - log_debug( _(" Pointers Required: %d\n Pointers Reported: %d\n"), - max_ptrs, ea_hdr->ea_num_ptrs); - } + if (max_ptrs > ea_hdr->ea_num_ptrs) { + log_err(_("EA (%s) has incorrect number of pointers.\n"), + ea_name); + log_err(_(" Required: %d\n Reported: %d\n"), + max_ptrs, ea_hdr->ea_num_ptrs); + return 1; + } else { + log_debug( _(" Pointers Required: %d\n Pointers Reported: %d\n"), + max_ptrs, ea_hdr->ea_num_ptrs); } return 0; }

1 0

gfs2-utils: master - fsck.gfs2: Don't remove duplicate eattr blocks
by Bob Peterson 29 Sep '15

29 Sep '15

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=647bf61ecc… Commit: 647bf61ecc78ba5bcfbacdc1ab95488cfe787aeb Parent: 28a73aac8c060378553cd161222e30c5b8d40865 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Sep 24 07:51:51 2015 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Sep 29 13:23:34 2015 -0500 fsck.gfs2: Don't remove duplicate eattr blocks Before this patch, if two dinodes referenced the same block, one of them in the extended attributes, it would delete the extended attributes. That's wrong because the other dinode referencing the block might be a valid reference, and the block should not be deleted out from under it. This patch changes the return code from 1 to 0 in this case, which causes pass1b to resolve whichever reference is proper and delete or not delete the other reference as appropriate. It also splits the clear_eas function into a separate function to complain about the problem; that way the duplicate errors above are still reported. --- gfs2/fsck/pass1.c | 76 +++++++++++++++++++++++++++++++++------------------- 1 files changed, 48 insertions(+), 28 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 653be59..f21886b 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -585,6 +585,24 @@ static int ask_remove_inode_eattr(struct gfs2_inode *ip, return 0; } +/* complain_eas - complain about extended attribute errors for an inode + * + * @ip - in core inode pointer + * block - the block that had the problem + * duplicate - if this is a duplicate block, don't set it "free" + * emsg - what to tell the user about the eas being checked + * Returns: 1 if the EA is fixed, else 0 if it was not fixed. + */ +static void complain_eas(struct gfs2_inode *ip, uint64_t block, + const char *emsg) +{ + log_err(_("Inode #%llu (0x%llx): %s"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr, emsg); + log_err(_(" at block #%lld (0x%llx).\n"), + (unsigned long long)block, (unsigned long long)block); +} + /* clear_eas - clear the extended attributes for an inode * * @ip - in core inode pointer @@ -597,11 +615,7 @@ static int ask_remove_inode_eattr(struct gfs2_inode *ip, static int clear_eas(struct gfs2_inode *ip, struct block_count *bc, uint64_t block, int duplicate, const char *emsg) { - log_err( _("Inode #%llu (0x%llx): %s"), - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr, emsg); - log_err( _(" at block #%lld (0x%llx).\n"), - (unsigned long long)block, (unsigned long long)block); + complain_eas(ip, block, emsg); if (query( _("Clear the bad Extended Attribute? (y/n) "))) { if (block == ip->i_di.di_eattr) { remove_inode_eattr(ip, bc); @@ -646,11 +660,15 @@ static int check_eattr_indir(struct gfs2_inode *ip, uint64_t indirect, if (q != GFS2_BLKST_FREE) { /* Duplicate? */ add_duplicate_ref(ip, indirect, ref_as_ea, 0, INODE_VALID); - if (!clear_eas(ip, bc, indirect, 1, - _("Bad indirect Extended Attribute " - "duplicate found"))) - bc->ea_count++; - return 1; + complain_eas(ip, indirect, + _("Bad indirect Extended Attribute " + "duplicate found")); + bc->ea_count++; + /* Return 0 here because if all that's wrong is a + duplicate block reference, we want pass1b to figure + it out. We don't want to delete all the extended + attributes as if they are in error. */ + return 0; } clear_eas(ip, bc, indirect, 0, _("Extended Attribute indirect block has incorrect " @@ -658,16 +676,11 @@ static int check_eattr_indir(struct gfs2_inode *ip, uint64_t indirect, return 1; } if (q != GFS2_BLKST_FREE) { /* Duplicate? */ - log_err( _("Inode #%llu (0x%llx): Duplicate Extended " - "Attribute indirect block found at #%llu " - "(0x%llx).\n"), - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)indirect, - (unsigned long long)indirect); add_duplicate_ref(ip, indirect, ref_as_ea, 0, INODE_VALID); + complain_eas(ip, indirect, + _("Duplicate Extended Attribute indirect block")); bc->ea_count++; - ret = 1; + ret = 0; /* For the same reason stated above. */ } else { fsck_blockmap_set(ip, indirect, _("indirect Extended Attribute"), sdp->gfs1 ? @@ -740,27 +753,34 @@ static int check_ealeaf_block(struct gfs2_inode *ip, uint64_t block, int btype, If it isn't, clear it but don't count it as a duplicate. */ leaf_bh = bread(sdp, block); if (gfs2_check_meta(leaf_bh, btype)) { + bc->ea_count++; if (q != GFS2_BLKST_FREE) { /* Duplicate? */ add_duplicate_ref(ip, block, ref_as_ea, 0, INODE_VALID); - clear_eas(ip, bc, block, 1, - _("Bad Extended Attribute duplicate found")); - } else { - clear_eas(ip, bc, block, 0, - _("Extended Attribute leaf block " - "has incorrect type")); + complain_eas(ip, block, _("Extended attribute leaf " + "duplicate found")); + /* Return 0 here because if all that's wrong is a + duplicate block reference, we want pass1b to figure + it out. We don't want to delete all the extended + attributes as if they are in error. */ + return 0; } + clear_eas(ip, bc, block, 0, _("Extended Attribute leaf block " + "has incorrect type")); brelse(leaf_bh); return 1; } if (q != GFS2_BLKST_FREE) { /* Duplicate? */ - log_debug( _("Duplicate block found at #%lld (0x%llx).\n"), - (unsigned long long)block, - (unsigned long long)block); + complain_eas(ip, block, _("Extended Attribute leaf " + "duplicate found")); add_duplicate_ref(ip, block, ref_as_data, 0, INODE_VALID); bc->ea_count++; brelse(leaf_bh); - return 1; + /* Return 0 here because if all that's wrong is a duplicate + block reference, we want pass1b to figure it out. We don't + want to delete all the extended attributes as if they are + in error. */ + return 0; } if (ip->i_di.di_eattr == 0) { /* Can only get in here if there were unrecoverable ea

1 0

gfs2-utils: master - fsck.gfs2: Remove redundancy in add_duplicate_ref
by Bob Peterson 29 Sep '15

29 Sep '15

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=28a73aac8c… Commit: 28a73aac8c060378553cd161222e30c5b8d40865 Parent: f2bef761d91128dcb586a2a67202cdc6ce646eed Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Sep 23 15:11:53 2015 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Sep 29 13:23:31 2015 -0500 fsck.gfs2: Remove redundancy in add_duplicate_ref Before this patch, when a duplicate reference was added to the list, function add_duplicate_ref would print out an info message that said: This brings the total to: X duplicate references. I found this message to be redundant and confusing. It's much more clear to simply say: This brings the total to: X references --- gfs2/fsck/util.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index a6a5cdc..b22abc6 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -402,8 +402,8 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, if (first) log_info( _("This is the original reference.\n")); else - log_info( _("This brings the total to: %d duplicate " - "references\n"), dt->refs); + log_info( _("This brings the total to: %d references\n"), + dt->refs); return meta_is_good; }

1 0

gfs2-utils: master - fsck.gfs2: Always restore saved value for di_eattr
by Bob Peterson 29 Sep '15

29 Sep '15

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=f2bef761d9… Commit: f2bef761d91128dcb586a2a67202cdc6ce646eed Parent: f44e9910a604ae4d81b4e7acb8e8e1248fb3dddc Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Sep 23 15:09:09 2015 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Sep 29 13:23:27 2015 -0500 fsck.gfs2: Always restore saved value for di_eattr There are times when function check_indirect_eattr saves off the value of di_eattr in order to properly process the extended attributes. The saved value must always be restored, if it exists, otherwise we can accidentally remove an inode's EA reference. --- gfs2/fsck/metawalk.c | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index bf187f9..bb03d1a 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1144,13 +1144,12 @@ static int check_indirect_eattr(struct gfs2_inode *ip, uint64_t indirect, } ea_leaf_ptr++; } + /* If we temporarily nuked the ea block to prevent checking past + a corrupt ea leaf, we need to restore the saved di_eattr block. */ + if (di_eattr_save != 0) + ip->i_di.di_eattr = di_eattr_save; if (pass->finish_eattr_indir) { if (!first_ea_is_bad) { - /* If the first ea is good but subsequent ones were - bad and deleted, we need to restore the saved - di_eattr block. */ - if (leaf_pointer_errors) - ip->i_di.di_eattr = di_eattr_save; pass->finish_eattr_indir(ip, leaf_pointers, leaf_pointer_errors, pass->private);

1 0

gfs2-utils: master - fsck.gfs2: Once an indirect ea error is found, flag all that follow
by Bob Peterson 29 Sep '15

29 Sep '15

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=f44e9910a6… Commit: f44e9910a604ae4d81b4e7acb8e8e1248fb3dddc Parent: 9944c79285b7a1bd65c7b5a27d0e682ed0fd9da3 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Sep 23 14:28:00 2015 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Sep 29 13:23:24 2015 -0500 fsck.gfs2: Once an indirect ea error is found, flag all that follow Before this patch, function check_indirect_eattr could find an error with an indirect extended attribute, but it would ignore subsequent EA errors and return a good return code. That's bad. Once you find an EA error in an indirect list, all the ones that follow should be flagged as errors and subsequently cleared. The return code should also reflect that an error was found during the processing. This patch adds a second "err" variable to keep track of whether new errors were found as opposed to whether _any_ errors were found in the list. It also prints new error messages where the error was found and on all pointers following the original error. --- gfs2/fsck/metawalk.c | 28 +++++++++++++++++++++++----- 1 files changed, 23 insertions(+), 5 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index b4330fb..bf187f9 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1081,7 +1081,7 @@ static int check_indirect_eattr(struct gfs2_inode *ip, uint64_t indirect, struct gfs2_buffer_head *indirect_buf, struct metawalk_fxns *pass) { - int error = 0; + int error = 0, err; uint64_t *ea_leaf_ptr, *end; uint64_t block; struct gfs2_sbd *sdp = ip->i_sbd; @@ -1096,11 +1096,29 @@ static int check_indirect_eattr(struct gfs2_inode *ip, uint64_t indirect, while (*ea_leaf_ptr && (ea_leaf_ptr < end)){ block = be64_to_cpu(*ea_leaf_ptr); leaf_pointers++; - error = check_leaf_eattr(ip, block, indirect, pass); - if (error) { + err = check_leaf_eattr(ip, block, indirect, pass); + if (err) { + error = err; + log_err(_("Error detected in leaf block %lld (0x%llx) " + "referenced by indirect block %lld (0x%llx)" + ".\n"), + (unsigned long long)block, + (unsigned long long)block, + (unsigned long long)indirect, + (unsigned long long)indirect); + log_err(_("Subsequent leaf block pointers should be " + "cleared.\n")); + } + if (error) { /* leaf blocks following an error must also be + treated as error blocks and cleared. */ leaf_pointer_errors++; - if (query( _("Fix the indirect block too? (y/n) "))) - *ea_leaf_ptr = 0; + log_err(_("Pointer to EA leaf block %lld (0x%llx) in " + "indirect block %lld (0x%llx) should be " + "cleared.\n"), + (unsigned long long)block, + (unsigned long long)block, + (unsigned long long)indirect, + (unsigned long long)indirect); } /* If the first eattr lead is bad, we can't have a hole, so we have to treat this as an unrecoverable eattr error and

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

cluster-commits September 2015