Re: [Qemu-block] [PATCH v3 01/15] block: Add "file" output parameter to block status query functions

2015-12-01 Thread Stefan Hajnoczi
On Mon, Nov 30, 2015 at 05:09:49PM +0800, Fam Zheng wrote:
> On Mon, 11/30 16:38, Stefan Hajnoczi wrote:
> > On Thu, Nov 26, 2015 at 01:05:21PM +0800, Fam Zheng wrote:
> > > @@ -1535,13 +1541,14 @@ static int64_t coroutine_fn 
> > > bdrv_co_get_block_status(BlockDriverState *bs,
> > >  }
> > >  }
> > >  
> > > -if (bs->file &&
> > > +if (*file && *file != bs &&
> > >  (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
> > >  (ret & BDRV_BLOCK_OFFSET_VALID)) {
> > 
> > What is the purpose of this change?
> 
> The code here is to sensibly detect "zero" by going into the "file" which the
> offset is valid for. Now the "file" is no longer always "bs->file", so we use
> the returned "file" pointer instead.

Thanks for explaining!

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-block] [PATCH v3 01/15] block: Add "file" output parameter to block status query functions

2015-11-30 Thread Fam Zheng
On Mon, 11/30 16:38, Stefan Hajnoczi wrote:
> On Thu, Nov 26, 2015 at 01:05:21PM +0800, Fam Zheng wrote:
> > @@ -1535,13 +1541,14 @@ static int64_t coroutine_fn 
> > bdrv_co_get_block_status(BlockDriverState *bs,
> >  }
> >  }
> >  
> > -if (bs->file &&
> > +if (*file && *file != bs &&
> >  (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
> >  (ret & BDRV_BLOCK_OFFSET_VALID)) {
> 
> What is the purpose of this change?

The code here is to sensibly detect "zero" by going into the "file" which the
offset is valid for. Now the "file" is no longer always "bs->file", so we use
the returned "file" pointer instead.

Fam



Re: [Qemu-block] [PATCH v3 01/15] block: Add "file" output parameter to block status query functions

2015-11-30 Thread Stefan Hajnoczi
On Thu, Nov 26, 2015 at 01:05:21PM +0800, Fam Zheng wrote:
> @@ -1535,13 +1541,14 @@ static int64_t coroutine_fn 
> bdrv_co_get_block_status(BlockDriverState *bs,
>  }
>  }
>  
> -if (bs->file &&
> +if (*file && *file != bs &&
>  (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
>  (ret & BDRV_BLOCK_OFFSET_VALID)) {

What is the purpose of this change?


signature.asc
Description: PGP signature


[Qemu-block] [PATCH v3 01/15] block: Add "file" output parameter to block status query functions

2015-11-25 Thread Fam Zheng
The added parameter can be used to return the BDS pointer which the
valid offset is referring to. It's value should be ignored unless
BDRV_BLOCK_OFFSET_VALID in ret is set.

Until block drivers fill in the right value, let's clear it explicitly
right before calling .bdrv_get_block_status.

Signed-off-by: Fam Zheng 
---
 block/io.c| 42 --
 block/iscsi.c |  6 --
 block/mirror.c|  3 ++-
 block/parallels.c |  2 +-
 block/qcow.c  |  2 +-
 block/qcow2.c |  2 +-
 block/qed.c   |  3 ++-
 block/raw-posix.c |  3 ++-
 block/raw_bsd.c   |  3 ++-
 block/sheepdog.c  |  2 +-
 block/vdi.c   |  2 +-
 block/vmdk.c  |  2 +-
 block/vpc.c   |  2 +-
 block/vvfat.c |  2 +-
 include/block/block.h |  6 --
 include/block/block_int.h |  3 ++-
 qemu-img.c|  7 +--
 17 files changed, 59 insertions(+), 33 deletions(-)

diff --git a/block/io.c b/block/io.c
index adc1eab..71930ed 100644
--- a/block/io.c
+++ b/block/io.c
@@ -656,6 +656,7 @@ int bdrv_write_zeroes(BlockDriverState *bs, int64_t 
sector_num,
 int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags)
 {
 int64_t target_sectors, ret, nb_sectors, sector_num = 0;
+BlockDriverState *file;
 int n;
 
 target_sectors = bdrv_nb_sectors(bs);
@@ -668,7 +669,7 @@ int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags 
flags)
 if (nb_sectors <= 0) {
 return 0;
 }
-ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n);
+ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n, &file);
 if (ret < 0) {
 error_report("error getting block status at sector %" PRId64 ": 
%s",
  sector_num, strerror(-ret));
@@ -1455,6 +1456,7 @@ int bdrv_flush_all(void)
 typedef struct BdrvCoGetBlockStatusData {
 BlockDriverState *bs;
 BlockDriverState *base;
+BlockDriverState **file;
 int64_t sector_num;
 int nb_sectors;
 int *pnum;
@@ -1479,7 +1481,8 @@ typedef struct BdrvCoGetBlockStatusData {
  */
 static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
  int64_t sector_num,
- int nb_sectors, int *pnum)
+ int nb_sectors, int *pnum,
+ BlockDriverState **file)
 {
 int64_t total_sectors;
 int64_t n;
@@ -1509,16 +1512,19 @@ static int64_t coroutine_fn 
bdrv_co_get_block_status(BlockDriverState *bs,
 return ret;
 }
 
-ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum);
+*file = NULL;
+ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum,
+file);
 if (ret < 0) {
 *pnum = 0;
+*file = NULL;
 return ret;
 }
 
 if (ret & BDRV_BLOCK_RAW) {
 assert(ret & BDRV_BLOCK_OFFSET_VALID);
 return bdrv_get_block_status(bs->file->bs, ret >> BDRV_SECTOR_BITS,
- *pnum, pnum);
+ *pnum, pnum, file);
 }
 
 if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
@@ -1535,13 +1541,14 @@ static int64_t coroutine_fn 
bdrv_co_get_block_status(BlockDriverState *bs,
 }
 }
 
-if (bs->file &&
+if (*file && *file != bs &&
 (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
 (ret & BDRV_BLOCK_OFFSET_VALID)) {
+BlockDriverState *file2;
 int file_pnum;
 
-ret2 = bdrv_co_get_block_status(bs->file->bs, ret >> BDRV_SECTOR_BITS,
-*pnum, &file_pnum);
+ret2 = bdrv_co_get_block_status(*file, ret >> BDRV_SECTOR_BITS,
+*pnum, &file_pnum, &file2);
 if (ret2 >= 0) {
 /* Ignore errors.  This is just providing extra information, it
  * is useful but not necessary.
@@ -1566,14 +1573,15 @@ static int64_t coroutine_fn 
bdrv_co_get_block_status_above(BlockDriverState *bs,
 BlockDriverState *base,
 int64_t sector_num,
 int nb_sectors,
-int *pnum)
+int *pnum,
+BlockDriverState **file)
 {
 BlockDriverState *p;
 int64_t ret = 0;
 
 assert(bs != base);
 for (p = bs; p != base; p = backing_bs(p)) {
-ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum);
+ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum, file);
 if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) {
 break;
 }
@@ -1592,7 +1600,8 @@ static void coroutine_fn 
bdrv_get_block_status_above_co_entry(void *opaque)
 data->ret = bdrv_co_get_block_status_above(data->bs, data->base,