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.