[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 09/21] null: Switch to .bdrv_co_block_status(
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v8 09/21] null: Switch to .bdrv_co_block_status() |
Date: |
Wed, 14 Feb 2018 13:05:25 +0100 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 13.02.2018 um 21:26 hat Eric Blake geschrieben:
> We are gradually moving away from sector-based interfaces, towards
> byte-based. Update the null driver accordingly.
>
> Signed-off-by: Eric Blake <address@hidden>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> Reviewed-by: Fam Zheng <address@hidden>
>
> ---
> v6-v7: no change
> v5: minor fix to type of 'ret'
> v4: rebase to interface tweak
> v3: no change
> v2: rebase to mapping parameter
> ---
> block/null.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/block/null.c b/block/null.c
> index 214d394fff4..806a8631e4d 100644
> --- a/block/null.c
> +++ b/block/null.c
> @@ -223,22 +223,23 @@ static int null_reopen_prepare(BDRVReopenState
> *reopen_state,
> return 0;
> }
>
> -static int64_t coroutine_fn null_co_get_block_status(BlockDriverState *bs,
> - int64_t sector_num,
> - int nb_sectors, int
> *pnum,
> - BlockDriverState **file)
> +static int coroutine_fn null_co_block_status(BlockDriverState *bs,
> + bool want_zero, int64_t offset,
> + int64_t bytes, int64_t *pnum,
> + int64_t *map,
> + BlockDriverState **file)
> {
> BDRVNullState *s = bs->opaque;
> - off_t start = sector_num * BDRV_SECTOR_SIZE;
> + int ret = BDRV_BLOCK_OFFSET_VALID;
>
> - *pnum = nb_sectors;
> + *pnum = bytes;
> + *map = offset;
> *file = bs;
>
> if (s->read_zeroes) {
> - return BDRV_BLOCK_OFFSET_VALID | start | BDRV_BLOCK_ZERO;
> - } else {
> - return BDRV_BLOCK_OFFSET_VALID | start;
> + ret |= BDRV_BLOCK_ZERO;
> }
> + return ret;
> }
Preexisting, but I think this return value is wrong. OFFSET_VALID
without DATA is to documented to have the following semantics:
* DATA ZERO OFFSET_VALID
* f t t sectors preallocated, read as zero, returned file not
* necessarily zero at offset
* f f t sectors preallocated but read from backing_hd,
* returned file contains garbage at offset
I'm not sure what OFFSET_VALID is even supposed to mean for null.
Or in fact, what it is supposed to mean for any protocol driver, because
normally it just means I can use this offset for accessing bs->file. But
protocol drivers don't have a bs->file, so it's interesting to see that
they still all set this flag.
OFFSET_VALID | DATA might be excusable because I can see that it's
convenient that a protocol driver refers to itself as *file instead of
returning NULL there and then the offset is valid (though it would be
pointless to actually follow the file pointer), but OFFSET_VALID without
DATA probably isn't.
Kevin
- [Qemu-devel] [PATCH v8 00/21] add byte-based block_status driver callbacks, Eric Blake, 2018/02/13
- [Qemu-devel] [PATCH v8 02/21] nvme: Drop pointless .bdrv_co_get_block_status(), Eric Blake, 2018/02/13
- [Qemu-devel] [PATCH v8 06/21] iscsi: Switch cluster_sectors to byte-based, Eric Blake, 2018/02/13
- [Qemu-devel] [PATCH v8 04/21] file-posix: Switch to .bdrv_co_block_status(), Eric Blake, 2018/02/13
- [Qemu-devel] [PATCH v8 08/21] iscsi: Switch to .bdrv_co_block_status(), Eric Blake, 2018/02/13
- [Qemu-devel] [PATCH v8 09/21] null: Switch to .bdrv_co_block_status(), Eric Blake, 2018/02/13
[Qemu-devel] [PATCH v8 14/21] raw: Switch to .bdrv_co_block_status(), Eric Blake, 2018/02/13
[Qemu-devel] [PATCH v8 07/21] iscsi: Switch iscsi_allocmap_update() to byte-based, Eric Blake, 2018/02/13
[Qemu-devel] [PATCH v8 05/21] gluster: Switch to .bdrv_co_block_status(), Eric Blake, 2018/02/13
[Qemu-devel] [PATCH v8 11/21] qcow: Switch to .bdrv_co_block_status(), Eric Blake, 2018/02/13