[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 01/15] block: Add "file" output parameter to
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v8 01/15] block: Add "file" output parameter to block status query functions |
Date: |
Mon, 25 Jan 2016 14:04:37 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 25.01.2016 um 03:44 hat Fam Zheng geschrieben:
> 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 <address@hidden>
> Reviewed-by: Max Reitz <address@hidden>
> ---
> block/io.c | 38 ++++++++++++++++++++++++++------------
> 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 | 11 +++++++----
> include/block/block_int.h | 3 ++-
> qemu-img.c | 13 +++++++++----
> 17 files changed, 64 insertions(+), 35 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index 5bb353a..0836991 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -664,6 +664,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);
> @@ -676,7 +677,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));
> @@ -1466,6 +1467,7 @@ int bdrv_flush_all(void)
> typedef struct BdrvCoGetBlockStatusData {
> BlockDriverState *bs;
> BlockDriverState *base;
> + BlockDriverState **file;
> int64_t sector_num;
> int nb_sectors;
> int *pnum;
> @@ -1490,7 +1492,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)
This function has a comment explaining all parameters. Except **file
now.
My impression at the moment is that it generally returns bs->file for
format drivers and bs itself for protocol drivers if the sector is
allocated in this layer.
> {
> int64_t total_sectors;
> int64_t n;
> @@ -1520,16 +1523,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;
You specified that file is to be ignored unless BDRV_BLOCK_OFFSET_VALID
is set. Why reset it here then? It's not reset in other error cases.
> 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)) {
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 2ea5a4a..b8d29e1 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -2884,7 +2884,7 @@ static coroutine_fn int vvfat_co_write(BlockDriverState
> *bs, int64_t sector_num,
> }
>
> static int64_t coroutine_fn vvfat_co_get_block_status(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors, int* n)
> + int64_t sector_num, int nb_sectors, int *n, BlockDriverState **file)
> {
> BDRVVVFATState* s = bs->opaque;
> *n = s->sector_count - sector_num;
This still returns NULL at the end of the series. Shouldn't it return bs
like other protocol drivers do?
Kevin
- [Qemu-devel] [PATCH v8 00/15] qemu-img map: Allow driver to return file of the allocated block, Fam Zheng, 2016/01/24
- [Qemu-devel] [PATCH v8 03/15] qcow2: Assign bs->file->bs to file in qcow2_co_get_block_status, Fam Zheng, 2016/01/24
- [Qemu-devel] [PATCH v8 04/15] raw: Assign bs to file in raw_co_get_block_status, Fam Zheng, 2016/01/24
- [Qemu-devel] [PATCH v8 05/15] iscsi: Assign bs to file in iscsi_co_get_block_status, Fam Zheng, 2016/01/24
- [Qemu-devel] [PATCH v8 06/15] parallels: Assign bs->file->bs to file in parallels_co_get_block_status, Fam Zheng, 2016/01/24
- [Qemu-devel] [PATCH v8 07/15] qed: Assign bs->file->bs to file in bdrv_qed_co_get_block_status, Fam Zheng, 2016/01/24
- [Qemu-devel] [PATCH v8 08/15] sheepdog: Assign bs to file in sd_co_get_block_status, Fam Zheng, 2016/01/24