On 05.01.2016 09:11, Fam Zheng wrote:
> The added parameter can be used to return the BDS pointer which the
> valid offset is referring to. Its 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.
>
> The "bs->file" condition in bdrv_co_get_block_status is kept now to keep
> iotest
> case 102 passing, and will be fixed once all drivers return the right file
> pointer.
>
> Signed-off-by: Fam Zheng <f...@redhat.com>
> ---
> 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 | 9 ++++++---
> include/block/block_int.h | 3 ++-
> qemu-img.c | 7 +++++--
> 17 files changed, 61 insertions(+), 34 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index 63e3678..492c291 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -663,6 +663,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);
> @@ -675,7 +676,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));
> @@ -1464,6 +1465,7 @@ int bdrv_flush_all(void)
> typedef struct BdrvCoGetBlockStatusData {
> BlockDriverState *bs;
> BlockDriverState *base;
> + BlockDriverState **file;
> int64_t sector_num;
> int nb_sectors;
> int *pnum;
> @@ -1488,7 +1490,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;
> @@ -1518,16 +1521,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)) {
> @@ -1544,13 +1550,14 @@ static int64_t coroutine_fn
> bdrv_co_get_block_status(BlockDriverState *bs,
> }
> }
>
> - if (bs->file &&
> + if (bs->file && *file && *file != bs &&
In order to keep iotest 102 working, this line should not be changed at
all in this patch. Because *file is NULL and not changed by any of the
block drivers, the condition will always be false now (breaking iotest 102).
> (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,
And instead of *file, this should remain bs->file->bs.
> + *pnum, &file_pnum, &file2);
> if (ret2 >= 0) {
> /* Ignore errors. This is just providing extra information, it
> * is useful but not necessary.
[...]
> diff --git a/include/block/block.h b/include/block/block.h
> index db8e096..ab966ad 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -113,7 +113,8 @@ typedef struct HDGeometry {
> * Allocation status flags
> * BDRV_BLOCK_DATA: data is read from bs->file or another file
> * BDRV_BLOCK_ZERO: sectors read as zero
> - * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
> + * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file or another file
> + * as raw data
Hm, this seems so unspecific. We do know exactly where the data is
stored, and that is in the *file BDS returned by
bdrv_get_block_status(). Can we make some reference to that? I don't
know whether "sector stored in *file as raw data" will be clear enough,
though. Maybe "sector stored in the BDS returned as *file by
bdrv_get_block_status()" will suffice.
Max
> * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
> * layer (as opposed to the backing file)
> * BDRV_BLOCK_RAW: used internally to indicate that the request
signature.asc
Description: OpenPGP digital signature
