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

[tristan]

Tao Ma wrote:
> 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.

It makes sense, fswreck is not a regular tools get frequently used. our 
aim is not to make it ideally perfect but try to cover all corruptions.

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

While if the 'multi-allocation-attempt' didn't hurt anything, and make 
it more endurable in extremely fragmented env, why not bother adding 
several lines to achieve this:)

Tristan.

>
> Regards,
> Tao