[Ocfs2-tools-devel] [PATCH] fsck.ocfs2: support fixing inode alloc group description
Eric Ren
zren at suse.com
Tue Jan 16 20:05:01 PST 2018
Hi Jun,
On 01/09/2018 05:43 PM, piaojun wrote:
> when inode_alloc's gd is corrupted, we may reinitialize it and then set
> its bitmap by iterating all files of root dir.
A quick look at the change. It seems the patch also checks
"global_inode_alloc"
and files under system files directory "//".
> Signed-off-by: Jun Piao <piaojun at huawei.com>
> ---
> fsck.ocfs2/pass0.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 173 insertions(+)
>
> diff --git a/fsck.ocfs2/pass0.c b/fsck.ocfs2/pass0.c
> index bfd11fb..8202f49 100644
> --- a/fsck.ocfs2/pass0.c
> +++ b/fsck.ocfs2/pass0.c
> @@ -1308,6 +1308,175 @@ out:
> return ret;
> }
>
> +struct walk_path {
> + const char *argv0;
> + char *path;
> + ocfs2_filesys *fs;
> + struct ocfs2_group_desc *bgs;
> + int corrupted_bgs;
> +};
> +
> +static int set_bitmap_func(struct ocfs2_dir_entry *dentry,
> + uint64_t blocknr,
> + int offset,
> + int blocksize,
> + char *buf,
> + void *priv_data)
> +{
> + struct walk_path *wp = priv_data;
> + struct ocfs2_group_desc *bg;
> + __le64 inode = dentry->inode;
> + __le64 bg_blkno;
> + errcode_t ret;
> + int len;
> + int reti = 0;
> + int i = 0;
> + char *old_path, *path = NULL;
> +
> + if (!strncmp(dentry->name, ".", dentry->name_len) ||
> + !strncmp(dentry->name, "..", dentry->name_len))
> + return 0;
> +
> + ret = ocfs2_malloc0(PATH_MAX, &path);
> + if (ret) {
> + com_err(wp->argv0, ret,
> + "while allocating path memory in %s\n", wp->path);
> + return OCFS2_DIRENT_ABORT;
> + }
> +
> + len = strlen(wp->path);
> + memcpy(path, wp->path, len);
> + memcpy(path + len, dentry->name, dentry->name_len);
> + if (dentry->file_type == OCFS2_FT_DIR)
> + path[len + dentry->name_len] = '/';
> +
> + /* set group desc bitmap */
> + for (i = 0; i < wp->corrupted_bgs; i++) {
> + bg = &wp->bgs[i];
> + bg_blkno = bg->bg_blkno;
> + if (inode > bg_blkno && inode <= bg_blkno + bg->bg_bits) {
> + ocfs2_set_bit(inode - bg_blkno, bg->bg_bitmap);
> + bg->bg_free_bits_count--;
> + }
> + }
> +
> + if (dentry->file_type == OCFS2_FT_DIR) {
> + old_path = wp->path;
> + wp->path = path;
> + ret = ocfs2_dir_iterate(wp->fs, inode, 0, NULL,
> + set_bitmap_func, wp);
In order to express set_bitmap_func() is a iteration function, I am
thinking if "iterate_set_bitmap" is
a better name. Anyway, it doesn't matter.
> + if (ret) {
> + com_err(wp->argv0, ret, "while walking %s", wp->path);
> + reti = OCFS2_DIRENT_ABORT;
> + }
> + wp->path = old_path;
> + }
> +
> + ocfs2_free(&path);
> +
> + return reti;
> +}
> +
> +static errcode_t verify_group_desc(o2fsck_state *ost,
> + struct ocfs2_dinode *di, int type)
> +{
> + uint16_t bits;
> + uint64_t blkno;
> + errcode_t ret = 0;
> + int corrupted_bgs = 0, i;
> + struct ocfs2_chain_list *cl = &di->id2.i_chain;
> + struct ocfs2_chain_rec *rec;
> + struct ocfs2_group_desc *bgs = NULL;
> +
> + ret = ocfs2_malloc_blocks(ost->ost_fs->fs_io,
> + cl->cl_next_free_rec, &bgs);
> + if (ret) {
> + com_err(whoami, ret, "while allocating block groups");
While allocating block group descriptors?
> + goto out;
> + }
> + memset(bgs, 0, ost->ost_fs->fs_blocksize * cl->cl_next_free_rec);
> +
> + for (i = 0; i < cl->cl_next_free_rec; i++) {
> + rec = &cl->cl_recs[i];
> + blkno = rec->c_blkno;
> + bits = rec->c_total;
> +
> + ret = ocfs2_read_group_desc(ost->ost_fs, blkno,
> + (char *)&bgs[corrupted_bgs]);
> + if ((ret == OCFS2_ET_BAD_GROUP_DESC_MAGIC) ||
> + (!ret && bgs[corrupted_bgs].bg_generation != ost->ost_fs_generation)) {
> + if (!prompt(ost, PY, PR_GROUP_EXPECTED_DESC,
> + "Block %"PRIu64" should be a group "
> + "descriptor for the bitmap chain allocator but it "
> + "wasn't found in any chains. Reinitialize it as "
"wasn't found in any chains" - should we use "but it was corrupted."
instead?
> + "a group desc and link it into the bitmap "
> + "allocator?", blkno))
> + continue;
> + ocfs2_init_group_desc(ost->ost_fs,
> + &bgs[corrupted_bgs],
> + blkno, ost->ost_fs_generation,
> + di->i_blkno, bits, i, 0);
Hmm, regarding the last parameter of ocfs2_init_group_desc(...,
suballoc), is it correct to always
set 0 no matter it's global_inode_alloc or inode_alloc?
> + corrupted_bgs++;
> + } else if (ret) {
> + com_err(whoami, ret, "while reading a cluster bitmap "
The bitmap in inode alloc is block bitmap, not cluster bitmap, right?
> + "group descriptor from block %"PRIu64,
> + blkno);
> + }
> + }
> +
> + /* traverse all inodes, and set group desc bitmap */
> + if (corrupted_bgs) {
> + /* Walk root dir */
> + struct walk_path wp;
> + uint64_t root_blkno;
> + char *path = NULL;
> +
> + switch (type) {
> + case GLOBAL_INODE_ALLOC_SYSTEM_INODE:
> + path = "//";
> + root_blkno = ost->ost_fs->fs_sysdir_blkno;
> + break;
> + case INODE_ALLOC_SYSTEM_INODE:
> + path = "/";
> + root_blkno = ost->ost_fs->fs_root_blkno;
> + break;
> + default:
> + ret = OCFS2_ET_INTERNAL_FAILURE;
> + com_err(whoami, ret, "while verifying group desc");
> + goto out;
> + }
> +
> + wp.argv0 = whoami;
> + wp.path = path;
> + wp.fs = ost->ost_fs;
> + wp.bgs = bgs;
> + wp.corrupted_bgs = corrupted_bgs;
> + ret = ocfs2_dir_iterate(ost->ost_fs,
> + root_blkno, 0, NULL,
> + set_bitmap_func, &wp);
> + if (ret) {
> + com_err(whoami, ret, "while walking root dir");
> + goto out;
> + }
> + }
> +
> + /* write back fixed bgs */
> + for (i = 0; i < corrupted_bgs; i++) {
> + ret = ocfs2_write_group_desc(ost->ost_fs,
> + bgs[i].bg_blkno,
> + (char *)&bgs[i]);
> + if (ret) {
> + com_err(whoami, ret, "while writing a cluster group "
> + "descriptor at block %"PRIu64, blkno);
> + ost->ost_saw_error = 1;
> + }
> + }
> +
> +out:
> + ocfs2_free(&bgs);
> + return ret;
> +}
> +
> /* this returns an error if it didn't leave the allocators in a state that
> * the iterators will be able to work with. There is probably some room
> * for more resiliance here. */
> @@ -1483,6 +1652,10 @@ retry_bitmap:
> verbosef("Caching inode alloc failed, err %d\n",
> (int)ret);
The following config in .gitconfig may help generate a easy-to-read
patch always with
function name:
"""
[diff "default"]
xfuncname = "^[[:alpha:]$_].*[^:]$"
"""
>
> + ret = verify_group_desc(ost, di, type);
> + if (ret)
> + goto out;
> +
> ret = verify_chain_alloc(ost, di,
> blocks + ost->ost_fs->fs_blocksize,
> blocks +
Off topic, but while looking through the code, I find there are some
places where log message is wrong. Could help fix them in another patch?
You can search with "reading inode alloc inode " string, eg.
"""
1345 ret = ocfs2_lookup_system_inode(fs, GLOBAL_BITMAP_SYSTEM_INODE,
1346 0, &blkno);
1347 if (ret) {
1348 com_err(whoami, ret, "while looking up the global
bitmap "
1349 "inode");
1350 goto out;
1351 }
1352
1353 ret = ocfs2_read_inode(ost->ost_fs, blkno, (char *)di);
1354 if (ret) {
1355 com_err(whoami, ret, "reading inode alloc inode "
1356 "%"PRIu64" for verification", blkno);
1357 goto out;
1358 }
1359
1360 verbosef("found inode alloc %"PRIu64" at block %"PRIu64"\n",
1361 (uint64_t)di->i_blkno, blkno);
"""
The global_bitmap doesn't correspond to "inode alloc" inode, right?
Thanks,
Eric
More information about the Ocfs2-tools-devel
mailing list