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

[Tao Ma]

tristan wrote:
> 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:)
yes, it is a perfect solution for a user space program, but now it is 
fswreck which we will only want to create a test env and corrupt it.
So if we can't allocate a cpg at a time, the system is really fragmented 
and I am not sure whether other fswreck test codes can survive.
And IIRC, the biggest user of fswreck is 
ocfs2-test/programs/fsck-tests/fsck-test.sh, and it will reformat the 
volume and then call fswreck every time. So it is really hard for it to 
fail in allocating cpg.

Having said that, if you really worry about the allocation, I am fine 
with your solution and feel free to add a patch for it later.

Regards,
Tao