[Ocfs2-tools-devel] [PATCH] fsck.ocfs2: support fixing inode alloc group description
piaojun
piaojun at huawei.com
Wed Jan 17 00:54:39 PST 2018
Hi Eric,
On 2018/1/17 12:05, Eric Ren wrote:
> 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 "//".
>
This patch only considers 'global_inode_alloc' and 'inode_alloc' which
contains 'struct ocfs2_group_desc'
>> 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?
>
Yes.
>> + 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?
>
Agree, I will fix it in patch v2.
>> + "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?
>
Good catch, here we should set 1 for 'suballocator'.
>> + 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?
>
Yes, I will fix it in patch v2.
>> + "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:]$_].*[^:]$"
> """
>
Sounds good, I'm glad to have a try.
>>
>> + 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?
>
You are right, I will try to fix these wrong message.
thanks,
Jun
> Thanks,
> Eric
>
> .
>
More information about the Ocfs2-tools-devel
mailing list