[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard |
Date: |
Mon, 11 May 2015 16:02:26 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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.
- [Qemu-devel] [PATCH 0/4] block: Mirror discarded sectors, Fam Zheng, 2015/05/05
- [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard, Fam Zheng, 2015/05/05
- Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard, Paolo Bonzini, 2015/05/05
- Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard, Fam Zheng, 2015/05/05
- Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard, Paolo Bonzini, 2015/05/06
- Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard, Fam Zheng, 2015/05/06
- Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard, Paolo Bonzini, 2015/05/06
- Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard,
Fam Zheng <=
- Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard, Paolo Bonzini, 2015/05/11
[Qemu-devel] [PATCH 2/4] block: Remove bdrv_reset_dirty, Fam Zheng, 2015/05/05
[Qemu-devel] [PATCH 3/4] qemu-iotests: Make block job methods common, Fam Zheng, 2015/05/05
[Qemu-devel] [PATCH 4/4] qemu-iotests: Add test case for mirror with unmap, Fam Zheng, 2015/05/05