Re: [Qemu-devel] [PATCH v8 11/15] vmdk: Return extent's file in bdrv_get_block_status

[Fam Zheng <famz@redhat.com>]

On Mon, 01/25 14:28, Kevin Wolf wrote:
> Am 25.01.2016 um 03:44 hat Fam Zheng geschrieben:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
> > ---
> >  block/vmdk.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index e1d3e27..f8f7fcf 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -1274,6 +1274,7 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
> >                               0, 0);
> >      qemu_co_mutex_unlock(&s->lock);
> >  
> > +    index_in_cluster = vmdk_find_index_in_cluster(extent, sector_num);
> >      switch (ret) {
> >      case VMDK_ERROR:
> >          ret = -EIO;
> > @@ -1286,14 +1287,15 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
> >          break;
> >      case VMDK_OK:
> >          ret = BDRV_BLOCK_DATA;
> > -        if (extent->file == bs->file && !extent->compressed) {
> > -            ret |= BDRV_BLOCK_OFFSET_VALID | offset;
> > +        if (!extent->compressed) {
> > +            ret |= BDRV_BLOCK_OFFSET_VALID;
> > +            ret |= (offset + (index_in_cluster << BDRV_SECTOR_BITS))
> > +                    & BDRV_BLOCK_OFFSET_MASK;
> >          }
> > -
> > +        *file = extent->file->bs;
> >          break;
> >      }
> >  
> > -    index_in_cluster = vmdk_find_index_in_cluster(extent, sector_num);
> >      n = extent->cluster_sectors - index_in_cluster;
> >      if (n > nb_sectors) {
> >          n = nb_sectors;
> 
> The commit message made me expect that this does exactly the same as the
> other patches, i.e. fill the file argument and nothing else.
> 
> Instead, this extends the functioniality to work on any kind of extents
> instead of just the the main VMDK file, and it changes the calculation
> of the offset (seems to be a hidden bug fix?)
> 
> This needs a non-empty commit message, and depending on what you're
> going to write there, possibly splitting the patch.

Will split it.

Fam