[Ocfs2-tools-devel] [PATCH 1/6] libocfs2: Make file truncate process return error in bad case.

[Tao Ma]

Sunil Mushran wrote:
> While this patch may work for this case, it may hurt us in other parts
> of the fsck. Especially code paths where we call ocfs2_free_clusters()
> because of an earlier error.
> 
> So instead if making it treat this condition as an error, we could add
> a conditional ocfs2_bitmap_test_generic() to test if all the bits we are
> clearing are indeed set. If not, then we will know to trigger a full fsck.
> 
> Thoughts?
Not sure of it. ocfs2_free_clusters and ocfs2_truncate are common APIs 
from libocfs2, so they really should return errors. I think it is fsck 
that should handle the return error gracefully.
I have gone through all the callers of ocfs2_free_clusters, thanks to 
god, fsck doesn't use it directly except my 2 following patches. As for 
ocfs2_truncate, there are only one place in mk_journal which may be run 
in fsck. If you really care this, we may remove the ret check in 
ocfs2_make_journal.

Regards,
Tao
> 
> Sunil
> 
> Tao Ma wrote:
>> ocfs2_truncate handles the issue of file truncate, but it doesn't have
>> a good error catch mechanism. So this patch try to add it.
>> 1. In ocfs2_free_clusters, it will call ocfs2_bitmap_clear_range_generic
>> to free the clusters to the system bitmap. The bad thing is that it never
>> return error when the bit is already freed in the map, so add the error
>> return in this function, so that ocfs2_free_clusters can use it to verify
>> whether the free process is successful or not.
>> 2. Add errcode in truncate_ctxt, so that it can carry back the real error
>> code from truncate_iterate.
>>
>> Signed-off-by: Tao Ma <tao.ma at oracle.com>
>> ---
>>  libocfs2/bitmap.c   |    8 ++++++--
>>  libocfs2/truncate.c |    7 +++++++
>>  2 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/libocfs2/bitmap.c b/libocfs2/bitmap.c
>> index a5fe511..f5e9669 100644
>> --- a/libocfs2/bitmap.c
>> +++ b/libocfs2/bitmap.c
>> @@ -740,13 +740,17 @@ errcode_t 
>> ocfs2_bitmap_clear_range_generic(ocfs2_bitmap *bitmap,
>>  {
>>      struct ocfs2_bitmap_region *br;
>>      uint64_t end;
>> +    int old_tmp;
>>  
>>      br = ocfs2_bitmap_lookup(bitmap, first_bit, len, NULL, NULL, NULL);
>>      if (!br)
>>          return OCFS2_ET_INVALID_BIT;
>>  
>> -    for (end = first_bit + len; first_bit < end; first_bit++)
>> -        clear_generic_shared(bitmap, br, first_bit);
>> +    for (end = first_bit + len; first_bit < end; first_bit++) {
>> +        old_tmp = clear_generic_shared(bitmap, br, first_bit);
>> +        if (!old_tmp)
>> +            return OCFS2_ET_INVALID_BIT;
>> +    }
>>  
>>      return 0;
>>  }
>> diff --git a/libocfs2/truncate.c b/libocfs2/truncate.c
>> index e67f3b7..cf64136 100644
>> --- a/libocfs2/truncate.c
>> +++ b/libocfs2/truncate.c
>> @@ -35,6 +35,7 @@
>>  struct truncate_ctxt {
>>      uint64_t new_size_in_clusters;
>>      uint32_t new_i_clusters;
>> +    errcode_t errcode;
>>  };
>>  
>>  /*
>> @@ -130,6 +131,8 @@ static int truncate_iterate(ocfs2_filesys *fs,
>>  bail:
>>      if (buf)
>>          ocfs2_free(&buf);
>> +    if (ret)
>> +        ctxt->errcode = ret;
>>      return func_ret;
>>  }
>>  
>> @@ -243,11 +246,15 @@ errcode_t 
>> ocfs2_zero_tail_and_truncate(ocfs2_filesys *fs,
>>      ctxt.new_i_clusters = ci->ci_inode->i_clusters;
>>      ctxt.new_size_in_clusters =
>>              ocfs2_clusters_in_blocks(fs, new_size_in_blocks);
>> +    ctxt.errcode = 0;
>>  
>>      ret = ocfs2_extent_iterate_inode(fs, ci->ci_inode,
>>                       OCFS2_EXTENT_FLAG_DEPTH_TRAVERSE,
>>                       NULL, truncate_iterate,
>>                       &ctxt);
>> +    if (!ret)
>> +        ret = ctxt.errcode;
>> +
>>      if (ret)
>>          goto out;
>>  
>>   
>