Re: [Qemu-devel] [PATCH] block: don't probe zeroes in bs->file by default on block_status

[Vladimir Sementsov-Ogievskiy]

11.01.2019 15:21, Kevin Wolf wrote:
> Am 11.01.2019 um 12:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 11.01.2019 13:41, Kevin Wolf wrote:
>>> Am 10.01.2019 um 14:20 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>>> drv_co_block_status digs bs->file for additional, more accurate search
>>>> for hole inside region, reported as DATA by bs since 5daa74a6ebc.
>>>>
>>>> This accuracy is not free: assume we have qcow2 disk. Actually, qcow2
>>>> knows, where are holes and where is data. But every block_status
>>>> request calls lseek additionally. Assume a big disk, full of
>>>> data, in any iterative copying block job (or img convert) we'll call
>>>> lseek(HOLE) on every iteration, and each of these lseeks will have to
>>>> iterate through all metadata up to the end of file. It's obviously
>>>> ineffective behavior. And for many scenarios we don't need this lseek
>>>> at all.
>>>>
>>>> So, let's "5daa74a6ebc" by default, leaving an option to return
>>>> previous behavior, which is needed for scenarios with preallocated
>>>> images.
>>>>
>>>> Add iotest illustrating new option semantics.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>>
>>> I still think that an option isn't a good solution and we should try use
>>> some heuristics instead.
>>
>> Do you think that heuristics would be better than fair cache for lseek
>> results?
> 
> I don't think lseek() results are cachable, at least for images that can
> share BLK_PERM_WRITE between processes.
> 
>> I don't see good heuristics, neither want to implement lseek optimizations.
>> In our cases we don't need lseek under qcow2 at all, and it's obviously 
>> better
>> just don't lseek in these cases.
> 
> I suggested one: Pass large contiguous allocated ranges to the protocol
> driver, but just assume that the allocation status is correct in the
> format layer if they are small.

So, for fully allocated image, we will call lseek always?

> 
>> Moreover, as I understand, the cases when we need lseek are those when qcow2 
>> thinks
>> that the cluster is data, but actually it's a hole on fs... Isn't it better 
>> to
>> support this thing in qcow2? A kind of allocated-zeroes cluster? We have 
>> enough
>> reserved bits in cluster descriptor to add this feature. And than we don't 
>> need
>> this kind of cheating when user "better knows" where are holes than format 
>> layer
>> which owns the data..
> 
> The point of metadata preallocation is that you don't have to update the
> metadata when you write to the image. Having to clear a bit in the L2
> entry would negate that.
> 
> Maybe another option would be to use a compatible feature flag in qcow2
> that is set during creation with preallocation=metadata, and we would
> only go to the lower layer if the flag is set. The downside side is that
> this would only be effective for new images, so copying old preallocated
> images would be slowed down considerably.
> 
> Kevin
> 


-- 
Best regards,
Vladimir