[Ocfs2-tools-devel] [PATCH] o2image: return error if pread64 failed in ocfs2_image_load_bitmap()

[Joseph Qi]

On 16/12/24 17:30, piaojun wrote:
> Hi Joseph,
>
> On 2016-12-23 17:38, Joseph Qi wrote:
>> Hi Jun,
>>
>>
>> On 16/12/21 17:33, piaojun wrote:
>>> In ocfs2_image_load_bitmap(), if pread64 failed, we need catch the
>>> errno and return errno to upper callers. otherwise we may get the
>>> incorrect 'arr_map' but 'o2image -I' execute successfully.
>>>
>>> Signed-off-by: Jun Piao <piaojun at huawei.com>
>>> ---
>>>    libocfs2/image.c | 15 ++++++++++-----
>>>    1 file changed, 10 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libocfs2/image.c b/libocfs2/image.c
>>> index fe95530..460616d 100644
>>> --- a/libocfs2/image.c
>>> +++ b/libocfs2/image.c
>>> @@ -163,7 +163,7 @@ errcode_t ocfs2_image_load_bitmap(ocfs2_filesys *ofs)
>>>        struct ocfs2_image_state *ost;
>>>        struct ocfs2_image_hdr *hdr;
>>>        uint64_t blk_off, bits_set;
>>> -    int count, i, j, fd;
>>> +    int count = 0, tmp, i, j, fd;
>>>        errcode_t ret;
>>>        char *blk;
>>>    @@ -216,10 +216,15 @@ errcode_t ocfs2_image_load_bitmap(ocfs2_filesys *ofs)
>>>             * we don't use io_read_block as ocfs2 image bitmap block size
>>>             * could be different from filesystem block size
>>>             */
>>> -        count = pread64(fd, ost->ost_bmparr[i].arr_map,
>>> -                ost->ost_bmpblksz, blk_off);
>>> -        if (count < ost->ost_bmpblksz)
>>> -            goto out;
>>> +        while (count < ost->ost_bmpblksz) {
>>> +            tmp = pread64(fd, ost->ost_bmparr[i].arr_map + count,
>>> +                ost->ost_bmpblksz - count, blk_off + count);
>>> +            if (tmp <= 0) {
>>> +                ret = OCFS2_ET_IO;
>>> +                goto out;
>>> +            }
>>> +            count += tmp;
>>> +        }
>> In which case we need loop read?
>> Or shall we just set the error code if read fails?
>>
>> Thanks,
>> Joseph
> The usage of pread64 refers to unix_io_read_block() in unix_io.c.
> I have not catch the case we need loop read but I think it is safer by
> loop reading in case of incomplete data.
>
> Thanks,
> Jun
If no real case, I'd prefer to keep it as simple as possible.

Thanks,
Joseph
>>>              /* add bits set in this bitmap */
>>>            for (j = 0; j < (ost->ost_bmpblksz * 8); j++)
>>
>>