May 2013 - cluster-commits - Fedora mailing-lists

cluster: RHEL6 - fsck.gfs2: shorten some debug messages in lost+found
by Bob Peterson 21 May '13

21 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=89e2e4a2f24f5… Commit: 89e2e4a2f24f54c14cf7b241daea375324a0a5eb Parent: 872b99e0a2569404c49f4483260b5a1b8023d372 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Mar 6 10:29:38 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Fri May 17 14:56:50 2013 -0500 fsck.gfs2: shorten some debug messages in lost+found This patch changes the debug output of lost+found such that it only prints the block number in hexadecimal. This shortens the output and makes debug output easier to read. rhbz#902920 --- gfs2/fsck/lost_n_found.c | 12 ++++-------- 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index 42d97ee..388787d 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -32,11 +32,9 @@ static void add_dotdot(struct gfs2_inode *ip) if (di && valid_block(sdp, di->dotdot_parent.no_addr)) { struct gfs2_inode *dip; - log_debug(_("Directory %lld (0x%llx) already had a " - "\"..\" link to %lld (0x%llx).\n"), + log_debug(_("Directory (0x%llx) already had a " + "\"..\" link to (0x%llx).\n"), (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)di->dotdot_parent.no_addr, (unsigned long long)di->dotdot_parent.no_addr); dip = fsck_load_inode(sdp, di->dotdot_parent.no_addr); if (dip->i_di.di_num.no_formal_ino == @@ -74,15 +72,13 @@ static void add_dotdot(struct gfs2_inode *ip) } else { if (di) log_debug(_("Couldn't find a valid \"..\" entry " - "for orphan directory %lld (0x%llx): " + "for orphan directory (0x%llx): " "'..' = 0x%llx\n"), (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)di->dotdot_parent.no_addr); else - log_debug(_("Couldn't find directory %lld (0x%llx) " + log_debug(_("Couldn't find directory (0x%llx) " "in directory tree.\n"), - (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); } if (gfs2_dirent_del(ip, "..", 2))

1 0

cluster: RHEL6 - fsck.gfs2: Check for formal inode mismatch when adding to lost+found
by Bob Peterson 21 May '13

21 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=872b99e0a2569… Commit: 872b99e0a2569404c49f4483260b5a1b8023d372 Parent: 1325bb4d27bffbcaa5c2170bb1300d8e2c3a9807 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Mar 6 10:28:20 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Fri May 17 14:56:44 2013 -0500 fsck.gfs2: Check for formal inode mismatch when adding to lost+found This patch adds a check to the code that adds inodes to lost+found so that dinodes with formal inode mismatches are logged, but not added. rhbz#902920 --- gfs2/fsck/lost_n_found.c | 44 ++++++++++++++++++++++++++++---------------- 1 files changed, 28 insertions(+), 16 deletions(-) diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index 751cbd8..42d97ee 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -38,24 +38,36 @@ static void add_dotdot(struct gfs2_inode *ip) (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)di->dotdot_parent.no_addr, (unsigned long long)di->dotdot_parent.no_addr); - decr_link_count(di->dotdot_parent.no_addr, - ip->i_di.di_num.no_addr, - _(".. unlinked, moving to lost+found")); dip = fsck_load_inode(sdp, di->dotdot_parent.no_addr); - if (dip->i_di.di_nlink > 0) { - dip->i_di.di_nlink--; - set_di_nlink(dip); /* keep inode tree in sync */ - log_debug(_("Decrementing its links to %d\n"), - dip->i_di.di_nlink); - bmodified(dip->i_bh); - } else if (!dip->i_di.di_nlink) { - log_debug(_("Its link count is zero.\n")); + if (dip->i_di.di_num.no_formal_ino == + di->dotdot_parent.no_formal_ino) { + decr_link_count(di->dotdot_parent.no_addr, + ip->i_di.di_num.no_addr, + _(".. unlinked, moving to lost+found")); + if (dip->i_di.di_nlink > 0) { + dip->i_di.di_nlink--; + set_di_nlink(dip); /* keep inode tree in sync */ + log_debug(_("Decrementing its links to %d\n"), + dip->i_di.di_nlink); + bmodified(dip->i_bh); + } else if (!dip->i_di.di_nlink) { + log_debug(_("Its link count is zero.\n")); + } else { + log_debug(_("Its link count is %d! Changing " + "it to 0.\n"), dip->i_di.di_nlink); + dip->i_di.di_nlink = 0; + set_di_nlink(dip); /* keep inode tree in sync */ + bmodified(dip->i_bh); + } } else { - log_debug(_("Its link count is %d! Changing " - "it to 0.\n"), dip->i_di.di_nlink); - dip->i_di.di_nlink = 0; - set_di_nlink(dip); /* keep inode tree in sync */ - bmodified(dip->i_bh); + log_debug(_("Directory (0x%llx)'s link to parent " + "(0x%llx) had a formal inode discrepancy: " + "was 0x%llx, expected 0x%llx\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)di->dotdot_parent.no_addr, + di->dotdot_parent.no_formal_ino, + dip->i_di.di_num.no_formal_ino); + log_debug(_("The parent directory was not changed.\n")); } fsck_inode_put(&dip); di = NULL;

1 0

cluster: RHEL6 - fsck.gfs2: Split out function to make sure lost+found exists
by Bob Peterson 21 May '13

21 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=1325bb4d27bff… Commit: 1325bb4d27bffbcaa5c2170bb1300d8e2c3a9807 Parent: 82558210e8ef16821170bb9787c0b46a1abf9bfd Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Mon Feb 25 10:18:55 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Fri May 17 14:56:33 2013 -0500 fsck.gfs2: Split out function to make sure lost+found exists This patch extracts a section of code from the lost+found functions and makes a new make_sure_lf_exists function that can be called from more places. rhbz#902920 --- gfs2/fsck/lost_n_found.c | 129 +++++++++++++++++++++++----------------------- gfs2/fsck/lost_n_found.h | 1 + 2 files changed, 66 insertions(+), 64 deletions(-) diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index 282d673..751cbd8 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -86,6 +86,70 @@ static void add_dotdot(struct gfs2_inode *ip) } } +void make_sure_lf_exists(struct gfs2_inode *ip) +{ + uint8_t q; + struct dir_info *di; + struct gfs2_sbd *sdp = ip->i_sbd; + uint32_t mode; + + if (lf_dip) + return; + + log_info( _("Locating/Creating lost+found directory\n")); + + /* if this is gfs1, we have to trick createi into using + no_formal_ino = no_addr, so we set next_inum to the + free block we're about to allocate. */ + if (sdp->gfs1) + sdp->md.next_inum = find_free_blk(sdp); + mode = (sdp->gfs1 ? DT2IF(GFS_FILE_DIR) : S_IFDIR) | 0700; + if (sdp->gfs1) + lf_dip = gfs_createi(sdp->md.rooti, "lost+found", mode, 0); + else + lf_dip = createi(sdp->md.rooti, "lost+found", + S_IFDIR | 0700, 0); + if (lf_dip == NULL) { + log_crit(_("Error creating lost+found: %s\n"), + strerror(errno)); + exit(FSCK_ERROR); + } + + /* createi will have incremented the di_nlink link count for the root + directory. We must set the nlink value in the hash table to keep + them in sync so that pass4 can detect and fix any descrepancies. */ + set_di_nlink(sdp->md.rooti); + + q = block_type(lf_dip->i_di.di_num.no_addr); + if (q != gfs2_inode_dir) { + /* This is a new lost+found directory, so set its block type + and increment link counts for the directories */ + /* FIXME: i'd feel better about this if fs_mkdir returned + whether it created a new directory or just found an old one, + and we used that instead of the block_type to run this */ + fsck_blockmap_set(ip, lf_dip->i_di.di_num.no_addr, + _("lost+found dinode"), gfs2_inode_dir); + dirtree_insert(lf_dip->i_di.di_num); + /* root inode links to lost+found */ + incr_link_count(sdp->md.rooti->i_di.di_num, lf_dip, _("root")); + /* lost+found link for '.' from itself */ + incr_link_count(lf_dip->i_di.di_num, lf_dip, "\".\""); + /* lost+found link for '..' back to root */ + incr_link_count(lf_dip->i_di.di_num, sdp->md.rooti, "\"..\""); + if (sdp->gfs1) + lf_dip->i_di.__pad1 = GFS_FILE_DIR; + } + log_info( _("lost+found directory is dinode %lld (0x%llx)\n"), + (unsigned long long)lf_dip->i_di.di_num.no_addr, + (unsigned long long)lf_dip->i_di.di_num.no_addr); + di = dirtree_find(lf_dip->i_di.di_num.no_addr); + if (di) { + log_info( _("Marking lost+found inode connected\n")); + di->checked = 1; + di = NULL; + } +} + /* add_inode_to_lf - Add dir entry to lost+found for the inode * @ip: inode to add to lost + found * @@ -100,73 +164,10 @@ int add_inode_to_lf(struct gfs2_inode *ip){ __be32 inode_type; uint64_t lf_blocks; struct gfs2_sbd *sdp = ip->i_sbd; - struct dir_info *di; int err = 0; uint32_t mode; - if (!lf_dip) { - uint8_t q; - - log_info( _("Locating/Creating lost+found directory\n")); - - /* if this is gfs1, we have to trick createi into using - no_formal_ino = no_addr, so we set next_inum to the - free block we're about to allocate. */ - if (sdp->gfs1) - sdp->md.next_inum = find_free_blk(sdp); - mode = (sdp->gfs1 ? DT2IF(GFS_FILE_DIR) : S_IFDIR) | 0700; - if (sdp->gfs1) - lf_dip = gfs_createi(sdp->md.rooti, "lost+found", - mode, 0); - else - lf_dip = createi(sdp->md.rooti, "lost+found", - S_IFDIR | 0700, 0); - if (lf_dip == NULL) { - log_crit(_("Error %d creating lost+found\n"), errno); - exit(FSCK_ERROR); - } - - /* createi will have incremented the di_nlink link count for - the root directory. We must set the nlink value - in the hash table to keep them in sync so that pass4 can - detect and fix any descrepancies. */ - set_di_nlink(sdp->md.rooti); - - q = block_type(lf_dip->i_di.di_num.no_addr); - if (q != gfs2_inode_dir) { - /* This is a new lost+found directory, so set its - * block type and increment link counts for - * the directories */ - /* FIXME: i'd feel better about this if - * fs_mkdir returned whether it created a new - * directory or just found an old one, and we - * used that instead of the block_type to run - * this */ - fsck_blockmap_set(ip, lf_dip->i_di.di_num.no_addr, - _("lost+found dinode"), - gfs2_inode_dir); - /* root inode links to lost+found */ - incr_link_count(sdp->md.rooti->i_di.di_num, - lf_dip, _("root")); - /* lost+found link for '.' from itself */ - incr_link_count(lf_dip->i_di.di_num, - lf_dip, "\".\""); - /* lost+found link for '..' back to root */ - incr_link_count(lf_dip->i_di.di_num, sdp->md.rooti, - "\"..\""); - if (sdp->gfs1) - lf_dip->i_di.__pad1 = GFS_FILE_DIR; - } - log_info( _("lost+found directory is dinode %lld (0x%llx)\n"), - (unsigned long long)lf_dip->i_di.di_num.no_addr, - (unsigned long long)lf_dip->i_di.di_num.no_addr); - di = dirtree_find(lf_dip->i_di.di_num.no_addr); - if (di) { - log_info( _("Marking lost+found inode connected\n")); - di->checked = 1; - di = NULL; - } - } + make_sure_lf_exists(ip); if (ip->i_di.di_num.no_addr == lf_dip->i_di.di_num.no_addr) { log_err( _("Trying to add lost+found to itself...skipping")); return 0; diff --git a/gfs2/fsck/lost_n_found.h b/gfs2/fsck/lost_n_found.h index f28a1d9..2b76cc2 100644 --- a/gfs2/fsck/lost_n_found.h +++ b/gfs2/fsck/lost_n_found.h @@ -4,5 +4,6 @@ #include "libgfs2.h" int add_inode_to_lf(struct gfs2_inode *ip); +void make_sure_lf_exists(struct gfs2_inode *ip); #endif /* __LOST_N_FOUND_H__ */

1 0

cluster: RHEL6 - fsck.gfs2: Move function find_free_blk to util.c
by Bob Peterson 21 May '13

21 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=82558210e8ef1… Commit: 82558210e8ef16821170bb9787c0b46a1abf9bfd Parent: 43729045dbdf085af140885b6af582851bf521f1 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Feb 21 09:43:22 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Fri May 17 14:55:59 2013 -0500 fsck.gfs2: Move function find_free_blk to util.c In a future patch to fsck, function find_free_blk will be called in order to correctly report blocks that will need to be allocated for things such as leaf splits. This patch moves function find_free_blk to a more centralized place, util.c, to that end. rhbz#902920 --- gfs2/fsck/lost_n_found.c | 39 --------------------------------------- gfs2/fsck/util.c | 39 +++++++++++++++++++++++++++++++++++++++ gfs2/fsck/util.h | 1 + 3 files changed, 40 insertions(+), 39 deletions(-) diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index 06ee485..282d673 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -86,45 +86,6 @@ static void add_dotdot(struct gfs2_inode *ip) } } -static uint64_t find_free_blk(struct gfs2_sbd *sdp) -{ - struct osi_node *n, *next = NULL; - struct rgrp_tree *rl = NULL; - struct gfs2_rindex *ri; - struct gfs2_rgrp *rg; - unsigned int block, bn = 0, x = 0, y = 0; - unsigned int state; - struct gfs2_buffer_head *bh; - - memset(&rg, 0, sizeof(rg)); - for (n = osi_first(&sdp->rgtree); n; n = next) { - next = osi_next(n); - rl = (struct rgrp_tree *)n; - if (rl->rg.rg_free) - break; - } - - if (n == NULL) - return 0; - - ri = &rl->ri; - rg = &rl->rg; - - for (block = 0; block < ri->ri_length; block++) { - bh = rl->bh[block]; - x = (block) ? sizeof(struct gfs2_meta_header) : sizeof(struct gfs2_rgrp); - - for (; x < sdp->bsize; x++) - for (y = 0; y < GFS2_NBBY; y++) { - state = (bh->b_data[x] >> (GFS2_BIT_SIZE * y)) & 0x03; - if (state == GFS2_BLKST_FREE) - return ri->ri_data0 + bn; - bn++; - } - } - return 0; -} - /* add_inode_to_lf - Add dir entry to lost+found for the inode * @ip: inode to add to lost + found * diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index 24bae6f..5e14dd4 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -532,3 +532,42 @@ bad_dinode: stack; return -EPERM; } + +uint64_t find_free_blk(struct gfs2_sbd *sdp) +{ + struct osi_node *n, *next = NULL; + struct rgrp_tree *rl = NULL; + struct gfs2_rindex *ri; + struct gfs2_rgrp *rg; + unsigned int block, bn = 0, x = 0, y = 0; + unsigned int state; + struct gfs2_buffer_head *bh; + + memset(&rg, 0, sizeof(rg)); + for (n = osi_first(&sdp->rgtree); n; n = next) { + next = osi_next(n); + rl = (struct rgrp_tree *)n; + if (rl->rg.rg_free) + break; + } + + if (n == NULL) + return 0; + + ri = &rl->ri; + rg = &rl->rg; + + for (block = 0; block < ri->ri_length; block++) { + bh = rl->bh[block]; + x = (block) ? sizeof(struct gfs2_meta_header) : sizeof(struct gfs2_rgrp); + + for (; x < sdp->bsize; x++) + for (y = 0; y < GFS2_NBBY; y++) { + state = (bh->b_data[x] >> (GFS2_BIT_SIZE * y)) & 0x03; + if (state == GFS2_BLKST_FREE) + return ri->ri_data0 + bn; + bn++; + } + } + return 0; +} diff --git a/gfs2/fsck/util.h b/gfs2/fsck/util.h index b56fe69..5ee38ad 100644 --- a/gfs2/fsck/util.h +++ b/gfs2/fsck/util.h @@ -180,4 +180,5 @@ extern void *gfs2_bmap_destroy(struct gfs2_sbd *sdp, struct gfs2_bmap *il); extern int gfs2_blockmap_set(struct gfs2_bmap *il, uint64_t block, enum gfs2_mark_block mark); extern int set_ip_blockmap(struct gfs2_inode *ip, int instree); +extern uint64_t find_free_blk(struct gfs2_sbd *sdp); #endif /* __UTIL_H__ */

1 0

cluster: RHEL6 - libgfs2: let dir_split_leaf receive a "broken" lindex
by Bob Peterson 21 May '13

21 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=43729045dbdf0… Commit: 43729045dbdf085af140885b6af582851bf521f1 Parent: 3cb07500ec76da42ff56fbbb01eca50e5358c10e Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Feb 21 09:36:01 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Fri May 17 14:42:48 2013 -0500 libgfs2: let dir_split_leaf receive a "broken" lindex For ordinary leaf blocks, the hash table must follow the rules, which means it needs to follow a power-of-two boundary. In other words, it needs to enforce that: start = (lindex & ~(len - 1)); But when doing repairs, fsck will need to detect when hash tables violate this rule and fix it. In that case, it may need to pass in an invalid starting offset for a leaf to split. This patch moves the responsibility for checking the starting block to the calling function. rhbz#902920 --- gfs2/libgfs2/fs_ops.c | 13 +++++++------ gfs2/libgfs2/libgfs2.h | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c index f88c48f..690d8dd 100644 --- a/gfs2/libgfs2/fs_ops.c +++ b/gfs2/libgfs2/fs_ops.c @@ -865,13 +865,13 @@ void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out) die("gfs2_put_leaf_nr: Bad internal write.\n"); } -void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, uint64_t leaf_no, +void dir_split_leaf(struct gfs2_inode *dip, uint32_t start, uint64_t leaf_no, struct gfs2_buffer_head *obh) { struct gfs2_buffer_head *nbh; struct gfs2_leaf *nleaf, *oleaf; struct gfs2_dirent *dent, *prev = NULL, *next = NULL, *new; - uint32_t start, len, half_len, divider; + uint32_t len, half_len, divider; uint64_t bn, *lp; uint32_t name_len; int x, moved = FALSE; @@ -897,8 +897,6 @@ void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, uint64_t leaf_no, len = 1 << (dip->i_di.di_depth - be16_to_cpu(oleaf->lf_depth)); half_len = len >> 1; - start = (lindex & ~(len - 1)); - lp = calloc(1, half_len * sizeof(uint64_t)); if (lp == NULL) { fprintf(stderr, "Out of memory in %s\n", __FUNCTION__); @@ -1091,7 +1089,7 @@ static int dir_e_add(struct gfs2_inode *dip, const char *filename, int len, struct gfs2_buffer_head *bh, *nbh; struct gfs2_leaf *leaf, *nleaf; struct gfs2_dirent *dent; - uint32_t lindex; + uint32_t lindex, llen; uint32_t hash; uint64_t leaf_no, bn; int err = 0; @@ -1113,7 +1111,10 @@ restart: if (dirent_alloc(dip, bh, len, &dent)) { if (be16_to_cpu(leaf->lf_depth) < dip->i_di.di_depth) { - dir_split_leaf(dip, lindex, leaf_no, bh); + llen = 1 << (dip->i_di.di_depth - + be16_to_cpu(leaf->lf_depth)); + dir_split_leaf(dip, lindex & ~(llen - 1), + leaf_no, bh); brelse(bh); goto restart; diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index 8354a14..d5de337 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -400,7 +400,7 @@ extern void block_map(struct gfs2_inode *ip, uint64_t lblock, int *new, extern void gfs2_get_leaf_nr(struct gfs2_inode *dip, uint32_t index, uint64_t *leaf_out); extern void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out); -extern void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, +extern void dir_split_leaf(struct gfs2_inode *dip, uint32_t start, uint64_t leaf_no, struct gfs2_buffer_head *obh); extern void gfs2_free_block(struct gfs2_sbd *sdp, uint64_t block); extern int gfs2_freedi(struct gfs2_sbd *sdp, uint64_t block);

1 0

cluster: RHEL6 - libgfs2: allow dir_split_leaf to receive a leaf buffer
by Bob Peterson 21 May '13

21 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=3cb07500ec76d… Commit: 3cb07500ec76da42ff56fbbb01eca50e5358c10e Parent: 6f85923316e5b1504a231cd7c8bde5bb6a059506 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Feb 21 09:33:24 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Fri May 17 14:30:57 2013 -0500 libgfs2: allow dir_split_leaf to receive a leaf buffer This is a small performance improvement. Rather than having function dir_split_leaf read in the leaf to be split, this patch lets the buffer_head to be passed in from the calling function, which has it read in anyway. rhbz#902920 --- gfs2/libgfs2/fs_ops.c | 9 ++++----- gfs2/libgfs2/libgfs2.h | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c index 5417efb..f88c48f 100644 --- a/gfs2/libgfs2/fs_ops.c +++ b/gfs2/libgfs2/fs_ops.c @@ -865,9 +865,10 @@ void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out) die("gfs2_put_leaf_nr: Bad internal write.\n"); } -void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, uint64_t leaf_no) +void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, uint64_t leaf_no, + struct gfs2_buffer_head *obh) { - struct gfs2_buffer_head *nbh, *obh; + struct gfs2_buffer_head *nbh; struct gfs2_leaf *nleaf, *oleaf; struct gfs2_dirent *dent, *prev = NULL, *next = NULL, *new; uint32_t start, len, half_len, divider; @@ -891,7 +892,6 @@ void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, uint64_t leaf_no) nleaf = (struct gfs2_leaf *)nbh->b_data; nleaf->lf_dirent_format = cpu_to_be32(GFS2_FORMAT_DE); - obh = bread(dip->i_sbd, leaf_no); oleaf = (struct gfs2_leaf *)obh->b_data; len = 1 << (dip->i_di.di_depth - be16_to_cpu(oleaf->lf_depth)); @@ -970,7 +970,6 @@ void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, uint64_t leaf_no) bmodified(dip->i_bh); bmodified(obh); /* Need to do this in case nothing was moved */ - brelse(obh); bmodified(nbh); brelse(nbh); } @@ -1114,8 +1113,8 @@ restart: if (dirent_alloc(dip, bh, len, &dent)) { if (be16_to_cpu(leaf->lf_depth) < dip->i_di.di_depth) { + dir_split_leaf(dip, lindex, leaf_no, bh); brelse(bh); - dir_split_leaf(dip, lindex, leaf_no); goto restart; } else if (dip->i_di.di_depth < GFS2_DIR_MAX_DEPTH) { diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index 9e3de8a..8354a14 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -401,7 +401,7 @@ extern void gfs2_get_leaf_nr(struct gfs2_inode *dip, uint32_t index, uint64_t *leaf_out); extern void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out); extern void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, - uint64_t leaf_no); + uint64_t leaf_no, struct gfs2_buffer_head *obh); extern void gfs2_free_block(struct gfs2_sbd *sdp, uint64_t block); extern int gfs2_freedi(struct gfs2_sbd *sdp, uint64_t block); extern int gfs2_get_leaf(struct gfs2_inode *dip, uint64_t leaf_no,

1 0

cluster: RHEL6 - libgfs2: externalize dir_split_leaf
by Bob Peterson 21 May '13

21 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=6f85923316e5b… Commit: 6f85923316e5b1504a231cd7c8bde5bb6a059506 Parent: c361fd8d3d7b1074107f0800fdae79820574b81d Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Feb 21 09:29:24 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Fri May 17 14:30:22 2013 -0500 libgfs2: externalize dir_split_leaf This patch makes libgfs2 externalize function dir_split_leaf so that fsck.gfs2 can split leafs in a future patch. rhbz#902920 --- gfs2/libgfs2/fs_ops.c | 3 +-- gfs2/libgfs2/libgfs2.h | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c index 1164a8c..5417efb 100644 --- a/gfs2/libgfs2/fs_ops.c +++ b/gfs2/libgfs2/fs_ops.c @@ -865,8 +865,7 @@ void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out) die("gfs2_put_leaf_nr: Bad internal write.\n"); } -static void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, - uint64_t leaf_no) +void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, uint64_t leaf_no) { struct gfs2_buffer_head *nbh, *obh; struct gfs2_leaf *nleaf, *oleaf; diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index bf834a5..9e3de8a 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -400,6 +400,8 @@ extern void block_map(struct gfs2_inode *ip, uint64_t lblock, int *new, extern void gfs2_get_leaf_nr(struct gfs2_inode *dip, uint32_t index, uint64_t *leaf_out); extern void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out); +extern void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, + uint64_t leaf_no); extern void gfs2_free_block(struct gfs2_sbd *sdp, uint64_t block); extern int gfs2_freedi(struct gfs2_sbd *sdp, uint64_t block); extern int gfs2_get_leaf(struct gfs2_inode *dip, uint64_t leaf_no,

1 0

cluster: RHEL6 - fsck: Speed up reading of dir leaf blocks
by Bob Peterson 21 May '13

