[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 13/31] file-posix: Switch to .bdrv_
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 13/31] file-posix: Switch to .bdrv_co_block_status() |
Date: |
Tue, 18 Apr 2017 15:56:12 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0 |
On 04/17/2017 08:33 PM, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based. Update the file protocol driver accordingly.
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
> block/file-posix.c | 47 +++++++++++++++++++++++------------------------
> 1 file changed, 23 insertions(+), 24 deletions(-)
This patch makes qemu-iotests 109 fail, due to an assertion in 6/31 that
the nested call to bdrv_block_status() [thanks to BDRV_BLOCK_RAW] is
still sector-aligned. The culprit:
> +static int64_t coroutine_fn raw_co_block_status(BlockDriverState *bs,
> + int64_t offset,
> + int64_t bytes, int64_t *pnum,
> + BlockDriverState **file)
> {
> - off_t start, data = 0, hole = 0;
> + off_t data = 0, hole = 0;
> int64_t total_size;
> int ret;
>
> @@ -1856,39 +1856,38 @@ static int64_t coroutine_fn
> raw_co_get_block_status(BlockDriverState *bs,
> return ret;
> }
>
> - start = sector_num * BDRV_SECTOR_SIZE;
> total_size = bdrv_getlength(bs);
total_size is always sector-aligned (bdrv_getlength() purposefully
widens the input file size, 560 bytes in the case of an empty qcow image
in test 109, out to a sector boundary)...
> if (total_size < 0) {
> return total_size;
> - } else if (start >= total_size) {
> + } else if (offset >= total_size) {
> *pnum = 0;
> return 0;
> - } else if (start + nb_sectors * BDRV_SECTOR_SIZE > total_size) {
> - nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
> + } else if (offset + bytes > total_size) {
> + bytes = total_size - offset;
> }
>
> - ret = find_allocation(bs, start, &data, &hole);
> + ret = find_allocation(bs, offset, &data, &hole);
...while find_allocation still obeys byte-granularity limits of the
underlying file system, and therefore reports a hole starting at offset
560 (the only time a hole starts at a non-sector boundary); pre-patch
that didn't matter (because we rounded before returning sectors through
*pnum), but post-patch, returning 560 instead of 1024 triggers problems.
I'm still debating whether the best fix is to tweak the file-posix
limits to just round up to sector boundaries at EOF, or to make the
block layer itself special-case EOF in case other protocols also have
byte-granularity support. Any advice you have while reviewing is
appreciated, but it does mean I'll need a v2 of the series one way or
another.
And I didn't spot it during my earlier testing because I was skipping
109 as failing prior to my patches even started testing it - but now
that Jeff and Fam have both posted patches for two separate problems in
test 109 (the need to munge "len", and failure to do proper cleanup), I
no longer have that excuse.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH 08/31] block: Switch BdrvCoGetBlockStatusData to byte-based, (continued)
- [Qemu-block] [PATCH 08/31] block: Switch BdrvCoGetBlockStatusData to byte-based, Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 09/31] block: Switch bdrv_co_get_block_status_above() to byte-based, Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 10/31] block: Convert bdrv_get_block_status_above() to bytes, Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 11/31] block: Add .bdrv_co_block_status() callback, Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 12/31] commit: Switch to .bdrv_co_block_status(), Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 14/31] gluster: Switch to .bdrv_co_block_status(), Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 13/31] file-posix: Switch to .bdrv_co_block_status(), Eric Blake, 2017/04/17
- Re: [Qemu-block] [Qemu-devel] [PATCH 13/31] file-posix: Switch to .bdrv_co_block_status(),
Eric Blake <=
- [Qemu-block] [PATCH 15/31] iscsi: Switch cluster_sectors to byte-based, Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 16/31] iscsi: Switch iscsi_allocmap_update() to byte-based, Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 17/31] iscsi: Switch to .bdrv_co_block_status(), Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 18/31] mirror: Switch to .bdrv_co_block_status(), Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 19/31] null: Switch to .bdrv_co_block_status(), Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 20/31] parallels: Switch to .bdrv_co_block_status(), Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 21/31] qcow: Switch to .bdrv_co_block_status(), Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 22/31] qcow2: Switch to .bdrv_co_block_status(), Eric Blake, 2017/04/17
- [Qemu-block] [PATCH 23/31] qed: Switch to .bdrv_co_block_status(), Eric Blake, 2017/04/17