Re: [Qemu-devel] [PATCH v4 09/20] parallels: Switch to .bdrv_co_block_status()

[Eric Blake]

On 11/30/2017 04:12 PM, Eric Blake wrote:

> > >       BDRVParallelsState *s = bs->opaque;
> > > -    int64_t offset;
> > > +    int count;
> > >
> > > +    assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
> > >       qemu_co_mutex_lock(&s->lock);
> > > -    offset = block_status(s, sector_num, nb_sectors, pnum);
> > > +    offset = block_status(s, offset >> BDRV_SECTOR_BITS,
> > > +                          bytes >> BDRV_SECTOR_BITS, &count);
> > >       qemu_co_mutex_unlock(&s->lock);
> > >
> > >       if (offset < 0) {
> >
> > pnum=count*BDRV_SECTOR_SIZE and map=0 setting missed here.
> >
> > >           return 0;
> > >       }
>
> Setting *map is only required when return value includes 
> BDRV_BLOCK_OFFSET_VALID, so that one was not necessary.  But you do 
> raise an interesting point about a pre-existing bug with pnum not being 
> set.  Commit 298a1665a guarantees that *pnum is 0 (and not uninitialized 
> garbage) - but that still violates the contract that we (want to) have 
> that all drivers either make progress or return an error (setting pnum 
> to 0 should only be done at EOF or on error).

Oh. The pre-existing code DID set pnum to a non-zero value, as a side 
effect of block_status(); the new code fails to do so.  So it is not 
pre-existing; good catch!

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org