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 <vsement...@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? > >> 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
