Re: [Qemu-block] [PATCH 6/9] block: pass qiov into before_write notifier
On 06/15/2016 12:21 PM, Stefan Hajnoczi wrote: On Tue, Jun 14, 2016 at 06:25:13PM +0300, Denis V. Lunev wrote: Signed-off-by: Denis V. Lunev Reviewed-by: Vladimir Sementsov-Ogievskiy CC: Stefan Hajnoczi CC: Fam Zheng CC: Kevin Wolf CC: Max Reitz CC: Jeff Cody CC: Eric Blake --- block/io.c| 12 +++- include/block/block_int.h | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) This patch is missing a commit description. Why is this necessary? If you add .qiov then can we remove the .bytes field? this would not be convenient unfortunately. here I have pointer to qiov and this pointer is not always available. Actually it is available only to the write operation. The purpose is to make data being written available for the notifier. Den
Re: [Qemu-block] [PATCH 6/9] block: pass qiov into before_write notifier
On Tue, Jun 14, 2016 at 06:25:13PM +0300, Denis V. Lunev wrote: > Signed-off-by: Denis V. Lunev > Reviewed-by: Vladimir Sementsov-Ogievskiy > CC: Stefan Hajnoczi > CC: Fam Zheng > CC: Kevin Wolf > CC: Max Reitz > CC: Jeff Cody > CC: Eric Blake > --- > block/io.c| 12 +++- > include/block/block_int.h | 1 + > 2 files changed, 8 insertions(+), 5 deletions(-) I read Eric's reply and realized the duplication with my earlier reply. Feel free to ignore mine and just respond to Eric. signature.asc Description: PGP signature
Re: [Qemu-block] [PATCH 6/9] block: pass qiov into before_write notifier
On Tue, Jun 14, 2016 at 06:25:13PM +0300, Denis V. Lunev wrote: > Signed-off-by: Denis V. Lunev > Reviewed-by: Vladimir Sementsov-Ogievskiy > CC: Stefan Hajnoczi > CC: Fam Zheng > CC: Kevin Wolf > CC: Max Reitz > CC: Jeff Cody > CC: Eric Blake > --- > block/io.c| 12 +++- > include/block/block_int.h | 1 + > 2 files changed, 8 insertions(+), 5 deletions(-) This patch is missing a commit description. Why is this necessary? If you add .qiov then can we remove the .bytes field? signature.asc Description: PGP signature
Re: [Qemu-block] [PATCH 6/9] block: pass qiov into before_write notifier
On 06/14/2016 09:25 AM, Denis V. Lunev wrote:
> Signed-off-by: Denis V. Lunev
The commit message says what, but not why. It's useful to give
reviewers a hint as to why a patch makes sense (such as a future patch
being able to use the write notifier to make mirroring more efficient if
it knows what is being mirrored).
> Reviewed-by: Vladimir Sementsov-Ogievskiy
> CC: Stefan Hajnoczi
> CC: Fam Zheng
> CC: Kevin Wolf
> CC: Max Reitz
> CC: Jeff Cody
> CC: Eric Blake
> ---
> block/io.c| 12 +++-
> include/block/block_int.h | 1 +
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> @@ -2228,7 +2230,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs,
> int64_t sector_num,
> return 0;
> }
>
> -tracked_request_begin(&req, bs, sector_num, nb_sectors,
> +tracked_request_begin(&req, bs, NULL, sector_num, nb_sectors,
>BDRV_TRACKED_DISCARD);
> bdrv_set_dirty(bs, sector_num, nb_sectors);
>
> @@ -2331,7 +2333,7 @@ static int bdrv_co_do_ioctl(BlockDriverState *bs, int
> req, void *buf)
> };
> BlockAIOCB *acb;
>
> -tracked_request_begin(&tracked_req, bs, 0, 0, BDRV_TRACKED_IOCTL);
> +tracked_request_begin(&tracked_req, bs, NULL, 0, 0, BDRV_TRACKED_IOCTL);
> if (!drv || !drv->bdrv_aio_ioctl) {
> co.ret = -ENOTSUP;
> goto out;
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 30a9717..72f463a 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -69,6 +69,7 @@ enum BdrvTrackedRequestType {
>
> typedef struct BdrvTrackedRequest {
> BlockDriverState *bs;
> +QEMUIOVector *qiov;
> int64_t offset;
> unsigned int bytes;
I guess bytes and qiov->size are not always redundant, because you pass
NULL for qiov for a zero or discard operation (an alternative would be
to always pass a qiov, even for zero/discard, so that we only need a
single size). But I've been pointing out our inconsistent use of qiov
for zeroes in multiple places, so I don't think it's worth changing in
this series, but in one of its own if we want to do it.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-block] [PATCH 6/9] block: pass qiov into before_write notifier
Signed-off-by: Denis V. Lunev
Reviewed-by: Vladimir Sementsov-Ogievskiy
CC: Stefan Hajnoczi
CC: Fam Zheng
CC: Kevin Wolf
CC: Max Reitz
CC: Jeff Cody
CC: Eric Blake
---
block/io.c| 12 +++-
include/block/block_int.h | 1 +
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/block/io.c b/block/io.c
index 2d832aa..d2ad09c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -368,12 +368,14 @@ static void tracked_request_end(BdrvTrackedRequest *req)
*/
static void tracked_request_begin(BdrvTrackedRequest *req,
BlockDriverState *bs,
+ QEMUIOVector *qiov,
int64_t offset,
unsigned int bytes,
enum BdrvTrackedRequestType type)
{
*req = (BdrvTrackedRequest){
.bs = bs,
+.qiov = qiov,
.offset = offset,
.bytes = bytes,
.type = type,
@@ -1073,7 +1075,7 @@ int coroutine_fn bdrv_co_preadv(BlockDriverState *bs,
bytes = ROUND_UP(bytes, align);
}
-tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_READ);
+tracked_request_begin(&req, bs, NULL, offset, bytes, BDRV_TRACKED_READ);
ret = bdrv_aligned_preadv(bs, &req, offset, bytes, align,
use_local_qiov ? &local_qiov : qiov,
flags);
@@ -1391,7 +1393,7 @@ int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs,
* Pad qiov with the read parts and be sure to have a tracked request not
* only for bdrv_aligned_pwritev, but also for the reads of the RMW cycle.
*/
-tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_WRITE);
+tracked_request_begin(&req, bs, qiov, offset, bytes, BDRV_TRACKED_WRITE);
if (!qiov) {
ret = bdrv_co_do_zero_pwritev(bs, offset, bytes, flags, &req);
@@ -2098,7 +2100,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
return 0;
}
-tracked_request_begin(&req, bs, 0, 0, BDRV_TRACKED_FLUSH);
+tracked_request_begin(&req, bs, NULL, 0, 0, BDRV_TRACKED_FLUSH);
/* Write back all layers by calling one driver function */
if (bs->drv->bdrv_co_flush) {
@@ -2228,7 +2230,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs,
int64_t sector_num,
return 0;
}
-tracked_request_begin(&req, bs, sector_num, nb_sectors,
+tracked_request_begin(&req, bs, NULL, sector_num, nb_sectors,
BDRV_TRACKED_DISCARD);
bdrv_set_dirty(bs, sector_num, nb_sectors);
@@ -2331,7 +2333,7 @@ static int bdrv_co_do_ioctl(BlockDriverState *bs, int
req, void *buf)
};
BlockAIOCB *acb;
-tracked_request_begin(&tracked_req, bs, 0, 0, BDRV_TRACKED_IOCTL);
+tracked_request_begin(&tracked_req, bs, NULL, 0, 0, BDRV_TRACKED_IOCTL);
if (!drv || !drv->bdrv_aio_ioctl) {
co.ret = -ENOTSUP;
goto out;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 30a9717..72f463a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -69,6 +69,7 @@ enum BdrvTrackedRequestType {
typedef struct BdrvTrackedRequest {
BlockDriverState *bs;
+QEMUIOVector *qiov;
int64_t offset;
unsigned int bytes;
enum BdrvTrackedRequestType type;
--
2.5.0
