[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v5 1/8] block: Add bdrv_get_block_status_above
From: |
Fam Zheng |
Subject: |
Re: [Qemu-block] [PATCH v5 1/8] block: Add bdrv_get_block_status_above |
Date: |
Tue, 26 May 2015 17:53:41 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Tue, 05/26 11:22, Paolo Bonzini wrote:
>
>
> On 26/05/2015 05:36, Fam Zheng wrote:
> > Like bdrv_is_allocated_above, this function follows the backing chain until
> > seeing
> > BDRV_BLOCK_ALLOCATED. Base is not included.
> >
> > Reimplement bdrv_is_allocated on top.
> >
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> > block/io.c | 53
> > ++++++++++++++++++++++++++++++++++++++++-----------
> > include/block/block.h | 4 ++++
> > 2 files changed, 46 insertions(+), 11 deletions(-)
> >
> > diff --git a/block/io.c b/block/io.c
> > index e394d92..a0d9990 100644
> > --- a/block/io.c
> > +++ b/block/io.c
> > @@ -1560,28 +1560,51 @@ static int64_t coroutine_fn
> > bdrv_co_get_block_status(BlockDriverState *bs,
> > return ret;
> > }
> >
> > -/* Coroutine wrapper for bdrv_get_block_status() */
> > -static void coroutine_fn bdrv_get_block_status_co_entry(void *opaque)
> > +static int64_t coroutine_fn
> > bdrv_co_get_block_status_above(BlockDriverState *bs,
> > + BlockDriverState *base,
> > + int64_t sector_num,
> > + int nb_sectors,
> > + int *pnum)
> > +{
> > + BlockDriverState *p;
> > + int64_t ret;
> > +
> > + assert(bs != base);
> > + for (p = bs; p != base; p = p->backing_hd) {
> > + ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum);
>
> Since pnum is an output parameter only, *pnum will be set by the last
> call in the loop.
>
> This is not what bdrv_is_allocated_above does: you have to set *pnum
> (roughly) to the _smallest_ value returned by the calls. Consider this
> case (base == NULL, bs->backing_hd->backing_hd == NULL):
>
> 1 2 3
> 123456789012345678901234567890
> bs ...........AAAAAAAAAAAA.......
> bs->backing_hd ...............AAAAAAAAAAAAAAA
>
> Your code would return *pnum == 15, but the right result is *pnum == 11.
Yes, this is the case I missed! Thanks for explaining!
Fam
- [Qemu-block] [PATCH v5 0/8] block: Mirror discarded sectors, Fam Zheng, 2015/05/25
- [Qemu-block] [PATCH v5 1/8] block: Add bdrv_get_block_status_above, Fam Zheng, 2015/05/25
- [Qemu-block] [PATCH v5 2/8] qmp: Add optional bool "unmap" to drive-mirror, Fam Zheng, 2015/05/25
- [Qemu-block] [PATCH v5 3/8] mirror: Do zero write on target if sectors not allocated, Fam Zheng, 2015/05/25
- [Qemu-block] [PATCH v5 4/8] block: Fix dirty bitmap in bdrv_co_discard, Fam Zheng, 2015/05/25
- [Qemu-block] [PATCH v5 5/8] block: Remove bdrv_reset_dirty, Fam Zheng, 2015/05/25
- [Qemu-block] [PATCH v5 6/8] qemu-iotests: Make block job methods common, Fam Zheng, 2015/05/25
- [Qemu-block] [PATCH v5 7/8] qemu-iotests: Add test case for mirror with unmap, Fam Zheng, 2015/05/25
- [Qemu-block] [PATCH v5 8/8] iotests: Use event_wait in wait_ready, Fam Zheng, 2015/05/25