On Wed, 05/06 12:21, Paolo Bonzini wrote: > > > On 06/05/2015 11:50, Fam Zheng wrote: > > # src can_write_zeroes_with_unmap target > > can_write_zeroes_with_unmap > > -------------------------------------------------------------------------------- > > 1 true true > > 2 true false > > 3 false true > > 4 false false > > 1 should replicate WRITE SAME, in case the unmap granularity of the > target is different from that of the source. In that case, a discard on > the target might leave some sectors untouched. Writing zeroes would > ensure matching data between the source and the target. > > 2 should _not_ discard: it should write zeroes even at the cost of > making the target fully provisioned. Perhaps you can optimize it by > looking at bdrv_get_block_status for the target, and checking the answer > for BDRV_ZERO. > > 3 and 4 can use discard on the target. > > So it looks like only the source setting matters. > > We need to check the cost of bdrv_co_get_block_status for "raw", too. > If it's too expensive, that can be a problem.
In my test, bdrv_is_allocated_above() takes 10~20us on ext4 raw image on my laptop SATA. And also note that: /* * ... * * 'pnum' is set to the number of sectors (including and immediately following * the specified sector) that are known to be in the same * allocated/unallocated state. * * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes * beyond the end of the disk image it will be clamped. */ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, So we currently don't coalesce the sequential dirty sectors. Was that intentional? Fam > > Paolo > > > For case 2 & 3 it's probably better to mirror the actual reading of source. > > > > I'm not sure about 4. > > Even in case 1, discard could be "UNMAP" and write zeroes could be > "WRITE SAME". If the unmap granularity of the target is For unaligned > sectors, UNMAP might leave some sectors aside while WRITE SAME will > write with zeroes.