21 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=c361fd8d3d7b1… Commit: c361fd8d3d7b1074107f0800fdae79820574b81d Parent: 0cf2544edfa377c47b0e904ecdce00194590593e Author: Steven Whitehouse <swhiteho(a)redhat.com> AuthorDate: Mon Feb 18 17:06:58 2013 +0000 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Fri May 17 14:29:20 2013 -0500 fsck: Speed up reading of dir leaf blocks This patch adds readahead for directory leaf blocks. It gives me a speed up of only around one second on my test filesystem, however that only has one directory with a reasonable number of files in it. So that is actually pretty good going for that small a filesystem. Due to the reading of the dir hash table in a single sweep, this reduces the number of calls to read dir hash table blocks considerably. The patch takes all the valid leaf block pointers, sorts them into disk block order and then issues readahead requests for the blocks in order that they are read in, in good time before they are needed. rhbz#902920 --- gfs2/fsck/metawalk.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 80 insertions(+), 6 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index fb461ae..ce80738 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -7,6 +7,7 @@ #include <unistd.h> #include <libintl.h> #include <ctype.h> +#include <fcntl.h> #define _(String) gettext(String) #include "libgfs2.h" @@ -640,24 +641,87 @@ out_copy_old_leaf: return 1; } +static uint64_t *get_dir_hash(struct gfs2_inode *ip) +{ + unsigned hsize = (1 << ip->i_di.di_depth) * sizeof(uint64_t); + int ret; + uint64_t *tbl = malloc(hsize); + + if (tbl == NULL) + return NULL; + + ret = gfs2_readi(ip, tbl, 0, hsize); + if (ret != hsize) { + free(tbl); + return NULL; + } + + return tbl; +} + +static int u64cmp(const void *p1, const void *p2) +{ + uint64_t a = *(uint64_t *)p1; + uint64_t b = *(uint64_t *)p2; + + if (a > b) + return 1; + if (b < b) + return -1; + + return 0; +} + +static void dir_leaf_reada(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize) +{ + uint64_t *t = alloca(hsize * sizeof(uint64_t)); + uint64_t leaf_no; + struct gfs2_sbd *sdp = ip->i_sbd; + unsigned n = 0; + unsigned i; + + for (i = 0; i < hsize; i++) { + leaf_no = be64_to_cpu(tbl[i]); + if (valid_block(ip->i_sbd, leaf_no)) + t[n++] = leaf_no * sdp->bsize; + } + qsort(t, n, sizeof(uint64_t), u64cmp); + for (i = 0; i < n; i++) + posix_fadvise(sdp->device_fd, t[i], sdp->bsize, POSIX_FADV_WILLNEED); +} + /* Checks exhash directory entries */ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) { int error; 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; struct gfs2_buffer_head *lbh; int lindex; struct gfs2_sbd *sdp = ip->i_sbd; int ref_count = 0, old_was_dup; + uint64_t *tbl; + + tbl = get_dir_hash(ip); + if (tbl == NULL) { + perror("get_dir_hash"); + return -1; + } + + /* Turn off system readahead */ + posix_fadvise(sdp->device_fd, 0, 0, POSIX_FADV_RANDOM); + + /* Readahead */ + dir_leaf_reada(ip, tbl, hsize); /* Find the first valid leaf pointer in range and use it as our "old" leaf. That way, bad blocks at the beginning will be overwritten with the first valid leaf. */ first_ok_leaf = leaf_no = -1; - for (lindex = 0; lindex < (1 << ip->i_di.di_depth); lindex++) { - gfs2_get_leaf_nr(ip, lindex, &leaf_no); + for (lindex = 0; lindex < hsize; lindex++) { + leaf_no = be64_to_cpu(tbl[lindex]); if (valid_block(ip->i_sbd, leaf_no)) { lbh = bread(sdp, leaf_no); /* Make sure it's really a valid leaf block. */ @@ -674,19 +738,22 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) "blocks\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); + free(tbl); + 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 < (1 << ip->i_di.di_depth); lindex++) { + for (lindex = 0; lindex < hsize; lindex++) { if (fsck_abort) break; - gfs2_get_leaf_nr(ip, lindex, &leaf_no); + 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; @@ -696,8 +763,11 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) } do { - if (fsck_abort) + if (fsck_abort) { + free(tbl); + posix_fadvise(sdp->device_fd, 0, 0, POSIX_FADV_NORMAL); return 0; + } /* If the old leaf was a duplicate referenced by a previous dinode, we can't check the number of pointers because the number of pointers may be for @@ -708,8 +778,10 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) &ref_count, &lindex, &oldleaf); - if (error) + if (error) { + free(tbl); return error; + } } error = check_leaf(ip, lindex, pass, &ref_count, &leaf_no, old_leaf, &bad_leaf, @@ -724,6 +796,8 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) (unsigned long long)leaf_no); } while (1); /* while we have chained leaf blocks */ } /* for every leaf block */ + free(tbl); + posix_fadvise(sdp->device_fd, 0, 0, POSIX_FADV_NORMAL); return 0; }

1 0

cluster: RHEL6 - libgfs2: Add readahead for rgrp headers
by Bob Peterson 21 May '13

