Re: [Qemu-devel] [PATCH] block/quorum: implement .bdrv_co_get_block_status

[Liu Yuan <namei.unix@gmail.com>]

On Fri, Jul 18, 2014 at 03:35:57PM +0200, Paolo Bonzini wrote:
> Il 17/07/2014 13:50, Liu Yuan ha scritto:
> > - allow drive-mirror to create sprase mirror on images like qcow2
> > - allow qemu-img map to work as expected on quorum driver
> > 
> > Cc: Benoit Canet <benoit@irqsave.net>
> > Cc: Kevin Wolf <kwolf@redhat.com>
> > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > Signed-off-by: Liu Yuan <namei.unix@gmail.com>
> > ---
> >  block/quorum.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/block/quorum.c b/block/quorum.c
> > index ebf5c71..f0d0a98 100644
> > --- a/block/quorum.c
> > +++ b/block/quorum.c
> > @@ -780,6 +780,21 @@ static coroutine_fn int quorum_co_flush(BlockDriverState *bs)
> >      return result;
> >  }
> >  
> > +static int64_t coroutine_fn quorum_co_get_block_status(BlockDriverState *bs,
> > +                                                       int64_t sector_num,
> > +                                                       int nb_sectors,
> > +                                                       int *pnum)
> > +{
> > +    BDRVQuorumState *s = bs->opaque;
> > +    BlockDriverState *child_bs = s->bs[0];
> > +
> > +    if (child_bs->drv->bdrv_co_get_block_status)
> > +        return child_bs->drv->bdrv_co_get_block_status(child_bs, sector_num,
> > +                                                       nb_sectors, pnum);
> 
> Is this "if" necessary?

Yes, otherwise bdrv_get_block_status() will be called multiple times and the
result for qcow2 won't return desired value im my test. Or we can simply call
bdrv_get_block_status() plus some tricks?

> 
> > +    return bdrv_get_block_status(child_bs, sector_num, nb_sectors, pnum);
> 
> Also, the definition of BDRV_BLOCK_OFFSET_VALID explicitly refers to
> bs->file, so you probably have to exclude it from the result as well as
> BDRV_BLOCK_RAW.
> 

Thanks for reminding.

Yuan