[Ocfs2-tools-devel] [PATCH 04/12] fsck.ocfs2: Add checking of directory trailers.
Sunil Mushran
sunil.mushran at oracle.com
Tue Jan 6 17:57:07 PST 2009
One typo. Else looks good.
Joel Becker wrote:
> If we have trailers, fsck needs to know about them.
>
> In pass2, we don't treat the trailer of a directory block as a regular
> dirent. We fix the trailer's compatibility fields if they are broken.
>
> Signed-off-by: Joel Becker <joel.becker at oracle.com>
> ---
> fsck.ocfs2/fsck.ocfs2.checks.8.in | 36 ++++++++++++++++
> fsck.ocfs2/pass2.c | 83 +++++++++++++++++++++++++++++++++---
> fsck.ocfs2/pass3.c | 1 -
> 3 files changed, 112 insertions(+), 8 deletions(-)
>
> diff --git a/fsck.ocfs2/fsck.ocfs2.checks.8.in b/fsck.ocfs2/fsck.ocfs2.checks.8.in
> index 725b90c..22982c0 100644
> --- a/fsck.ocfs2/fsck.ocfs2.checks.8.in
> +++ b/fsck.ocfs2/fsck.ocfs2.checks.8.in
> @@ -598,6 +598,42 @@ Answering yes will try to repair the directory entry. This runs a very good
> chance of invalidating all the entries in the directory block. Orphaned
> inodes may appear in lost+found.
>
> +.SS "DIR_TRAILER_INODE"
> +A directory block trailer is a fake directory entry at the end of the
> +block. The trailer has compatibility fields for when it is viewed as a
> +directory entry. The inode field must be zero.
> +
> +Answering yes will set the inode field to zero.
> +
> +.SS "DIR_TRAILER_NAME_LEN"
> +A directory block trailer is a fake directory entry at the end of the
> +block. The trailer has compatibility fields for when it is viewed as a
> +directory entry. The name length field must be zero.
> +
> +Answering yes will set the name length field to zero.
> +
> +.SS "DIR_TRAILER_REC_LEN"
> +A directory block trailer is a fake directory entry at the end of the
> +block. The trailer has compatibility fields for when it is viewed as a
> +directory entry. The record length field must be equal to the size of
> +the trailer.
> +
> +Answering yes will set the record length field to the size of the trailer.
> +
> +.SS "DIR_TRAILER_BLKNO"
> +A directory block trailer is a fake directory entry at the end of the
> +block. The self-referential block number is incorrect.
> +
> +Answering yes will set the block number to the correct block on disk.
> +
> +.SS "DIR_TRAILER_PARENT_INODE"
> +A directory block trailer is a fake directory entry at the end of the
> +block. It has a pointer to the directory inode it belongs to. This
> +pointer is incorrect.
> +
> +Answering yes will set the parent inode pointer to the inode referencing
> +this directory block.
> +
> \" pass3.c
>
> .SS "ROOT_DIR_MISSING"
> diff --git a/fsck.ocfs2/pass2.c b/fsck.ocfs2/pass2.c
> index 8239c10..b3a522b 100644
> --- a/fsck.ocfs2/pass2.c
> +++ b/fsck.ocfs2/pass2.c
> @@ -193,6 +193,68 @@ out:
> return ret;
> }
>
> +/*
> + * The directory trailer has compatibility fields so it can be treated
> + * as an empty (deleted) dirent. We need to make sure those are correct.
> + */
> +static void fix_dir_trailer(o2fsck_state *ost, o2fsck_dirblock_entry *dbe,
> + struct ocfs2_dir_block_trailer *trailer,
> + unsigned int *flags)
> +{
> + if (trailer->db_compat_inode &&
> + prompt(ost, PY, PR_DIR_TRAILER_INODE,
> + "Directory block trailer for logical block %"PRIu64" "
> + "physcal block %"PRIu64" in directory inode %"PRIu64" "
> + "has a non-zero inode number. Clear it?",
> + dbe->e_blkcount, dbe->e_blkno, dbe->e_ino)) {
> + trailer->db_compat_inode = 0;
> + *flags |= OCFS2_DIRENT_CHANGED;
> + }
> +
> + if (trailer->db_compat_name_len &&
> + prompt(ost, PY, PR_DIR_TRAILER_NAME_LEN,
> + "Directory block trailer for logical block %"PRIu64" "
> + "physcal block %"PRIu64" in directory inode %"PRIu64" "
> + "has a non-zero name_len. Clear it?",
> + dbe->e_blkcount, dbe->e_blkno, dbe->e_ino)) {
> + trailer->db_compat_name_len = 0;
> + *flags |= OCFS2_DIRENT_CHANGED;
> + }
> +
> + if ((trailer->db_compat_rec_len !=
> + sizeof(struct ocfs2_dir_block_trailer)) &&
> + prompt(ost, PY, PR_DIR_TRAILER_REC_LEN,
> + "Directory block trailer for logical block %"PRIu64" "
> + "physcal block %"PRIu64" in directory inode %"PRIu64" "
> + "has an invalid rec_len. Fix it?",
> + dbe->e_blkcount, dbe->e_blkno, dbe->e_ino)) {
> + trailer->db_compat_rec_len = 0;
> + *flags |= OCFS2_DIRENT_CHANGED;
> + }
>
Shouldn't it be:
+ trailer->db_compat_rec_len = sizeof(struct ocfs2_dir_block_trailer);
> +
> + if ((trailer->db_blkno != dbe->e_blkno) &&
> + prompt(ost, PY, PR_DIR_TRAILER_BLKNO,
> + "Directory block trailer for logical block %"PRIu64" "
> + "physcal block %"PRIu64" in directory inode %"PRIu64" "
> + "has an invalid db_blkno of %"PRIu64". Fix it?",
> + dbe->e_blkcount, dbe->e_blkno, dbe->e_ino,
> + trailer->db_blkno)) {
> + trailer->db_blkno = dbe->e_blkno;
> + *flags |= OCFS2_DIRENT_CHANGED;
> + }
> +
> + if ((trailer->db_parent_dinode != dbe->e_ino) &&
> + prompt(ost, PY, PR_DIR_TRAILER_PARENT_INODE,
> + "Directory block trailer for logical block %"PRIu64" "
> + "physcal block %"PRIu64" in directory inode %"PRIu64" "
> + "claims it belongs to inoe %"PRIu64". Fix it?",
> + dbe->e_blkcount, dbe->e_blkno, dbe->e_ino,
> + trailer->db_parent_dinode)) {
> + trailer->db_parent_dinode = dbe->e_ino;
> + *flags |= OCFS2_DIRENT_CHANGED;
> + }
> +}
> +
> static int dirent_leaves_partial(struct ocfs2_dir_entry *dirent, int left)
> {
> left -= dirent->rec_len;
> @@ -612,7 +674,7 @@ static unsigned pass2_dir_block_iterate(o2fsck_dirblock_entry *dbe,
> {
> struct dirblock_data *dd = priv_data;
> struct ocfs2_dir_entry *dirent, *prev = NULL;
> - unsigned int offset = 0, ret_flags = 0;
> + unsigned int offset = 0, ret_flags = 0, end = dd->fs->fs_blocksize;
> struct ocfs2_dinode *di = (struct ocfs2_dinode *)dd->inoblock_buf;
> errcode_t ret = 0;
>
> @@ -663,9 +725,12 @@ static unsigned pass2_dir_block_iterate(o2fsck_dirblock_entry *dbe,
> dbe->e_blkno);
> goto out;
> }
> +
> + if (ocfs2_dir_has_trailer(dd->fs, di))
> + end = ocfs2_dir_trailer_blk_off(dd->fs);
> }
>
> - while (offset < dd->fs->fs_blocksize) {
> + while (offset < end) {
> dirent = (struct ocfs2_dir_entry *)(dd->dirblock_buf + offset);
>
> verbosef("checking dirent offset %d, rec_len %"PRIu16" "
> @@ -679,7 +744,7 @@ static unsigned pass2_dir_block_iterate(o2fsck_dirblock_entry *dbe,
> /* if we can't trust this dirent then fix it up or skip
> * the whole block */
> if (corrupt_dirent_lengths(dirent,
> - dd->fs->fs_blocksize - offset)) {
> + end - offset)) {
> if (!prompt(dd->ost, PY, PR_DIRENT_LENGTH,
> "Directory inode %"PRIu64" "
> "corrupted in logical block %"PRIu64" "
> @@ -691,8 +756,7 @@ static unsigned pass2_dir_block_iterate(o2fsck_dirblock_entry *dbe,
>
> /* we edit the dirent in place so we try to parse
> * it again after fixing it */
> - fix_dirent_lengths(dirent,
> - dd->fs->fs_blocksize - offset,
> + fix_dirent_lengths(dirent, end - offset,
> prev, &ret_flags);
> continue;
>
> @@ -708,8 +772,7 @@ static unsigned pass2_dir_block_iterate(o2fsck_dirblock_entry *dbe,
> * XXX should verify that ocfs2 reclaims entries like that.
> */
> ret = fix_dirent_dots(dd->ost, dbe, dirent, offset,
> - dd->fs->fs_blocksize - offset,
> - &ret_flags);
> + end - offset, &ret_flags);
> if (ret)
> goto out;
> if (dirent->inode == 0)
> @@ -752,6 +815,12 @@ next:
> prev = dirent;
> }
>
> + if (ocfs2_dir_has_trailer(dd->fs, di))
> + fix_dir_trailer(dd->ost, dbe,
> + ocfs2_dir_trailer_from_block(dd->fs,
> + dd->dirblock_buf),
> + &ret_flags);
> +
> if (ret_flags & OCFS2_DIRENT_CHANGED) {
> if (di->i_dyn_features & OCFS2_INLINE_DATA_FL) {
> memcpy(dd->inoblock_buf, dd->dirblock_buf,
> diff --git a/fsck.ocfs2/pass3.c b/fsck.ocfs2/pass3.c
> index 7c9d2bd..457f312 100644
> --- a/fsck.ocfs2/pass3.c
> +++ b/fsck.ocfs2/pass3.c
> @@ -280,7 +280,6 @@ void o2fsck_reconnect_file(o2fsck_state *ost, uint64_t inode)
> if (ret)
> goto out;
>
> - /* XXX I gotta say, ocfs2_link_and_expand() seems pretty reasonable */
> ret = ocfs2_link(ost->ost_fs, ost->ost_lostfound_ino, iname, inode,
> type);
> if (ret) {
>
More information about the Ocfs2-tools-devel
mailing list