[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 02/20] block: Switch passthrough drivers to .

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v4 02/20] block: Switch passthrough drivers to .bdrv_co_block_status()
Date: Tue, 28 Nov 2017 10:05:55 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

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 <address@hidden>

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;
-           (sector_num << BDRV_SECTOR_BITS);

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]