21 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=0cf2544edfa37… Commit: 0cf2544edfa377c47b0e904ecdce00194590593e Parent: 62a5ab048683b4ce77f0eeaba81d3330da574d41 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Sun Feb 17 13:08:04 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Fri May 17 14:26:27 2013 -0500 libgfs2: Add readahead for rgrp headers This adds readahead to rgrp headers, greatly improving the speed with which they can be read in during fsck. Also, the multiple reads which were used before are replaced with a single read per resource group. This is an example of the kinds of speed up which may well be possible elsewhere in the code. I started with this example simply because it was the easiest one to do. An alternative implementation might O_DIRECT and aio, but I'm not sure that there would be much benefit compared with this method. A further thought would be to use drop behind in places where we know that we will not be looking at the data again. Taking timings for just the rgrp reading section of fsck, I see almost a 10x speed up for that section of code using this patch on a 500G filesystem. rhbz#902920 --- gfs2/fsck/main.c | 4 ++ gfs2/libgfs2/buf.c | 83 ++++++++++++++++++++++++++++++++++++++++------- gfs2/libgfs2/libgfs2.h | 10 +++++- gfs2/libgfs2/rgrp.c | 6 ++-- gfs2/libgfs2/super.c | 35 ++++++++++++++++++++ 5 files changed, 120 insertions(+), 18 deletions(-) diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c index 676b676..ec44c83 100644 --- a/gfs2/fsck/main.c +++ b/gfs2/fsck/main.c @@ -35,6 +35,8 @@ struct osi_root inodetree = (struct osi_root) { NULL, }; int dups_found = 0, dups_found_first = 0; struct gfs_sb *sbd1 = NULL; +extern FILE *brdfp; + /* This function is for libgfs2's sake. */ void print_it(const char *label, const char *fmt, const char *fmt2, ...) { @@ -232,6 +234,8 @@ int main(int argc, char **argv) memset(sdp, 0, sizeof(*sdp)); +/* brdfp = fopen("brd-log.txt", "w"); */ + if ((error = read_cmdline(argc, argv, &opts))) exit(error); setbuf(stdout, NULL); diff --git a/gfs2/libgfs2/buf.c b/gfs2/libgfs2/buf.c index 5a0f718..fdd6e3d 100644 --- a/gfs2/libgfs2/buf.c +++ b/gfs2/libgfs2/buf.c @@ -5,6 +5,7 @@ #include <inttypes.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/time.h> #include <fcntl.h> #include <unistd.h> #include <errno.h> @@ -12,6 +13,61 @@ #include "libgfs2.h" +FILE *brdfp = NULL; + +int __breadm(struct gfs2_sbd *sdp, struct gfs2_buffer_head **bhs, size_t n, + uint64_t block, int line, const char *caller) +{ + struct iovec *iov = alloca(n * sizeof(struct iovec)); + struct iovec *iovbase = iov; + struct timeval st, et, tt; + uint64_t b = block; + size_t size = 0; + size_t i; + int ret; + + for (i = 0; i < n; i++) { + bhs[i] = bget(sdp, b++); + if (bhs[i] == NULL) + return -1; + *iov++ = bhs[i]->iov; + size += bhs[i]->iov.iov_len; + } + + gettimeofday(&st, NULL); + ret = preadv(sdp->device_fd, iovbase, n, block * sdp->bsize); + gettimeofday(&et, NULL); + + if (ret != size) { + fprintf(stderr, "bad read: %s from %s:%d: block " + "%llu (0x%llx)\n", strerror(errno), + caller, line, (unsigned long long)block, + (unsigned long long)block); + exit(-1); + } + + if (brdfp) { + unsigned long usecs; + timersub(&et, &st, &tt); + usecs = tt.tv_usec + 1000000*tt.tv_sec; + fprintf(brdfp, "%lu\t%d\t%s\n", usecs, line, caller); + } + + return 0; +} + +struct gfs2_buffer_head *__bread(struct gfs2_sbd *sdp, uint64_t num, int line, + const char *caller) +{ + struct gfs2_buffer_head *bh; + int ret; + + ret = __breadm(sdp, &bh, 1, num, line, caller); + if (ret >= 0) + return bh; + return NULL; +} + struct gfs2_buffer_head *__bget_generic(struct gfs2_sbd *sdp, uint64_t num, int read_disk, int line, const char *caller) @@ -46,27 +102,28 @@ struct gfs2_buffer_head *__bget_generic(struct gfs2_sbd *sdp, uint64_t num, return bh; } -struct gfs2_buffer_head *__bget(struct gfs2_sbd *sdp, uint64_t num, int line, - const char *caller) +struct gfs2_buffer_head *bget(struct gfs2_sbd *sdp, uint64_t num) { - return __bget_generic(sdp, num, FALSE, line, caller); -} + struct gfs2_buffer_head *bh; -struct gfs2_buffer_head *__bread(struct gfs2_sbd *sdp, uint64_t num, int line, - const char *caller) -{ - return __bget_generic(sdp, num, TRUE, line, caller); + bh = calloc(1, sizeof(struct gfs2_buffer_head) + sdp->bsize); + if (bh == NULL) + return NULL; + + bh->b_blocknr = num; + bh->sdp = sdp; + bh->iov.iov_base = (char *)bh + sizeof(struct gfs2_buffer_head); + bh->iov.iov_len = sdp->bsize; + + return bh; } int bwrite(struct gfs2_buffer_head *bh) { struct gfs2_sbd *sdp = bh->sdp; - if (lseek(sdp->device_fd, bh->b_blocknr * sdp->bsize, SEEK_SET) != - bh->b_blocknr * sdp->bsize) { - return -1; - } - if (write(sdp->device_fd, bh->b_data, sdp->bsize) != sdp->bsize) + if (pwritev(sdp->device_fd, &bh->iov, 1, bh->b_blocknr * sdp->bsize) != + bh->iov.iov_len) return -1; sdp->writes++; bh->b_modified = 0; diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index 1b0b98d..bf834a5 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -10,6 +10,7 @@ #include <stdlib.h> #include <string.h> #include <sys/types.h> +#include <sys/uio.h> #include <linux/types.h> #include <linux/limits.h> #include <endian.h> @@ -112,7 +113,10 @@ struct gfs2_buffer_head { osi_list_t b_altlist; /* alternate list */ uint64_t b_blocknr; int b_modified; - char *b_data; + union { + struct iovec iov; + char *b_data; + }; struct gfs2_sbd *sdp; }; @@ -291,6 +295,8 @@ extern struct gfs2_buffer_head *__bget(struct gfs2_sbd *sdp, uint64_t num, int line, const char *caller); extern struct gfs2_buffer_head *__bread(struct gfs2_sbd *sdp, uint64_t num, int line, const char *caller); +extern struct gfs2_buffer_head *bget(struct gfs2_sbd *sdp, uint64_t num); +extern int __breadm(struct gfs2_sbd *sdp, struct gfs2_buffer_head **bhs, size_t n, uint64_t block, int line, const char *caller); extern int bwrite(struct gfs2_buffer_head *bh); extern int brelse(struct gfs2_buffer_head *bh); @@ -299,8 +305,8 @@ extern int brelse(struct gfs2_buffer_head *bh); #define bget_generic(bl, num, find, read) __bget_generic(bl, num, find, read, \ __LINE__, \ __FUNCTION__) -#define bget(bl, num) __bget(bl, num, __LINE__, __FUNCTION__) #define bread(bl, num) __bread(bl, num, __LINE__, __FUNCTION__) +#define breadm(bl, bhs, n, block) __breadm(bl, bhs, n, block, __LINE__, __FUNCTION__) #define bsync(bl) do { __bsync(bl, __LINE__, __FUNCTION__); } while(0) #define bcommit(bl) do { __bcommit(bl, __LINE__, __FUNCTION__); } while(0) diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c index 0fcea3c..64703e0 100644 --- a/gfs2/libgfs2/rgrp.c +++ b/gfs2/libgfs2/rgrp.c @@ -129,10 +129,10 @@ uint64_t gfs2_rgrp_read(struct gfs2_sbd *sdp, struct rgrp_tree *rgd) return -1; if (gfs2_check_range(sdp, rgd->ri.ri_addr)) return -1; + if (breadm(sdp, rgd->bh, length, rgd->ri.ri_addr)) + return -1; for (x = 0; x < length; x++){ - rgd->bh[x] = bread(sdp, rgd->ri.ri_addr + x); - if(gfs2_check_meta(rgd->bh[x], - (x) ? GFS2_METATYPE_RB : GFS2_METATYPE_RG)) + if(gfs2_check_meta(rgd->bh[x], (x) ? GFS2_METATYPE_RB : GFS2_METATYPE_RG)) { uint64_t error; diff --git a/gfs2/libgfs2/super.c b/gfs2/libgfs2/super.c index f8de2e6..8f49dff 100644 --- a/gfs2/libgfs2/super.c +++ b/gfs2/libgfs2/super.c @@ -5,6 +5,7 @@ #include <stdlib.h> #include <string.h> #include <errno.h> +#include <fcntl.h> #include "libgfs2.h" #include "osi_list.h" @@ -206,6 +207,29 @@ int rindex_read(struct gfs2_sbd *sdp, int fd, int *count1, int *sane) return 0; } +#define RA_WINDOW 32 + +static unsigned gfs2_rgrp_reada(struct gfs2_sbd *sdp, unsigned cur_window, + struct osi_node *n) +{ + struct rgrp_tree *rgd; + unsigned i; + off_t start, len; + + for (i = 0; i < RA_WINDOW; i++, n = osi_next(n)) { + if (n == NULL) + return i; + if (i < cur_window) + continue; + rgd = (struct rgrp_tree *)n; + start = rgd->ri.ri_addr * sdp->bsize; + len = rgd->ri.ri_length * sdp->bsize; + posix_fadvise(sdp->device_fd, start, len, POSIX_FADV_WILLNEED); + } + + return i; +} + /** * ri_update - attach rgrps to the super block * @sdp: incore superblock data @@ -226,15 +250,24 @@ static int __ri_update(struct gfs2_sbd *sdp, int fd, int *rgcount, int *sane, uint64_t errblock = 0; uint64_t rmax = 0; struct osi_node *n, *next = NULL; + unsigned ra_window = 0; + + /* Turn off generic readhead */ + posix_fadvise(sdp->device_fd, 0, 0, POSIX_FADV_RANDOM); if (rindex_read(sdp, fd, &count1, sane)) goto fail; for (n = osi_first(&sdp->rgtree); n; n = next) { next = osi_next(n); rgd = (struct rgrp_tree *)n; + /* Readahead resource group headers */ + if (ra_window < RA_WINDOW/2) + ra_window = gfs2_rgrp_reada(sdp, ra_window, n); + /* Read resource group header */ errblock = gfs2_rgrp_read(sdp, rgd); if (errblock) return errblock; + ra_window--; count2++; if (!quiet && count2 % 100 == 0) { printf("."); @@ -250,9 +283,11 @@ static int __ri_update(struct gfs2_sbd *sdp, int fd, int *rgcount, int *sane, if (count1 != count2) goto fail; + posix_fadvise(sdp->device_fd, 0, 0, POSIX_FADV_NORMAL); return 0; fail: + posix_fadvise(sdp->device_fd, 0, 0, POSIX_FADV_NORMAL); gfs2_rgrp_free(&sdp->rgtree); return -1; }

1 0

cluster: RHEL510 - Med: Don't preserve SELinux context when copying files to /var/lib/nfs/sm
by Ryan McCabe 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=0c3782fd3fa61… Commit: 0c3782fd3fa61e9ceea938cc4f61e8572c6916c8 Parent: d45a4fe2fa265a19a4130161caa4ce92367f9072 Author: Ryan McCabe <rmccabe(a)redhat.com> AuthorDate: Mon May 20 15:16:02 2013 -0400 Committer: Ryan McCabe <rmccabe(a)redhat.com> CommitterDate: Mon May 20 23:46:49 2013 -0400 Med: Don't preserve SELinux context when copying files to /var/lib/nfs/sm Pass the flags -Rdpf instead of -af to cp when copying files to /var/lib/nfs/sm so that the SELinux context is inherited from the target directory and not preserved from the files being copied. Resolves: rhbz#907898 Signed-off-by: Ryan McCabe <rmccabe(a)redhat.com> --- rgmanager/src/resources/svclib_nfslock | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rgmanager/src/resources/svclib_nfslock b/rgmanager/src/resources/svclib_nfslock index 2ec7bdd..fbfa94e 100644 --- a/rgmanager/src/resources/svclib_nfslock +++ b/rgmanager/src/resources/svclib_nfslock @@ -228,7 +228,7 @@ notify_list_store() fi owner=$(ls -dl /var/lib/nfs/statd/sm | awk '{print $3"."$4}') - cp -af /var/lib/nfs/statd/sm/* $nl_dir/sm + cp -Rdpf /var/lib/nfs/statd/sm/* $nl_dir/sm chown -R $owner $nl_dir return 0 elif [ -d "/var/lib/nfs/sm" ]; then @@ -238,7 +238,7 @@ notify_list_store() fi owner=$(ls -dl /var/lib/nfs/sm | awk '{print $3"."$4}') - cp -af /var/lib/nfs/sm/* $nl_dir/sm + cp -Rdpf /var/lib/nfs/sm/* $nl_dir/sm chown -R $owner $nl_dir return 0 fi @@ -265,12 +265,12 @@ notify_list_merge() if [ -d "/var/lib/nfs/statd/sm" ]; then owner=$(ls -dl /var/lib/nfs/statd/sm | awk '{print $3"."$4}') - cp -af $nl_dir/sm/* /var/lib/nfs/statd/sm + cp -Rdpf $nl_dir/sm/* /var/lib/nfs/statd/sm chown -R $owner $nl_dir return 0 elif [ -d "/var/lib/nfs/sm" ]; then owner=$(ls -dl /var/lib/nfs/sm | awk '{print $3"."$4}') - cp -af $nl_dir/sm/* /var/lib/nfs/sm + cp -Rdpf $nl_dir/sm/* /var/lib/nfs/sm chown -R $owner $nl_dir return 0 fi

1 0

cluster: RHEL510 - HA LVM should only remove missing PVs on stop when they belong to mirrors
by Ryan McCabe 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=d45a4fe2fa265… Commit: d45a4fe2fa265a19a4130161caa4ce92367f9072 Parent: 1d7a0335016cae0246f455a707f78bfc5d1ff880 Author: John Ruemker <jruemker(a)redhat.com> AuthorDate: Tue May 14 15:19:19 2013 -0500 Committer: Ryan McCabe <rmccabe(a)redhat.com> CommitterDate: Mon May 20 23:46:49 2013 -0400 HA LVM should only remove missing PVs on stop when they belong to mirrors This adds --mirrorsonly to the 3 'vgreduce --removemissing' calls in the LVM agents. You'll also notice that it adds another self_fence check after we fail to remove tags. In my previous comment, I pointed out that in the case of single-host by_lv, after we vgreduce we then can't deactivate the logical volume again because it doesn't exist. This results in us executing self_fence, which may have just been a happy accident. But when we avoid making metadata changes by adding --mirrorsonly, the subsequent deactivation is still successful, and thus we miss the self_fence logic. So, I added another check so we still catch the failure and fence ourselves in this situation. Resolves: rhbz#962376 Signed-off-by: John Ruemker <jruemker(a)redhat.com> Signed-off-by: Jonthan Brassow <jbrassow(a)redhat.com> Signed-off-by: Ryan McCabe <rmccabe(a)redhat.com> --- rgmanager/src/resources/lvm_by_lv.sh | 14 ++++++++++---- rgmanager/src/resources/lvm_by_vg.sh | 4 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/rgmanager/src/resources/lvm_by_lv.sh b/rgmanager/src/resources/lvm_by_lv.sh index 4971173..7140076 100644 --- a/rgmanager/src/resources/lvm_by_lv.sh +++ b/rgmanager/src/resources/lvm_by_lv.sh @@ -243,13 +243,19 @@ lv_activate_and_tag() # be removed from the VG via a separate call before # the tag can be removed. ocf_log err "Attempting volume group clean-up and retry" - vgreduce --removemissing --force $OCF_RESKEY_vg_name + vgreduce --removemissing --mirrorsonly --force $OCF_RESKEY_vg_name # Retry tag deletion lvchange --deltag $tag $lv_path if [ $? -ne 0 ]; then - ocf_log err "Failed to delete tag from $lv_path" - return $OCF_ERR_GENERIC + if [ "$self_fence" ]; then + ocf_log err "Failed to delete tag from $lv_path: REBOOTING" + sync + reboot -fn + else + ocf_log err "Failed to delete tag from $lv_path" + fi + return $OCF_ERR_GENERIC fi fi @@ -322,7 +328,7 @@ lv_activate() ocf_log notice "Attempting cleanup of $OCF_RESKEY_vg_name" - if vgreduce --removemissing --force --config \ + if vgreduce --removemissing --mirrorsonly --force --config \ "activation { volume_list = \"$OCF_RESKEY_vg_name\" }" \ $OCF_RESKEY_vg_name; then ocf_log notice "$OCF_RESKEY_vg_name now consistent" diff --git a/rgmanager/src/resources/lvm_by_vg.sh b/rgmanager/src/resources/lvm_by_vg.sh index 0dd2aaa..819d0b8 100755 --- a/rgmanager/src/resources/lvm_by_vg.sh +++ b/rgmanager/src/resources/lvm_by_vg.sh @@ -202,7 +202,7 @@ function vg_start_clustered ocf_log err "Failed to activate volume group, $OCF_RESKEY_vg_name" ocf_log notice "Attempting cleanup of $OCF_RESKEY_vg_name" - if ! vgreduce --removemissing --force $OCF_RESKEY_vg_name; then + if ! vgreduce --removemissing --mirrorsonly --force $OCF_RESKEY_vg_name; then ocf_log err "Failed to make $OCF_RESKEY_vg_name consistent" return $OCF_ERR_GENERIC fi @@ -398,7 +398,7 @@ function vg_stop_single # Shut down the volume group # Do we need to make this resilient? - vgchange -an $OCF_RESKEY_vg_name + vgchange -aln $OCF_RESKEY_vg_name # Make sure all the logical volumes are inactive results=(`lvs -o name,attr --noheadings $OCF_RESKEY_vg_name 2> /dev/null`)

1 0

cluster: RHEL510 - Med: Don't preserve SELinux context when copying files to /var/lib/nfs/sm
by Ryan McCabe 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=0c3782fd3fa61… Commit: 0c3782fd3fa61e9ceea938cc4f61e8572c6916c8 Parent: d45a4fe2fa265a19a4130161caa4ce92367f9072 Author: Ryan McCabe <rmccabe(a)redhat.com> AuthorDate: Mon May 20 15:16:02 2013 -0400 Committer: Ryan McCabe <rmccabe(a)redhat.com> CommitterDate: Mon May 20 23:46:49 2013 -0400 Med: Don't preserve SELinux context when copying files to /var/lib/nfs/sm Pass the flags -Rdpf instead of -af to cp when copying files to /var/lib/nfs/sm so that the SELinux context is inherited from the target directory and not preserved from the files being copied. Resolves: rhbz#907898 Signed-off-by: Ryan McCabe <rmccabe(a)redhat.com> --- rgmanager/src/resources/svclib_nfslock | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rgmanager/src/resources/svclib_nfslock b/rgmanager/src/resources/svclib_nfslock index 2ec7bdd..fbfa94e 100644 --- a/rgmanager/src/resources/svclib_nfslock +++ b/rgmanager/src/resources/svclib_nfslock @@ -228,7 +228,7 @@ notify_list_store() fi owner=$(ls -dl /var/lib/nfs/statd/sm | awk '{print $3"."$4}') - cp -af /var/lib/nfs/statd/sm/* $nl_dir/sm + cp -Rdpf /var/lib/nfs/statd/sm/* $nl_dir/sm chown -R $owner $nl_dir return 0 elif [ -d "/var/lib/nfs/sm" ]; then @@ -238,7 +238,7 @@ notify_list_store() fi owner=$(ls -dl /var/lib/nfs/sm | awk '{print $3"."$4}') - cp -af /var/lib/nfs/sm/* $nl_dir/sm + cp -Rdpf /var/lib/nfs/sm/* $nl_dir/sm chown -R $owner $nl_dir return 0 fi @@ -265,12 +265,12 @@ notify_list_merge() if [ -d "/var/lib/nfs/statd/sm" ]; then owner=$(ls -dl /var/lib/nfs/statd/sm | awk '{print $3"."$4}') - cp -af $nl_dir/sm/* /var/lib/nfs/statd/sm + cp -Rdpf $nl_dir/sm/* /var/lib/nfs/statd/sm chown -R $owner $nl_dir return 0 elif [ -d "/var/lib/nfs/sm" ]; then owner=$(ls -dl /var/lib/nfs/sm | awk '{print $3"."$4}') - cp -af $nl_dir/sm/* /var/lib/nfs/sm + cp -Rdpf $nl_dir/sm/* /var/lib/nfs/sm chown -R $owner $nl_dir return 0 fi

1 0

cluster: RHEL510 - HA LVM should only remove missing PVs on stop when they belong to mirrors
by Ryan McCabe 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=d45a4fe2fa265… Commit: d45a4fe2fa265a19a4130161caa4ce92367f9072 Parent: 1d7a0335016cae0246f455a707f78bfc5d1ff880 Author: John Ruemker <jruemker(a)redhat.com> AuthorDate: Tue May 14 15:19:19 2013 -0500 Committer: Ryan McCabe <rmccabe(a)redhat.com> CommitterDate: Mon May 20 23:46:49 2013 -0400 HA LVM should only remove missing PVs on stop when they belong to mirrors This adds --mirrorsonly to the 3 'vgreduce --removemissing' calls in the LVM agents. You'll also notice that it adds another self_fence check after we fail to remove tags. In my previous comment, I pointed out that in the case of single-host by_lv, after we vgreduce we then can't deactivate the logical volume again because it doesn't exist. This results in us executing self_fence, which may have just been a happy accident. But when we avoid making metadata changes by adding --mirrorsonly, the subsequent deactivation is still successful, and thus we miss the self_fence logic. So, I added another check so we still catch the failure and fence ourselves in this situation. Resolves: rhbz#962376 Signed-off-by: John Ruemker <jruemker(a)redhat.com> Signed-off-by: Jonthan Brassow <jbrassow(a)redhat.com> Signed-off-by: Ryan McCabe <rmccabe(a)redhat.com> --- rgmanager/src/resources/lvm_by_lv.sh | 14 ++++++++++---- rgmanager/src/resources/lvm_by_vg.sh | 4 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/rgmanager/src/resources/lvm_by_lv.sh b/rgmanager/src/resources/lvm_by_lv.sh index 4971173..7140076 100644 --- a/rgmanager/src/resources/lvm_by_lv.sh +++ b/rgmanager/src/resources/lvm_by_lv.sh @@ -243,13 +243,19 @@ lv_activate_and_tag() # be removed from the VG via a separate call before # the tag can be removed. ocf_log err "Attempting volume group clean-up and retry" - vgreduce --removemissing --force $OCF_RESKEY_vg_name + vgreduce --removemissing --mirrorsonly --force $OCF_RESKEY_vg_name # Retry tag deletion lvchange --deltag $tag $lv_path if [ $? -ne 0 ]; then - ocf_log err "Failed to delete tag from $lv_path" - return $OCF_ERR_GENERIC + if [ "$self_fence" ]; then + ocf_log err "Failed to delete tag from $lv_path: REBOOTING" + sync + reboot -fn + else + ocf_log err "Failed to delete tag from $lv_path" + fi + return $OCF_ERR_GENERIC fi fi @@ -322,7 +328,7 @@ lv_activate() ocf_log notice "Attempting cleanup of $OCF_RESKEY_vg_name" - if vgreduce --removemissing --force --config \ + if vgreduce --removemissing --mirrorsonly --force --config \ "activation { volume_list = \"$OCF_RESKEY_vg_name\" }" \ $OCF_RESKEY_vg_name; then ocf_log notice "$OCF_RESKEY_vg_name now consistent" diff --git a/rgmanager/src/resources/lvm_by_vg.sh b/rgmanager/src/resources/lvm_by_vg.sh index 0dd2aaa..819d0b8 100755 --- a/rgmanager/src/resources/lvm_by_vg.sh +++ b/rgmanager/src/resources/lvm_by_vg.sh @@ -202,7 +202,7 @@ function vg_start_clustered ocf_log err "Failed to activate volume group, $OCF_RESKEY_vg_name" ocf_log notice "Attempting cleanup of $OCF_RESKEY_vg_name" - if ! vgreduce --removemissing --force $OCF_RESKEY_vg_name; then + if ! vgreduce --removemissing --mirrorsonly --force $OCF_RESKEY_vg_name; then ocf_log err "Failed to make $OCF_RESKEY_vg_name consistent" return $OCF_ERR_GENERIC fi @@ -398,7 +398,7 @@ function vg_stop_single # Shut down the volume group # Do we need to make this resilient? - vgchange -an $OCF_RESKEY_vg_name + vgchange -aln $OCF_RESKEY_vg_name # Make sure all the logical volumes are inactive results=(`lvs -o name,attr --noheadings $OCF_RESKEY_vg_name 2> /dev/null`)

1 0

cluster: RHEL510 - cman: enable and document logging of CMAN subsystem (additional fix)
by Jan Pokorný 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=1d7a0335016ca… Commit: 1d7a0335016cae0246f455a707f78bfc5d1ff880 Parent: 7473b8cf0199b7f0e819cd78bd53f215ed371095 Author: Jan Pokorný <jpokorny(a)redhat.com> AuthorDate: Mon May 20 20:51:42 2013 +0200 Committer: Jan Pokorný <jpokorny(a)redhat.com> CommitterDate: Mon May 20 20:51:42 2013 +0200 cman: enable and document logging of CMAN subsystem (additional fix) Related: rhbz#963251 Signed-off-by: Jan Pokorný <jpokorny(a)redhat.com> --- cman/daemon/logging.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/cman/daemon/logging.c b/cman/daemon/logging.c index 4f6e731..44c3d0f 100644 --- a/cman/daemon/logging.c +++ b/cman/daemon/logging.c @@ -81,6 +81,7 @@ void log_debug(int subsys, int stamp, const char *fmt, ...) strcpy(newfmt, "ais: "); break; default: + newfmt[0] = '\0'; break; } }

1 0

cluster: RHEL59 - rgmanager: Update the Oracle resource agents to support Oracle 11g
by Ryan McCabe 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=3ac6fa1287778… Commit: 3ac6fa12877780c28b07c9d2402687f878ba374e Parent: c830187908b759b5b5203efc0fa441fc401b7f1b Author: Ryan McCabe <rmccabe(a)redhat.com> AuthorDate: Thu May 16 16:51:55 2013 -0400 Committer: Ryan McCabe <rmccabe(a)redhat.com> CommitterDate: Mon May 20 14:21:13 2013 -0400 rgmanager: Update the Oracle resource agents to support Oracle 11g Add Oracle 11g support to the orainstance, oralistener, and oracledb resource agents. Resolves: rhbz#964991 Signed-off-by: Ryan McCabe <rmccabe(a)redhat.com> --- rgmanager/src/resources/oracledb.sh | 428 ++++++++++++++++++++------------ rgmanager/src/resources/orainstance.sh | 298 +++++++++++++--------- rgmanager/src/resources/oralistener.sh | 224 ++++++++++------- 3 files changed, 578 insertions(+), 372 deletions(-) diff --git a/rgmanager/src/resources/oracledb.sh b/rgmanager/src/resources/oracledb.sh index bcb367f..2bbb807 100755 --- a/rgmanager/src/resources/oracledb.sh +++ b/rgmanager/src/resources/oracledb.sh @@ -1,22 +1,27 @@ #!/bin/bash # -# Copyright 2003-2004, 2006-2008 Red Hat, Inc. +# Copyright (C) 1997-2003 Sistina Software, Inc. All rights reserved. +# Copyright (C) 2004-2013 Red Hat, Inc. All rights reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. # # Author(s): # Hardy Merrill <hmerrill at redhat.com> # Lon Hohberger <lhh at redhat.com> # Michael Moon <Michael dot Moon at oracle.com> -# -# This program is Open Source software. You may modify and/or redistribute -# it persuant to the terms of the Open Software License version 2.1, which -# is available from the following URL and is included herein by reference: -# -# http://opensource.org/licenses/osl-2.1.php -# -# chkconfig: 345 99 01 -# description: Service script for starting/stopping \ -# Oracle(R) Database 10g on \ -# Red Hat Enterprise Linux 5 +# Ryan McCabe <rmccabe at redhat.com> # # NOTES: # @@ -35,15 +40,16 @@ # Oracle is a registered trademark of Oracle Corporation. # Oracle9i is a trademark of Oracle Corporation. # Oracle10g is a trademark of Oracle Corporation. +# Oracle11g is a trademark of Oracle Corporation. # All other trademarks are property of their respective owners. # -. /etc/init.d/functions +. $(dirname $0)/ocf-shellfuncs +. $(dirname $0)/utils/config-utils.sh +. $(dirname $0)/utils/messages.sh +. $(dirname $0)/utils/ra-skelet.sh -# -# Source stuff from /etc/sysconfig, but this may be overridden if -# this is being called as a cluster resource agent instead. -#. /etc/sysconfig/oracledb +. /etc/init.d/functions declare SCRIPT="`basename $0`" declare SCRIPTDIR="`dirname $0`" @@ -122,6 +128,15 @@ declare -r DB_PROCNAMES="pmon" declare -r LSNR_PROCNAME="tnslsnr" #declare -r LSNR_PROCNAME="tnslsnrXX" # testing +# clulog will not log messages when run by the oracle user. +# This is a hack to work around that. +if [ "`id -u`" = "`id -u $ORACLE_USER`" ]; then + ocf_log() { + prio=$1 + shift + logger -i -p daemon."$prio" -- "$*" + } +fi ########################################################## # (Hopefully) No user-serviceable parts below this line. # @@ -134,10 +149,10 @@ meta_data() <version>1.0</version> <longdesc lang="en"> - Oracle 10g Failover Instance + Oracle 10g/11g Failover Instance </longdesc> <shortdesc lang="en"> - Oracle 10g Failover Instance + Oracle 10g/11g Failover Instance </shortdesc> <parameters> @@ -164,7 +179,6 @@ meta_data() <content type="string"/> </parameter> - <parameter name="user" required="1"> <longdesc lang="en"> Oracle user name. This is the user name of the Oracle @@ -191,8 +205,10 @@ meta_data() <longdesc lang="en"> This is the Oracle installation type: base - Database Instance and Listener only + base-11g - Oracle11g Database Instance and Listener Only base-em (or 10g) - Database, Listener, Enterprise Manager, and iSQL*Plus + base-em-11g - Database, Listener, Enterprise Manager dbconsole ias (or 10g-ias) - Internet Application Server (Infrastructure) </longdesc> <shortdesc lang="en"> @@ -230,7 +246,7 @@ meta_data() <action name="monitor" depth="10" timeout="30" interval="30"/> <action name="meta-data" timeout="5"/> - <action name="verify-all" timeout="5"/> + <action name="validate-all" timeout="5"/> </actions> <special tag="rgmanager"> @@ -240,84 +256,55 @@ meta_data() EOT } - -# -# "action"-like macro supporting functions -# -faction() -{ - echo -n "$1" - shift - $* - if [ $? -eq 0 ]; then - echo_success - echo - return 0 - fi - - echo_failure - echo - return 1 -} - - # -# Start Oracle9i (database portion) +# Start Oracle9i/10g/11g (database portion) # start_db() { - declare tmpfile - declare logfile declare -i rv + declare startup_cmd + declare startup_stdout - tmpfile=$(mktemp /tmp/$SCRIPT-start.tmp.XXXXXX) - logfile=$(mktemp /tmp/$SCRIPT-start.log.XXXXXX) + ocf_log info "Starting Oracle DB $ORACLE_SID" - # # Set up our sqlplus script. Basically, we're trying to # capture output in the hopes that it's useful in the case # that something doesn't work properly. - # - echo "startup" > $tmpfile - echo "quit" >> $tmpfile - - sqlplus "/ as sysdba" < $tmpfile &> $logfile + startup_cmd="set heading off;\nstartup;\nquit;\n" + startup_stdout=$(echo -e "$startup_cmd" | sqlplus -S "/ as sysdba") rv=$? - # Dump logfile to /var/log/messages - initlog -q -c "cat $logfile" - + # Dump output to syslog for debugging + ocf_log debug "[$ORACLE_SID] [$rv] sent $startup_cmd" + ocf_log debug "[$ORACLE_SID] [$rv] got $startup_stdout" + if [ $rv -ne 0 ]; then - echo "ORACLE_HOME Incorrectly set?" - echo "See $logfile for more information." - return 1 + ocf_log error "Starting Oracle DB $ORACLE_SID failed, sqlplus returned $rv" + return 1 fi - # # If we see: # ORA-.....: failure, we failed - # - - rm -f $tmpfile - grep -q "^ORA-" $logfile - if [ $? -eq 0 ]; then - rm -f $tmpfile - echo "ORACLE_SID Incorrectly set?" - echo "See $logfile for more information." - return 1 + # Troubleshooting: + # ORA-00845 - Try rm -f /dev/shm/ora_* + # ORA-01081 - Try echo -e 'shutdown abort;\nquit;'|sqlplus "/ as sysdba" + if [[ "$startup_stdout" =~ "ORA-" ]] || [[ "$startup_stdout" =~ "failure" ]]; then + ocf_log error "Starting Oracle DB $ORACLE_SID failed, found errors in stdout" + return 1 fi + ocf_log info "Started Oracle DB $ORACLE_SID successfully" return 0 } # -# Stop Oracle9i (database portion) +# Stop Oracle (database portion) # stop_db() { - declare tmpfile - declare logfile + declare stop_cmd + declare stop_stdout declare -i rv declare how_shutdown="$1" @@ -325,38 +312,30 @@ stop_db() how_shutdown="immediate" fi - tmpfile=$(mktemp /tmp/$SCRIPT-stop.tmp.XXXXXX) - logfile=$(mktemp /tmp/$SCRIPT-stop.log.XXXXXX) + ocf_log info "Stopping Oracle DB $ORACLE_SID $how_shutdown" # Setup for Stop ... - echo "shutdown $how_shutdown" > $tmpfile - echo "quit" >> $tmpfile - - sqlplus "/ as sysdba" < $tmpfile &> $logfile + stop_cmd="set heading off;\nshutdown $how_shutdown;\nquit;\n" + stop_stdout=$(echo -e "$stop_cmd" | sqlplus -S "/ as sysdba") rv=$? - # Dump logfile to /var/log/messages - initlog -q -c "cat $logfile" - + # Log stdout of the stop command + ocf_log debug "[$ORACLE_SID] sent stop command $stop_cmd" + ocf_log debug "[$ORACLE_SID] got $stop_stdout" + + # sqlplus returned failure. We'll return failed to rhcs if [ $rv -ne 0 ]; then - echo "ORACLE_HOME Incorrectly set?" - echo "See $logfile for more information." - return 1 + ocf_log error "Stopping Oracle DB $ORACLE_SID failed, sqlplus returned $rv" + return 1 fi - # - # If we see 'failure' in the log, we're done. - # - rm -f $tmpfile - grep -q "^ORA-" $logfile - if [ $? -eq 0 ]; then - echo_failure - echo - echo "Possible reason: ORACLE_SID Incorrectly set." - echo "See $logfile for more information." - return 1 + # If we see 'ORA-' or 'failure' in stdout, we're done. + if [[ "$startup_stdout" =~ "ORA-" ]] || [[ "$startup_stdout" =~ "failure" ]]; then + ocf_log error "Stopping Oracle DB $ORACLE_SID failed, errors in stdout" + return 1 fi + ocf_log info "Stopped Oracle DB $ORACLE_SID successfully" return 0 } @@ -372,12 +351,15 @@ force_cleanup() # Patch from Shane Bradley to fix 471266 pids=`ps ax | grep $ORACLE_HOME | grep "ora_.*_${ORACLE_SID}" | grep -v grep | awk '{print $1}'` - initlog -n $SCRIPT -s "<err> Not all Oracle processes exited cleanly, killing" - + ocf_log error "Not all Oracle processes for $ORACLE_SID exited cleanly, killing" + for pid in $pids; do kill -9 $pid - if [ $? -eq 0 ]; then - initlog -n $SCRIPT -s "Killed $pid" + rv=$? + if [ $rv -eq 0 ]; then + ocf_log info "Cleanup $ORACLE_SID Killed PID $pid" + else + ocf_log error "Cleanup $ORACLE_SID Kill PID $pid failed: $rv" fi done @@ -392,14 +374,19 @@ force_cleanup() exit_idle() { declare -i n=0 + + ocf_log debug "Waiting for Oracle processes for $ORACLE_SID to terminate..." while ps ax | grep $ORACLE_HOME | grep -q -v grep; do if [ $n -ge 90 ]; then + ocf_log debug "Timed out while waiting for Oracle processes for $ORACLE_SID to terminate" force_cleanup return 0 fi sleep 1 ((n++)) done + + ocf_log debug "All Oracle processes for $ORACLE_SID have terminated" return 0 } @@ -438,24 +425,27 @@ get_db_status() for (( i=$RESTART_RETRIES ; i; i-- )) ; do # this db process is down - stop and # (re)start all ora_XXXX_$ORACLE_SID processes - initlog -q -n $SCRIPT -s "Restarting Oracle Database..." + ocf_log info "Restarting Oracle Database $ORACLE_SID" stop_db immediate - if [ $? != 0 ] ; then + if [ $? -ne 0 ] ; then # stop failed - return 1 + ocf_log error "Error stopping Oracle Database $ORACLE_SID" return 1 fi start_db - if [ $? == 0 ] ; then + if [ $? -eq 0 ] ; then # ora_XXXX_$ORACLE_SID processes started # successfully, so break out of the # stop/start # 'for' loop + ocf_log info "Restarted Oracle Database $ORACLE_SID successfully" break fi done if [ $i -eq 0 ]; then # stop/start's failed - return 1 (failure) + ocf_log error "Failed to restart Oracle Database $ORACLE_SID after $RESTART_RETRIES tries" return 1 fi done @@ -471,42 +461,46 @@ get_lsnr_status() declare -i subsys_lock=$1 declare -i rv - status $LSNR_PROCNAME + ocf_log debug "Checking status for listener $ORACLE_LISTENER" + lsnrctl status "$ORACLE_LISTENER" >& /dev/null rv=$? - if [ $rv == 0 ] ; then + if [ $rv -eq 0 ] ; then + ocf_log debug "Listener $ORACLE_LISTENER is up" return 0 # Listener is running fine fi - # # We're not supposed to be running, and we are, # in fact, not running. Return 3 - # if [ $subsys_lock -ne 0 ]; then + ocf_log debug "Listener $ORACLE_LISTENER is stopped as expected" return 3 fi - # # Listener is NOT running (but should be) - try to restart - # for (( i=$RESTART_RETRIES ; i; i-- )) ; do - - action "Restarting Oracle listener:" lsnrctl start \ - $ORACLE_LISTENER - lsnrctl status $ORACLE_LISTENER >& /dev/null - if [ $? == 0 ] ; then + ocf_log info "Listener $ORACLE_LISTENER is down, attempting to restart" + lsnrctl start "$ORACLE_LISTENER" >& /dev/null + lsnrctl status "$ORACLE_LISTENER" >& /dev/null + if [ $? -eq 0 ] ; then + ocf_log info "Listener $ORACLE_LISTENER was restarted successfully" break # Listener was (re)started and is running fine fi done if [ $i -eq 0 ]; then # stop/start's failed - return 1 (failure) + ocf_log error "Failed to restart listener $ORACLE_LISTENER after $RESTART_RETRIES tries" return 1 fi - status $LSNR_PROCNAME - if [ $? != 0 ] ; then + lsnrctl_stdout=$(lsnrctl status "$ORACLE_LISTENER") + rv=$? + if [ $rv -ne 0 ] ; then + ocf_log error "Starting listener $ORACLE_LISTENER failed: $rv output $lsnrctl_stdout" return 1 # Problem restarting the Listener fi + + ocf_log info "Listener $ORACLE_LISTENER started successfully" return 0 # Success restarting the Listener } @@ -541,12 +535,12 @@ get_opmn_proc_status() _status=`echo $_status | cut -f2 -d' '` if [ "${_status}" == "Alive" ] || [ "${_status}" == "Init" ]; then if [ $i -lt $RESTART_RETRIES ] ; then - echo " $comp$type_pretty restarted" + ocf_log info "$comp$type_pretty restarted" fi - echo " $comp$type_pretty (pid $_pid) is running..." + ocf_log info "$comp$type_pretty (pid $_pid) is running..." break else - echo " $comp$type_pretty is stopped" + ocf_log info "$comp$type_pretty is stopped" # # Try to restart it, but don't worry if we fail. OPMN @@ -564,6 +558,7 @@ get_opmn_proc_status() if [ $i -eq 0 ]; then # restarts failed - return 1 (failure) + ocf_log error "Failed to restart OPMN process $comp" return 1 fi @@ -584,7 +579,7 @@ get_opmn_status() # # OPMN not running?? # - echo "opmn is stopped" + ocf_log info "OPMN is stopped" if [ $subsys_lock -eq 0 ]; then # @@ -600,8 +595,8 @@ get_opmn_status() # # Print out the PIDs for everyone. # - echo "opmn is running..." - echo "opmn components:" + ocf_log info "OPMN is running..." + ocf_log info "opmn components:" # # Check the OPMN-managed processes @@ -615,6 +610,7 @@ get_opmn_status() # restarted. # if [ $ct_errors -ne 0 ]; then + ocf_log error "$ct_errors errors occurred while restarting OPMN-managed processes" return 1 fi return 0 @@ -659,10 +655,7 @@ update_status() # oops() { - echo "Please configure this script ($0) to" - echo "match your installation." - echo - echo " $1 failed validation checks." + ocf_log error "$ORACLE_SID: Fatal: $1 failed validation checks" exit 1 } @@ -673,6 +666,8 @@ oops() # validation_checks() { + ocf_log debug "Validating configuration for $ORACLE_SID" + # # If the oracle user doesn't exist, we're done. # @@ -701,8 +696,12 @@ validation_checks() ORACLE_TYPE="base-em" elif [ "$ORACLE_TYPE" = "10g-ias" ] || [ "$ORACLE_TYPE" = "ias" ]; then ORACLE_TYPE="ias" + elif [ "$ORACLE_TYPE" = "11g" ] || [ "$ORACLE_TYPE" = "base-em-11g" ]; then + ORACLE_TYPE="base-em-11g" + elif [ "$ORACLE_TYPE" = "base-11g" ]; then + ORACLE_TYPE="base-11g" else - oops ORACLE_TYPE + oops "ORACLE_TYPE $ORACLE_TYPE" fi # @@ -715,7 +714,7 @@ validation_checks() # Oracle needs to be run as the Oracle user, not root! # if [ "`id -u`" = "0" ]; then - echo "Restarting $0 as $ORACLE_USER." + #echo "Restarting $0 as $ORACLE_USER." # # Breaks on RHEL5 # exec sudo -u $ORACLE_USER $0 $* @@ -727,75 +726,191 @@ validation_checks() # # If we're not root and not the Oracle user, we're done. # - [ "`id -u`" = "`id -u $ORACLE_USER`" ] || exit 1 - [ "`id -g`" = "`id -g $ORACLE_USER`" ] || exit 1 + [ "`id -u`" = "`id -u $ORACLE_USER`" ] || oops "not ORACLE_USER after su" + [ "`id -g`" = "`id -g $ORACLE_USER`" ] || oops "not ORACLE_GROUP after su" # # Go home. # - cd $ORACLE_HOME + cd "$ORACLE_HOME" + ocf_log debug "Validation checks for $ORACLE_SID succeeded" return 0 } # -# Start Oracle9i Application Server Infrastructure +# Start Oracle 9i/10g/11g Application Server Infrastructure # start_oracle() { - faction "Starting Oracle Database:" start_db || return 1 - action "Starting Oracle Listener:" lsnrctl start $ORACLE_LISTENER || return 1 + ocf_log info "Starting service $ORACLE_SID" + + start_db + rv=$? + if [ $rv -ne 0 ]; then + ocf_log error "Starting service $ORACLE_SID failed" + return 1 + fi + + ocf_log info "Starting listener $ORACLE_LISTENER" + lsnrctl_stdout=$(lsnrctl start "$ORACLE_LISTENER") + rv=$? + if [ $rv -ne 0 ]; then + ocf_log debug "[$ORACLE_SID] Listener $ORACLE_LISTENER start returned $rv output $lsnrctl_stdout" + ocf_log error "Starting service $ORACLE_SID failed" + return 1 + fi if [ "$ORACLE_TYPE" = "base-em" ]; then - action "Starting iSQL*Plus:" isqlplusctl start || return 1 - action "Starting Oracle EM DB Console:" emctl start dbconsole || return 1 + ocf_log info "Starting iSQL*Plus for $ORACLE_SID" + isqlplusctl start + if [ $? -ne 0 ]; then + ocf_log error "iSQL*Plus startup for $ORACLE_SID failed" + ocf_log error "Starting service $ORACLE_SID failed" + return 1 + else + ocf_log info "iSQL*Plus startup for $ORACLE_SID succeeded" + fi + + ocf_log info "Starting Oracle EM DB Console for $ORACLE_SID" + emctl start dbconsole + if [ $? -ne 0 ]; then + ocf_log error "Oracle EM DB Console startup for $ORACLE_SID failed" + ocf_log error "Starting service $ORACLE_SID failed" + return 1 + else + ocf_log info "Oracle EM DB Console startup for $ORACLE_SID succeeded" + fi elif [ "$ORACLE_TYPE" = "ias" ]; then - action "Starting Oracle EM:" emctl start em || return 1 - action "Starting iAS Infrastructure:" opmnctl startall || return 1 + ocf_log info "Starting Oracle EM for $ORACLE_SID" + emctl start em + if [ $? -ne 0 ]; then + ocf_log error "Oracle EM startup for $ORACLE_SID failed" + ocf_log error "Starting service $ORACLE_SID failed" + return 1 + else + ocf_log info "Oracle EM startup for $ORACLE_SID succeeded" + fi + + ocf_log info "Starting iAS Infrastructure for $ORACLE_SID" + opmnctl startall + if [ $? -ne 0 ]; then + ocf_log error "iAS Infrastructure startup for $ORACLE_SID failed" + ocf_log error "Starting service $ORACLE_SID failed" + return 1 + else + ocf_log info "iAS Infrastructure startup for $ORACLE_SID succeeded" + fi + elif [ "$ORACLE_TYPE" = "base-em-11g" ]; then + ocf_log info "Starting Oracle EM DB Console for $ORACLE_SID" + emctl start dbconsole + if [ $? -ne 0 ]; then + ocf_log error "Oracle EM DB Console startup for $ORACLE_SID failed" + ocf_log error "Starting service $ORACLE_SID failed" + return 1 + else + ocf_log info "Oracle EM DB Console startup for $ORACLE_SID succeeded" + fi fi if [ -n "$LOCKFILE" ]; then - touch $LOCKFILE + touch "$LOCKFILE" fi + + ocf_log info "Starting service $ORACLE_SID completed successfully" return 0 } # -# Stop Oracle9i Application Server Infrastructure +# Stop Oracle 9i/10g/11g Application Server Infrastructure # stop_oracle() { + ocf_log info "Stopping service $ORACLE_SID" + if ! [ -e "$ORACLE_HOME/bin/lsnrctl" ]; then - echo "Oracle Listener Control is not available" - echo " ($ORACLE_HOME not mounted?)" + ocf_log error "Oracle Listener Control is not available ($ORACLE_HOME not mounted?)" return 0 fi if [ "$ORACLE_TYPE" = "base-em" ]; then - action "Stopping Oracle EM DB Console:" emctl stop dbconsole || return 1 - action "Stopping iSQL*Plus:" isqlplusctl stop || return 1 + ocf_log info "Stopping Oracle EM DB Console for $ORACLE_SID" + emctl stop dbconsole + if [ $? -ne 0 ]; then + ocf_log error "Stopping Oracle EM DB Console for $ORACLE_SID failed" + ocf_log error "Stopping service $ORACLE_SID failed" + return 1 + else + ocf_log info "Stopping Oracle EM DB Console for $ORACLE_SID succeeded" + fi + + ocf_log info "Stopping iSQL*Plus for $ORACLE_SID" + isqlplusctl stop + if [ $? -ne 0 ]; then + ocf_log error "Stopping iSQL*Plus for $ORACLE_SID failed" + ocf_log error "Stopping service $ORACLE_SID failed" + return 1 + else + ocf_log info "Stopping iSQL*Plus for $ORACLE_SID succeeded" + fi elif [ "$ORACLE_TYPE" = "ias" ]; then - action "Stopping iAS Infrastructure:" opmnctl stopall || return 1 - action "Stopping Oracle EM:" emctl stop em || return 1 + ocf_log info "Stopping iAS Infrastructure for $ORACLE_SID" + opmnctl stopall + if [ $? -ne 0 ]; then + ocf_log error "Stopping iAS Infrastructure for $ORACLE_SID failed" + ocf_log error "Stopping service $ORACLE_SID failed" + return 1 + else + ocf_log info "Stopping iAS Infrastructure for $ORACLE_SID succeeded" + fi + + ocf_log info "Stopping Oracle EM for $ORACLE_SID" + emctl stop em + if [ $? -ne 0 ]; then + ocf_log error "Stopping Oracle EM for $ORACLE_SID failed" + ocf_log error "Stopping service $ORACLE_SID failed" + return 1 + else + ocf_log info "Stopping Oracle EM for $ORACLE_SID succeeded" + fi + elif [ "$ORACLE_TYPE" = "base-em-11g" ]; then + ocf_log info "Stopping Oracle EM DB Console for $ORACLE_SID" + emctl stop dbconsole + if [ $? -ne 0 ]; then + ocf_log error "Stopping Oracle EM DB Console for $ORACLE_SID failed" + ocf_log error "Stopping service $ORACLE_SID failed" + return 1 + else + ocf_log info "Stopping Oracle EM DB Console for $ORACLE_SID succeeded" + fi fi - faction "Stopping Oracle Database:" stop_db immediate + stop_db immediate || stop_db abort if [ $? -ne 0 ]; then - faction "Stopping Oracle Database (hard):" stop_db abort || return 1 + ocf_log error "Stopping service $ORACLE_SID failed" + return 1 fi - action "Stopping Oracle Listener:" lsnrctl stop $ORACLE_LISTENER - faction "Waiting for all Oracle processes to exit:" exit_idle + ocf_log info "Stopping listener $ORACLE_LISTENER for $ORACLE_SID" + lsnrctl_stdout=$(lsnrctl stop "$ORACLE_LISTENER") + rv=$? + if [ $? -ne 0 ]; then + ocf_log error "Listener $ORACLE_LISTENER stop failed for $ORACLE_SID: $rv output $lsnrctl_stdout" + # XXX - failure? + fi + exit_idle if [ $? -ne 0 ]; then - echo "WARNING: Not all Oracle processes exited cleanly" + ocf_log warning "WARNING: Not all Oracle processes exited cleanly for $ORACLE_SID" fi if [ -n "$LOCKFILE" ]; then - rm -f $LOCKFILE + rm -f "$LOCKFILE" fi + + ocf_log info "Stopping service $ORACLE_SID succeeded" return 0 } @@ -823,10 +938,12 @@ status_oracle() declare -i subsys_lock=1 declare -i last + ocf_log debug "Checking status for $ORACLE_SID depth $depth" + # # Check for lock file. Crude and rudimentary, but it works # - if [ -z "$LOCKFILE" ] || [ -f $LOCKFILE ]; then + if [ -z "$LOCKFILE" ] || [ -f "$LOCKFILE" ]; then subsys_lock=0 fi @@ -840,9 +957,9 @@ status_oracle() update_status $? $last last=$? - if [ "$ORACLE_TYPE" = "base-em" ]; then + if [ "$ORACLE_TYPE" = "base-em" ] || [ "$ORACLE_TYPE" = "base-em-11g" ]; then # XXX Add isqlplus status check?! - emctl status dbconsole 2>&1 | grep "is running" + emctl status dbconsole >&/dev/null update_status $? $last last=$? elif [ "$ORACLE_TYPE" = "ias" ]; then @@ -857,9 +974,10 @@ status_oracle() # file back. XXX - this kosher? # if [ $last -eq 0 ] && [ $subsys_lock -ne 0 ]; then - touch $LOCKFILE + touch "$LOCKFILE" fi + ocf_log debug "Status returning $last for $ORACLE_SID" return $last } diff --git a/rgmanager/src/resources/orainstance.sh b/rgmanager/src/resources/orainstance.sh index a9f690d..ac71a92 100755 --- a/rgmanager/src/resources/orainstance.sh +++ b/rgmanager/src/resources/orainstance.sh @@ -1,11 +1,12 @@ #!/bin/bash # -# Copyright 2003-2004, 2006-2007 Red Hat, Inc. +# Copyright 2003-2004, 2006-2013 Red Hat, Inc. # # Author(s): # Hardy Merrill <hmerrill at redhat.com> # Lon Hohberger <lhh at redhat.com> # Michael Moon <Michael dot Moon at oracle.com> +# Ryan McCabe <rmccabe at redhat.com> # # This program is Open Source software. You may modify and/or redistribute # it persuant to the terms of the Open Software License version 2.1, which @@ -13,11 +14,6 @@ # # http://opensource.org/licenses/osl-2.1.php # -# chkconfig: 345 99 01 -# description: Service script for starting/stopping \ -# Oracle(R) Database 10g on \ -# Red Hat Enterprise Linux 5 -# # NOTES: # # (1) You can comment out the LOCKFILE declaration below. This will prevent @@ -35,6 +31,7 @@ # Oracle is a registered trademark of Oracle Corporation. # Oracle9i is a trademark of Oracle Corporation. # Oracle10g is a trademark of Oracle Corporation. +# Oracle11g is a trademark of Oracle Corporation. # All other trademarks are property of their respective owners. # # @@ -48,6 +45,12 @@ # - SysV init support removed. Only usable with rgmanager # +# Grab the global RHCS helper functions +. $(dirname $0)/ocf-shellfuncs +. $(dirname $0)/utils/config-utils.sh +. $(dirname $0)/utils/messages.sh +. $(dirname $0)/utils/ra-skelet.sh + . /etc/init.d/functions declare SCRIPT="`basename $0`" @@ -60,60 +63,64 @@ ORACLE_SID=$OCF_RESKEY_name # Optional parameters with default values LISTENERS=$OCF_RESKEY_listeners -LOCKFILE="/tmp/.oracle10g-${ORACLE_SID}.lock" +LOCKFILE="$ORACLE_HOME/.orainstance-${ORACLE_SID}.lock" [ -n "$OCF_RESKEY_lockfile" ] && LOCKFILE=$OCF_RESKEY_lockfile export LISTENERS ORACLE_USER ORACLE_HOME ORACLE_SID LOCKFILE export LD_LIBRARY_PATH=$ORACLE_HOME/lib -export PATH=$ORACLE_HOME/bin:$PATH +export PATH=$ORACLE_HOME/bin:/bin:/sbin:/usr/bin:/usr/sbin declare -i RESTART_RETRIES=3 declare -r DB_PROCNAMES="pmon" declare -r LSNR_PROCNAME="tnslsnr" +# clulog will not log messages when run by the oracle user. +# This is a hack to work around that. +if [ "`id -u`" = "`id -u $ORACLE_USER`" ]; then + ocf_log() { + prio=$1 + shift + logger -i -p daemon."$prio" -- "$*" + } +fi # # Start Oracle (database portion) # start_db() { - declare tmpfile - declare logfile declare -i rv + declare startup_cmd + declare startup_stdout - tmpfile=/tmp/$SCRIPT-start.$$ - logfile=/tmp/$SCRIPT-start.log.$$ + ocf_log info "Starting Oracle DB $ORACLE_SID" # Set up our sqlplus script. Basically, we're trying to # capture output in the hopes that it's useful in the case # that something doesn't work properly. - echo "startup" > $tmpfile - echo "quit" >> $tmpfile - - sqlplus "/ as sysdba" < $tmpfile > $logfile + startup_cmd="set heading off;\nstartup;\nquit;\n" + startup_stdout=$(echo -e "$startup_cmd" | sqlplus -S "/ as sysdba") rv=$? - rm -f $tmpfile - - # Dump logfile to /var/log/messages - initlog -q -c "cat $logfile" + # Dump output to syslog for debugging + ocf_log debug "[$ORACLE_SID] [$rv] sent $startup_cmd" + ocf_log debug "[$ORACLE_SID] [$rv] got $startup_stdout" if [ $rv -ne 0 ]; then - rm -f $logfile - initlog -n $SCRIPT -q -s "sqlplus returned 1, failed" + ocf_log error "Starting Oracle DB $ORACLE_SID failed, sqlplus returned $rv" return 1 fi # If we see: # ORA-.....: failure, we failed - grep -q "^ORA-" $logfile - rv=$? - - rm -f $logfile - if [ $rv -eq 0 ]; then - initlog -n $SCRIPT -q -s "found failure in stdout, returning 1" + # Troubleshooting: + # ORA-00845 - Try rm -f /dev/shm/ora_* + # ORA-01081 - Try echo -e 'shutdown abort;\nquit;'|sqlplus "/ as sysdba" + if [[ "$startup_stdout" =~ "ORA-" ]] || [[ "$startup_stdout" =~ "failure" ]]; then + ocf_log error "Starting Oracle DB $ORACLE_SID failed, found errors in stdout" return 1 fi + ocf_log info "Started Oracle DB $ORACLE_SID successfully" return 0 } @@ -122,49 +129,47 @@ start_db() { # Stop Oracle (database portion) # stop_db() { - declare tmpfile - declare logfile + declare stop_cmd + declare stop_stdout declare -i rv + declare how_shutdown="$1" - tmpfile=/tmp/$SCRIPT-stop.$$ - logfile=/tmp/$SCRIPT-stop.log.$$ + if [ -z "$1" ]; then + how_shutdown="immediate" + fi - ora_procname="ora_${DB_PROCNAMES}_${ORACLE_SID}" - status $ora_procname - if [ $? -ne 0 ]; then - # No pmon process found, db already down - return 0 - fi + ocf_log info "Stopping Oracle DB $ORACLE_SID $how_shutdown" - # Setup for Stop ... - echo "shutdown immediate" > $tmpfile - echo "quit" >> $tmpfile + ora_procname="ora_${DB_PROCNAMES}_${ORACLE_SID}" + status $ora_procname + if [ $? -ne 0 ]; then + ocf_log debug "no pmon process -- DB $ORACLE_SID already stopped" + # No pmon process found, db already down + return 0 + fi - sqlplus "/ as sysdba" < $tmpfile > $logfile + # Setup for Stop ... + stop_cmd="set heading off;\nshutdown $how_shutdown;\nquit;\n" + stop_stdout=$(echo -e "$stop_cmd" | sqlplus -S "/ as sysdba") rv=$? - rm -f $tmpfile - - # Dump logfile to /var/log/messages - initlog -q -c "cat $logfile" + # Log stdout of the stop command + ocf_log debug "[$ORACLE_SID] sent stop command $stop_cmd" + ocf_log debug "[$ORACLE_SID] got $stop_stdout" - # sqlplus returned failure. We'll return failed to rhcs + # sqlplus returned failure. We'll return failed to rhcs if [ $rv -ne 0 ]; then - rm -f $logfile - initlog -n $SCRIPT -q -s "sqlplus returned 1, failed" + ocf_log error "Stopping Oracle DB $ORACLE_SID failed, sqlplus returned $rv" return 1 fi - grep -q "^ORA-" $logfile - rv=$? - rm -f $logfile - - # If we see 'failure' in the log, we're done. - if [ $rv -eq 0 ]; then - initlog -n $SCRIPT -q -s "found failure in stdout, returning 1" + # If we see 'ORA-' or 'failure' in stdout, we're done. + if [[ "$startup_stdout" =~ "ORA-" ]] || [[ "$startup_stdout" =~ "failure" ]]; then + ocf_log error "Stopping Oracle DB $ORACLE_SID failed, errors in stdout" return 1 fi + ocf_log info "Stopped Oracle DB $ORACLE_SID successfully" return 0 } @@ -176,14 +181,17 @@ force_cleanup() { declare pids declare pid - pids=`ps ax | grep $ORACLE_SID | grep -v grep | awk '{print $1}'` - - initlog -n $SCRIPT -s "<err> Not all Oracle processes exited cleanly, killing" + ocf_log error "Not all Oracle processes for $ORACLE_SID exited cleanly, killing" + pids=`ps ax | grep "ora_.*_${ORACLE_SID}" | grep -v grep | awk '{print $1}'` + for pid in $pids; do kill -9 $pid - if [ $? -eq 0 ]; then - initlog -n $SCRIPT -s "Killed $pid" + rv=$? + if [ $rv -eq 0 ]; then + ocf_log info "Cleanup $ORACLE_SID Killed PID $pid" + else + ocf_log error "Cleanup $ORACLE_SID Kill PID $pid failed: $rv" fi done @@ -197,14 +205,18 @@ force_cleanup() { exit_idle() { declare -i n=0 + ocf_log debug "Waiting for Oracle processes for $ORACLE_SID to terminate..." while ps ax | grep $ORACLE_SID | grep -q -v $LSNR_PROCNAME | grep -q -v grep; do if [ $n -ge 90 ]; then + ocf_log debug "Timed out while waiting for Oracle processes for $ORACLE_SID to terminate" force_cleanup return 0 fi sleep 1 ((n++)) done + + ocf_log debug "All Oracle processes for $ORACLE_SID have terminated" return 0 } @@ -219,6 +231,8 @@ get_db_status() { declare -i rv=0 declare ora_procname + ocf_log debug "Checking status of DB $ORACLE_SID" + for procname in $DB_PROCNAMES ; do ora_procname="ora_${procname}_${ORACLE_SID}" @@ -231,30 +245,34 @@ get_db_status() { # We're not supposed to be running, and we are, # in fact, not running... if [ $subsys_lock -ne 0 ]; then + ocf_log debug "DB $ORACLE_SID is already stopped" return 3 fi for (( i=$RESTART_RETRIES ; i; i-- )) ; do # this db process is down - stop and # (re)start all ora_XXXX_$ORACLE_SID processes - initlog -q -n $SCRIPT -s "Restarting Oracle Database..." + ocf_log info "Restarting Oracle Database $ORACLE_SID" stop_db start_db - if [ $? == 0 ] ; then + if [ $? -eq 0 ] ; then # ora_XXXX_$ORACLE_SID processes started # successfully, so break out of the # stop/start # 'for' loop + ocf_log info "Restarted Oracle DB $ORACLE_SID successfully" break fi done if [ $i -eq 0 ]; then # stop/start's failed - return 1 (failure) - initlog -q -n $SCRIPT -s "Restart failed, retuning 1" + ocf_log error "Failed to restart Oracle DB $ORACLE_SID after $RESTART_RETRIES tries" return 1 fi done + + ocf_log debug "Checking status of DB $ORACLE_SID success" return 0 } @@ -265,41 +283,48 @@ get_db_status() { get_lsnr_status() { declare -i subsys_lock=$1 declare -i rv - declare -r LISTENER=$3 + declare -r LISTENER=$3 - lsnrctl status $LISTENER >& /dev/null + ocf_log debug "Checking status for listener $LISTENER" + lsnrctl status "$LISTENER" >& /dev/null rv=$? - if [ $rv == 0 ] ; then + if [ $rv -eq 0 ] ; then + ocf_log debug "Listener $LISTENER is up" return 0 # Listener is running fine fi # We're not supposed to be running, and we are, # in fact, not running. Return 3 if [ $subsys_lock -ne 0 ]; then + ocf_log debug "Listener $LISTENER is stopped as expected" return 3 fi # Listener is NOT running (but should be) - try to restart for (( i=$RESTART_RETRIES ; i; i-- )) ; do - initlog -n $SCRIPT -q -s "Restarting Oracle listener ($LISTENER)" - lsnrctl start $LISTENER - lsnrctl status $LISTENER >& /dev/null - if [ $? == 0 ] ; then + ocf_log info "Listener $LISTENER is down, attempting to restart" + lsnrctl start "$LISTENER" >& /dev/null + lsnrctl status "$LISTENER" >& /dev/null + if [ $? -eq 0 ]; then + ocf_log info "Listener $LISTENER was restarted successfully" break # Listener was (re)started and is running fine fi done if [ $i -eq 0 ]; then # stop/start's failed - return 1 (failure) - initlog -n $SCRIPT -q -s "Listener restart failed, retuning 1" + ocf_log error "Failed to restart listener $LISTENER after $RESTART_RETRIES tries" return 1 fi - lsnrctl status $LISTENER >& /dev/null - if [ $? != 0 ] ; then - initlog -n $SCRIPT -q -s "Listener status failed, retuning 1" + lsnrctl_stdout=$(lsnrctl status "$LISTENER") + rv=$? + if [ $rv -ne 0 ] ; then + ocf_log error "Starting listener $LISTENER failed: $rv output $lsnrctl_stdout" return 1 # Problem restarting the Listener fi + + ocf_log info "Listener $LISTENER started successfully" return 0 # Success restarting the Listener } @@ -329,7 +354,7 @@ update_status() { fi if [ $old_status -ne $new_status ]; then - initlog -n $SCRIPT -q -s "$old_status vs $new_status - returning 1" + ocf_log error "Error: $old_status vs $new_status for $ORACLE_SID - returning 1" return 1 fi @@ -341,11 +366,7 @@ update_status() { # Print an error message to the user and exit. # oops() { - #echo "Please configure this script ($0) to" - #echo "match your installation." - #echo - #echo " $1 failed validation checks." - initlog -n $SCRIPT -q -s "$1 failed validation checks" + ocf_log error "$ORACLE_SID: Fatal: $1 failed validation checks" exit 1 } @@ -355,16 +376,18 @@ oops() { # script. # validation_checks() { + ocf_log debug "Validating configuration for $ORACLE_SID" + # If the oracle user doesn't exist, we're done. [ -n "$ORACLE_USER" ] || oops "ORACLE_USER" id -u $ORACLE_USER > /dev/null || oops "ORACLE_USER" - id -g $ORACLE_USER > /dev/null || oops "ORACLE_USER" + id -g $ORACLE_USER > /dev/null || oops "ORACLE_GROUP" # If the oracle home isn't a directory, we're done - [ -n "$ORACLE_HOME" ] || oops ORACLE_HOME + [ -n "$ORACLE_HOME" ] || oops "ORACLE_HOME" # If the oracle SID is NULL, we're done - [ -n "$ORACLE_SID" ] || oops ORACLE_SID + [ -n "$ORACLE_SID" ] || oops "ORACLE_SID" # Super user? Automatically change UID and exec as oracle user. # Oracle needs to be run as the Oracle user, not root! @@ -374,12 +397,13 @@ validation_checks() { fi # If we're not root and not the Oracle user, we're done. - [ "`id -u`" = "`id -u $ORACLE_USER`" ] || exit 1 - [ "`id -g`" = "`id -g $ORACLE_USER`" ] || exit 1 + [ "`id -u`" = "`id -u $ORACLE_USER`" ] || oops "not ORACLE_USER after su" + [ "`id -g`" = "`id -g $ORACLE_USER`" ] || oops "not ORACLE_GROUP after su" # Go home. - cd $ORACLE_HOME + cd "$ORACLE_HOME" + ocf_log debug "Validation checks for $ORACLE_SID succeeded" return 0 } @@ -388,20 +412,31 @@ validation_checks() { # Start Oracle # start_oracle() { - initlog -n $SCRIPT -q -s "Starting Oracle Database" - start_db || return 1 - - for LISTENER in ${LISTENERS}; do - logfile=/tmp/$SCRIPT-lsn-$$.log - initlog -n $SCRIPT -q -s "Starting Oracle Listener $LISTENER" - lsnrctl start $LISTENER > $logfile - initlog -q -c "cat $logfile" - rm -f $logfile - done + ocf_log info "Starting service $ORACLE_SID" + + start_db + rv=$? + if [ $rv -ne 0 ]; then + ocf_log error "Starting service $ORACLE_SID failed" + return 1 + fi + + for LISTENER in ${LISTENERS}; do + ocf_log info "Starting listener $LISTENER" + lsnrctl_stdout=$(lsnrctl start "$LISTENER") + rv=$? + if [ $rv -ne 0 ]; then + ocf_log debug "[$ORACLE_SID] Listener $LISTENER start returned $rv output $lsnrctl_stdout" + ocf_log error "Starting service $ORACLE_SID failed" + return 1 + fi + done if [ -n "$LOCKFILE" ]; then - touch $LOCKFILE + touch "$LOCKFILE" fi + + ocf_log info "Starting service $ORACLE_SID completed successfully" return 0 } @@ -410,30 +445,42 @@ start_oracle() { # Stop Oracle # stop_oracle() { + ocf_log info "Stopping service $ORACLE_SID" + if ! [ -e "$ORACLE_HOME/bin/lsnrctl" ]; then - initlog -n $SCRIPT -q -s "Oracle Listener Control is not available ($ORACLE_HOME not mounted?)" + ocf_log error "Oracle Listener Control is not available ($ORACLE_HOME not mounted?)" + # XXX should this return 1? return 0 fi - initlog -n $SCRIPT -q -s "Stopping Oracle Database" - stop_db || return 1 + stop_db || stop_db abort + if [ $? -ne 0 ]; then + ocf_log error "Unable to stop DB for $ORACLE_SID" + return 1 + fi - - for LISTENER in ${LISTENERS}; do - initlog -n $SCRIPT -q -s "Stopping Oracle Listener $LISTENER" - lsnrctl stop $LISTENER - done + for LISTENER in ${LISTENERS}; do + ocf_log info "Stopping listener $LISTENER for $ORACLE_SID" + lsnrctl_stdout=$(lsnrctl stop "$LISTENER") + rv=$? + if [ $? -ne 0 ]; then + ocf_log error "Listener $LISTENER stop failed for $ORACLE_SID: $rv output $lsnrctl_stdout" + # XXX - failure? + fi + done - initlog -n $SCRIPT -q -s "Waiting for all Oracle processes to exit" - exit_idle + exit_idle if [ $? -ne 0 ]; then - initlog -n $SCRIPT -q -s "WARNING: Not all Oracle processes exited cleanly" + ocf_log error "WARNING: Not all Oracle processes exited cleanly for $ORACLE_SID" + # XXX - failure? fi if [ -n "$LOCKFILE" ]; then - rm -f $LOCKFILE + rm -f "$LOCKFILE" fi + + ocf_log info "Stopping service $ORACLE_SID succeeded" return 0 } @@ -461,8 +508,10 @@ status_oracle() { declare -i last declare -i depth=$1 + ocf_log debug "Checking status for $ORACLE_SID depth $depth" + # Check for lock file. Crude and rudimentary, but it works - if [ -z "$LOCKFILE" ] || [ -f $LOCKFILE ]; then + if [ -z "$LOCKFILE" ] || [ -f "$LOCKFILE" ]; then subsys_lock=0 fi @@ -472,18 +521,19 @@ status_oracle() { last=$? # Check & report listener status - for LISTENER in ${LISTENERS}; do - get_lsnr_status $subsys_lock $depth $LISTENER - update_status $? $last - last=$? - done + for LISTENER in ${LISTENERS}; do + get_lsnr_status $subsys_lock $depth "$LISTENER" + update_status $? $last + last=$? + done # No lock file, but everything's running. Put the lock # file back. XXX - this kosher? if [ $last -eq 0 ] && [ $subsys_lock -ne 0 ]; then - touch $LOCKFILE + touch "$LOCKFILE" fi + ocf_log debug "Status returning $last for $ORACLE_SID" return $last } @@ -493,22 +543,22 @@ status_oracle() { ######################## case $1 in - meta-data) - cat `echo $0 | sed 's/^$.*$\.sh$/\1.metadata/'` - exit 0 - ;; + meta-data) + cat `echo $0 | sed 's/^$.*$\.sh$/\1.metadata/'` + exit 0 + ;; start) - validation_checks $* + validation_checks $* start_oracle exit $? ;; stop) - validation_checks $* + validation_checks $* stop_oracle exit $? ;; status|monitor) - validation_checks $* + validation_checks $* status_oracle $OCF_CHECK_LEVEL exit $? ;; diff --git a/rgmanager/src/resources/oralistener.sh b/rgmanager/src/resources/oralistener.sh index 3d6b839..30b8e06 100755 --- a/rgmanager/src/resources/oralistener.sh +++ b/rgmanager/src/resources/oralistener.sh @@ -1,7 +1,5 @@ #!/bin/bash # -# $Id: oralistener.sh 127 2009-08-21 09:17:52Z hevirtan $ -# # Red Hat Cluster Suite resource agent for controlling Oracle 10g # listener instances. This script will start, stop and monitor running # listeners. @@ -12,6 +10,24 @@ # # monitor: Will check that the listener is OK by calling lsnrctl status # +# +# Copyright (C) 1997-2003 Sistina Software, Inc. All rights reserved. +# Copyright (C) 2004-2013 Red Hat, Inc. All rights reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. +# # Grab the global RHCS helper functions . $(dirname $0)/ocf-shellfuncs @@ -27,126 +43,148 @@ LISTENER=$OCF_RESKEY_name LC_ALL=C LANG=C -PATH=/bin:/sbin:/usr/bin:/usr/sbin:$ORACLE_HOME/bin -export LC_ALL LANG PATH ORACLE_HOME +PATH=$ORACLE_HOME/bin:/bin:/sbin:/usr/bin:/usr/sbin +export LC_ALL LANG PATH ORACLE_USER ORACLE_HOME + +# clulog will not log messages when run by the oracle user. +# This is a hack to work around that. +if [ "`id -u`" = "`id -u $ORACLE_USER`" ]; then + ocf_log() { + prio=$1 + shift + logger -i -p daemon."$prio" -- "$*" + } +fi verify_all() { - clog_service_verify $CLOG_INIT - - if [ -z "$OCF_RESKEY_name" ]; then - clog_service_verify $CLOG_FAILED "Invalid name of service (listener name)" - return $OCF_ERR_ARGS - fi - - if [ -z "$OCF_RESKEY_home" ]; then - clog_service_verify $CLOG_FAILED "No Oracle home specified." - return $OCF_ERR_ARGS - fi - - if [ -z "$OCF_RESKEY_user" ]; then - clog_service_verify $CLOG_FAILED "No Oracle username specified." - return $OCF_ERR_ARGS - fi - - # Make sure the lsnrctl binary is in our $PATH - if [ ! -x $(which lsnrctl) ]; then - clog_service_verify $CLOG_FAILED "oralistener:${OCF_RESKEY_home}: Unable to locate lsnrctl command from path! ($PATH)" - return $OCF_ERR_GENERIC - fi - - clog_service_verify $CLOG_SUCCEED - return 0 -} + ocf_log debug "Validating configuration for $LISTENER" -start () { - clog_service_start $CLOG_INIT - - logfile="/tmp/oracle_lsn.$$" - su -p - $ORACLE_USER -c "lsnrctl start $LISTENER > $logfile" + if [ -z "$OCF_RESKEY_name" ]; then + ocf_log error "Validation for $LISTENER failed: Invalid name of service (listener name)" + return $OCF_ERR_ARGS + fi - initlog -q -c "cat $logfile" - rm -f $logfile + if [ -z "$OCF_RESKEY_home" ]; then + ocf_log error "Validation for $LISTENER failed: No Oracle home specified." + return $OCF_ERR_ARGS + fi - clog_service_start $CLOG_SUCCEED - return 0 + if [ -z "$OCF_RESKEY_user" ]; then + ocf_log error "Validation for $LISTENER failed: No Oracle username specified." + return $OCF_ERR_ARGS + fi + + # Super user? Automatically change UID and exec as oracle user. + # Oracle needs to be run as the Oracle user, not root! + if [ "`id -u`" = "0" ]; then + su $OCF_RESKEY_user -c "$0 $*" + exit $? + fi + + # Make sure the lsnrctl binary is in our $PATH + if [ ! -x $(which lsnrctl) ]; then + ocf_log error "Validation for $LISTENER failed: Unable to locate lsnrctl command from path! ($PATH)" + return $OCF_ERR_GENERIC + fi + + ocf_log debug "Validation checks for $LISTENER succeeded" + return 0 +} + +start() { + ocf_log info "Starting listener $LISTENER" + lsnrctl_stdout=$(lsnrctl start "$LISTENER") + if [ $? -ne 0 ]; then + ocf_log error "start listener $LISTENER failed $lsnrctl_stdout" + return $OCF_ERR_GENERIC + fi + + ocf_log info "Listener $LISTENER started successfully" + return 0 } -stop () { - clog_service_stop $CLOG_INIT - - logfile="/tmp/oracle_lsn.$$" - su -p - $ORACLE_USER -c "lsnrctl stop $LISTENER > $logfile" +stop() { + ocf_log info "Stopping listener $LISTENER" - initlog -q -c "cat $logfile" - rm -f $logfile + lsnrctl_stdout=$(lsnrctl stop "$LISTENER") + if [ $? -ne 0 ]; then + ocf_log debug "stop listener $LISTENER failed $lsnrctl_stdout" + return $OCF_ERR_GENERIC + fi - clog_service_stop $CLOG_SUCCEED - return 0 + ocf_log info "Listener $LISTENER stopped successfully" + return 0 } -monitor () { - clog_service_status $CLOG_INIT - - su -p - $ORACLE_USER -c "lsnrctl status $LISTENER" - rv=$? - if [ $rv == 0 ]; then - clog_service_status $CLOG_SUCCEED - return 0 # Listener is running fine - else - clog_service_status $CLOG_FAILED - return $OCF_ERR_GENERIC - fi +monitor() { + declare -i depth=$1 + + ocf_log debug "Checking status for listener $LISTENER depth $depth" + lsnrctl status "$LISTENER" >& /dev/null + if [ $? -ne 0 ]; then + ocf_log error "Listener $LISTENER not running" + return $OCF_ERR_GENERIC + fi + + ocf_log debug "Listener $LISTENER is up" + return 0 # Listener is running fine } recover() { + ocf_log debug "Recovering listener $LISTENER" + for (( i=$RESTART_RETRIES ; i; i-- )); do start - if [ $? == 0 ] ; then - break + if [ $? -eq 0 ] ; then + ocf_log debug "Restarted listener $LISTENER successfully" + break fi done if [ $i -eq 0 ]; then # stop/start's failed - return 1 (failure) + ocf_log debug "Failed to restart listener $LISTENER after $RESTART_RETRIES tries" return 1 fi - status - if [ $? != 0 ] ; then + status + if [ $? -ne 0 ] ; then + ocf_log debug "Failed to restart listener $LISTENER" return 1 # Problem restarting the Listener fi + ocf_log debug "Restarted listener $LISTENER successfully" return 0 # Success restarting the Listener } case $1 in - meta-data) - cat `echo $0 | sed 's/^$.*$\.sh$/\1.metadata/'` - exit 0 - ;; - verify-all) - verify_all - exit $? - ;; - start) - verify_all && start - exit $? - ;; - stop) - verify_all && stop - exit $? - ;; - recover) - verify_all && recover - exit $? - ;; - status|monitor) - verify_all - monitor - exit $? - ;; - *) - echo "Usage: $0 {start|stop|recover|monitor|status|meta-data|verify-all}" - exit $OCF_ERR_GENERIC - ;; + meta-data) + cat `echo $0 | sed 's/^$.*$\.sh$/\1.metadata/'` + exit 0 + ;; + verify-all) + verify_all $* + exit $? + ;; + start) + verify_all $* && start + exit $? + ;; + stop) + verify_all $* && stop + exit $? + ;; + recover) + verify_all $* && recover + exit $? + ;; + status|monitor) + verify_all $* + monitor $OCF_CHECK_LEVEL + exit $? + ;; + *) + echo "Usage: $0 {start|stop|recover|monitor|status|meta-data|verify-all}" + exit $OCF_ERR_GENERIC + ;; esac

1 0

gfs2-utils: master - fsck.gfs2: delete all duplicates from unrecoverable damaged dinodes
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=982979702a… Commit: 982979702a7f23b069e31f171cc38fc4c87a018a Parent: e8d58780c43e0befeacab299c6d099e196bc83b9 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Tue May 14 09:51:28 2013 -0500 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: delete all duplicates from unrecoverable damaged dinodes When pass1 encounters a dinode with unrecoverable damage, it tries to "undo" the metadata and data block designations it marked in the blockmap prior to finding the damage. That's all fine, but if the damaged dinode has a duplicate reference, we also need to delete that from the duplicate reference list. Otherwise pass1b may try to resolve the duplicate reference and reinstate the damaged dinode. --- gfs2/fsck/metawalk.c | 5 ++++ gfs2/fsck/pass1b.c | 60 -------------------------------------------------- gfs2/fsck/util.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ gfs2/fsck/util.h | 1 + 4 files changed, 65 insertions(+), 60 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 44b5c66..22f3334 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1530,6 +1530,11 @@ undo_metalist: brelse(bh); } } + /* There may be leftover duplicate records, so we need to delete them. + For example, if a metadata block was found to be a duplicate, we + may not have added it to the metalist, which means it's not there + to undo. */ + delete_all_dups(ip); /* Set the dinode as "bad" so it gets deleted */ fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, _("corrupt"), gfs2_block_free); diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 9a23197..0dcb306 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -52,66 +52,6 @@ static void log_inode_reference(struct duptree *dt, osi_list_t *tmp, int inval) } /* delete_all_dups - delete all duplicate records for a given inode */ -static void delete_all_dups(struct gfs2_inode *ip) -{ - struct osi_node *n, *next; - struct duptree *dt; - osi_list_t *tmp, *x; - struct inode_with_dups *id; - int found; - - for (n = osi_first(&dup_blocks); n; n = next) { - next = osi_next(n); - dt = (struct duptree *)n; - - found = 0; - id = NULL; - - osi_list_foreach_safe(tmp, &dt->ref_invinode_list, x) { - id = osi_list_entry(tmp, struct inode_with_dups, list); - if (id->block_no == ip->i_di.di_num.no_addr) { - dup_listent_delete(dt, id); - found = 1; - } - } - osi_list_foreach_safe(tmp, &dt->ref_inode_list, x) { - id = osi_list_entry(tmp, struct inode_with_dups, list); - if (id->block_no == ip->i_di.di_num.no_addr) { - dup_listent_delete(dt, id); - found = 1; - } - } - if (!found) - continue; - - if (dt->refs == 0) { - log_debug(_("This was the last reference: 0x%llx is " - "no longer a duplicate.\n"), - (unsigned long long)dt->block); - dup_delete(dt); /* not duplicate now */ - } else { - log_debug(_("%d references remain to 0x%llx\n"), - dt->refs, (unsigned long long)dt->block); - if (dt->refs > 1) - continue; - - id = NULL; - osi_list_foreach(tmp, &dt->ref_invinode_list) - id = osi_list_entry(tmp, - struct inode_with_dups, - list); - osi_list_foreach(tmp, &dt->ref_inode_list) - id = osi_list_entry(tmp, - struct inode_with_dups, - list); - if (id) - log_debug("Last reference is from inode " - "0x%llx\n", - (unsigned long long)id->block_no); - } - } -} - /* * resolve_dup_references - resolve all but the last dinode that has a * duplicate reference to a given block. diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index 9d6f163..fd1b292 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -725,3 +725,62 @@ uint64_t *get_dir_hash(struct gfs2_inode *ip) return tbl; } +void delete_all_dups(struct gfs2_inode *ip) +{ + struct osi_node *n, *next; + struct duptree *dt; + osi_list_t *tmp, *x; + struct inode_with_dups *id; + int found; + + for (n = osi_first(&dup_blocks); n; n = next) { + next = osi_next(n); + dt = (struct duptree *)n; + + found = 0; + id = NULL; + + osi_list_foreach_safe(tmp, &dt->ref_invinode_list, x) { + id = osi_list_entry(tmp, struct inode_with_dups, list); + if (id->block_no == ip->i_di.di_num.no_addr) { + dup_listent_delete(dt, id); + found = 1; + } + } + osi_list_foreach_safe(tmp, &dt->ref_inode_list, x) { + id = osi_list_entry(tmp, struct inode_with_dups, list); + if (id->block_no == ip->i_di.di_num.no_addr) { + dup_listent_delete(dt, id); + found = 1; + } + } + if (!found) + continue; + + if (dt->refs == 0) { + log_debug(_("This was the last reference: 0x%llx is " + "no longer a duplicate.\n"), + (unsigned long long)dt->block); + dup_delete(dt); /* not duplicate now */ + } else { + log_debug(_("%d references remain to 0x%llx\n"), + dt->refs, (unsigned long long)dt->block); + if (dt->refs > 1) + continue; + + id = NULL; + osi_list_foreach(tmp, &dt->ref_invinode_list) + id = osi_list_entry(tmp, + struct inode_with_dups, + list); + osi_list_foreach(tmp, &dt->ref_inode_list) + id = osi_list_entry(tmp, + struct inode_with_dups, + list); + if (id) + log_debug("Last reference is from inode " + "0x%llx\n", + (unsigned long long)id->block_no); + } + } +} diff --git a/gfs2/fsck/util.h b/gfs2/fsck/util.h index 361b1a2..580acd8 100644 --- a/gfs2/fsck/util.h +++ b/gfs2/fsck/util.h @@ -187,6 +187,7 @@ extern char generic_interrupt(const char *caller, const char *where, extern char gfs2_getch(void); extern uint64_t find_free_blk(struct gfs2_sbd *sdp); extern uint64_t *get_dir_hash(struct gfs2_inode *ip); +extern void delete_all_dups(struct gfs2_inode *ip); #define stack log_debug("<backtrace> - %s()\n", __func__)

1 0

gfs2-utils: master - fsck.gfs2: take hash table start boundaries into account
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=e8d58780c4… Commit: e8d58780c43e0befeacab299c6d099e196bc83b9 Parent: 58a213659fb8afd5d25fa25d7ec3ec0a8d5e21dd Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Fri Apr 19 09:25:51 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: take hash table start boundaries into account When checking the hash table in pass2, we can't just keep doubling the length for each consecutive check because the number of pointer copies (aka length) is also tied to the starting offset. If the starting offset is invalid for the length, it might treat a chunk of the hash table as bigger than it should, eventually overwriting good entries. Along the same lines, while we're trying to determine the length, it's not good enough to double the length and check if the hash table entry matches. The reason is: there can be several values overwritten with the same value, 0x00, that indicates places where pass1 found an invalid leaf block pointer. To avoid that, we need to check intermediate values as well, and stop if we find a gap. --- gfs2/fsck/metawalk.c | 5 +++-- gfs2/fsck/pass2.c | 43 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index ffc3555..44b5c66 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -473,11 +473,12 @@ static int check_entries(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, if ((char *)dent + de.de_rec_len >= bh_end){ log_debug( _("Last entry processed for %lld->%lld " - "(0x%llx->0x%llx).\n"), + "(0x%llx->0x%llx), di_blocks=%llu.\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)bh->b_blocknr, (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)bh->b_blocknr); + (unsigned long long)bh->b_blocknr, + (unsigned long long)ip->i_di.di_blocks); break; } diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index a24edbe..3d0bb49 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -370,9 +370,10 @@ static int wrong_leaf(struct gfs2_inode *ip, struct gfs2_inum *entry, 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)"), + "is: %llu (0x%llx). di_blocks=%llu\n"), (unsigned long long)real_leaf, - (unsigned long long)real_leaf); + (unsigned long long)real_leaf, + (unsigned long long)ip->i_di.di_blocks); fsck_blockmap_set(ip, real_leaf, _("split leaf"), gfs2_indir_blk); } @@ -1032,6 +1033,7 @@ static int basic_check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, log_err( _("Bad directory entry '%s' cleared.\n"), tmp_name); return 1; } else { + (*count)++; return 0; } } @@ -1150,11 +1152,13 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize, /* Look at the first dirent and check its hash value to see if it's at the proper starting offset. */ hash_index = hash_table_index(dentry.de_hash, ip); + /* Need to use len here, not *proper_len because the leaf block may + be valid within the range, but starts too soon in the hash table. */ if (hash_index < lindex || hash_index > lindex + len) { log_err(_("This leaf block has hash index %d, which is out of " "bounds for where it appears in the hash table " "(%d - %d)\n"), - hash_index, lindex, lindex + len); + hash_index, lindex, lindex + *proper_len); error = lost_leaf(ip, tbl, leafblk, len, lindex, lbh); brelse(lbh); return error; @@ -1291,6 +1295,8 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, struct gfs2_buffer_head *lbh; int factor; uint32_t proper_start; + uint32_t next_proper_start; + int anomaly; lindex = 0; while (lindex < hsize) { @@ -1299,10 +1305,23 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, len = 1; factor = 0; leafblk = be64_to_cpu(tbl[lindex]); + next_proper_start = lindex; + anomaly = 0; while (lindex + (len << 1) - 1 < hsize) { if (be64_to_cpu(tbl[lindex + (len << 1) - 1]) != leafblk) break; + next_proper_start = (lindex & ~((len << 1) - 1)); + if (lindex != next_proper_start) + anomaly = 1; + /* Check if there are other values written between + here and the next factor. */ + for (i = len; !anomaly && i + lindex < hsize && + i < (len << 1); i++) + if (be64_to_cpu(tbl[lindex + i]) != leafblk) + anomaly = 1; + if (anomaly) + break; len <<= 1; factor++; } @@ -1344,8 +1363,10 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, proper_start = (lindex & ~(proper_len - 1)); if (lindex != proper_start) { log_debug(_("lindex 0x%llx is not a proper starting " - "point for this leaf: 0x%llx\n"), + "point for leaf %llu (0x%llx): 0x%llx\n"), (unsigned long long)lindex, + (unsigned long long)leafblk, + (unsigned long long)leafblk, (unsigned long long)proper_start); changes = fix_hashtable(ip, tbl, hsize, leafblk, lindex, proper_start, len, @@ -1368,9 +1389,11 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, depth, and adjust the hash table accordingly. */ if (len != proper_len) { log_err(_("Length %d (0x%x) is not a proper length " - "for this leaf. Valid boundary assumed to " - "be %d (0x%x).\n"), - len, len, proper_len, proper_len); + "for leaf %llu (0x%llx). Valid boundary " + "assumed to be %d (0x%x).\n"), len, len, + (unsigned long long)leafblk, + (unsigned long long)leafblk, + proper_len, proper_len); lbh = bread(ip->i_sbd, leafblk); gfs2_leaf_in(&leaf, lbh); if (gfs2_check_meta(lbh, GFS2_METATYPE_LF) || @@ -1419,8 +1442,10 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, proper_len = 1 << (ip->i_di.di_depth - leaf.lf_depth); if (proper_len != len) { log_debug(_("Length 0x%x is not proper for " - "this leaf: 0x%x"), - len, proper_len); + "leaf %llu (0x%llx): 0x%x"), + len, (unsigned long long)leafblk, + (unsigned long long)leafblk, + proper_len); changes = fix_hashtable(ip, tbl, hsize, leafblk, lindex, lindex, len,

1 0

gfs2-utils: master - fsck.gfs2: Don't allocate leaf blocks in pass1
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=58a213659f… Commit: 58a213659fb8afd5d25fa25d7ec3ec0a8d5e21dd Parent: c2a39034d9f2888dc0a9431cea86998a929c30ba Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Apr 17 14:09:30 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: Don't allocate leaf blocks in pass1 Before this patch, if leaf blocks were found to be corrupt, pass1 tried to fix them by allocating new leaf blocks in place of the bad ones. That's a bad idea, because pass1 populates the blockmap and sets the bitmap accordingly. In other words, it's dynamically changing. Say, for example, that you're checking a directory a dinode 0x1234, and it has a corrupt hash table, and needs new leaf blocks inserted. Now suppose you have a second directory that occurs later in the bitmap, say at block 0x2345, and it references leaf block 0x2346, but for some reason that block (0x2346) is improperly set to "free" in the bitmap. If pass1 goes out looking for a free block in order to allocate a new leaf for 0x1234, it will naturally find block 0x2346, because it's marked free. It writes a new leaf at that block and adds a new reference in the hash table of 0x1234. Later, when pass1 processes directory 0x2345, it discovers the reference to 0x2346. Not only has it wiped out the perfectly good leaf block, it has also created a duplicate block reference that it needs to sort out in pass1b, which will likely keep the replaced reference and throw the good one we had. Thus, we introduced corruption into the file system when we should have kept the only good reference to 0x2346 and fixed the bitmap. The solution provided by this patch is to simply zero out the bad hash table entries when pass1 comes across them. Later, when pass2 discovers the zero leaf blocks, it can safely allocate new blocks (since pass1 synced the bitmap according to the blockmap) for the new leaf blocks and replace the zeros with valid block references. --- gfs2/fsck/metawalk.c | 31 ++++++++++++++++++++++++++++++- gfs2/fsck/metawalk.h | 2 +- gfs2/fsck/pass1.c | 9 ++------- gfs2/fsck/pass2.c | 2 +- 4 files changed, 34 insertions(+), 10 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 161c183..ffc3555 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1955,7 +1955,7 @@ int write_new_leaf(struct gfs2_inode *dip, int start_lindex, int num_copies, * leaf a bit, but it's better than deleting the whole directory, * which is what used to happen before. */ int repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, int lindex, - int ref_count, const char *msg) + int ref_count, const char *msg, int allow_alloc) { int new_leaf_blks = 0, error, refs; uint64_t bn = 0; @@ -1970,6 +1970,35 @@ int repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, int lindex, log_err( _("Bad leaf left in place.\n")); goto out; } + if (!allow_alloc) { + uint64_t *cpyptr; + char *padbuf; + int pad_size, i; + + padbuf = malloc(ref_count * sizeof(uint64_t)); + cpyptr = (uint64_t *)padbuf; + for (i = 0; i < ref_count; i++) { + *cpyptr = 0; + cpyptr++; + } + pad_size = ref_count * sizeof(uint64_t); + log_err(_("Writing zeros to the hash table of directory %lld " + "(0x%llx) at index: 0x%x for 0x%x pointers.\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr, + lindex, ref_count); + if (ip->i_sbd->gfs1) + gfs1_writei(ip, padbuf, lindex * sizeof(uint64_t), + pad_size); + else + gfs2_writei(ip, padbuf, lindex * sizeof(uint64_t), + pad_size); + free(padbuf); + log_err( _("Directory Inode %llu (0x%llx) patched.\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + goto out; + } /* We can only write leafs in quantities that are factors of two, since leaves are doubled, not added sequentially. So if we have a hole that's not a factor of 2, we have to diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index aacb962..a5a51c2 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -61,7 +61,7 @@ extern int write_new_leaf(struct gfs2_inode *dip, int start_lindex, int num_copies, const char *before_or_after, uint64_t *bn); extern int repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, int lindex, - int ref_count, const char *msg); + int ref_count, const char *msg, int allow_alloc); #define is_duplicate(dblock) ((dupfind(dblock)) ? 1 : 0) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 2c1c046..df778ef 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -84,13 +84,8 @@ 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; + repair_leaf(ip, leaf_no, lindex, ref_count, msg, 0); + return 0; } struct metawalk_fxns pass1_fxns = { diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 5767c4d..a24edbe 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -1040,7 +1040,7 @@ 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); + return repair_leaf(ip, leaf_no, lindex, ref_count, msg, 1); } /* The purpose of leafck_fxns is to provide a means for function fix_hashtable

1 0

gfs2-utils: master - fsck.gfs2: Stop "undo" process when error data block is reached
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=c2a39034d9… Commit: c2a39034d9f2888dc0a9431cea86998a929c30ba Parent: fb2ef82d8dd9b4c5304d377b9d2fa1ad3da1a82c Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Fri Apr 12 08:42:34 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: Stop "undo" process when error data block is reached When fsck.gfs2 discovers a data block in error, it flags the error and especially in pass1, it tries to "undo" the block designations it previously marked in the blockmap. Before this patch, the "undo" functions didn't know when to stop. So it could "undo" designations in the blockmap that it hadn't "done" in the first place. With this patch, if an error is encountered while processing data blocks (not counting duplicate references--for example, blocks marked as 'data' that are really dinodes which it hasn't gotten to yet) it saves off the block where the error occurred. Later, during the "undo" processing, it stops when it reaches the block that flagged the error. --- gfs2/fsck/metawalk.c | 36 +++++++++++++++++++++++++++--------- gfs2/fsck/pass1.c | 2 +- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 6e9e593..161c183 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1325,7 +1325,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, */ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, struct gfs2_buffer_head *bh, int head_size, - uint64_t *blks_checked) + uint64_t *blks_checked, uint64_t *error_blk) { int error = 0, rc = 0; uint64_t block, *ptr; @@ -1349,8 +1349,13 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, 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, + log_info("\n"); + if (rc < 0) { + *error_blk = block; + log_info(_("Unrecoverable ")); + } + log_info(_("data block error %d on block %llu " + "(0x%llx).\n"), rc, (unsigned long long)block, (unsigned long long)block); } @@ -1362,7 +1367,8 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, } static int undo_check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, - uint64_t *ptr_start, char *ptr_end) + uint64_t *ptr_start, char *ptr_end, + uint64_t error_blk) { int rc = 0; uint64_t block, *ptr; @@ -1375,6 +1381,8 @@ static int undo_check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, if (skip_this_pass || fsck_abort) return 1; block = be64_to_cpu(*ptr); + if (block == error_blk) + return 1; rc = pass->undo_check_data(ip, block, pass->private); if (rc < 0) return rc; @@ -1415,6 +1423,8 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) uint64_t blks_checked = 0; int error, rc; int metadata_clean = 0; + uint64_t error_blk = 0; + int hit_error_blk = 0; if (!height && !is_dir(&ip->i_di, ip->i_sbd->gfs1)) return 0; @@ -1460,7 +1470,7 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) if (pass->check_data) error = check_data(ip, pass, bh, head_size, - &blks_checked); + &blks_checked, &error_blk); if (pass->big_file_msg && ip->i_di.di_blocks > COMFORTABLE_BLKS) pass->big_file_msg(ip, blks_checked); } @@ -1498,12 +1508,20 @@ undo_metalist: i, pass->private); else rc = 0; - if (metadata_clean && rc == 0 && i == height - 1) { + if (metadata_clean && rc == 0 && i == height - 1 && + !hit_error_blk) { head_size = hdr_size(bh, height); - if (head_size) - undo_check_data(ip, pass, (uint64_t *) + if (head_size) { + rc = undo_check_data(ip, pass, + (uint64_t *) (bh->b_data + head_size), - (bh->b_data + ip->i_sbd->bsize)); + (bh->b_data + ip->i_sbd->bsize), + error_blk); + if (rc > 0) { + hit_error_blk = 1; + rc = 0; + } + } } if (bh == ip->i_bh) osi_list_del(&bh->b_altlist); diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index ee828d8..2c1c046 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -462,7 +462,7 @@ static int check_data(struct gfs2_inode *ip, uint64_t metablock, fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, _("bad (out of range) data"), gfs2_bad_block); - return 1; + return -1; } bc->data_count++; /* keep the count sane anyway */ q = block_type(block);

