Re: [Qemu-devel] [PATCH 10/16] block/parallels: add get_block_status

[Kevin Wolf]

Am 15.12.2014 um 12:52 hat Denis V. Lunev geschrieben:
> On 15/12/14 11:27, Denis V. Lunev wrote:
> >From: Roman Kagan <address@hidden>
> >
> >Implement VFS method for get_block_status to Parallels format driver.
> >
> >Signed-off-by: Roman Kagan <address@hidden>
> >Signed-off-by: Denis V. Lunev <address@hidden>
> >CC: Jeff Cody <address@hidden>
> >CC: Kevin Wolf <address@hidden>
> >CC: Stefan Hajnoczi <address@hidden>
> >---
> >  block/parallels.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> >diff --git a/block/parallels.c b/block/parallels.c
> >index a05bf39..2d3e962 100644
> >--- a/block/parallels.c
> >+++ b/block/parallels.c
> >@@ -384,6 +384,26 @@ static int cluster_remainder(BDRVParallelsState *s, 
> >int64_t sector_num,
> >      return MIN(nb_sectors, ret);
> >  }
> >+static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState 
> >*bs,
> >+        int64_t sector_num, int nb_sectors, int *pnum)
> >+{
> >+    BDRVParallelsState *s = bs->opaque;
> >+    int64_t offset;
> >+
> >+    qemu_co_mutex_lock(&s->lock);
> >+    offset = seek_to_sector(s, sector_num);
> I have mistaken here at porting Roman's changes
> on top of my changes. "padding" is not applied here
> while it should.
> 
> Shame on me :(
> 
> Something safe should be invented to avoid this
> mess.

Perhaps the application of padding should be moved into
seek_to_sector(). In this case, the following changes to parallels_read()
would follow:

* Reading from the backing file would not apply the padding. I think the
  padding is an internal property of this specific image file, so this
  is what actually should happen. In other words, a bug fix that you
  would have to do anyway.

* n = cluster_remainder(...) must be changed to be based on position
  (i.e. the position in the image file) rather than sector_num (i.e. the
  virtual offset as seen by the guest)

Kevin