On Mon, Dec 24, 2018 at 5:50 PM Vladimir Sementsov-Ogievskiy < vsement...@virtuozzo.com> wrote:
> Hi all! > > bdrv_co_block_status digs bs->file for additional, more accurate search > for hole > inside region, reported as DATA by bs since long-ago > > commit 5daa74a6ebce7543aaad178c4061dc087bb4c705 > Author: Paolo Bonzini <pbonz...@redhat.com> > Date: Wed Sep 4 19:00:38 2013 +0200 > > block: look for zero blocks in bs->file > > 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. Recently (and not the first time) we have faced the > situation, > when lseek works slow. Of course, it is more about kernel implementation > of it, > but why to involve lseek at all, if we have the information on qcow2 level? > 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. I'm thinking about, how to properly workaround this thing, and I have some > questions. > > What are the cases, when we really need digging for zeros in bs->file? > > Why in mirror we care about discard vs write_zeros, keeping in mind, that > block_status will return ZERO instead for unallocated regions in some > cases, not > related to guest view of disk (bdrv_unallocated_blocks_are_zero, backing > file is > smaller than request.start)? > > And finally, what to do with the problem? > > Obviously, the simplest thing is just revert 5daa74a. What will it break? > > Otherwise, I need to "revert" it in some cases, and, may be, it should be a > user-defined parameter.. Should it? And what are the cases? We need to > "free" > of 5daa74a at least qcow2.. But again, should it be format-specific > option, or > something more generic? > > Then, if it would be separate parameter, how should it interfere with > want_zeros, or even, should it be want_zeros, reworked a bit to be > want_lseek? > How about having 2 passes: 1. seek DATA/HOLE for entire image, keep result in memory 2. use the result when modifying ranges This can also help allocation of clusters in a destination image: 1. seek DATA/HOLE in source image 2. allocate blocks in destination 3. convert using out of order writes *without* causing fragmentation in the destination image. Here is an example using this approach when copying images to qem-nbd. This is a pretty simplistic single threaded implementation in python, using "qemu-img map" to find the data and zero ranges. It is faster than qemu-img convert :-) https://github.com/oVirt/ovirt-imageio/blob/master/examples/nbd-client https://github.com/oVirt/ovirt-imageio/commit/bec7cb677e33c6d0a7645c367af3d56b70b666db Nir