1 0

gfs2-utils: master - fsck.gfs2: double-check transitions from dinode to data
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=fb2ef82d8d… Commit: fb2ef82d8dd9b4c5304d377b9d2fa1ad3da1a82c Parent: a3c643a9c98dd68138ff6b623fd86923a16fc626 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Thu Apr 11 07:22:33 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: double-check transitions from dinode to data If a corrupt dinode references a bunch of blocks as data blocks, and those blocks occur later in the bitmap (as is usually the case) but they're really dinodes, we have a problem. Before it finds the corruption, it can change the bitmap markings from 'dinode' to 'data' blocks. Later, when it determines the dinode is corrupt. It tries to "undo" all those data blocks, but since pass1 hasn't processed them yet, it marks them as 'free' in the bitmap, and we've lost the fact that they're dinodes. The result is that the files/dinodes being improperly referenced are deleted by mistake. This patch adds a check for bitmap transitions in pass1 from 'dinode' to 'data', where the block hasn't been checked yet. We don't care about transitions from dinode to free because that's a normal delete of a dinode. We also don't care about transitions between dinode to metadata, because all those checks validate that the metadata type is the correct type of metadata, so we know we're making the right decision. So the only issue are data blocks referencing dinodes. What this patch does is: when the bitmap is making a transition from 'dinode' to 'data' in pass1, it basically puts up a red flag. The block is read in and checked to see if it really looks like a dinode. We have to be careful here, because customer data is allowed to look like a dinode. If the block really seems to be a dinode, we DO NOT want to treat it as a data block and assume the duplicate reference handler in pass1b will handle it, because the dinode's metadata blocks will not have been checked in pass1. Instead, we want to flag it as corruption in the referencing file dinode, not change the bitmap or blockmap, and allow pass1 to treat it properly as a dinode when it gets there. The corrupt dinode referencing the dinode as 'data' should be deleted and the work done thusfar should be backed out by the pass1 'undo' functions. --- gfs2/fsck/metawalk.c | 21 +++++++++++++--- gfs2/fsck/metawalk.h | 14 +++++++---- gfs2/fsck/pass1.c | 65 +++++++++++++++++++++++++++++++++++++++++++++---- gfs2/fsck/pass1b.c | 2 +- gfs2/fsck/pass2.c | 2 +- gfs2/fsck/pass3.c | 4 +- 6 files changed, 89 insertions(+), 19 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 22b16ee..6e9e593 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -27,7 +27,7 @@ is used to set the latter. The two must be kept in sync, otherwise you'll get bitmap mismatches. This function checks the status of the bitmap whenever the blockmap changes, and fixes it accordingly. */ -int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, +int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, int error_on_dinode, enum gfs2_mark_block new_blockmap_state) { int old_bitmap_state, new_bitmap_state; @@ -49,6 +49,16 @@ int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, /* gfs1 descriptions: */ {"free", "data", "free meta", "metadata", "reserved"}}; + if (error_on_dinode && old_bitmap_state == GFS2_BLKST_DINODE && + new_bitmap_state != GFS2_BLKST_FREE) { + log_debug(_("Reference as '%s' to block %llu (0x%llx) " + "which was marked as dinode. Needs " + "further investigation.\n"), + allocdesc[sdp->gfs1][new_bitmap_state], + (unsigned long long)blk, + (unsigned long long)blk); + return 1; + } /* Keep these messages as short as possible, or the output gets to be huge and unmanageable. */ log_err( _("Block %llu (0x%llx) was '%s', should be %s.\n"), @@ -106,6 +116,7 @@ int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, */ int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock, const char *btype, enum gfs2_mark_block mark, + int error_on_dinode, const char *caller, int fline) { int error; @@ -164,9 +175,11 @@ int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock, /* First, check the rgrp bitmap against what we think it should be. If that fails, it's an invalid block--part of an rgrp. */ - error = check_n_fix_bitmap(ip->i_sbd, bblock, mark); + error = check_n_fix_bitmap(ip->i_sbd, bblock, error_on_dinode, mark); if (error) { - log_err( _("This block is not represented in the bitmap.\n")); + if (error < 0) + log_err( _("This block is not represented in the " + "bitmap.\n")); return error; } @@ -517,7 +530,7 @@ int check_leaf(struct gfs2_inode *ip, int lindex, struct metawalk_fxns *pass, if (pass->check_leaf) { error = pass->check_leaf(ip, *leaf_no, pass->private); - if (error) { + if (error == -EEXIST) { log_info(_("Previous reference to leaf %lld (0x%llx) " "has already checked it; skipping.\n"), (unsigned long long)*leaf_no, diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index 56f57d9..aacb962 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -45,10 +45,12 @@ extern int delete_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, void *private); extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock, - const char *btype, enum gfs2_mark_block mark, - const char *caller, int line); + const char *btype, enum gfs2_mark_block mark, + int error_on_dinode, + const char *caller, int line); extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, - enum gfs2_mark_block new_blockmap_state); + int error_on_dinode, + enum gfs2_mark_block new_blockmap_state); extern void reprocess_inode(struct gfs2_inode *ip, const char *desc); extern struct duptree *dupfind(uint64_t block); extern struct gfs2_inode *fsck_system_inode(struct gfs2_sbd *sdp, @@ -63,8 +65,10 @@ extern int repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, int lindex, #define is_duplicate(dblock) ((dupfind(dblock)) ? 1 : 0) -#define fsck_blockmap_set(ip, b, bt, m) _fsck_blockmap_set(ip, b, bt, m, \ - __FUNCTION__, __LINE__) +#define fsck_blockmap_set(ip, b, bt, m) \ + _fsck_blockmap_set(ip, b, bt, m, 0, __FUNCTION__, __LINE__) +#define fsck_blkmap_set_noino(ip, b, bt, m) \ + _fsck_blockmap_set(ip, b, bt, m, 1, __FUNCTION__, __LINE__) enum meta_check_rc { meta_error = -1, diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index ad6690b..ee828d8 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -150,7 +150,7 @@ static int resuscitate_metalist(struct gfs2_inode *ip, uint64_t block, 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); + check_n_fix_bitmap(ip->i_sbd, block, 0, gfs2_indir_blk); bc->indir_count++; return meta_is_good; } @@ -204,7 +204,7 @@ static int resuscitate_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, if (fsck_system_inode(sdp, block)) fsck_blockmap_set(ip, block, _("system file"), dinode_type); else - check_n_fix_bitmap(sdp, block, dinode_type); + check_n_fix_bitmap(sdp, block, 0, dinode_type); /* Return the number of leaf entries so metawalk doesn't flag this leaf as having none. */ *count = be16_to_cpu(((struct gfs2_leaf *)bh->b_data)->lf_entries); @@ -339,6 +339,8 @@ static int undo_reference(struct gfs2_inode *ip, uint64_t block, int meta, struct block_count *bc = (struct block_count *)private; struct duptree *dt; struct inode_with_dups *id; + int old_bitmap_state = 0; + struct rgrp_tree *rgd; if (!valid_block(ip->i_sbd, block)) { /* blk outside of FS */ fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, @@ -367,6 +369,12 @@ static int undo_reference(struct gfs2_inode *ip, uint64_t block, int meta, return 1; } } + if (!meta) { + rgd = gfs2_blk2rgrpd(ip->i_sbd, block); + old_bitmap_state = lgfs2_get_bitmap(ip->i_sbd, block, rgd); + if (old_bitmap_state == GFS2_BLKST_DINODE) + return -1; + } fsck_blockmap_set(ip, block, meta ? _("bad indirect") : _("referenced data"), gfs2_block_free); @@ -385,6 +393,51 @@ static int undo_check_data(struct gfs2_inode *ip, uint64_t block, return undo_reference(ip, block, 0, private); } +/* blockmap_set_as_data - set block as 'data' in the blockmap, if not dinode + * + * This function tries to set a block that's referenced as data as 'data' + * in the fsck blockmap. But if that block is marked as 'dinode' in the + * rgrp bitmap, it does additional checks to see if it looks like a dinode. + * Note that previous checks were done for duplicate references, so this + * is checking for dinodes that we haven't processed yet. + */ +static int blockmap_set_as_data(struct gfs2_inode *ip, uint64_t block) +{ + int error; + struct gfs2_buffer_head *bh; + struct gfs2_dinode *di; + + error = fsck_blkmap_set_noino(ip, block, _("data"), gfs2_block_used); + if (!error) + return 0; + + error = 0; + /* The bitmap says it's a dinode, but a block reference begs to differ. + So which is it? */ + bh = bread(ip->i_sbd, block); + if (gfs2_check_meta(bh, GFS2_METATYPE_DI) != 0) + goto out; + + /* The meta header agrees it's a dinode. But it might be data in + disguise, so do some extra checks. */ + di = (struct gfs2_dinode *)bh->b_data; + if (be64_to_cpu(di->di_num.no_addr) != block) + goto out; + + log_err(_("Inode %lld (0x%llx) has a reference to block %lld (0x%llx) " + "as a data block, but it appears to be a dinode we " + "haven't checked yet.\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); + error = -1; +out: + if (!error) + fsck_blockmap_set(ip, block, _("data"), gfs2_block_used); + brelse(bh); + return error; +} + static int check_data(struct gfs2_inode *ip, uint64_t metablock, uint64_t block, void *private) { @@ -469,7 +522,7 @@ static int check_data(struct gfs2_inode *ip, uint64_t metablock, (unsigned long long)block, (unsigned long long)block); fsck_blockmap_set(ip, block, _("jdata"), gfs2_jdata); } else - fsck_blockmap_set(ip, block, _("data"), gfs2_block_used); + return blockmap_set_as_data(ip, block); return 0; } @@ -1199,7 +1252,7 @@ static int check_system_inode(struct gfs2_sbd *sdp, (unsigned long long)iblock, (unsigned long long)iblock); gfs2_blockmap_set(bl, iblock, gfs2_block_free); - check_n_fix_bitmap(sdp, iblock, gfs2_block_free); + check_n_fix_bitmap(sdp, iblock, 0, gfs2_block_free); inode_put(sysinode); } } @@ -1486,7 +1539,7 @@ 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); - check_n_fix_bitmap(sdp, block, gfs2_block_free); + check_n_fix_bitmap(sdp, block, 0, gfs2_block_free); } else if (handle_di(sdp, bh) < 0) { stack; brelse(bh); @@ -1596,7 +1649,7 @@ int pass1(struct gfs2_sbd *sdp) } /* rgrps and bitmaps don't have bits to represent their blocks, so don't do this: - check_n_fix_bitmap(sdp, rgd->ri.ri_addr + i, + check_n_fix_bitmap(sdp, rgd->ri.ri_addr + i, 0, gfs2_meta_rgrp);*/ } diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 9c76eda..9a23197 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -501,7 +501,7 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *dt) dup_delete(dh.dt); /* Now fix the block type of the block in question. */ gfs2_blockmap_set(bl, dup_blk, gfs2_block_free); - check_n_fix_bitmap(sdp, dup_blk, gfs2_block_free); + check_n_fix_bitmap(sdp, dup_blk, 0, gfs2_block_free); } } return 0; diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index dc99869..5767c4d 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -1713,7 +1713,7 @@ int pass2(struct gfs2_sbd *sdp) /* Can't use fsck_blockmap_set here because we don't have an inode in memory. */ gfs2_blockmap_set(bl, dirblk, gfs2_inode_invalid); - check_n_fix_bitmap(sdp, dirblk, gfs2_inode_invalid); + check_n_fix_bitmap(sdp, dirblk, 0, gfs2_inode_invalid); } ip = fsck_load_inode(sdp, dirblk); if (!ds.dotdir) { diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c index 53052b6..4894d8c 100644 --- a/gfs2/fsck/pass3.c +++ b/gfs2/fsck/pass3.c @@ -275,7 +275,7 @@ int pass3(struct gfs2_sbd *sdp) gfs2_blockmap_set(bl, di->dinode.no_addr, gfs2_block_free); check_n_fix_bitmap(sdp, di->dinode.no_addr, - gfs2_block_free); + 0, gfs2_block_free); break; } else log_err( _("Unlinked directory with bad block remains\n")); @@ -299,7 +299,7 @@ int pass3(struct gfs2_sbd *sdp) because we don't have ip */ gfs2_blockmap_set(bl, di->dinode.no_addr, gfs2_block_free); - check_n_fix_bitmap(sdp, di->dinode.no_addr, + check_n_fix_bitmap(sdp, di->dinode.no_addr, 0, gfs2_block_free); log_err( _("The block was cleared\n")); break;

1 0

gfs2-utils: master - fsck.gfs2: Remove unused variable
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=a3c643a9c9… Commit: a3c643a9c98dd68138ff6b623fd86923a16fc626 Parent: 1bae9562287edc528adcb1f39a750a5cb4a5e286 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Mon Apr 8 06:30:44 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: Remove unused variable This patch removes a variable that wasn't being used. --- gfs2/fsck/pass1b.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 15a3f3a..9c76eda 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -146,7 +146,6 @@ static void resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt, enum dup_ref_type this_ref; struct inode_info *ii; int found_good_ref = 0; - uint64_t dup_block; uint8_t q; osi_list_foreach_safe(tmp, ref_list, x) { @@ -228,7 +227,6 @@ static void resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt, (unsigned long long)id->block_no, (unsigned long long)id->block_no); - dup_block = id->block_no; ip = fsck_load_inode(sdp, id->block_no); /* If we've already deleted this dinode, don't try to delete it again. That could free blocks that used to be duplicate

1 0

gfs2-utils: master - fsck.gfs2: Remove all bad eattr blocks
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=1bae956228… Commit: 1bae9562287edc528adcb1f39a750a5cb4a5e286 Parent: 98544a4b3a1a7251a8230a24e7efdcce306c99bb Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Apr 3 11:52:05 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: Remove all bad eattr blocks Before this patch, bad extended attributes were not properly removed from a dinode, and blocks were not freed. This patch properly removes them all. --- gfs2/fsck/pass1.c | 12 ++++++++---- gfs2/fsck/pass1c.c | 8 +++++++- gfs2/fsck/pass2.c | 18 ++++++++++-------- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index ee7e2c5..ad6690b 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -487,6 +487,8 @@ static int remove_inode_eattr(struct gfs2_inode *ip, struct block_count *bc) static int ask_remove_inode_eattr(struct gfs2_inode *ip, struct block_count *bc) { + if (ip->i_di.di_eattr == 0) + return 0; /* eattr was removed prior to this call */ log_err( _("Inode %lld (0x%llx) has unrecoverable Extended Attribute " "errors.\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); @@ -1074,11 +1076,13 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip) if (fsck_abort) return 0; - error = check_inode_eattr(ip, &pass1_fxns); + if (!error) { + error = check_inode_eattr(ip, &pass1_fxns); - if (error && - !(ip->i_di.di_flags & GFS2_DIF_EA_INDIRECT)) - ask_remove_inode_eattr(ip, &bc); + if (error && + !(ip->i_di.di_flags & GFS2_DIF_EA_INDIRECT)) + ask_remove_inode_eattr(ip, &bc); + } if (ip->i_di.di_blocks != (1 + bc.indir_count + bc.data_count + bc.ea_count)) { diff --git a/gfs2/fsck/pass1c.c b/gfs2/fsck/pass1c.c index 26d47d5..b918de1 100644 --- a/gfs2/fsck/pass1c.c +++ b/gfs2/fsck/pass1c.c @@ -12,6 +12,12 @@ #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, @@ -62,7 +68,7 @@ static int ask_remove_eattr_entry(struct gfs2_sbd *sdp, static int ask_remove_eattr(struct gfs2_inode *ip) { if (query( _("Remove the bad Extended Attribute? (y/n) "))) { - ip->i_di.di_eattr = 0; + check_inode_eattr(ip, &pass1c_fxns_delete); bmodified(ip->i_bh); log_err( _("Bad Extended Attribute removed.\n")); return 1; diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index b79e999..dc99869 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -20,11 +20,13 @@ #define MAX_FILENAME 256 -struct metawalk_fxns clear_eattrs = { +struct metawalk_fxns pass2_fxns; + +struct metawalk_fxns delete_eattrs = { .check_eattr_indir = delete_eattr_indir, .check_eattr_leaf = delete_eattr_leaf, - .check_eattr_entry = clear_eattr_entry, - .check_eattr_extentry = clear_eattr_extentry, + .check_eattr_entry = delete_eattr_entry, + .check_eattr_extentry = delete_eattr_extentry, }; /* Set children's parent inode in dir_info structure - ext2 does not set @@ -599,7 +601,7 @@ static int basic_dentry_checks(struct gfs2_inode *ip, struct gfs2_dirent *dent, entry_ip = ip; else entry_ip = fsck_load_inode(sdp, entry->no_addr); - check_inode_eattr(entry_ip, &clear_eattrs); + check_inode_eattr(entry_ip, &delete_eattrs); if (entry_ip != ip) fsck_inode_put(&entry_ip); return 1; @@ -683,7 +685,7 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, entry_ip = ip; else entry_ip = fsck_load_inode(sdp, entry.no_addr); - check_inode_eattr(entry_ip, &clear_eattrs); + check_inode_eattr(entry_ip, &delete_eattrs); if (entry_ip != ip) fsck_inode_put(&entry_ip); goto nuke_dentry; @@ -714,7 +716,7 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, entry_ip = ip; else entry_ip = fsck_load_inode(sdp, entry.no_addr); - check_inode_eattr(entry_ip, &clear_eattrs); + check_inode_eattr(entry_ip, &delete_eattrs); if (entry_ip != ip) fsck_inode_put(&entry_ip); goto nuke_dentry; @@ -744,7 +746,7 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, entry_ip = ip; else entry_ip = fsck_load_inode(sdp, entry.no_addr); - check_inode_eattr(entry_ip, &clear_eattrs); + check_inode_eattr(entry_ip, &delete_eattrs); if (entry_ip != ip) fsck_inode_put(&entry_ip); @@ -764,7 +766,7 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, entry_ip = ip; else entry_ip = fsck_load_inode(sdp, entry.no_addr); - check_inode_eattr(entry_ip, &clear_eattrs); + check_inode_eattr(entry_ip, &delete_eattrs); if (entry_ip != ip) fsck_inode_put(&entry_ip);

1 0

gfs2-utils: master - fsck.gfs2: major duplicate reference reform
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=98544a4b3a… Commit: 98544a4b3a1a7251a8230a24e7efdcce306c99bb Parent: 390b7b663762b32a68fff13c214ec6361f63f3bf Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Apr 3 10:50:24 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: major duplicate reference reform This patch is a large set of changes designed to rework how pass1b resolves duplicate block references. There are basically two major changes with this patch: First, the metawalk functions were trying to attribute too much information to the return codes of its callback functions: (1) Was there an error? (2) Was the inode valid? (3) Was a duplicate block reference encountered? (4) Should we keep going and process more of its metadata? This often led to bad decisions made by metawalk: For example, it would stop processing metadata when it should have continued, thereby forgetting to mark blocks free that were no longer in use. This patch introduces two new variables to the metatree functions, *is_valid and *was_duplicate. The first one indicates whether the dinode was valid or whether there is good cause to delete it. The second indicates whether a duplicate block reference was encountered. With this patch, the return code indicates simply whether metadata processing should be skipped or not, and nothing more. This is especially useful in pass1. For example, if it encounters major corruption in a dinode, it doesn't do any good to mark all its blocks as duplicates and have the undo functions try to reverse all those decisions. The second major change with this patch has to do with the philosophy of how duplicate references are resolved. Before, pass1 would flag the duplicates and pass1b would try to resolve them all, marking dinodes that should be deleted as "bad", and pass2 would delete the bad dinodes. This becomes very problematic and messy in pass1b, especially in cases where you have a number of duplicate references that are common between multiple dinodes. For example, suppose files A, B and C share some of the same blocks, but not others: A - 0x3000 0x3001 0x1233 0x1234 0x3004 B - 0x4000 0x4001 0x4002 0x1234 0x1235 C - 0x1231 0x1232 0x1233 0x1234 0x1235 The old strategy that got us into trouble was to log the three duplicate blocks, delete invalid dinodes A and B, but leave the duplicate reference structure around for 0x1233, 0x1234 and 0x1235 so that C would be left intact with the only references to all five blocks. But in cleaning up the leftover duplicate structure often led to bad decisions where C wouldn't have all its blocks marked as referenced. Often, you would end up with blocks that were marked as free which were still in use, and blocks that were marked as in use that should have been freed, and it was all due to the existence of those duplicate structures that were still on the list until pass2. The new strategy is to resolve-as-you-go. In other words, pass1b considers the three duplicate blocks, but when it decides that file A should be deleted, it removes all its references from the list, thereby making the decision between B and C easier: it no longer has to worry about block 1233, and there's only one thing to consider about block 0x1234 and 0x1235. When B is deleted, it removes all its duplicate references, so block 0x1235 is no longer considered to be in conflict. Once a file is deleted, all its duplicate reference structures are removed so as not to confuse other duplicates being resolved. The duplicate handler structure, struct dup_handler, is revised with every reference that's resolved so it's not working off a long list of possibles, most of which were already taken care of by previous actions. --- gfs2/fsck/fsck.h | 2 - gfs2/fsck/initialize.c | 2 +- gfs2/fsck/metawalk.c | 217 +++++++++---- gfs2/fsck/metawalk.h | 31 ++- gfs2/fsck/pass1.c | 101 ++++--- gfs2/fsck/pass1b.c | 810 ++++++++++++++++++++---------------------------- gfs2/fsck/pass2.c | 60 ---- gfs2/fsck/util.c | 37 ++- gfs2/fsck/util.h | 3 +- 9 files changed, 611 insertions(+), 652 deletions(-) diff --git a/gfs2/fsck/fsck.h b/gfs2/fsck/fsck.h index b21a670..6d888af 100644 --- a/gfs2/fsck/fsck.h +++ b/gfs2/fsck/fsck.h @@ -112,11 +112,9 @@ extern int pass4(struct gfs2_sbd *sdp); extern int pass5(struct gfs2_sbd *sdp); extern int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane); -extern void gfs2_dup_free(void); 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 *dt); extern void dirtree_delete(struct dir_info *b); diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c index 7d64b0a..b01b240 100644 --- a/gfs2/fsck/initialize.c +++ b/gfs2/fsck/initialize.c @@ -66,7 +66,7 @@ static int block_mounters(struct gfs2_sbd *sdp, int block_em) return 0; } -void gfs2_dup_free(void) +static void gfs2_dup_free(void) { struct osi_node *n; struct duptree *dt; diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index dc585ae..22b16ee 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -950,7 +950,8 @@ int delete_block(struct gfs2_inode *ip, uint64_t block, /** * find_remove_dup - find out if this is a duplicate ref. If so, remove it. - * Returns: 0 if not a duplicate reference, 1 if it is. + * + * Returns: 1 if there are any remaining references to this block, else 0. */ int find_remove_dup(struct gfs2_inode *ip, uint64_t block, const char *btype) { @@ -964,41 +965,18 @@ int find_remove_dup(struct gfs2_inode *ip, uint64_t block, const char *btype) /* remove the inode reference id structure for this reference. */ id = find_dup_ref_inode(dt, ip); if (!id) - return 0; + goto more_refs; - dup_listent_delete(id); - log_err( _("Removing duplicate status of block %llu (0x%llx) " - "referenced as %s by dinode %llu (0x%llx)\n"), - (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); - dt->refs--; /* one less reference */ - if (dt->refs == 1) { - log_info( _("This leaves only one reference: it's " - "no longer a duplicate.\n")); + dup_listent_delete(dt, id); + if (dt->refs == 0) { + log_info( _("This was the last reference: it's no longer a " + "duplicate.\n")); dup_delete(dt); /* not duplicate now */ - } else - log_info( _("%d block reference(s) remain.\n"), - dt->refs); - return 1; /* but the original ref still exists so do not free it. */ -} - -/** - * free_block_if_notdup - free blocks associated with an inode, but if it's a - * duplicate, just remove that designation instead. - * Returns: 1 if the block was freed, 0 if a duplicate reference was removed - * Note: The return code is handled this way because there are places in - * metawalk.c that assume "1" means "change was made" and "0" means - * change was not made. - */ -int free_block_if_notdup(struct gfs2_inode *ip, uint64_t block, - const char *btype) -{ - if (!find_remove_dup(ip, block, btype)) { /* not a dup */ - fsck_blockmap_set(ip, block, btype, gfs2_block_free); - return meta_skip_further; + return 0; } - return meta_is_good; +more_refs: + log_info( _("%d block reference(s) remain.\n"), dt->refs); + return 1; /* references still exist so do not free the block. */ } /** @@ -1010,7 +988,8 @@ int free_block_if_notdup(struct gfs2_inode *ip, uint64_t block, */ static int delete_block_if_notdup(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, - const char *btype, void *private) + const char *btype, int *was_duplicate, + void *private) { uint8_t q; @@ -1027,7 +1006,19 @@ static int delete_block_if_notdup(struct gfs2_inode *ip, uint64_t block, (unsigned long long)ip->i_di.di_num.no_addr); return meta_is_good; } - return free_block_if_notdup(ip, block, btype); + if (find_remove_dup(ip, block, btype)) { /* a dup */ + if (was_duplicate) + *was_duplicate = 1; + log_err( _("Not clearing duplicate reference in inode " + "at block #%llu (0x%llx) to block #%llu (0x%llx) " + "because it's referenced by another inode.\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); + } else { + fsck_blockmap_set(ip, block, btype, gfs2_block_free); + } + return meta_is_good; } /** @@ -1197,7 +1188,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, osi_list_t *prev_list, *cur_list, *tmp; int h, head_size, iblk_type; uint64_t *ptr, block; - int error = 0, err; + int error, was_duplicate, is_valid; osi_list_add(&metabh->b_altlist, &mlp[0]); @@ -1211,7 +1202,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, /* if (<there are no indirect blocks to check>) */ if (height < 2) - return 0; + return meta_is_good; for (h = 1; h < height; h++) { if (h > 1) { if (is_dir(&ip->i_di, ip->i_sbd->gfs1) && @@ -1243,7 +1234,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, ptr++) { if (skip_this_pass || fsck_abort) { free_metalist(ip, mlp); - return FSCK_OK; + return meta_is_good; } nbh = NULL; @@ -1251,19 +1242,41 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, continue; block = be64_to_cpu(*ptr); - err = pass->check_metalist(ip, block, &nbh, h, - pass->private); + was_duplicate = 0; + error = pass->check_metalist(ip, block, &nbh, + h, &is_valid, + &was_duplicate, + pass->private); /* check_metalist should hold any buffers it gets with "bread". */ - if (err == meta_error) { + if (error == meta_error) { stack; - error = err; + log_info(_("\nSerious metadata " + "error on block %llu " + "(0x%llx).\n"), + (unsigned long long)block, + (unsigned long long)block); return error; } - if (err == meta_skip_further) { - if (!error) - error = err; - log_debug( _("Skipping block %llu (0x%llx)\n"), + if (error == meta_skip_further) { + log_info(_("\nUnrecoverable metadata " + "error on block %llu " + "(0x%llx). Further metadata" + " will be skipped.\n"), + (unsigned long long)block, + (unsigned long long)block); + return error; + } + if (!is_valid) { + log_debug( _("Skipping rejected block " + "%llu (0x%llx)\n"), + (unsigned long long)block, + (unsigned long long)block); + continue; + } + if (was_duplicate) { + log_debug( _("Skipping duplicate %llu " + "(0x%llx)\n"), (unsigned long long)block, (unsigned long long)block); continue; @@ -1590,34 +1603,52 @@ int remove_dentry_from_dir(struct gfs2_sbd *sdp, uint64_t dir, } int delete_metadata(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, void *private) + struct gfs2_buffer_head **bh, int h, int *is_valid, + int *was_duplicate, void *private) { - return delete_block_if_notdup(ip, block, bh, _("metadata"), private); + *is_valid = 1; + *was_duplicate = 0; + return delete_block_if_notdup(ip, block, bh, _("metadata"), + was_duplicate, private); } int delete_leaf(struct gfs2_inode *ip, uint64_t block, void *private) { - return delete_block_if_notdup(ip, block, NULL, _("leaf"), private); + return delete_block_if_notdup(ip, block, NULL, _("leaf"), NULL, + 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); + return delete_block_if_notdup(ip, block, NULL, _("data"), NULL, + private); } -int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, - struct gfs2_buffer_head **bh, void *private) +static int del_eattr_generic(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, struct gfs2_buffer_head **bh, + void *private, const char *eatype) { - int ret; + int ret = 0; + int was_free = 0; + uint8_t q; - ret = delete_block_if_notdup(ip, block, NULL, - _("indirect extended attribute"), - private); + if (valid_block(ip->i_sbd, block)) { + q = block_type(block); + if (q == gfs2_block_free) + was_free = 1; + ret = delete_block_if_notdup(ip, block, NULL, eatype, + NULL, private); + if (!ret) { + *bh = bread(ip->i_sbd, block); + if (!was_free) + ip->i_di.di_blocks--; + bmodified(ip->i_bh); + } + } /* Even if it's a duplicate reference, we want to eliminate the reference itself, and adjust di_blocks accordingly. */ if (ip->i_di.di_eattr) { - ip->i_di.di_blocks--; if (block == ip->i_di.di_eattr) ip->i_di.di_eattr = 0; bmodified(ip->i_bh); @@ -1625,24 +1656,74 @@ int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, return ret; } +int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, + struct gfs2_buffer_head **bh, void *private) +{ + return del_eattr_generic(ip, block, parent, bh, private, + _("extended attribute")); +} + int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private) { - int ret; + return del_eattr_generic(ip, block, parent, bh, private, + _("indirect extended attribute")); +} - ret = delete_block_if_notdup(ip, block, NULL, _("extended attribute"), - private); - if (ip->i_di.di_eattr) { - ip->i_di.di_blocks--; - if (block == ip->i_di.di_eattr) - ip->i_di.di_eattr = 0; - bmodified(ip->i_bh); +int delete_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 avail_size; + int max_ptrs; + + if (!ea_hdr->ea_name_len){ + /* Skip this entry for now */ + return 1; } - return ret; + + 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))){ + /* Skip invalid entry */ + return 1; + } + + 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; + + if (max_ptrs > ea_hdr->ea_num_ptrs) + return 1; + + log_debug( _(" Pointers Required: %d\n Pointers Reported: %d\n"), + max_ptrs, ea_hdr->ea_num_ptrs); + + return 0; +} + +int delete_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, + struct gfs2_buffer_head *leaf_bh, + struct gfs2_ea_header *ea_hdr, + struct gfs2_ea_header *ea_hdr_prev, void *private) +{ + uint64_t block = be64_to_cpu(*ea_data_ptr); + + return delete_block_if_notdup(ip, block, NULL, _("extended attribute"), + NULL, private); } static int alloc_metalist(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, void *private) + struct gfs2_buffer_head **bh, int h, int *is_valid, + int *was_duplicate, void *private) { uint8_t q; const char *desc = (const char *)private; @@ -1650,6 +1731,8 @@ static int alloc_metalist(struct gfs2_inode *ip, uint64_t block, /* No need to range_check here--if it was added, it's in range. */ /* We can't check the bitmap here because this function is called after the bitmap has been set but before the blockmap has. */ + *is_valid = 1; + *was_duplicate = 0; *bh = bread(ip->i_sbd, block); q = block_type(block); if (blockmap_to_bitmap(q, ip->i_sbd->gfs1) == GFS2_BLKST_FREE) { diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index 49217cc..56f57d9 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -24,7 +24,8 @@ extern int delete_block(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, const char *btype, void *private); extern int delete_metadata(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, void *private); + struct gfs2_buffer_head **bh, int h, int *is_valid, + int *was_duplicate, 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 metablock, uint64_t block, void *private); @@ -32,6 +33,17 @@ extern int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t pa struct gfs2_buffer_head **bh, void *private); extern int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private); +extern int delete_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); +extern int delete_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, + struct gfs2_buffer_head *leaf_bh, + struct gfs2_ea_header *ea_hdr, + struct gfs2_ea_header *ea_hdr_prev, + void *private); + extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock, const char *btype, enum gfs2_mark_block mark, const char *caller, int line); @@ -48,8 +60,6 @@ extern int write_new_leaf(struct gfs2_inode *dip, int start_lindex, uint64_t *bn); extern int repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no, int lindex, int ref_count, const char *msg); -extern int free_block_if_notdup(struct gfs2_inode *ip, uint64_t block, - const char *btype); #define is_duplicate(dblock) ((dupfind(dblock)) ? 1 : 0) @@ -83,8 +93,23 @@ struct metawalk_fxns { int ref_count, struct gfs2_buffer_head *lbh); int (*check_leaf) (struct gfs2_inode *ip, uint64_t block, void *private); + /* parameters to the check_metalist sub-functions: + ip: incore inode pointer + block: block number of the metadata block to be checked + bh: buffer_head to be returned + h: height + is_valid: returned as 1 if the metadata block is valid and should + be added to the metadata list for further processing. + was_duplicate: returns as 1 if the metadata block was determined + to be a duplicate reference, in which case we want to + skip adding it to the metadata list. + private: Pointer to pass-specific data + returns: 0 - everything is good, but there may be duplicates + 1 - skip further processing + */ int (*check_metalist) (struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private); int (*check_data) (struct gfs2_inode *ip, uint64_t metablock, uint64_t block, void *private); diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index df10089..ee7e2c5 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -37,7 +37,8 @@ struct block_count { 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); + struct gfs2_buffer_head **bh, int h, int *is_valid, + int *was_duplicate, 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 metablock, @@ -64,6 +65,7 @@ static int finish_eattr_indir(struct gfs2_inode *ip, int leaf_pointers, int leaf_pointer_errors, void *private); static int invalidate_metadata(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private); static int invalidate_leaf(struct gfs2_inode *ip, uint64_t block, void *private); @@ -127,10 +129,13 @@ struct metawalk_fxns invalidate_fxns = { */ static int resuscitate_metalist(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { struct block_count *bc = (struct block_count *)private; + *is_valid = 1; + *was_duplicate = 0; *bh = NULL; if (!valid_block(ip->i_sbd, block)){ /* blk outside of FS */ fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, @@ -139,7 +144,8 @@ 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 meta_skip_further; + *is_valid = 0; + return meta_is_good; } if (fsck_system_inode(ip->i_sbd, block)) fsck_blockmap_set(ip, block, _("system file"), gfs2_indir_blk); @@ -241,16 +247,19 @@ 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) + struct gfs2_buffer_head **bh, int h, int *is_valid, + int *was_duplicate, void *private) { uint8_t q; - int found_dup = 0, iblk_type; + int iblk_type; struct gfs2_buffer_head *nbh; struct block_count *bc = (struct block_count *)private; const char *blktypedesc; *bh = NULL; + *was_duplicate = 0; + *is_valid = 0; if (!valid_block(ip->i_sbd, block)) { /* blk outside of FS */ /* The bad dinode should be invalidated later due to "unrecoverable" errors. The inode itself should be @@ -282,12 +291,13 @@ 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, q, block_type_string(q)); - add_duplicate_ref(ip, block, ref_as_meta, 0, INODE_VALID); - found_dup = 1; + *was_duplicate = 1; } nbh = bread(ip->i_sbd, block); - if (gfs2_check_meta(nbh, iblk_type)){ + *is_valid = (gfs2_check_meta(nbh, iblk_type) == 0); + + if (!(*is_valid)) { log_err( _("Inode %lld (0x%llx) has a bad indirect block " "pointer %lld (0x%llx) (points to something " "that is not %s).\n"), @@ -295,31 +305,23 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)block, (unsigned long long)block, blktypedesc); - if (!found_dup) { - fsck_blockmap_set(ip, block, _("bad indirect"), - gfs2_meta_inval); - brelse(nbh); - nbh = NULL; - return meta_skip_further; - } brelse(nbh); - nbh = NULL; - } else /* blk check ok */ - *bh = nbh; + return meta_skip_further; + } bc->indir_count++; - if (found_dup) { - if (nbh) { - brelse(nbh); - nbh = NULL; - *bh = NULL; - } - return meta_skip_further; /* don't process the metadata again */ - } else - fsck_blockmap_set(ip, block, _("indirect"), - gfs2_indir_blk); + if (*was_duplicate) { + add_duplicate_ref(ip, block, ref_as_meta, 0, + *is_valid ? INODE_VALID : INODE_INVALID); + brelse(nbh); + } else { + *bh = nbh; + fsck_blockmap_set(ip, block, _("indirect"), gfs2_indir_blk); + } - return meta_is_good; + if (*is_valid) + return meta_is_good; + return meta_skip_further; } /* undo_reference - undo previously processed data or metadata @@ -354,7 +356,7 @@ static int undo_reference(struct gfs2_inode *ip, uint64_t block, int meta, if (!id) break; - dup_listent_delete(id); + dup_listent_delete(dt, id); } while (id); if (dt->refs) { @@ -827,7 +829,8 @@ static int check_eattr_entries(struct gfs2_inode *ip, * delete_block_if_notdup. */ static int mark_block_invalid(struct gfs2_inode *ip, uint64_t block, - enum dup_ref_type reftype, const char *btype) + enum dup_ref_type reftype, const char *btype, + int *is_valid, int *was_duplicate) { uint8_t q; @@ -838,11 +841,20 @@ static int mark_block_invalid(struct gfs2_inode *ip, uint64_t block, * referenced elsewhere (duplicates) won't be flagged as such, * 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)) + if (is_valid) + *is_valid = 1; + if (was_duplicate) + *was_duplicate = 0; + if (!valid_block(ip->i_sbd, block)) { + if (is_valid) + *is_valid = 0; return meta_is_good; + } q = block_type(block); if (q != gfs2_block_free) { + if (was_duplicate) + *was_duplicate = 1; add_duplicate_ref(ip, block, reftype, 0, INODE_INVALID); log_info( _("%s block %lld (0x%llx), part of inode " "%lld (0x%llx), was previously referenced so " @@ -859,21 +871,27 @@ static int mark_block_invalid(struct gfs2_inode *ip, uint64_t block, static int invalidate_metadata(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { - return mark_block_invalid(ip, block, ref_as_meta, _("metadata")); + *is_valid = 1; + *was_duplicate = 0; + return mark_block_invalid(ip, block, ref_as_meta, _("metadata"), + is_valid, was_duplicate); } static int invalidate_leaf(struct gfs2_inode *ip, uint64_t block, void *private) { - return mark_block_invalid(ip, block, ref_as_meta, _("leaf")); + return mark_block_invalid(ip, block, ref_as_meta, _("leaf"), + NULL, NULL); } 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")); + return mark_block_invalid(ip, block, ref_as_data, _("data"), + NULL, NULL); } static int invalidate_eattr_indir(struct gfs2_inode *ip, uint64_t block, @@ -881,7 +899,8 @@ static int invalidate_eattr_indir(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, void *private) { return mark_block_invalid(ip, block, ref_as_ea, - _("indirect extended attribute")); + _("indirect extended attribute"), + NULL, NULL); } static int invalidate_eattr_leaf(struct gfs2_inode *ip, uint64_t block, @@ -889,7 +908,8 @@ static int invalidate_eattr_leaf(struct gfs2_inode *ip, uint64_t block, void *private) { return mark_block_invalid(ip, block, ref_as_ea, - _("extended attribute")); + _("extended attribute"), + NULL, NULL); } /** @@ -924,7 +944,7 @@ 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 meta_skip_further; + return meta_is_good; else return meta_error; /* Exits check_metatree quicker */ } @@ -939,7 +959,7 @@ 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 meta_skip_further; + return meta_is_good; else return meta_error; /* Exits check_metatree quicker */ } @@ -948,8 +968,11 @@ static int rangecheck_block(struct gfs2_inode *ip, uint64_t block, static int rangecheck_metadata(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 rangecheck_block(ip, block, bh, btype_meta, private); } @@ -1048,7 +1071,7 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip) /* 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) + if (fsck_abort) return 0; error = check_inode_eattr(ip, &pass1_fxns); diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index b5da200..15a3f3a 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -23,386 +23,10 @@ struct fxn_info { struct dup_handler { struct duptree *dt; - struct inode_with_dups *id; int ref_inode_count; int ref_count; }; -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, - 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); -static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block, - uint64_t parent, struct gfs2_buffer_head **bh, - void *private); -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); -static int check_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, - struct gfs2_buffer_head *leaf_bh, - struct gfs2_ea_header *ea_hdr, - struct gfs2_ea_header *ea_hdr_prev, - 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, int lindex, - void *priv); - -struct metawalk_fxns find_refs = { - .private = NULL, - .check_leaf = check_leaf_refs, - .check_metalist = check_metalist, - .check_data = check_data, - .check_eattr_indir = check_eattr_indir, - .check_eattr_leaf = check_eattr_leaf, - .check_dentry = NULL, - .check_eattr_entry = check_eattr_entry, - .check_eattr_extentry = check_eattr_extentry, -}; - -struct metawalk_fxns find_dirents = { - .private = NULL, - .check_leaf = NULL, - .check_metalist = NULL, - .check_data = NULL, - .check_eattr_indir = NULL, - .check_eattr_leaf = NULL, - .check_dentry = find_dentry, - .check_eattr_entry = NULL, - .check_eattr_extentry = NULL, -}; - -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); -} - -static int check_metalist(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, void *private) -{ - return add_duplicate_ref(ip, block, ref_as_meta, 1, INODE_VALID); -} - -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); -} - -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; - int error; - - error = add_duplicate_ref(ip, block, ref_as_ea, 1, INODE_VALID); - if (!error) - *bh = bread(sdp, block); - - return error; -} - -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; - int error; - - error = add_duplicate_ref(ip, block, ref_as_ea, 1, INODE_VALID); - if (!error) - *bh = bread(sdp, block); - return error; -} - -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) -{ - return 0; -} - -static int check_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, - struct gfs2_buffer_head *leaf_bh, - struct gfs2_ea_header *ea_hdr, - struct gfs2_ea_header *ea_hdr_prev, - void *private) -{ - uint64_t block = be64_to_cpu(*ea_data_ptr); - - return add_duplicate_ref(ip, block, ref_as_ea, 1, INODE_VALID); -} - -/* - * check_dir_dup_ref - check for a directory entry duplicate reference - * and if found, set the name into the id. - * Returns: 1 if filename was found, otherwise 0 - */ -static int check_dir_dup_ref(struct gfs2_inode *ip, struct gfs2_dirent *de, - osi_list_t *tmp2, char *filename) -{ - struct inode_with_dups *id; - - id = osi_list_entry(tmp2, struct inode_with_dups, list); - if (id->name) - /* We can only have one parent of inodes that contain duplicate - * blocks...no need to keep looking for this one. */ - return 1; - if (id->block_no == de->de_inum.no_addr) { - id->name = strdup(filename); - id->parent = ip->i_di.di_num.no_addr; - log_debug( _("Duplicate block %llu (0x%llx" - ") is in file or directory %llu" - " (0x%llx) named %s\n"), - (unsigned long long)id->block_no, - (unsigned long long)id->block_no, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr, - filename); - /* If there are duplicates of duplicates, I guess we'll miss - them here. */ - return 1; - } - return 0; -} - -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, int lindex, void *priv) -{ - struct osi_node *n, *next = NULL; - osi_list_t *tmp2; - struct duptree *dt; - int found; - - for (n = osi_first(&dup_blocks); n; n = next) { - next = osi_next(n); - dt = (struct duptree *)n; - found = 0; - 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, &dt->ref_inode_list) { - if (check_dir_dup_ref(ip, de, tmp2, filename)) - break; - } - } - } - /* Return the number of leaf entries so metawalk doesn't flag this - leaf as having none. */ - *count = be16_to_cpu(((struct gfs2_leaf *)bh->b_data)->lf_entries); - return 0; -} - -static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - void *private) -{ - struct dup_handler *dh = (struct dup_handler *) private; - struct duptree *dt; - - if (!valid_block(ip->i_sbd, block)) - 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 - somewhere in its metadata. We know this block is listed as data - or metadata for this inode, but it may or may not be one of the - actual duplicate references that caused the problem. If it's not - a duplicate, it's normal metadata that isn't referenced anywhere - else, but we're deleting the inode out from under it, so we need - 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. */ - dt = dupfind(block); - if (!dt) { - fsck_blockmap_set(ip, block, _("no longer valid"), - gfs2_block_free); - return meta_is_good; - } - /* This block, having failed the above test, is duplicated somewhere */ - 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"), - dh->id->name ? dh->id->name : _("unknown name"), - (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); - log_err( _("Inode %s is in directory %llu (0x%llx)\n"), - dh->id->name ? dh->id->name : "", - (unsigned long long)dh->id->parent, - (unsigned long long)dh->id->parent); - } - /* We return 1 not 0 because we need build_and_check_metalist to - bypass adding the metadata below it to the metalist. If that - were to happen, all the indirect blocks pointed to by the - duplicate block would be processed twice, which means it might - 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 meta_skip_further; -} - -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); -} - -static int clear_leaf(struct gfs2_inode *ip, uint64_t block, void *private) -{ - return clear_dup_metalist(ip, block, NULL, 0, private); -} - -static int clear_dup_eattr_indir(struct gfs2_inode *ip, uint64_t block, - uint64_t parent, struct gfs2_buffer_head **bh, - void *private) -{ - return clear_dup_metalist(ip, block, NULL, 0, private); -} - -static int clear_dup_eattr_leaf(struct gfs2_inode *ip, uint64_t block, - uint64_t parent, struct gfs2_buffer_head **bh, - void *private) -{ - return clear_dup_metalist(ip, block, NULL, 0, private); -} - -static int clear_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]; - - if (!ea_hdr->ea_name_len){ - /* Skip this entry for now */ - return 1; - } - - 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))){ - /* Skip invalid entry */ - return 1; - } - - 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) - return 1; - else { - log_debug( _(" Pointers Required: %d\n Pointers Reported: %d\n"), - max_ptrs, ea_hdr->ea_num_ptrs); - } - } - return 0; -} - -static int clear_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, - struct gfs2_buffer_head *leaf_bh, - struct gfs2_ea_header *ea_hdr, - struct gfs2_ea_header *ea_hdr_prev, - void *private) -{ - uint64_t block = be64_to_cpu(*ea_data_ptr); - - return clear_dup_metalist(ip, block, NULL, 0, private); -} - -/* Finds all references to duplicate blocks in the metadata */ -static int find_block_ref(struct gfs2_sbd *sdp, uint64_t inode) -{ - struct gfs2_inode *ip; - int error = 0; - - ip = fsck_load_inode(sdp, inode); /* bread, inode_get */ - /* double-check the meta header just to be sure it's metadata */ - if (ip->i_di.di_header.mh_magic != GFS2_MAGIC || - ip->i_di.di_header.mh_type != GFS2_METATYPE_DI) { - log_debug( _("Block %lld (0x%llx) is not gfs2 metadata.\n"), - (unsigned long long)inode, - (unsigned long long)inode); - fsck_inode_put(&ip); - return 1; - } - /* Check to see if this inode was referenced by another by mistake */ - add_duplicate_ref(ip, inode, ref_is_inode, 1, INODE_VALID); - - /* Check this dinode's metadata for references to known duplicates */ - error = check_metatree(ip, &find_refs); - if (error < 0) { - stack; - fsck_inode_put(&ip); /* out, brelse, free */ - return error; - } - - /* Exhash dir leafs will be checked by check_metatree (right after - the "end:" label.) But if this is a linear directory we need to - check the dir with check_linear_dir. */ - if (is_dir(&ip->i_di, sdp->gfs1) && - !(ip->i_di.di_flags & GFS2_DIF_EXHASH)) - error = check_linear_dir(ip, ip->i_bh, &find_dirents); - - /* Check for ea references in the inode */ - if (!error) - error = check_inode_eattr(ip, &find_refs); - - fsck_inode_put(&ip); /* out, brelse, free */ - - return error; -} - -/* get_ref_type - figure out if all duplicate references from this inode - are the same type, and if so, return the type. */ -static enum dup_ref_type get_ref_type(struct inode_with_dups *id) -{ - enum dup_ref_type t, i; - int found_type_with_ref; - int found_other_types; - - for (t = ref_as_data; t < ref_types; t++) { - found_type_with_ref = 0; - found_other_types = 0; - for (i = ref_as_data; i < ref_types; i++) { - if (id->reftypecount[i]) { - if (t == i) - found_type_with_ref = 1; - else - found_other_types = 1; - } - } - if (found_type_with_ref) - return found_other_types ? ref_types : t; - } - return ref_types; -} - static void log_inode_reference(struct duptree *dt, osi_list_t *tmp, int inval) { char reftypestring[32]; @@ -426,12 +50,74 @@ static void log_inode_reference(struct duptree *dt, osi_list_t *tmp, int inval) (unsigned long long)dt->block, (unsigned long long)dt->block, reftypestring); } + +/* delete_all_dups - delete all duplicate records for a given inode */ +static void delete_all_dups(struct gfs2_inode *ip) +{ + struct osi_node *n, *next; + struct duptree *dt; + osi_list_t *tmp, *x; + struct inode_with_dups *id; + int found; + + for (n = osi_first(&dup_blocks); n; n = next) { + next = osi_next(n); + dt = (struct duptree *)n; + + found = 0; + id = NULL; + + osi_list_foreach_safe(tmp, &dt->ref_invinode_list, x) { + id = osi_list_entry(tmp, struct inode_with_dups, list); + if (id->block_no == ip->i_di.di_num.no_addr) { + dup_listent_delete(dt, id); + found = 1; + } + } + osi_list_foreach_safe(tmp, &dt->ref_inode_list, x) { + id = osi_list_entry(tmp, struct inode_with_dups, list); + if (id->block_no == ip->i_di.di_num.no_addr) { + dup_listent_delete(dt, id); + found = 1; + } + } + if (!found) + continue; + + if (dt->refs == 0) { + log_debug(_("This was the last reference: 0x%llx is " + "no longer a duplicate.\n"), + (unsigned long long)dt->block); + dup_delete(dt); /* not duplicate now */ + } else { + log_debug(_("%d references remain to 0x%llx\n"), + dt->refs, (unsigned long long)dt->block); + if (dt->refs > 1) + continue; + + id = NULL; + osi_list_foreach(tmp, &dt->ref_invinode_list) + id = osi_list_entry(tmp, + struct inode_with_dups, + list); + osi_list_foreach(tmp, &dt->ref_inode_list) + id = osi_list_entry(tmp, + struct inode_with_dups, + list); + if (id) + log_debug("Last reference is from inode " + "0x%llx\n", + (unsigned long long)id->block_no); + } + } +} + /* * resolve_dup_references - resolve all but the last dinode that has a * duplicate reference to a given block. * * @sdp - pointer to the superblock structure - * @b - pointer to the duplicate reference rbtree to use + * @dt - pointer to the duplicate reference rbtree to use * @ref_list - list of duplicate references to be resolved (invalid or valid) * @dh - duplicate handler * inval - The references on this ref_list are invalid. We prefer to delete @@ -439,40 +125,42 @@ static void log_inode_reference(struct duptree *dt, 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 *dt, - osi_list_t *ref_list, struct dup_handler *dh, - int inval, int acceptable_ref) +static void resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt, + osi_list_t *ref_list, + struct dup_handler *dh, + int inval, int acceptable_ref) { struct gfs2_inode *ip; struct inode_with_dups *id; osi_list_t *tmp, *x; - struct metawalk_fxns clear_dup_fxns = { + struct metawalk_fxns pass1b_fxns_delete = { .private = NULL, - .check_leaf = clear_leaf, - .check_metalist = clear_dup_metalist, - .check_data = clear_dup_data, - .check_eattr_indir = clear_dup_eattr_indir, - .check_eattr_leaf = clear_dup_eattr_leaf, - .check_dentry = NULL, - .check_eattr_entry = clear_eattr_entry, - .check_eattr_extentry = clear_eattr_extentry, + .check_metalist = delete_metadata, + .check_data = delete_data, + .check_leaf = delete_leaf, + .check_eattr_indir = delete_eattr_indir, + .check_eattr_leaf = delete_eattr_leaf, + .check_eattr_entry = delete_eattr_entry, + .check_eattr_extentry = delete_eattr_extentry, }; enum dup_ref_type this_ref; struct inode_info *ii; int found_good_ref = 0; + uint64_t dup_block; + uint8_t q; osi_list_foreach_safe(tmp, ref_list, x) { if (skip_this_pass || fsck_abort) - return FSCK_OK; + return; id = osi_list_entry(tmp, struct inode_with_dups, list); dh->dt = dt; - dh->id = id; if (dh->ref_inode_count == 1) /* down to the last reference */ - return 1; + return; this_ref = get_ref_type(id); + q = block_type(id->block_no); if (inval) log_warn( _("Invalid ")); /* FIXME: If we already found an acceptable reference to this @@ -484,11 +172,8 @@ static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt, type and */ this_ref == acceptable_ref && /* this ref is acceptable */ !found_good_ref) { /* We haven't found a good reference */ - uint8_t q; - /* If this is an invalid inode, but not on the invalid list, it's better to delete it. */ - q = block_type(id->block_no); if (q != gfs2_inode_invalid) { found_good_ref = 1; log_warn( _("Inode %s (%lld/0x%llx)'s " @@ -526,69 +211,124 @@ static int resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt, (unsigned long long)id->block_no))) { log_warn( _("The bad inode was not cleared.")); /* delete the list entry so we don't leak memory but - leave the reference count. If the decrement the + leave the reference count. If we decrement the ref count, we could get down to 1 and the dinode would be changed without a 'Yes' answer. */ /* (dh->ref_inode_count)--;*/ - dup_listent_delete(id); + dup_listent_delete(dt, id); continue; } - log_warn( _("Clearing inode %lld (0x%llx)...\n"), - (unsigned long long)id->block_no, - (unsigned long long)id->block_no); - + if (q == gfs2_block_free) + log_warn( _("Inode %lld (0x%llx) was previously " + "deleted.\n"), + (unsigned long long)id->block_no, + (unsigned long long)id->block_no); + else + log_warn(_("Pass1b is deleting inode %lld (0x%llx).\n"), + (unsigned long long)id->block_no, + (unsigned long long)id->block_no); + + dup_block = id->block_no; ip = fsck_load_inode(sdp, id->block_no); - 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); - /* If the dup was in data or metadata, clear the dinode */ - if (id->reftypecount[ref_as_data] || - id->reftypecount[ref_as_meta]) { - check_metatree(ip, &clear_dup_fxns); - fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, - _("duplicate referencing bad"), - gfs2_inode_invalid); + /* If we've already deleted this dinode, don't try to delete + it again. That could free blocks that used to be duplicate + references that are now resolved (and gone). */ + if (q != gfs2_block_free) { + /* Clear the EAs for the inode first */ + check_inode_eattr(ip, &pass1b_fxns_delete); + /* If the reference was as metadata or data, we've got + a corrupt dinode that will be deleted. */ + if (inval || id->reftypecount[ref_as_data] || + id->reftypecount[ref_as_meta]) { + /* Remove the inode from the inode tree */ + ii = inodetree_find(ip->i_di.di_num.no_addr); + if (ii) + inodetree_delete(ii); + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("duplicate referencing bad"), + gfs2_inode_invalid); + /* We delete the dup_handler inode count and + duplicate id BEFORE clearing the metadata, + because if this is the last reference to + this metadata block, we need to traverse the + tree and free the data blocks it references. + However, we don't want to delete other + duplicates that may be used by other + dinodes. */ + (dh->ref_inode_count)--; + /* FIXME: other option should be to duplicate + the block for each duplicate and point the + metadata at the cloned blocks */ + check_metatree(ip, &pass1b_fxns_delete); + } } + /* Now we've got to go through an delete any other duplicate + references from this dinode we're deleting. If we don't, + pass1b will discover the other duplicate record, try to + delete this dinode a second time, and this time its earlier + duplicate references won't be seen as duplicates anymore + (because they were eliminated earlier in pass1b). And so + the blocks will be mistakenly freed, when, in fact, they're + still being referenced by a valid dinode. */ + delete_all_dups(ip); fsck_inode_put(&ip); /* out, brelse, free */ - (dh->ref_inode_count)--; - /* FIXME: other option should be to duplicate the - * block for each duplicate and point the metadata at - * the cloned blocks */ - dup_listent_delete(id); } - if (dh->ref_inode_count == 1) /* down to the last reference */ - return 1; - return 0; + return; } -static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *dt) +/* 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) { - struct gfs2_inode *ip; osi_list_t *tmp; + struct duptree *dt; struct inode_with_dups *id; - struct dup_handler dh = {0}; - int last_reference = 0; - struct gfs2_buffer_head *bh; - uint32_t cmagic, ctype; - enum dup_ref_type acceptable_ref; + 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; + 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; + dh->ref_inode_count++; + dh->ref_count += id->dup_count; } +} + +/* handle_dup_blk - handle a duplicate block reference. + * + * This function should resolve and delete the duplicate block reference given, + * iow dt. + */ +static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *dt) +{ + osi_list_t *tmp; + struct gfs2_inode *ip; + struct inode_with_dups *id; + struct dup_handler dh = {0}; + struct gfs2_buffer_head *bh; + uint32_t cmagic, ctype; + enum dup_ref_type acceptable_ref; + uint64_t dup_blk; + + dup_blk = dt->block; + revise_dup_handler(dup_blk, &dh); /* Log the duplicate references */ log_notice( _("Block %llu (0x%llx) has %d inodes referencing it" @@ -642,77 +382,67 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *dt) invalidated for other reasons, such as bad pointers. So we need to make sure at this point that any inode deletes reverse out any duplicate reference before we get to this point. */ - if (dh.ref_count == 1) - last_reference = 1; /* Step 1 - eliminate references from inodes that are not valid. * This may be because they were deleted due to corruption. * All block types are unacceptable, so we use ref_types. */ - if (!last_reference) { + if (dh.ref_count > 1) { log_debug( _("----------------------------------------------\n" "Step 1: Eliminate references to block %llu " "(0x%llx) that were previously marked " "invalid.\n"), (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); + resolve_dup_references(sdp, dt, &dt->ref_invinode_list, + &dh, 1, ref_types); + revise_dup_handler(dup_blk, &dh); } /* Step 2 - eliminate reference from inodes that reference it as the * wrong type. For example, a data file referencing it as * a data block, but it's really a metadata block. Or a * directory inode referencing a data block as a leaf block. */ - if (!last_reference) { + if (dh.ref_count > 1) { log_debug( _("----------------------------------------------\n" "Step 2: Eliminate references to block %llu " "(0x%llx) that need the wrong block type.\n"), (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); + resolve_dup_references(sdp, dt, &dt->ref_inode_list, &dh, 0, + acceptable_ref); + revise_dup_handler(dup_blk, &dh); } /* Step 3 - We have multiple dinodes referencing it as the correct * type. Just blast one of them. * All block types are fair game, so we use ref_types. */ - if (!last_reference) { + if (dh.ref_count > 1) { log_debug( _("----------------------------------------------\n" "Step 3: Choose one reference to block %llu " "(0x%llx) to keep.\n"), (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(&dt->ref_inode_list)) { - log_notice( _("Block %llu (0x%llx) has no more references; " - "Marking as 'free'.\n"), - (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; + resolve_dup_references(sdp, dt, &dt->ref_inode_list, &dh, 0, + ref_types); + revise_dup_handler(dup_blk, &dh); } - if (last_reference) { + /* If there's still a last remaining reference, and it's a valid + reference, use it to determine the correct block type for our + blockmap and bitmap. */ + if (dh.ref_count == 1 && !osi_list_empty(&dt->ref_inode_list)) { uint8_t q; log_notice( _("Block %llu (0x%llx) has only one remaining " - "reference.\n"), - (unsigned long long)dt->block, - (unsigned long long)dt->block); + "valid reference.\n"), + (unsigned long long)dup_blk, + (unsigned long long)dup_blk); /* 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 = (&dt->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 " @@ -753,13 +483,147 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *dt) "attribute"), gfs2_meta_eattr); fsck_inode_put(&ip); /* out, brelse, free */ + log_debug(_("Done with duplicate reference to block 0x%llx\n"), + (unsigned long long)dt->block); + dup_delete(dt); } else { /* They may have answered no and not fixed all references. */ - log_debug( _("All duplicate references were processed.\n")); + log_debug( _("All duplicate references to block 0x%llx were " + "processed.\n"), (unsigned long long)dup_blk); + if (dh.ref_count) { + log_debug(_("Done with duplicate reference to block " + "0x%llx, but %d references remain.\n"), + (unsigned long long)dup_blk, dh.ref_count); + } else { + log_notice( _("Block %llu (0x%llx) has no more " + "references; Marking as 'free'.\n"), + (unsigned long long)dup_blk, + (unsigned long long)dup_blk); + if (dh.dt) + dup_delete(dh.dt); + /* Now fix the block type of the block in question. */ + gfs2_blockmap_set(bl, dup_blk, gfs2_block_free); + check_n_fix_bitmap(sdp, dup_blk, gfs2_block_free); + } } return 0; } +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); +} + +static int check_metalist_refs(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, + void *private) +{ + *was_duplicate = 0; + *is_valid = 1; + return add_duplicate_ref(ip, block, ref_as_meta, 1, INODE_VALID); +} + +static int check_data_refs(struct gfs2_inode *ip, uint64_t metablock, + uint64_t block, void *private) +{ + return add_duplicate_ref(ip, block, ref_as_data, 1, INODE_VALID); +} + +static int check_eattr_indir_refs(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, + struct gfs2_buffer_head **bh, void *private) +{ + struct gfs2_sbd *sdp = ip->i_sbd; + int error; + + error = add_duplicate_ref(ip, block, ref_as_ea, 1, INODE_VALID); + if (!error) + *bh = bread(sdp, block); + + return error; +} + +static int check_eattr_leaf_refs(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, struct gfs2_buffer_head **bh, + void *private) +{ + struct gfs2_sbd *sdp = ip->i_sbd; + int error; + + error = add_duplicate_ref(ip, block, ref_as_ea, 1, INODE_VALID); + if (!error) + *bh = bread(sdp, block); + return error; +} + +static int check_eattr_entry_refs(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) +{ + return 0; +} + +static int check_eattr_extentry_refs(struct gfs2_inode *ip, + uint64_t *ea_data_ptr, + struct gfs2_buffer_head *leaf_bh, + struct gfs2_ea_header *ea_hdr, + struct gfs2_ea_header *ea_hdr_prev, + void *private) +{ + uint64_t block = be64_to_cpu(*ea_data_ptr); + + return add_duplicate_ref(ip, block, ref_as_ea, 1, INODE_VALID); +} + +/* Finds all references to duplicate blocks in the metadata */ +/* Finds all references to duplicate blocks in the metadata */ +static int find_block_ref(struct gfs2_sbd *sdp, uint64_t inode) +{ + struct gfs2_inode *ip; + int error = 0; + struct metawalk_fxns find_refs = { + .private = NULL, + .check_leaf = check_leaf_refs, + .check_metalist = check_metalist_refs, + .check_data = check_data_refs, + .check_eattr_indir = check_eattr_indir_refs, + .check_eattr_leaf = check_eattr_leaf_refs, + .check_eattr_entry = check_eattr_entry_refs, + .check_eattr_extentry = check_eattr_extentry_refs, + }; + + ip = fsck_load_inode(sdp, inode); /* bread, inode_get */ + + /* double-check the meta header just to be sure it's metadata */ + if (ip->i_di.di_header.mh_magic != GFS2_MAGIC || + ip->i_di.di_header.mh_type != GFS2_METATYPE_DI) { + log_debug( _("Block %lld (0x%llx) is not gfs2 metadata.\n"), + (unsigned long long)inode, + (unsigned long long)inode); + error = 1; + goto out; + } + /* Check to see if this inode was referenced by another by mistake */ + add_duplicate_ref(ip, inode, ref_is_inode, 1, INODE_VALID); + + /* Check this dinode's metadata for references to known duplicates */ + error = check_metatree(ip, &find_refs); + if (error < 0) + stack; + + /* Check for ea references in the inode */ + if (!error) + error = check_inode_eattr(ip, &find_refs); + +out: + fsck_inode_put(&ip); /* out, brelse, free */ + return error; +} + /* Pass 1b handles finding the previous inode for a duplicate block * When found, store the inodes pointing to the duplicate block for * use in pass2 */ @@ -768,7 +632,7 @@ int pass1b(struct gfs2_sbd *sdp) struct duptree *dt; uint64_t i; uint8_t q; - struct osi_node *n, *next = NULL; + struct osi_node *n; int rc = FSCK_OK; log_info( _("Looking for duplicate blocks...\n")); @@ -819,17 +683,11 @@ int pass1b(struct gfs2_sbd *sdp) * it later */ log_info( _("Handling duplicate blocks\n")); out: - for (n = osi_first(&dup_blocks); n; n = next) { - next = osi_next(n); + /* Resolve all duplicates by clearing out the dup tree */ + while ((n = osi_first(&dup_blocks))) { dt = (struct duptree *)n; if (!skip_this_pass && !rc) /* no error & not asked to skip the rest */ 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. - This is especially true for pass2 where it may delete "bad" - inodes, and we can't delete an inode's indirect block if - it was a duplicate (therefore in use by another dinode). */ } return rc; } diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 77718be..b79e999 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -169,59 +169,6 @@ static int check_file_type(uint8_t de_type, uint8_t blk_type, int gfs1) return 0; } -static int delete_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]; - - if (!ea_hdr->ea_name_len){ - /* Skip this entry for now */ - return 1; - } - - 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))){ - /* Skip invalid entry */ - return 1; - } - - 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) - return 1; - else { - log_debug( _(" Pointers Required: %d\n Pointers Reported: %d\n"), - max_ptrs, ea_hdr->ea_num_ptrs); - } - } - return 0; -} - -static int delete_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, - struct gfs2_buffer_head *leaf_bh, - struct gfs2_ea_header *ea_hdr, - struct gfs2_ea_header *ea_hdr_prev, - void *private) -{ - uint64_t block = be64_to_cpu(*ea_data_ptr); - - return delete_metadata(ip, block, NULL, 0, private); -} - struct metawalk_fxns pass2_fxns_delete = { .private = NULL, .check_metalist = delete_metadata, @@ -1836,12 +1783,5 @@ int pass2(struct gfs2_sbd *sdp) } fsck_inode_put(&ip); /* does a gfs2_dinode_out, brelse */ } - /* Now that we've deleted the inodes marked "bad" we can safely - get rid of the duplicate block list. If we do it any sooner, - we won't discover that a given block is a duplicate and avoid - deleting it from both inodes referencing it. Note: The other - returns from this function are premature exits of the program - and gfs2_block_list_destroy should get rid of the list for us. */ - gfs2_dup_free(); return FSCK_OK; } diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index ef59e6e..9d6f163 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -466,8 +466,39 @@ struct dir_info *dirtree_find(uint64_t block) return NULL; } -void dup_listent_delete(struct inode_with_dups *id) +/* get_ref_type - figure out if all duplicate references from this inode + are the same type, and if so, return the type. */ +enum dup_ref_type get_ref_type(struct inode_with_dups *id) { + enum dup_ref_type t, i; + int found_type_with_ref; + int found_other_types; + + for (t = ref_as_data; t < ref_types; t++) { + found_type_with_ref = 0; + found_other_types = 0; + for (i = ref_as_data; i < ref_types; i++) { + if (id->reftypecount[i]) { + if (t == i) + found_type_with_ref = 1; + else + found_other_types = 1; + } + } + if (found_type_with_ref) + return found_other_types ? ref_types : t; + } + return ref_types; +} + +void dup_listent_delete(struct duptree *dt, struct inode_with_dups *id) +{ + log_err( _("Removing duplicate reference to block %llu (0x%llx) " + "referenced as %s by dinode %llu (0x%llx)\n"), + (unsigned long long)dt->block, (unsigned long long)dt->block, + reftypes[get_ref_type(id)], (unsigned long long)id->block_no, + (unsigned long long)id->block_no); + dt->refs--; /* one less reference */ if (id->name) free(id->name); osi_list_del(&id->list); @@ -482,12 +513,12 @@ void dup_delete(struct duptree *dt) 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); + dup_listent_delete(dt, id); } 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); + dup_listent_delete(dt, id); } osi_erase(&dt->node, &dup_blocks); free(dt); diff --git a/gfs2/fsck/util.h b/gfs2/fsck/util.h index 00c2239..361b1a2 100644 --- a/gfs2/fsck/util.h +++ b/gfs2/fsck/util.h @@ -19,7 +19,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, enum dup_ref_type reftype, int first, int inode_valid); extern struct inode_with_dups *find_dup_ref_inode(struct duptree *dt, struct gfs2_inode *ip); -extern void dup_listent_delete(struct inode_with_dups *id); +extern void dup_listent_delete(struct duptree *dt, struct inode_with_dups *id); extern const char *reftypes[ref_types + 1]; @@ -174,6 +174,7 @@ static inline uint32_t gfs_to_gfs2_mode(struct gfs2_inode *ip) } } +extern enum dup_ref_type get_ref_type(struct inode_with_dups *id); extern struct gfs2_bmap *gfs2_bmap_create(struct gfs2_sbd *sdp, uint64_t size, uint64_t *addl_mem_needed); extern void *gfs2_bmap_destroy(struct gfs2_sbd *sdp, struct gfs2_bmap *il);

