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

[Tao Ma]

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. ;)

>
>> + 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