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 13:59 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 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 <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.
> 
> So, for fully allocated image, we will call lseek always?

If they are fully contiguous, yes. But that's a single lseek() call per
image then instead of an lseek() for every 64k, so not a big problem.

In the more realistic case, you will still call lseek() occasionally
because you will have some fragmentation, but the fragments can be
rather large. But it should still significantly reduce them compared to
now because you skip it for those parts with small contiguous
allocations where lseek() would be called a lot today.

Kevin