[Ocfs2-tools-devel] [PATCH 3/3] fswreck: Add corrupt codes for extent list check.

[tristan]

Tao Ma wrote:
>
>
> On 07/21/2010 05:14 PM, tristan wrote:
>> Tao Ma wrote:
>>> Signed-off-by: Tao Ma <tao.ma at oracle.com>
>>> ---
> <snip>
>>> +static void create_discontig_bg(ocfs2_filesys *fs,
>>> + uint16_t slotnum,
>>> + char *gd_buf)
>>> +{
>>> + char *buf = NULL;
>>> + errcode_t ret;
>>> + char sysfile[OCFS2_MAX_FILENAME_LEN];
>>> + struct ocfs2_super_block *sb = OCFS2_RAW_SB(fs->fs_super);
>>> + struct ocfs2_chain_list *cl;
>>> + struct ocfs2_dinode *di;
>>> + struct ocfs2_group_desc *gd;
>>> + struct ocfs2_chain_rec *rec;
>>> + uint32_t clusters;
>>> + uint64_t gd_blkno, di_blkno, old_blkno;
>>> + uint16_t chain;
>>> +
>>> + ret = ocfs2_malloc_block(fs->fs_io, &buf);
>>> + if (ret)
>>> + FSWRK_COM_FATAL(progname, ret);
>>> +
>>> + if (slotnum == UINT16_MAX)
>>> + slotnum = 0;
>>> +
>>> + snprintf(sysfile, sizeof(sysfile),
>>> + ocfs2_system_inodes[INODE_ALLOC_SYSTEM_INODE].si_name,
>>> + slotnum);
>>> +
>>> + ret = ocfs2_lookup(fs, sb->s_system_dir_blkno, sysfile,
>>> + strlen(sysfile), NULL, &di_blkno);
>>> + if (ret)
>>> + FSWRK_COM_FATAL(progname, ret);
>>> +
>>> + ret = ocfs2_read_inode(fs, di_blkno, buf);
>>> + if (ret)
>>> + FSWRK_COM_FATAL(progname, ret);
>>> +
>>> + di = (struct ocfs2_dinode *)buf;
>>> + cl = &di->id2.i_chain;
>>> +
>>> + chain = cl->cl_next_free_rec;
>>> + if (chain == cl->cl_count)
>>> + chain = 0;
>>> +
>>> + ret = ocfs2_new_clusters(fs, cl->cl_cpg, cl->cl_cpg,
>>> + &gd_blkno, &clusters);
>>
>> Hi tao,
>>
>> You are trying to create a contiguous block group here:) Actually we'll
>> still have a chance to proceed even if ocfs2_new_clusters failed to
>> allocate a contiguous region of clusters. how about using following
>> strategy:?
> yes, I allocate a contiguous cluster, but you can let them look like 
> discontiguous in the block group. ;) Look at create_discontig_bg_list. 
> It will insert divide these contiguous clusters into several parts and 
> add many extent records for them. It is quite easier, does it?
>>
>> /*
>> * we know there is 244 extent records in discontig-bg at most for sure
>> */
>> uint64_t clusters_region_blknos[244];
>> uint32_t wanted_clusters, left_clusters, cluster_regions = 0;
>> int i = 0;
>>
>> ret = ocfs2_malloc_block(&gd_blkno,...);
>>
>> wanted_clusters = cl->cl_cpg;
>> left_clusters = wanted_clusters;
>>
>> while (left_clusters) {
>>
>> ret = ocfs2_new_clusters(fs, wanted_clusters, 
>> &cluster_region_blknos[i]);
>> if (ret == -ENOMEM) {
>> if (allocsize == OCFS2_CLUSTERSIZE)
>> FSWRK_COM_FATAL(Programe, ret);
>>
>> wanted_clusters >>= 1;
>>
>> }
>>
>> left_clusters -= wanted_clusters;
>> i++;
>> }
>>
>> cluster_regions = i;
>>
>> /*
>> * Then we pass the array clusters_region_blknos[] and its size to
>> 'create_discontig_bg_list'
>> * so that it can fill its extent list by array members.
>> */
>>
>> That way, we're doing in a more 'discontig' way:)
> You just call ocfs2_new_clusters iterately, so the clusters you 
> allocated will also be physically contiguous, right? So what you do 
> here is just divide them into an array and then call 
> create_discontig_bg_list while my old method is allocate a big one and 
> then divide them in create_discontig_bg_list. ;)

Yes, calling ocfs2_new_clusters iterately may also allocate us a 
physically contiguous clusters region. while what if we're failed to 
allocate this in one time? then my method provides a chance to allocate 
a given number of clusters by scaling down the clusters we allocate in a 
time, which can incrementally add up to the number we expected:)

>
>>
>>> + if (ret || (clusters != cl->cl_cpg))
>>> + FSWRK_COM_FATAL(progname, ret);
>>> +
>>> + gd = (struct ocfs2_group_desc *)gd_buf;
>>> + ocfs2_init_group_desc(fs, gd, gd_blkno, 
>>> fs->fs_super->i_fs_generation,
>>> + di->i_blkno,
>>> + di->id2.i_chain.cl_cpg * di->id2.i_chain.cl_bpc,
>>> + chain, 1);
>>> +
>>> + create_discontig_bg_list(fs, gd, gd_blkno, clusters);
>>> +
>>> + rec = &di->id2.i_chain.cl_recs[chain];
>>> + old_blkno = rec->c_blkno;
>>> + gd->bg_next_group = old_blkno;
>>> +
>>> + ret = ocfs2_write_group_desc(fs, gd_blkno, gd_buf);
>>> + if (ret)
>>> + FSWRK_COM_FATAL(progname, ret);
>>
>> Suddenly, I found there may be a memory leak all around the fswreck
>> codes, if we exit there by FSWRK_COM_FATAL,
>> We've just got no chance to free the 'buf'.
> yes, fswreck is only a test program and FSWRK_COM_FATAL will just 
> error out. So please don't be much nervous about it. ;)
>
> Regards,
> Tao