On Mon, 05/25 16:43, Paolo Bonzini wrote:
>
>
> On 22/05/2015 05:40, Fam Zheng wrote:
> > +{
> > + 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);
>
> It's a bit ugly to have different parameters for bdrv_get_block_status
> and bdrv_co_get_block_status.
I'll add bdrv_get_block_status_above and rewrite bdrv_get_block_status body on
top.
>
> > + if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) {
> > + break;
> > + }
> > + }
> > + return ret;
> > +}
> > +
>
> I think BDRV_BLOCK_ALLOCATED will always be true if passing NULL as the
> base, so patch 3 still needs a tweak.
OK, will use BDRV_BLOCK_DATA there.
>
> But, more important, you need extra logic to correct the output value of
> pnum, as in bdrv_is_allocated_above.
bdrv_co_get_block_status should fix pnum.
Fam
