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

[Kevin Wolf <kwolf@redhat.com>]

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 <vsementsov@virtuozzo.com>
> > 
> > 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.

> 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