Re: [Qemu-devel] callout to *file in bdrv_co_get_block_status

[Peter Lieven]

Am 17.03.2017 um 11:59 schrieb Paolo Bonzini:
>
> On 17/03/2017 11:45, Peter Lieven wrote:
>> Hi,
>>
>>
>> I tried to debug why qemu-img convert with a VMDK source laying on a tmpfs 
>> is horrible slow.
>>
>> For some reason a lseek on a tmpfs is slow. Strictly speaking the lseek in 
>> find_allocation in file-posix.c
>>
>> is slow.
>>
>>
>> When qemu-img convert iterates over all sectors of a VMDK file to check 
>> their allocation status it ends
>>
>> up checking allocation status of all allocated sectors due to the following 
>> condition in bdrv_co_get_block_status:
>>
>>
>>     if (*file && *file != bs &&
>>         (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
>>         (ret & BDRV_BLOCK_OFFSET_VALID)) {
>>         BlockDriverState *file2;
>>         int file_pnum;
>>         ret2 = bdrv_co_get_block_status(*file, ret >> BDRV_SECTOR_BITS,
>>                                         *pnum, &file_pnum, &file2);
>>         if (ret2 >= 0) {
>>             /* Ignore errors.  This is just providing extra information, it
>>              * is useful but not necessary.
>>              */
>>             if (!file_pnum) {
>>                 /* !file_pnum indicates an offset at or beyond the EOF; it is
>>                  * perfectly valid for the format block driver to point to 
>> such
>>                  * offsets, so catch it and mark everything as zero */
>>                 ret |= BDRV_BLOCK_ZERO;
>>             } else {
>>                 /* Limit request to the range reported by the protocol 
>> driver */
>>                 *pnum = file_pnum;
>>                 ret |= (ret2 & BDRV_BLOCK_ZERO);
>>             }
>>         }
>>     }
>>
>>
>> Does anybody remember for what circumstances this case this was added? In 
>> case of an container format
>>
>> like VMDK or QCOW2 shouldn't we trust the information from the l2 tables in 
>> the VMDK or QCOW2?
> It provides additional information, for example it works better with
> prealloc=metadata.

Okay, understood. Can you imagine of a away to conditionally avoid this second 
callout? In my case we have an additional
lseek for each cluster. For a 20GB file this are approx. 327k calls to lseek. 
And if the file has no preallocated metadata
it will likely not improve anything. And even if the metadata is prealloced 
what is the allocation status of the clusters?

Peter