On 11/28/2017 02:19 AM, Vladimir Sementsov-Ogievskiy wrote:
12.10.2017 21:58, Eric Blake wrote:
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the generic helpers, and all passthrough clients
(blkdebug, commit, mirror, throttle) accordingly.
Signed-off-by: Eric Blake <ebl...@redhat.com>
BlockDriverState **file)
+int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs,
+ bool want_zero,
+ int64_t offset,
+ int64_t bytes,
+ int64_t *pnum,
+ int64_t *map,
+ BlockDriverState
**file)
{
assert(bs->backing && bs->backing->bs);
- *pnum = nb_sectors;
+ *pnum = bytes;
+ *map = offset;
*file = bs->backing->bs;
- return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
- (sector_num << BDRV_SECTOR_BITS);
+ return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID;
}
hmm. not related question, but can you please explain, why we use such
stubs
instead of really call bdrv_co_block_status on bs->file->bs or
bs->backing->bs ?
Returning BDRV_BLOCK_RAW is what tells io.c to call
bdrv_co_block_status() on whatever got assigned into *file. The two
stubs make it so there are fewer conditionals in io.c; perhaps it could
be reworked to avoid the stubs and have io.c have all the smarts when
.bdrv_co_block_status is NULL, but as you say, that would be a separate
patch (although it was Manos' work earlier this year that even created
the common helper stubs rather than duplicating code in each callback in
the first place).
+static int coroutine_fn blkdebug_co_block_status(BlockDriverState *bs,
+ assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));
looks like this is obviously guaranteed by block layer, so no needs for
special
function. and we can use
.bdrv_co_block_status = bdrv_co_block_status_from_backing
like for other drivers.
We could, but we don't want to. The point of the blkdebug driver is to
explicitly test that certain preconditions are being satisfied, so that
even if the block layer in io.c changes, our iotests make sure that
drivers are provided with certain guarantees. In other words, the
assert() added here is added value that we cannot stick in the common
helper, but something that we want to keep associated with the blkdebug
driver.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