1 0

gfs2-utils: master - fsck.gfs2: When flagging a duplicate reference, show valid or invalid
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=390b7b6637… Commit: 390b7b663762b32a68fff13c214ec6361f63f3bf Parent: 37480679e413e503de72d873021ce2c808f0d8cc Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Apr 3 10:03:49 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: When flagging a duplicate reference, show valid or invalid This patch changes the logging when duplicate block references are flagged. The idea is to print whether or not the inode with the reference is valid or invalid, which helps in diagnosing problems when duplicate block references are resolved. --- gfs2/fsck/util.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index fc3a0ec..ef59e6e 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -399,9 +399,10 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, id->reftypecount[reftype]++; id->dup_count++; log_info( _("Found %d reference(s) to block %llu" - " (0x%llx) as %s in inode #%llu (0x%llx)\n"), + " (0x%llx) as %s in %s inode #%llu (0x%llx)\n"), id->dup_count, (unsigned long long)block, (unsigned long long)block, reftypes[reftype], + inode_valid ? _("valid") : _("invalid"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); if (first)

1 0

gfs2-utils: master - fsck.gfs2: check for duplicate first references
by Bob Peterson 20 May '13

20 May '13

Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=37480679e4… Commit: 37480679e413e503de72d873021ce2c808f0d8cc Parent: 5c93311761c14819ed20087d23fe23f9a9ab2c21 Author: Bob Peterson <rpeterso(a)redhat.com> AuthorDate: Wed Apr 3 09:47:31 2013 -0700 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Mon May 20 11:12:47 2013 -0500 fsck.gfs2: check for duplicate first references Before this patch, fsck.gfs2 could get into situations where it's in pass1b searching for the first reference to a block that it knows has been referenced twice. However, for one reason or another, the first reference has been deleted. It may seem unlikely because pass1 tries to "undo" its references when it deletes a bad dinode. But it can still happen, for example, when pass1b decides to delete a dinode because of a _different_ duplicate reference within the same dinode. If the first reference was deleted prior to searching for the original reference, pass1b won't find the original reference. So prior to this patch, it would just keep on looking, until it found the second reference. In other words, it would mistake the second reference for the first reference. Then it would get confused and treat the reference as a duplicate of itself. Later, it would choose which reference to delete, and delete its dinode. But since they're the same reference, it could delete a dinode with a perfectly good reference (the first invalid reference having already been deleted). The solution that this patch implements is to check if the first reference we found is actually the second reference, and if so, treat it as a first reference. That way, it avoids creating a second duplicate reference structure, and later when it resolves the references, it finds there's only one, and it doesn't need to delete the valid dinode. --- gfs2/fsck/util.c | 24 ++++++++++++++++++++++-- 1 files changed, 22 insertions(+), 2 deletions(-) diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index 078d5f6..fc3a0ec 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -330,6 +330,28 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, if (dt->first_ref_found) return meta_is_good; + /* Check for a previous reference to this duplicate */ + id = find_dup_ref_inode(dt, ip); + + /* We have to be careful here. The original referencing dinode may have + deemed to be bad and deleted/freed in pass1. In that case, pass1b + wouldn't discover the correct [deleted] original reference. In + that case, we don't want to be confused and consider this second + reference the same as the first. If we do, we'll never be able to + resolve it. The first reference can't be the second reference. */ + if (id && first && !dt->first_ref_found) { + log_info(_("Original reference to block %llu (0x%llx) was " + "previously found to be bad and deleted.\n"), + (unsigned long long)block, + (unsigned long long)block); + log_info(_("I'll consider the reference from inode %llu " + "(0x%llx) the first reference.\n"), + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + dt->first_ref_found = 1; + 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 reference, we don't want to increment the reference count because @@ -341,8 +363,6 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, dt->refs++; } - /* Check for a previous reference to this duplicate */ - id = find_dup_ref_inode(dt, ip); if (id == NULL) { /* Check for the inode on the invalid inode reference list. */ uint8_t q;

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

cluster-commits May 2013