* [Qemu-devel] [PATCH v2 01/12] util/iov: introduce qemu_iovec_init_extended
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-05 11:54 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 02/12] util/iov: improve qemu_iovec_is_zero Vladimir Sementsov-Ogievskiy
` (14 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
Introduce new initialization API, to create requests with padding. Will
be used in the following patch. New API uses qemu_iovec_init_buf if
resulting io vector has only one element, to avoid extra allocations.
So, we need to update qemu_iovec_destroy to support destroying such
QIOVs.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/qemu/iov.h | 7 +++
util/iov.c | 112 +++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 114 insertions(+), 5 deletions(-)
diff --git a/include/qemu/iov.h b/include/qemu/iov.h
index 48b45987b7..f3787a0cf7 100644
--- a/include/qemu/iov.h
+++ b/include/qemu/iov.h
@@ -199,6 +199,13 @@ static inline void *qemu_iovec_buf(QEMUIOVector *qiov)
void qemu_iovec_init(QEMUIOVector *qiov, int alloc_hint);
void qemu_iovec_init_external(QEMUIOVector *qiov, struct iovec *iov, int niov);
+void qemu_iovec_init_extended(
+ QEMUIOVector *qiov,
+ void *head_buf, size_t head_len,
+ QEMUIOVector *mid_qiov, size_t mid_offset, size_t mid_len,
+ void *tail_buf, size_t tail_len);
+void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source,
+ size_t offset, size_t len);
void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len);
void qemu_iovec_concat(QEMUIOVector *dst,
QEMUIOVector *src, size_t soffset, size_t sbytes);
diff --git a/util/iov.c b/util/iov.c
index 74e6ca8ed7..39b6e31494 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -353,6 +353,103 @@ void qemu_iovec_concat(QEMUIOVector *dst,
qemu_iovec_concat_iov(dst, src->iov, src->niov, soffset, sbytes);
}
+/*
+ * qiov_find_iov
+ *
+ * Return pointer to iovec structure, where byte at @offset in original vector
+ * @iov exactly is.
+ * Set @remaining_offset to be offset inside that iovec to the same byte.
+ */
+static struct iovec *iov_skip_offset(struct iovec *iov, size_t offset,
+ size_t *remaining_offset)
+{
+ while (offset > 0 && offset >= iov->iov_len) {
+ offset -= iov->iov_len;
+ iov++;
+ }
+ *remaining_offset = offset;
+
+ return iov;
+}
+
+/*
+ * qiov_slice
+ *
+ * Find subarray of iovec's, containing requested range. @head would
+ * be offset in first iov (returned by the function), @tail would be
+ * count of extra bytes in last iovec (returned iov + @niov - 1).
+ */
+static struct iovec *qiov_slice(QEMUIOVector *qiov,
+ size_t offset, size_t len,
+ size_t *head, size_t *tail, int *niov)
+{
+ struct iovec *iov, *end_iov;
+
+ assert(offset + len <= qiov->size);
+
+ iov = iov_skip_offset(qiov->iov, offset, head);
+ end_iov = iov_skip_offset(iov, *head + len, tail);
+
+ if (*tail > 0) {
+ assert(*tail < end_iov->iov_len);
+ *tail = end_iov->iov_len - *tail;
+ end_iov++;
+ }
+
+ *niov = end_iov - iov;
+
+ return iov;
+}
+
+/*
+ * Compile new iovec, combining @head_buf buffer, sub-qiov of @mid_qiov,
+ * and @tail_buf buffer into new qiov.
+ */
+void qemu_iovec_init_extended(
+ QEMUIOVector *qiov,
+ void *head_buf, size_t head_len,
+ QEMUIOVector *mid_qiov, size_t mid_offset, size_t mid_len,
+ void *tail_buf, size_t tail_len)
+{
+ size_t mid_head, mid_tail;
+ int total_niov, mid_niov;
+ struct iovec *p, *mid_iov;
+
+ if (mid_len) {
+ mid_iov = qiov_slice(mid_qiov, mid_offset, mid_len,
+ &mid_head, &mid_tail, &mid_niov);
+ }
+
+ total_niov = !!head_len + mid_niov + !!tail_len;
+ if (total_niov == 1) {
+ qemu_iovec_init_buf(qiov, NULL, 0);
+ p = &qiov->local_iov;
+ } else {
+ qiov->niov = qiov->nalloc = total_niov;
+ qiov->size = head_len + mid_len + tail_len;
+ p = qiov->iov = g_new(struct iovec, qiov->niov);
+ }
+
+ if (head_len) {
+ p->iov_base = head_buf;
+ p->iov_len = head_len;
+ p++;
+ }
+
+ if (mid_len) {
+ memcpy(p, mid_iov, mid_niov * sizeof(*p));
+ p[0].iov_base = (uint8_t *)p[0].iov_base + mid_head;
+ p[0].iov_len -= mid_head;
+ p[mid_niov - 1].iov_len -= mid_tail;
+ p += mid_niov;
+ }
+
+ if (tail_len) {
+ p->iov_base = tail_buf;
+ p->iov_len = tail_len;
+ }
+}
+
/*
* Check if the contents of the iovecs are all zero
*/
@@ -374,14 +471,19 @@ bool qemu_iovec_is_zero(QEMUIOVector *qiov)
return true;
}
+void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source,
+ size_t offset, size_t len)
+{
+ qemu_iovec_init_extended(qiov, NULL, 0, source, offset, len, NULL, 0);
+}
+
void qemu_iovec_destroy(QEMUIOVector *qiov)
{
- assert(qiov->nalloc != -1);
+ if (qiov->nalloc != -1) {
+ g_free(qiov->iov);
+ }
- qemu_iovec_reset(qiov);
- g_free(qiov->iov);
- qiov->nalloc = 0;
- qiov->iov = NULL;
+ memset(qiov, 0, sizeof(*qiov));
}
void qemu_iovec_reset(QEMUIOVector *qiov)
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 01/12] util/iov: introduce qemu_iovec_init_extended
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 01/12] util/iov: introduce qemu_iovec_init_extended Vladimir Sementsov-Ogievskiy
@ 2019-06-05 11:54 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-05 11:54 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: fam, kwolf, Denis Lunev, mreitz, stefanha, jsnow
04.06.2019 19:15, Vladimir Sementsov-Ogievskiy wrote:
> Introduce new initialization API, to create requests with padding. Will
> be used in the following patch. New API uses qemu_iovec_init_buf if
> resulting io vector has only one element, to avoid extra allocations.
> So, we need to update qemu_iovec_destroy to support destroying such
> QIOVs.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> include/qemu/iov.h | 7 +++
> util/iov.c | 112 +++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 114 insertions(+), 5 deletions(-)
>
> diff --git a/include/qemu/iov.h b/include/qemu/iov.h
> index 48b45987b7..f3787a0cf7 100644
> --- a/include/qemu/iov.h
> +++ b/include/qemu/iov.h
> @@ -199,6 +199,13 @@ static inline void *qemu_iovec_buf(QEMUIOVector *qiov)
>
> void qemu_iovec_init(QEMUIOVector *qiov, int alloc_hint);
> void qemu_iovec_init_external(QEMUIOVector *qiov, struct iovec *iov, int niov);
> +void qemu_iovec_init_extended(
> + QEMUIOVector *qiov,
> + void *head_buf, size_t head_len,
> + QEMUIOVector *mid_qiov, size_t mid_offset, size_t mid_len,
> + void *tail_buf, size_t tail_len);
> +void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source,
> + size_t offset, size_t len);
> void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len);
> void qemu_iovec_concat(QEMUIOVector *dst,
> QEMUIOVector *src, size_t soffset, size_t sbytes);
> diff --git a/util/iov.c b/util/iov.c
> index 74e6ca8ed7..39b6e31494 100644
> --- a/util/iov.c
> +++ b/util/iov.c
> @@ -353,6 +353,103 @@ void qemu_iovec_concat(QEMUIOVector *dst,
> qemu_iovec_concat_iov(dst, src->iov, src->niov, soffset, sbytes);
> }
>
> +/*
> + * qiov_find_iov
> + *
> + * Return pointer to iovec structure, where byte at @offset in original vector
> + * @iov exactly is.
> + * Set @remaining_offset to be offset inside that iovec to the same byte.
> + */
> +static struct iovec *iov_skip_offset(struct iovec *iov, size_t offset,
> + size_t *remaining_offset)
> +{
> + while (offset > 0 && offset >= iov->iov_len) {
> + offset -= iov->iov_len;
> + iov++;
> + }
> + *remaining_offset = offset;
> +
> + return iov;
> +}
> +
> +/*
> + * qiov_slice
> + *
> + * Find subarray of iovec's, containing requested range. @head would
> + * be offset in first iov (returned by the function), @tail would be
> + * count of extra bytes in last iovec (returned iov + @niov - 1).
> + */
> +static struct iovec *qiov_slice(QEMUIOVector *qiov,
> + size_t offset, size_t len,
> + size_t *head, size_t *tail, int *niov)
> +{
> + struct iovec *iov, *end_iov;
> +
> + assert(offset + len <= qiov->size);
> +
> + iov = iov_skip_offset(qiov->iov, offset, head);
> + end_iov = iov_skip_offset(iov, *head + len, tail);
> +
> + if (*tail > 0) {
> + assert(*tail < end_iov->iov_len);
> + *tail = end_iov->iov_len - *tail;
> + end_iov++;
> + }
> +
> + *niov = end_iov - iov;
> +
> + return iov;
> +}
> +
> +/*
> + * Compile new iovec, combining @head_buf buffer, sub-qiov of @mid_qiov,
> + * and @tail_buf buffer into new qiov.
> + */
> +void qemu_iovec_init_extended(
> + QEMUIOVector *qiov,
> + void *head_buf, size_t head_len,
> + QEMUIOVector *mid_qiov, size_t mid_offset, size_t mid_len,
> + void *tail_buf, size_t tail_len)
> +{
> + size_t mid_head, mid_tail;
> + int total_niov, mid_niov;
Oops, clang is right, mid_niov may be uninitialized. So, here should be "mid_niov = 0".
> + struct iovec *p, *mid_iov;
> +
> + if (mid_len) {
> + mid_iov = qiov_slice(mid_qiov, mid_offset, mid_len,
> + &mid_head, &mid_tail, &mid_niov);
> + }
> +
> + total_niov = !!head_len + mid_niov + !!tail_len;
> + if (total_niov == 1) {
> + qemu_iovec_init_buf(qiov, NULL, 0);
> + p = &qiov->local_iov;
> + } else {
> + qiov->niov = qiov->nalloc = total_niov;
> + qiov->size = head_len + mid_len + tail_len;
> + p = qiov->iov = g_new(struct iovec, qiov->niov);
> + }
> +
> + if (head_len) {
> + p->iov_base = head_buf;
> + p->iov_len = head_len;
> + p++;
> + }
> +
> + if (mid_len) {
> + memcpy(p, mid_iov, mid_niov * sizeof(*p));
> + p[0].iov_base = (uint8_t *)p[0].iov_base + mid_head;
> + p[0].iov_len -= mid_head;
> + p[mid_niov - 1].iov_len -= mid_tail;
> + p += mid_niov;
> + }
> +
> + if (tail_len) {
> + p->iov_base = tail_buf;
> + p->iov_len = tail_len;
> + }
> +}
> +
> /*
> * Check if the contents of the iovecs are all zero
> */
> @@ -374,14 +471,19 @@ bool qemu_iovec_is_zero(QEMUIOVector *qiov)
> return true;
> }
>
> +void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source,
> + size_t offset, size_t len)
> +{
> + qemu_iovec_init_extended(qiov, NULL, 0, source, offset, len, NULL, 0);
> +}
> +
> void qemu_iovec_destroy(QEMUIOVector *qiov)
> {
> - assert(qiov->nalloc != -1);
> + if (qiov->nalloc != -1) {
> + g_free(qiov->iov);
> + }
>
> - qemu_iovec_reset(qiov);
> - g_free(qiov->iov);
> - qiov->nalloc = 0;
> - qiov->iov = NULL;
> + memset(qiov, 0, sizeof(*qiov));
> }
>
> void qemu_iovec_reset(QEMUIOVector *qiov)
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 02/12] util/iov: improve qemu_iovec_is_zero
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 01/12] util/iov: introduce qemu_iovec_init_extended Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 03/12] block/io: refactor padding Vladimir Sementsov-Ogievskiy
` (13 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
We'll need to check a part of qiov soon, so implement it now.
Optimization with align down to 4 * sizeof(long) is dropped due to:
1. It is strange: it aligns length of the buffer, but where is a
guarantee that buffer pointer is aligned itself?
2. buffer_is_zero() is a better place for optimizations and it has
them.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/qemu/iov.h | 2 +-
block/io.c | 2 +-
util/iov.c | 31 +++++++++++++++++++------------
3 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/include/qemu/iov.h b/include/qemu/iov.h
index f3787a0cf7..29957c8a72 100644
--- a/include/qemu/iov.h
+++ b/include/qemu/iov.h
@@ -212,7 +212,7 @@ void qemu_iovec_concat(QEMUIOVector *dst,
size_t qemu_iovec_concat_iov(QEMUIOVector *dst,
struct iovec *src_iov, unsigned int src_cnt,
size_t soffset, size_t sbytes);
-bool qemu_iovec_is_zero(QEMUIOVector *qiov);
+bool qemu_iovec_is_zero(QEMUIOVector *qiov, size_t qiov_offeset, size_t bytes);
void qemu_iovec_destroy(QEMUIOVector *qiov);
void qemu_iovec_reset(QEMUIOVector *qiov);
size_t qemu_iovec_to_buf(QEMUIOVector *qiov, size_t offset,
diff --git a/block/io.c b/block/io.c
index 3134a60a48..d02fded3b1 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1651,7 +1651,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
if (!ret && bs->detect_zeroes != BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF &&
!(flags & BDRV_REQ_ZERO_WRITE) && drv->bdrv_co_pwrite_zeroes &&
- qemu_iovec_is_zero(qiov)) {
+ qemu_iovec_is_zero(qiov, 0, qiov->size)) {
flags |= BDRV_REQ_ZERO_WRITE;
if (bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP) {
flags |= BDRV_REQ_MAY_UNMAP;
diff --git a/util/iov.c b/util/iov.c
index 39b6e31494..d988a533ce 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -451,23 +451,30 @@ void qemu_iovec_init_extended(
}
/*
- * Check if the contents of the iovecs are all zero
+ * Check if the contents of subrange of qiov data is all zeroes.
*/
-bool qemu_iovec_is_zero(QEMUIOVector *qiov)
+bool qemu_iovec_is_zero(QEMUIOVector *qiov, size_t offset, size_t bytes)
{
- int i;
- for (i = 0; i < qiov->niov; i++) {
- size_t offs = QEMU_ALIGN_DOWN(qiov->iov[i].iov_len, 4 * sizeof(long));
- uint8_t *ptr = qiov->iov[i].iov_base;
- if (offs && !buffer_is_zero(qiov->iov[i].iov_base, offs)) {
+ struct iovec *iov;
+ size_t current_offset;
+
+ assert(offset + bytes <= qiov->size);
+
+ iov = iov_skip_offset(qiov->iov, offset, ¤t_offset);
+
+ while (bytes) {
+ uint8_t *base = (uint8_t *)iov->iov_base + current_offset;
+ size_t len = MIN(iov->iov_len - current_offset, bytes);
+
+ if (!buffer_is_zero(base, len)) {
return false;
}
- for (; offs < qiov->iov[i].iov_len; offs++) {
- if (ptr[offs]) {
- return false;
- }
- }
+
+ current_offset = 0;
+ bytes -= len;
+ iov++;
}
+
return true;
}
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 03/12] block/io: refactor padding
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 01/12] util/iov: introduce qemu_iovec_init_extended Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 02/12] util/iov: improve qemu_iovec_is_zero Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 04/12] block: define .*_part io handlers in BlockDriver Vladimir Sementsov-Ogievskiy
` (12 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
We have similar padding code in bdrv_co_pwritev,
bdrv_co_do_pwrite_zeroes and bdrv_co_preadv. Let's combine and unify
it.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 355 ++++++++++++++++++++++++++++-------------------------
1 file changed, 190 insertions(+), 165 deletions(-)
diff --git a/block/io.c b/block/io.c
index d02fded3b1..ddd5902dc1 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1344,28 +1344,167 @@ out:
}
/*
- * Handle a read request in coroutine context
+ * Request padding
+ *
+ * |<---- align ----->| |<----- align ---->|
+ * |<- head ->|<------------- bytes ------------->|<-- tail -->|
+ * | | | | | |
+ * -*----------$-------*-------- ... --------*-----$------------*---
+ * | | | | | |
+ * | offset | | end |
+ * ALIGN_DOWN(offset) ALIGN_UP(offset) ALIGN_DOWN(end) ALIGN_UP(end)
+ * [buf ... ) [tail_buf )
+ *
+ * @buf is an aligned allocation needed to store @head and @tail paddings. @head
+ * is placed at the beginning of @buf and @tail at the @end.
+ *
+ * @tail_buf is a pointer to sub-buffer, corresponding to align-sized chunk
+ * around tail, if tail exists.
+ *
+ * @merge_reads is true for small requests,
+ * if @buf_len == @head + bytes + @tail. In this case it is possible that both
+ * head and tail exist but @buf_len == align and @tail_buf == @buf.
*/
+typedef struct BdrvRequestPadding {
+ uint8_t *buf;
+ size_t buf_len;
+ uint8_t *tail_buf;
+ size_t head;
+ size_t tail;
+ bool merge_reads;
+ QEMUIOVector local_qiov;
+} BdrvRequestPadding;
+
+static bool bdrv_init_padding(BlockDriverState *bs,
+ int64_t offset, int64_t bytes,
+ BdrvRequestPadding *pad)
+{
+ uint64_t align = bs->bl.request_alignment;
+ size_t sum;
+
+ memset(pad, 0, sizeof(*pad));
+
+ pad->head = offset & (align - 1);
+ pad->tail = ((offset + bytes) & (align - 1));
+ if (pad->tail) {
+ pad->tail = align - pad->tail;
+ }
+
+ if ((!pad->head && !pad->tail) || !bytes) {
+ return false;
+ }
+
+ sum = pad->head + bytes + pad->tail;
+ pad->buf_len = (sum > align && pad->head && pad->tail) ? 2 * align : align;
+ pad->buf = qemu_blockalign(bs, pad->buf_len);
+ pad->merge_reads = sum == pad->buf_len;
+ if (pad->tail) {
+ pad->tail_buf = pad->buf + pad->buf_len - align;
+ }
+
+ return true;
+}
+
+static int bdrv_padding_rmw_read(BdrvChild *child,
+ BdrvTrackedRequest *req,
+ BdrvRequestPadding *pad,
+ bool zero_middle)
+{
+ QEMUIOVector local_qiov;
+ BlockDriverState *bs = child->bs;
+ uint64_t align = bs->bl.request_alignment;
+ int ret;
+
+ assert(req->serialising && pad->buf);
+
+ if (pad->head || pad->merge_reads) {
+ uint64_t bytes = pad->merge_reads ? pad->buf_len : align;
+
+ qemu_iovec_init_buf(&local_qiov, pad->buf, bytes);
+
+ bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
+ ret = bdrv_aligned_preadv(child, req, req->overlap_offset, bytes,
+ align, &local_qiov, 0);
+ if (ret < 0) {
+ return ret;
+ }
+ bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
+
+ if (pad->merge_reads) {
+ goto zero_mem;
+ }
+ }
+
+ if (pad->tail) {
+ qemu_iovec_init_buf(&local_qiov, pad->tail_buf, align);
+
+ bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
+ ret = bdrv_aligned_preadv(
+ child, req,
+ req->overlap_offset + req->overlap_bytes - align,
+ align, align, &local_qiov, 0);
+ if (ret < 0) {
+ return ret;
+ }
+ bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
+ }
+
+zero_mem:
+ if (zero_middle) {
+ memset(pad->buf + pad->head, 0, pad->buf_len - pad->head - pad->tail);
+ }
+
+ return 0;
+}
+
+static void bdrv_padding_destroy(BdrvRequestPadding *pad)
+{
+ if (pad->buf) {
+ qemu_vfree(pad->buf);
+ qemu_iovec_destroy(&pad->local_qiov);
+ }
+}
+
+/*
+ * bdrv_pad_request
+ *
+ * Exchange request parameters with padded request if needed. Don't include RMW
+ * read of padding, bdrv_padding_rmw_read() should be called separately if
+ * needed.
+ *
+ * All parameters except @bs are in-out: they represent original request at
+ * function call and padded (if padding needed) at function finish.
+ *
+ * Function always succeeds.
+ */
+static bool bdrv_pad_request(BlockDriverState *bs, QEMUIOVector **qiov,
+ int64_t *offset, unsigned int *bytes,
+ BdrvRequestPadding *pad)
+{
+ if (!bdrv_init_padding(bs, *offset, *bytes, pad)) {
+ return false;
+ }
+
+ qemu_iovec_init_extended(&pad->local_qiov, pad->buf, pad->head,
+ *qiov, 0, *bytes,
+ pad->buf + pad->buf_len - pad->tail, pad->tail);
+ *bytes += pad->head + pad->tail;
+ *offset -= pad->head;
+ *qiov = &pad->local_qiov;
+
+ return true;
+}
+
int coroutine_fn bdrv_co_preadv(BdrvChild *child,
int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
BdrvRequestFlags flags)
{
BlockDriverState *bs = child->bs;
- BlockDriver *drv = bs->drv;
BdrvTrackedRequest req;
-
- uint64_t align = bs->bl.request_alignment;
- uint8_t *head_buf = NULL;
- uint8_t *tail_buf = NULL;
- QEMUIOVector local_qiov;
- bool use_local_qiov = false;
+ BdrvRequestPadding pad;
int ret;
- trace_bdrv_co_preadv(child->bs, offset, bytes, flags);
-
- if (!drv) {
- return -ENOMEDIUM;
- }
+ trace_bdrv_co_preadv(bs, offset, bytes, flags);
ret = bdrv_check_byte_request(bs, offset, bytes);
if (ret < 0) {
@@ -1379,43 +1518,16 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
flags |= BDRV_REQ_COPY_ON_READ;
}
- /* Align read if necessary by padding qiov */
- if (offset & (align - 1)) {
- head_buf = qemu_blockalign(bs, align);
- qemu_iovec_init(&local_qiov, qiov->niov + 2);
- qemu_iovec_add(&local_qiov, head_buf, offset & (align - 1));
- qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
- use_local_qiov = true;
-
- bytes += offset & (align - 1);
- offset = offset & ~(align - 1);
- }
-
- if ((offset + bytes) & (align - 1)) {
- if (!use_local_qiov) {
- qemu_iovec_init(&local_qiov, qiov->niov + 1);
- qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
- use_local_qiov = true;
- }
- tail_buf = qemu_blockalign(bs, align);
- qemu_iovec_add(&local_qiov, tail_buf,
- align - ((offset + bytes) & (align - 1)));
-
- bytes = ROUND_UP(bytes, align);
- }
+ bdrv_pad_request(bs, &qiov, &offset, &bytes, &pad);
tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_READ);
- ret = bdrv_aligned_preadv(child, &req, offset, bytes, align,
- use_local_qiov ? &local_qiov : qiov,
- flags);
+ ret = bdrv_aligned_preadv(child, &req, offset, bytes,
+ bs->bl.request_alignment,
+ qiov, flags);
tracked_request_end(&req);
bdrv_dec_in_flight(bs);
- if (use_local_qiov) {
- qemu_iovec_destroy(&local_qiov);
- qemu_vfree(head_buf);
- qemu_vfree(tail_buf);
- }
+ bdrv_padding_destroy(&pad);
return ret;
}
@@ -1711,44 +1823,34 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child,
BdrvTrackedRequest *req)
{
BlockDriverState *bs = child->bs;
- uint8_t *buf = NULL;
QEMUIOVector local_qiov;
uint64_t align = bs->bl.request_alignment;
- unsigned int head_padding_bytes, tail_padding_bytes;
int ret = 0;
+ bool padding;
+ BdrvRequestPadding pad;
- head_padding_bytes = offset & (align - 1);
- tail_padding_bytes = (align - (offset + bytes)) & (align - 1);
-
-
- assert(flags & BDRV_REQ_ZERO_WRITE);
- if (head_padding_bytes || tail_padding_bytes) {
- buf = qemu_blockalign(bs, align);
- qemu_iovec_init_buf(&local_qiov, buf, align);
- }
- if (head_padding_bytes) {
- uint64_t zero_bytes = MIN(bytes, align - head_padding_bytes);
-
- /* RMW the unaligned part before head. */
+ padding = bdrv_init_padding(bs, offset, bytes, &pad);
+ if (padding) {
mark_request_serialising(req, align);
wait_serialising_requests(req);
- bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
- ret = bdrv_aligned_preadv(child, req, offset & ~(align - 1), align,
- align, &local_qiov, 0);
- if (ret < 0) {
- goto fail;
- }
- bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
- memset(buf + head_padding_bytes, 0, zero_bytes);
- ret = bdrv_aligned_pwritev(child, req, offset & ~(align - 1), align,
- align, &local_qiov,
- flags & ~BDRV_REQ_ZERO_WRITE);
- if (ret < 0) {
- goto fail;
+ bdrv_padding_rmw_read(child, req, &pad, true);
+
+ if (pad.head || pad.merge_reads) {
+ int64_t aligned_offset = offset & ~(align - 1);
+ int64_t write_bytes = pad.merge_reads ? pad.buf_len : align;
+
+ qemu_iovec_init_buf(&local_qiov, pad.buf, write_bytes);
+ ret = bdrv_aligned_pwritev(child, req, aligned_offset, write_bytes,
+ align, &local_qiov,
+ flags & ~BDRV_REQ_ZERO_WRITE);
+ if (ret < 0 || pad.merge_reads) {
+ /* Error or all work is done */
+ goto out;
+ }
+ offset += write_bytes - pad.head;
+ bytes -= write_bytes - pad.head;
}
- offset += zero_bytes;
- bytes -= zero_bytes;
}
assert(!bytes || (offset & (align - 1)) == 0);
@@ -1758,7 +1860,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child,
ret = bdrv_aligned_pwritev(child, req, offset, aligned_bytes, align,
NULL, flags);
if (ret < 0) {
- goto fail;
+ goto out;
}
bytes -= aligned_bytes;
offset += aligned_bytes;
@@ -1766,26 +1868,17 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child,
assert(!bytes || (offset & (align - 1)) == 0);
if (bytes) {
- assert(align == tail_padding_bytes + bytes);
- /* RMW the unaligned part after tail. */
- mark_request_serialising(req, align);
- wait_serialising_requests(req);
- bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
- ret = bdrv_aligned_preadv(child, req, offset, align,
- align, &local_qiov, 0);
- if (ret < 0) {
- goto fail;
- }
- bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
+ assert(align == pad.tail + bytes);
- memset(buf, 0, bytes);
+ qemu_iovec_init_buf(&local_qiov, pad.tail_buf, align);
ret = bdrv_aligned_pwritev(child, req, offset, align, align,
&local_qiov, flags & ~BDRV_REQ_ZERO_WRITE);
}
-fail:
- qemu_vfree(buf);
- return ret;
+out:
+ bdrv_padding_destroy(&pad);
+
+ return ret;
}
/*
@@ -1798,10 +1891,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
BlockDriverState *bs = child->bs;
BdrvTrackedRequest req;
uint64_t align = bs->bl.request_alignment;
- uint8_t *head_buf = NULL;
- uint8_t *tail_buf = NULL;
- QEMUIOVector local_qiov;
- bool use_local_qiov = false;
+ BdrvRequestPadding pad;
int ret;
trace_bdrv_co_pwritev(child->bs, offset, bytes, flags);
@@ -1828,86 +1918,21 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
goto out;
}
- if (offset & (align - 1)) {
- QEMUIOVector head_qiov;
-
+ if (bdrv_pad_request(bs, &qiov, &offset, &bytes, &pad)) {
mark_request_serialising(&req, align);
wait_serialising_requests(&req);
-
- head_buf = qemu_blockalign(bs, align);
- qemu_iovec_init_buf(&head_qiov, head_buf, align);
-
- bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
- ret = bdrv_aligned_preadv(child, &req, offset & ~(align - 1), align,
- align, &head_qiov, 0);
- if (ret < 0) {
- goto fail;
- }
- bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
-
- qemu_iovec_init(&local_qiov, qiov->niov + 2);
- qemu_iovec_add(&local_qiov, head_buf, offset & (align - 1));
- qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
- use_local_qiov = true;
-
- bytes += offset & (align - 1);
- offset = offset & ~(align - 1);
-
- /* We have read the tail already if the request is smaller
- * than one aligned block.
- */
- if (bytes < align) {
- qemu_iovec_add(&local_qiov, head_buf + bytes, align - bytes);
- bytes = align;
- }
- }
-
- if ((offset + bytes) & (align - 1)) {
- QEMUIOVector tail_qiov;
- size_t tail_bytes;
- bool waited;
-
- mark_request_serialising(&req, align);
- waited = wait_serialising_requests(&req);
- assert(!waited || !use_local_qiov);
-
- tail_buf = qemu_blockalign(bs, align);
- qemu_iovec_init_buf(&tail_qiov, tail_buf, align);
-
- bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
- ret = bdrv_aligned_preadv(child, &req, (offset + bytes) & ~(align - 1),
- align, align, &tail_qiov, 0);
- if (ret < 0) {
- goto fail;
- }
- bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
-
- if (!use_local_qiov) {
- qemu_iovec_init(&local_qiov, qiov->niov + 1);
- qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
- use_local_qiov = true;
- }
-
- tail_bytes = (offset + bytes) & (align - 1);
- qemu_iovec_add(&local_qiov, tail_buf + tail_bytes, align - tail_bytes);
-
- bytes = ROUND_UP(bytes, align);
+ bdrv_padding_rmw_read(child, &req, &pad, false);
}
ret = bdrv_aligned_pwritev(child, &req, offset, bytes, align,
- use_local_qiov ? &local_qiov : qiov,
- flags);
+ qiov, flags);
-fail:
+ bdrv_padding_destroy(&pad);
- if (use_local_qiov) {
- qemu_iovec_destroy(&local_qiov);
- }
- qemu_vfree(head_buf);
- qemu_vfree(tail_buf);
out:
tracked_request_end(&req);
bdrv_dec_in_flight(bs);
+
return ret;
}
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 04/12] block: define .*_part io handlers in BlockDriver
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 03/12] block/io: refactor padding Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 05/12] block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset Vladimir Sementsov-Ogievskiy
` (11 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
Add handlers supporting qiov_offset parameter:
bdrv_co_preadv_part
bdrv_co_pwritev_part
bdrv_co_pwritev_compressed_part
This is used to reduce need of defining local_qiovs and hd_qiovs in all
corners of block layer code. The following patches will increase usage
of this new API part by part.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 15 ++++++
block/backup.c | 2 +-
block/io.c | 96 +++++++++++++++++++++++++++++++--------
qemu-img.c | 4 +-
4 files changed, 95 insertions(+), 22 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1eebc7c8f3..46e17d2e2f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -211,6 +211,9 @@ struct BlockDriver {
*/
int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
+ int coroutine_fn (*bdrv_co_preadv_part)(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset, int flags);
int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int flags);
/**
@@ -230,6 +233,9 @@ struct BlockDriver {
*/
int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
+ int coroutine_fn (*bdrv_co_pwritev_part)(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset, int flags);
/*
* Efficiently zero a region of the disk image. Typically an image format
@@ -340,6 +346,9 @@ struct BlockDriver {
int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
+ int coroutine_fn (*bdrv_co_pwritev_compressed_part)(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes, QEMUIOVector *qiov,
+ size_t qiov_offset);
int (*bdrv_snapshot_create)(BlockDriverState *bs,
QEMUSnapshotInfo *sn_info);
@@ -564,6 +573,12 @@ struct BlockDriver {
const char *const *strong_runtime_opts;
};
+static inline bool block_driver_can_compress(BlockDriver *drv)
+{
+ return drv->bdrv_co_pwritev_compressed ||
+ drv->bdrv_co_pwritev_compressed_part;
+}
+
typedef struct BlockLimits {
/* Alignment requirement, in bytes, for offset/length of I/O
* requests. Must be a power of 2 less than INT_MAX; defaults to
diff --git a/block/backup.c b/block/backup.c
index 00f4f8af53..af821b8990 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -569,7 +569,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
return NULL;
}
- if (compress && target->drv->bdrv_co_pwritev_compressed == NULL) {
+ if (compress && !block_driver_can_compress(target->drv)) {
error_setg(errp, "Compression is not supported for this drive %s",
bdrv_get_device_name(target));
return NULL;
diff --git a/block/io.c b/block/io.c
index ddd5902dc1..7e53a11492 100644
--- a/block/io.c
+++ b/block/io.c
@@ -128,7 +128,8 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
/* Default alignment based on whether driver has byte interface */
bs->bl.request_alignment = (drv->bdrv_co_preadv ||
- drv->bdrv_aio_preadv) ? 1 : 512;
+ drv->bdrv_aio_preadv ||
+ drv->bdrv_co_preadv_part) ? 1 : 512;
/* Take some limits from the children as a default */
if (bs->file) {
@@ -979,11 +980,14 @@ static void bdrv_co_io_em_complete(void *opaque, int ret)
static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ QEMUIOVector *qiov,
+ size_t qiov_offset, int flags)
{
BlockDriver *drv = bs->drv;
int64_t sector_num;
unsigned int nb_sectors;
+ QEMUIOVector local_qiov;
+ int ret;
assert(!(flags & ~BDRV_REQ_MASK));
assert(!(flags & BDRV_REQ_NO_FALLBACK));
@@ -992,8 +996,19 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
return -ENOMEDIUM;
}
+ if (drv->bdrv_co_preadv_part) {
+ return drv->bdrv_co_preadv_part(bs, offset, bytes, qiov, qiov_offset,
+ flags);
+ }
+
+ if (qiov_offset > 0 || bytes != qiov->size) {
+ qemu_iovec_init_slice(&local_qiov, qiov, qiov_offset, bytes);
+ qiov = &local_qiov;
+ }
+
if (drv->bdrv_co_preadv) {
- return drv->bdrv_co_preadv(bs, offset, bytes, qiov, flags);
+ ret = drv->bdrv_co_preadv(bs, offset, bytes, qiov, flags);
+ goto out;
}
if (drv->bdrv_aio_preadv) {
@@ -1005,10 +1020,12 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
acb = drv->bdrv_aio_preadv(bs, offset, bytes, qiov, flags,
bdrv_co_io_em_complete, &co);
if (acb == NULL) {
- return -EIO;
+ ret = -EIO;
+ goto out;
} else {
qemu_coroutine_yield();
- return co.ret;
+ ret = co.ret;
+ goto out;
}
}
@@ -1020,16 +1037,25 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
assert(bytes <= BDRV_REQUEST_MAX_BYTES);
assert(drv->bdrv_co_readv);
- return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+ ret = drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+
+out:
+ if (qiov == &local_qiov) {
+ qemu_iovec_destroy(&local_qiov);
+ }
+
+ return ret;
}
static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ QEMUIOVector *qiov,
+ size_t qiov_offset, int flags)
{
BlockDriver *drv = bs->drv;
int64_t sector_num;
unsigned int nb_sectors;
+ QEMUIOVector local_qiov;
int ret;
assert(!(flags & ~BDRV_REQ_MASK));
@@ -1039,6 +1065,18 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
return -ENOMEDIUM;
}
+ if (drv->bdrv_co_pwritev_part) {
+ ret = drv->bdrv_co_pwritev_part(bs, offset, bytes, qiov, qiov_offset,
+ flags & bs->supported_write_flags);
+ flags &= ~bs->supported_write_flags;
+ goto emulate_flags;
+ }
+
+ if (qiov_offset > 0 || bytes != qiov->size) {
+ qemu_iovec_init_slice(&local_qiov, qiov, qiov_offset, bytes);
+ qiov = &local_qiov;
+ }
+
if (drv->bdrv_co_pwritev) {
ret = drv->bdrv_co_pwritev(bs, offset, bytes, qiov,
flags & bs->supported_write_flags);
@@ -1082,24 +1120,44 @@ emulate_flags:
ret = bdrv_co_flush(bs);
}
+ if (qiov == &local_qiov) {
+ qemu_iovec_destroy(&local_qiov);
+ }
+
return ret;
}
static int coroutine_fn
bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov)
+ uint64_t bytes, QEMUIOVector *qiov,
+ size_t qiov_offset)
{
BlockDriver *drv = bs->drv;
+ QEMUIOVector local_qiov;
+ int ret;
if (!drv) {
return -ENOMEDIUM;
}
- if (!drv->bdrv_co_pwritev_compressed) {
+ if (!block_driver_can_compress(drv)) {
return -ENOTSUP;
}
- return drv->bdrv_co_pwritev_compressed(bs, offset, bytes, qiov);
+ if (drv->bdrv_co_pwritev_compressed_part) {
+ return drv->bdrv_co_pwritev_compressed_part(bs, offset, bytes,
+ qiov, qiov_offset);
+ }
+
+ if (qiov_offset == 0) {
+ return drv->bdrv_co_pwritev_compressed(bs, offset, bytes, qiov);
+ }
+
+ qemu_iovec_init_slice(&local_qiov, qiov, qiov_offset, bytes);
+ ret = drv->bdrv_co_pwritev_compressed(bs, offset, bytes, &local_qiov);
+ qemu_iovec_destroy(&local_qiov);
+
+ return ret;
}
static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
@@ -1183,7 +1241,7 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
qemu_iovec_init_buf(&local_qiov, bounce_buffer, pnum);
ret = bdrv_driver_preadv(bs, cluster_offset, pnum,
- &local_qiov, 0);
+ &local_qiov, 0, 0);
if (ret < 0) {
goto err;
}
@@ -1201,7 +1259,7 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
* necessary to flush even in cache=writethrough mode.
*/
ret = bdrv_driver_pwritev(bs, cluster_offset, pnum,
- &local_qiov,
+ &local_qiov, 0,
BDRV_REQ_WRITE_UNCHANGED);
}
@@ -1221,7 +1279,7 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
qemu_iovec_init(&local_qiov, qiov->niov);
qemu_iovec_concat(&local_qiov, qiov, progress, pnum - skip_bytes);
ret = bdrv_driver_preadv(bs, offset + progress, local_qiov.size,
- &local_qiov, 0);
+ &local_qiov, 0, 0);
qemu_iovec_destroy(&local_qiov);
if (ret < 0) {
goto err;
@@ -1309,7 +1367,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
max_bytes = ROUND_UP(MAX(0, total_bytes - offset), align);
if (bytes <= max_bytes && bytes <= max_transfer) {
- ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0);
+ ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0, 0);
goto out;
}
@@ -1325,7 +1383,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
qemu_iovec_concat(&local_qiov, qiov, bytes - bytes_remaining, num);
ret = bdrv_driver_preadv(bs, offset + bytes - bytes_remaining,
- num, &local_qiov, 0);
+ num, &local_qiov, 0, 0);
max_bytes -= num;
qemu_iovec_destroy(&local_qiov);
} else {
@@ -1620,7 +1678,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
}
qemu_iovec_init_buf(&qiov, buf, num);
- ret = bdrv_driver_pwritev(bs, offset, num, &qiov, write_flags);
+ ret = bdrv_driver_pwritev(bs, offset, num, &qiov, 0, write_flags);
/* Keep bounce buffer around if it is big enough for all
* all future requests.
@@ -1776,10 +1834,10 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
bdrv_debug_event(bs, BLKDBG_PWRITEV_ZERO);
ret = bdrv_co_do_pwrite_zeroes(bs, offset, bytes, flags);
} else if (flags & BDRV_REQ_WRITE_COMPRESSED) {
- ret = bdrv_driver_pwritev_compressed(bs, offset, bytes, qiov);
+ ret = bdrv_driver_pwritev_compressed(bs, offset, bytes, qiov, 0);
} else if (bytes <= max_transfer) {
bdrv_debug_event(bs, BLKDBG_PWRITEV);
- ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, flags);
+ ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, 0, flags);
} else {
bdrv_debug_event(bs, BLKDBG_PWRITEV);
while (bytes_remaining) {
@@ -1798,7 +1856,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
qemu_iovec_concat(&local_qiov, qiov, bytes - bytes_remaining, num);
ret = bdrv_driver_pwritev(bs, offset + bytes - bytes_remaining,
- num, &local_qiov, local_flags);
+ num, &local_qiov, 0, local_flags);
qemu_iovec_destroy(&local_qiov);
if (ret < 0) {
break;
diff --git a/qemu-img.c b/qemu-img.c
index b0535919b1..8a62a75a71 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2329,7 +2329,7 @@ static int img_convert(int argc, char **argv)
const char *preallocation =
qemu_opt_get(opts, BLOCK_OPT_PREALLOC);
- if (drv && !drv->bdrv_co_pwritev_compressed) {
+ if (drv && !block_driver_can_compress(drv)) {
error_report("Compression not supported for this file format");
ret = -1;
goto out;
@@ -2398,7 +2398,7 @@ static int img_convert(int argc, char **argv)
}
out_bs = blk_bs(s.target);
- if (s.compressed && !out_bs->drv->bdrv_co_pwritev_compressed) {
+ if (s.compressed && !block_driver_can_compress(out_bs->drv)) {
error_report("Compression not supported for this file format");
ret = -1;
goto out;
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 05/12] block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 04/12] block: define .*_part io handlers in BlockDriver Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 06/12] block/io: bdrv_co_do_copy_on_readv: lazy allocation Vladimir Sementsov-Ogievskiy
` (10 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
Use and support new API in bdrv_co_do_copy_on_readv. Note that in case
of allocated-in-top we need to shrink read size to MIN(..) by hand, as
pre-patch this was actually done implicitly by qemu_iovec_concat (and
we used local_qiov.size).
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/block/io.c b/block/io.c
index 7e53a11492..efd2b80293 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1161,7 +1161,8 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
}
static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
- int64_t offset, unsigned int bytes, QEMUIOVector *qiov)
+ int64_t offset, unsigned int bytes,
+ QEMUIOVector *qiov, size_t qiov_offset)
{
BlockDriverState *bs = child->bs;
@@ -1173,7 +1174,6 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
void *bounce_buffer;
BlockDriver *drv = bs->drv;
- QEMUIOVector local_qiov;
int64_t cluster_offset;
int64_t cluster_bytes;
size_t skip_bytes;
@@ -1236,6 +1236,8 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
assert(skip_bytes < pnum);
if (ret <= 0) {
+ QEMUIOVector local_qiov;
+
/* Must copy-on-read; use the bounce buffer */
pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
qemu_iovec_init_buf(&local_qiov, bounce_buffer, pnum);
@@ -1272,15 +1274,14 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
goto err;
}
- qemu_iovec_from_buf(qiov, progress, bounce_buffer + skip_bytes,
+ qemu_iovec_from_buf(qiov, qiov_offset + progress,
+ bounce_buffer + skip_bytes,
pnum - skip_bytes);
} else {
/* Read directly into the destination */
- qemu_iovec_init(&local_qiov, qiov->niov);
- qemu_iovec_concat(&local_qiov, qiov, progress, pnum - skip_bytes);
- ret = bdrv_driver_preadv(bs, offset + progress, local_qiov.size,
- &local_qiov, 0, 0);
- qemu_iovec_destroy(&local_qiov);
+ ret = bdrv_driver_preadv(bs, offset + progress,
+ MIN(pnum - skip_bytes, bytes - progress),
+ qiov, qiov_offset + progress, 0);
if (ret < 0) {
goto err;
}
@@ -1353,7 +1354,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
}
if (!ret || pnum != bytes) {
- ret = bdrv_co_do_copy_on_readv(child, offset, bytes, qiov);
+ ret = bdrv_co_do_copy_on_readv(child, offset, bytes, qiov, 0);
goto out;
}
}
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 06/12] block/io: bdrv_co_do_copy_on_readv: lazy allocation
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 05/12] block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 07/12] block/io: bdrv_aligned_preadv: use and support qiov_offset Vladimir Sementsov-Ogievskiy
` (9 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
Allocate bounce_buffer only if it is really needed. Also, sub-optimize
allocation size (why not?).
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/block/io.c b/block/io.c
index efd2b80293..a4f67aca47 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1171,7 +1171,7 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
* modifying the image file. This is critical for zero-copy guest I/O
* where anything might happen inside guest memory.
*/
- void *bounce_buffer;
+ void *bounce_buffer = NULL;
BlockDriver *drv = bs->drv;
int64_t cluster_offset;
@@ -1206,14 +1206,6 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
cluster_offset, cluster_bytes);
- bounce_buffer = qemu_try_blockalign(bs,
- MIN(MIN(max_transfer, cluster_bytes),
- MAX_BOUNCE_BUFFER));
- if (bounce_buffer == NULL) {
- ret = -ENOMEM;
- goto err;
- }
-
while (cluster_bytes) {
int64_t pnum;
@@ -1240,6 +1232,17 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
/* Must copy-on-read; use the bounce buffer */
pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
+ if (!bounce_buffer) {
+ int64_t max_we_need = MAX(pnum, cluster_bytes - pnum);
+ int64_t max_allowed = MIN(max_transfer, MAX_BOUNCE_BUFFER);
+ int64_t bounce_buffer_len = MIN(max_we_need, max_allowed);
+
+ bounce_buffer = qemu_try_blockalign(bs, bounce_buffer_len);
+ if (!bounce_buffer) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ }
qemu_iovec_init_buf(&local_qiov, bounce_buffer, pnum);
ret = bdrv_driver_preadv(bs, cluster_offset, pnum,
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 07/12] block/io: bdrv_aligned_preadv: use and support qiov_offset
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 06/12] block/io: bdrv_co_do_copy_on_readv: lazy allocation Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 08/12] block/io: bdrv_aligned_pwritev: " Vladimir Sementsov-Ogievskiy
` (8 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
Use and support new API in bdrv_co_do_copy_on_readv.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/block/io.c b/block/io.c
index a4f67aca47..0cc80e2d5a 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1309,7 +1309,7 @@ err:
*/
static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
- int64_t align, QEMUIOVector *qiov, int flags)
+ int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
{
BlockDriverState *bs = child->bs;
int64_t total_bytes, max_bytes;
@@ -1320,7 +1320,6 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
assert(is_power_of_2(align));
assert((offset & (align - 1)) == 0);
assert((bytes & (align - 1)) == 0);
- assert(!qiov || bytes == qiov->size);
assert((bs->open_flags & BDRV_O_NO_IO) == 0);
max_transfer = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_transfer, INT_MAX),
align);
@@ -1357,7 +1356,8 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
}
if (!ret || pnum != bytes) {
- ret = bdrv_co_do_copy_on_readv(child, offset, bytes, qiov, 0);
+ ret = bdrv_co_do_copy_on_readv(child, offset, bytes,
+ qiov, qiov_offset);
goto out;
}
}
@@ -1371,7 +1371,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
max_bytes = ROUND_UP(MAX(0, total_bytes - offset), align);
if (bytes <= max_bytes && bytes <= max_transfer) {
- ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0, 0);
+ ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, 0);
goto out;
}
@@ -1379,17 +1379,12 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
int num;
if (max_bytes) {
- QEMUIOVector local_qiov;
-
num = MIN(bytes_remaining, MIN(max_bytes, max_transfer));
assert(num);
- qemu_iovec_init(&local_qiov, qiov->niov);
- qemu_iovec_concat(&local_qiov, qiov, bytes - bytes_remaining, num);
ret = bdrv_driver_preadv(bs, offset + bytes - bytes_remaining,
- num, &local_qiov, 0, 0);
+ num, qiov, bytes - bytes_remaining, 0);
max_bytes -= num;
- qemu_iovec_destroy(&local_qiov);
} else {
num = bytes_remaining;
ret = qemu_iovec_memset(qiov, bytes - bytes_remaining, 0,
@@ -1486,7 +1481,7 @@ static int bdrv_padding_rmw_read(BdrvChild *child,
bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
ret = bdrv_aligned_preadv(child, req, req->overlap_offset, bytes,
- align, &local_qiov, 0);
+ align, &local_qiov, 0, 0);
if (ret < 0) {
return ret;
}
@@ -1504,7 +1499,7 @@ static int bdrv_padding_rmw_read(BdrvChild *child,
ret = bdrv_aligned_preadv(
child, req,
req->overlap_offset + req->overlap_bytes - align,
- align, align, &local_qiov, 0);
+ align, align, &local_qiov, 0, 0);
if (ret < 0) {
return ret;
}
@@ -1585,7 +1580,7 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_READ);
ret = bdrv_aligned_preadv(child, &req, offset, bytes,
bs->bl.request_alignment,
- qiov, flags);
+ qiov, 0, flags);
tracked_request_end(&req);
bdrv_dec_in_flight(bs);
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 08/12] block/io: bdrv_aligned_pwritev: use and support qiov_offset
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (6 preceding siblings ...)
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 07/12] block/io: bdrv_aligned_preadv: use and support qiov_offset Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 09/12] block/io: introduce bdrv_co_p{read, write}v_part Vladimir Sementsov-Ogievskiy
` (7 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
Use and support new API in bdrv_aligned_pwritev.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/block/io.c b/block/io.c
index 0cc80e2d5a..660c96527d 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1792,7 +1792,7 @@ bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, uint64_t bytes,
*/
static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
- int64_t align, QEMUIOVector *qiov, int flags)
+ int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
{
BlockDriverState *bs = child->bs;
BlockDriver *drv = bs->drv;
@@ -1812,7 +1812,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
assert(is_power_of_2(align));
assert((offset & (align - 1)) == 0);
assert((bytes & (align - 1)) == 0);
- assert(!qiov || bytes == qiov->size);
+ assert(!qiov || qiov_offset + bytes <= qiov->size);
max_transfer = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_transfer, INT_MAX),
align);
@@ -1820,7 +1820,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
if (!ret && bs->detect_zeroes != BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF &&
!(flags & BDRV_REQ_ZERO_WRITE) && drv->bdrv_co_pwrite_zeroes &&
- qemu_iovec_is_zero(qiov, 0, qiov->size)) {
+ qemu_iovec_is_zero(qiov, qiov_offset, bytes)) {
flags |= BDRV_REQ_ZERO_WRITE;
if (bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP) {
flags |= BDRV_REQ_MAY_UNMAP;
@@ -1833,15 +1833,15 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
bdrv_debug_event(bs, BLKDBG_PWRITEV_ZERO);
ret = bdrv_co_do_pwrite_zeroes(bs, offset, bytes, flags);
} else if (flags & BDRV_REQ_WRITE_COMPRESSED) {
- ret = bdrv_driver_pwritev_compressed(bs, offset, bytes, qiov, 0);
+ ret = bdrv_driver_pwritev_compressed(bs, offset, bytes,
+ qiov, qiov_offset);
} else if (bytes <= max_transfer) {
bdrv_debug_event(bs, BLKDBG_PWRITEV);
- ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, 0, flags);
+ ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, qiov_offset, flags);
} else {
bdrv_debug_event(bs, BLKDBG_PWRITEV);
while (bytes_remaining) {
int num = MIN(bytes_remaining, max_transfer);
- QEMUIOVector local_qiov;
int local_flags = flags;
assert(num);
@@ -1851,12 +1851,10 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
* need to flush on the last iteration */
local_flags &= ~BDRV_REQ_FUA;
}
- qemu_iovec_init(&local_qiov, qiov->niov);
- qemu_iovec_concat(&local_qiov, qiov, bytes - bytes_remaining, num);
ret = bdrv_driver_pwritev(bs, offset + bytes - bytes_remaining,
- num, &local_qiov, 0, local_flags);
- qemu_iovec_destroy(&local_qiov);
+ num, qiov, bytes - bytes_remaining,
+ local_flags);
if (ret < 0) {
break;
}
@@ -1899,7 +1897,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child,
qemu_iovec_init_buf(&local_qiov, pad.buf, write_bytes);
ret = bdrv_aligned_pwritev(child, req, aligned_offset, write_bytes,
- align, &local_qiov,
+ align, &local_qiov, 0,
flags & ~BDRV_REQ_ZERO_WRITE);
if (ret < 0 || pad.merge_reads) {
/* Error or all work is done */
@@ -1915,7 +1913,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child,
/* Write the aligned part in the middle. */
uint64_t aligned_bytes = bytes & ~(align - 1);
ret = bdrv_aligned_pwritev(child, req, offset, aligned_bytes, align,
- NULL, flags);
+ NULL, 0, flags);
if (ret < 0) {
goto out;
}
@@ -1929,7 +1927,8 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child,
qemu_iovec_init_buf(&local_qiov, pad.tail_buf, align);
ret = bdrv_aligned_pwritev(child, req, offset, align, align,
- &local_qiov, flags & ~BDRV_REQ_ZERO_WRITE);
+ &local_qiov, 0,
+ flags & ~BDRV_REQ_ZERO_WRITE);
}
out:
@@ -1982,7 +1981,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
}
ret = bdrv_aligned_pwritev(child, &req, offset, bytes, align,
- qiov, flags);
+ qiov, 0, flags);
bdrv_padding_destroy(&pad);
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 09/12] block/io: introduce bdrv_co_p{read, write}v_part
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (7 preceding siblings ...)
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 08/12] block/io: bdrv_aligned_pwritev: " Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 10/12] block/qcow2: refactor qcow2_co_preadv to use buffer-based io Vladimir Sementsov-Ogievskiy
` (6 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
Introduce extended variants of bdrv_co_preadv and bdrv_co_pwritev
with qiov_offset parameter.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 6 ++++++
block/io.c | 29 +++++++++++++++++++++++------
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 46e17d2e2f..8b7b5d5b84 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -940,9 +940,15 @@ extern BlockDriver bdrv_qcow2;
int coroutine_fn bdrv_co_preadv(BdrvChild *child,
int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
BdrvRequestFlags flags);
+int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
+ int64_t offset, unsigned int bytes,
+ QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
BdrvRequestFlags flags);
+int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
+ int64_t offset, unsigned int bytes,
+ QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
static inline int coroutine_fn bdrv_co_pread(BdrvChild *child,
int64_t offset, unsigned int bytes, void *buf, BdrvRequestFlags flags)
diff --git a/block/io.c b/block/io.c
index 660c96527d..82051730fd 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1534,7 +1534,8 @@ static void bdrv_padding_destroy(BdrvRequestPadding *pad)
*
* Function always succeeds.
*/
-static bool bdrv_pad_request(BlockDriverState *bs, QEMUIOVector **qiov,
+static bool bdrv_pad_request(BlockDriverState *bs,
+ QEMUIOVector **qiov, size_t *qiov_offset,
int64_t *offset, unsigned int *bytes,
BdrvRequestPadding *pad)
{
@@ -1543,11 +1544,12 @@ static bool bdrv_pad_request(BlockDriverState *bs, QEMUIOVector **qiov,
}
qemu_iovec_init_extended(&pad->local_qiov, pad->buf, pad->head,
- *qiov, 0, *bytes,
+ *qiov, *qiov_offset, *bytes,
pad->buf + pad->buf_len - pad->tail, pad->tail);
*bytes += pad->head + pad->tail;
*offset -= pad->head;
*qiov = &pad->local_qiov;
+ *qiov_offset = 0;
return true;
}
@@ -1555,6 +1557,14 @@ static bool bdrv_pad_request(BlockDriverState *bs, QEMUIOVector **qiov,
int coroutine_fn bdrv_co_preadv(BdrvChild *child,
int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
BdrvRequestFlags flags)
+{
+ return bdrv_co_preadv_part(child, offset, bytes, qiov, 0, flags);
+}
+
+int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
+ int64_t offset, unsigned int bytes,
+ QEMUIOVector *qiov, size_t qiov_offset,
+ BdrvRequestFlags flags)
{
BlockDriverState *bs = child->bs;
BdrvTrackedRequest req;
@@ -1575,12 +1585,12 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
flags |= BDRV_REQ_COPY_ON_READ;
}
- bdrv_pad_request(bs, &qiov, &offset, &bytes, &pad);
+ bdrv_pad_request(bs, &qiov, &qiov_offset, &offset, &bytes, &pad);
tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_READ);
ret = bdrv_aligned_preadv(child, &req, offset, bytes,
bs->bl.request_alignment,
- qiov, 0, flags);
+ qiov, qiov_offset, flags);
tracked_request_end(&req);
bdrv_dec_in_flight(bs);
@@ -1943,6 +1953,13 @@ out:
int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
BdrvRequestFlags flags)
+{
+ return bdrv_co_pwritev_part(child, offset, bytes, qiov, 0, flags);
+}
+
+int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
+ int64_t offset, unsigned int bytes, QEMUIOVector *qiov, size_t qiov_offset,
+ BdrvRequestFlags flags)
{
BlockDriverState *bs = child->bs;
BdrvTrackedRequest req;
@@ -1974,14 +1991,14 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
goto out;
}
- if (bdrv_pad_request(bs, &qiov, &offset, &bytes, &pad)) {
+ if (bdrv_pad_request(bs, &qiov, &qiov_offset, &offset, &bytes, &pad)) {
mark_request_serialising(&req, align);
wait_serialising_requests(&req);
bdrv_padding_rmw_read(child, &req, &pad, false);
}
ret = bdrv_aligned_pwritev(child, &req, offset, bytes, align,
- qiov, 0, flags);
+ qiov, qiov_offset, flags);
bdrv_padding_destroy(&pad);
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 10/12] block/qcow2: refactor qcow2_co_preadv to use buffer-based io
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (8 preceding siblings ...)
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 09/12] block/io: introduce bdrv_co_p{read, write}v_part Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 11/12] block/qcow2: implement .bdrv_co_preadv_part Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
Use buffer based io in encrypted case.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index f2cb131048..8a033ae08c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2047,19 +2047,15 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
}
assert(cur_bytes <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
- qemu_iovec_reset(&hd_qiov);
- qemu_iovec_add(&hd_qiov, cluster_data, cur_bytes);
- }
- BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
- ret = bdrv_co_preadv(s->data_file,
- cluster_offset + offset_in_cluster,
- cur_bytes, &hd_qiov, 0);
- if (ret < 0) {
- goto fail;
- }
- if (bs->encrypted) {
- assert(s->crypto);
+ BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+ ret = bdrv_co_pread(s->data_file,
+ cluster_offset + offset_in_cluster,
+ cur_bytes, cluster_data, 0);
+ if (ret < 0) {
+ goto fail;
+ }
+
assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
if (qcow2_co_decrypt(bs, cluster_offset, offset,
@@ -2068,6 +2064,14 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
goto fail;
}
qemu_iovec_from_buf(qiov, bytes_done, cluster_data, cur_bytes);
+ } else {
+ BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+ ret = bdrv_co_preadv(s->data_file,
+ cluster_offset + offset_in_cluster,
+ cur_bytes, &hd_qiov, 0);
+ if (ret < 0) {
+ goto fail;
+ }
}
break;
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 11/12] block/qcow2: implement .bdrv_co_preadv_part
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (9 preceding siblings ...)
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 10/12] block/qcow2: refactor qcow2_co_preadv to use buffer-based io Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 12/12] block/qcow2: implement .bdrv_co_pwritev(_compressed)_part Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
Implement and use new interface to get rid of hd_qiov.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2-cluster.c | 5 +++--
block/qcow2.c | 49 +++++++++++++++++++------------------------
2 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index cf892f37a8..1159d6ed2f 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -453,8 +453,9 @@ static int coroutine_fn do_perform_cow_read(BlockDriverState *bs,
* interface. This avoids double I/O throttling and request tracking,
* which can lead to deadlock when block layer copy-on-read is enabled.
*/
- ret = bs->drv->bdrv_co_preadv(bs, src_cluster_offset + offset_in_cluster,
- qiov->size, qiov, 0);
+ ret = bs->drv->bdrv_co_preadv_part(bs,
+ src_cluster_offset + offset_in_cluster,
+ qiov->size, qiov, 0, 0);
if (ret < 0) {
return ret;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 8a033ae08c..c389ef07b0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -75,7 +75,8 @@ qcow2_co_preadv_compressed(BlockDriverState *bs,
uint64_t file_cluster_offset,
uint64_t offset,
uint64_t bytes,
- QEMUIOVector *qiov);
+ QEMUIOVector *qiov,
+ size_t qiov_offset);
static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
{
@@ -1955,21 +1956,18 @@ out:
return ret;
}
-static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
- int flags)
+static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov,
+ size_t qiov_offset, int flags)
{
BDRVQcow2State *s = bs->opaque;
int offset_in_cluster;
int ret;
unsigned int cur_bytes; /* number of bytes in current iteration */
uint64_t cluster_offset = 0;
- uint64_t bytes_done = 0;
- QEMUIOVector hd_qiov;
uint8_t *cluster_data = NULL;
- qemu_iovec_init(&hd_qiov, qiov->niov);
-
while (bytes != 0) {
/* prepare next request */
@@ -1988,34 +1986,31 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
offset_in_cluster = offset_into_cluster(s, offset);
- qemu_iovec_reset(&hd_qiov);
- qemu_iovec_concat(&hd_qiov, qiov, bytes_done, cur_bytes);
-
switch (ret) {
case QCOW2_CLUSTER_UNALLOCATED:
if (bs->backing) {
BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
- ret = bdrv_co_preadv(bs->backing, offset, cur_bytes,
- &hd_qiov, 0);
+ ret = bdrv_co_preadv_part(bs->backing, offset, cur_bytes,
+ qiov, qiov_offset, 0);
if (ret < 0) {
goto fail;
}
} else {
/* Note: in this case, no need to wait */
- qemu_iovec_memset(&hd_qiov, 0, 0, cur_bytes);
+ qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);
}
break;
case QCOW2_CLUSTER_ZERO_PLAIN:
case QCOW2_CLUSTER_ZERO_ALLOC:
- qemu_iovec_memset(&hd_qiov, 0, 0, cur_bytes);
+ qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);
break;
case QCOW2_CLUSTER_COMPRESSED:
ret = qcow2_co_preadv_compressed(bs, cluster_offset,
offset, cur_bytes,
- &hd_qiov);
+ qiov, qiov_offset);
if (ret < 0) {
goto fail;
}
@@ -2063,12 +2058,12 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
ret = -EIO;
goto fail;
}
- qemu_iovec_from_buf(qiov, bytes_done, cluster_data, cur_bytes);
+ qemu_iovec_from_buf(qiov, qiov_offset, cluster_data, cur_bytes);
} else {
BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
- ret = bdrv_co_preadv(s->data_file,
- cluster_offset + offset_in_cluster,
- cur_bytes, &hd_qiov, 0);
+ ret = bdrv_co_preadv_part(s->data_file,
+ cluster_offset + offset_in_cluster,
+ cur_bytes, qiov, qiov_offset, 0);
if (ret < 0) {
goto fail;
}
@@ -2083,12 +2078,11 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
bytes -= cur_bytes;
offset += cur_bytes;
- bytes_done += cur_bytes;
+ qiov_offset += cur_bytes;
}
ret = 0;
fail:
- qemu_iovec_destroy(&hd_qiov);
qemu_vfree(cluster_data);
return ret;
@@ -4087,7 +4081,8 @@ qcow2_co_preadv_compressed(BlockDriverState *bs,
uint64_t file_cluster_offset,
uint64_t offset,
uint64_t bytes,
- QEMUIOVector *qiov)
+ QEMUIOVector *qiov,
+ size_t qiov_offset)
{
BDRVQcow2State *s = bs->opaque;
int ret = 0, csize, nb_csectors;
@@ -4118,7 +4113,7 @@ qcow2_co_preadv_compressed(BlockDriverState *bs,
goto fail;
}
- qemu_iovec_from_buf(qiov, 0, out_buf + offset_in_cluster, bytes);
+ qemu_iovec_from_buf(qiov, qiov_offset, out_buf + offset_in_cluster, bytes);
fail:
qemu_vfree(out_buf);
@@ -4638,8 +4633,8 @@ static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
BDRVQcow2State *s = bs->opaque;
BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD);
- return bs->drv->bdrv_co_preadv(bs, qcow2_vm_state_offset(s) + pos,
- qiov->size, qiov, 0);
+ return bs->drv->bdrv_co_preadv_part(bs, qcow2_vm_state_offset(s) + pos,
+ qiov->size, qiov, 0, 0);
}
/*
@@ -5179,7 +5174,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_block_status = qcow2_co_block_status,
- .bdrv_co_preadv = qcow2_co_preadv,
+ .bdrv_co_preadv_part = qcow2_co_preadv_part,
.bdrv_co_pwritev = qcow2_co_pwritev,
.bdrv_co_flush_to_os = qcow2_co_flush_to_os,
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 12/12] block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (10 preceding siblings ...)
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 11/12] block/qcow2: implement .bdrv_co_preadv_part Vladimir Sementsov-Ogievskiy
@ 2019-06-04 16:15 ` Vladimir Sementsov-Ogievskiy
2019-06-04 17:48 ` [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io no-reply
` (3 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-06-04 16:15 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: fam, kwolf, vsementsov, mreitz, stefanha, den, jsnow
Implement and use new interface to get rid of hd_qiov.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2.h | 1 +
include/qemu/iov.h | 1 +
block/qcow2-cluster.c | 9 ++++---
block/qcow2.c | 60 +++++++++++++++++++++----------------------
util/iov.c | 10 ++++++++
5 files changed, 48 insertions(+), 33 deletions(-)
diff --git a/block/qcow2.h b/block/qcow2.h
index 567375e56c..ea3bdb0699 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -417,6 +417,7 @@ typedef struct QCowL2Meta
* from @cow_start and @cow_end into one single write operation.
*/
QEMUIOVector *data_qiov;
+ size_t data_qiov_offset;
/** Pointer to next L2Meta of the same write request */
struct QCowL2Meta *next;
diff --git a/include/qemu/iov.h b/include/qemu/iov.h
index 29957c8a72..bffc151282 100644
--- a/include/qemu/iov.h
+++ b/include/qemu/iov.h
@@ -206,6 +206,7 @@ void qemu_iovec_init_extended(
void *tail_buf, size_t tail_len);
void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source,
size_t offset, size_t len);
+int qemu_iovec_subvec_niov(QEMUIOVector *qiov, size_t offset, size_t len);
void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len);
void qemu_iovec_concat(QEMUIOVector *dst,
QEMUIOVector *src, size_t soffset, size_t sbytes);
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1159d6ed2f..00b1a9ab8d 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -830,7 +830,6 @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
assert(start->nb_bytes <= UINT_MAX - end->nb_bytes);
assert(start->nb_bytes + end->nb_bytes <= UINT_MAX - data_bytes);
assert(start->offset + start->nb_bytes <= end->offset);
- assert(!m->data_qiov || m->data_qiov->size == data_bytes);
if ((start->nb_bytes == 0 && end->nb_bytes == 0) || m->skip_cow) {
return 0;
@@ -862,7 +861,11 @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
/* The part of the buffer where the end region is located */
end_buffer = start_buffer + buffer_size - end->nb_bytes;
- qemu_iovec_init(&qiov, 2 + (m->data_qiov ? m->data_qiov->niov : 0));
+ qemu_iovec_init(&qiov, 2 + (m->data_qiov ?
+ qemu_iovec_subvec_niov(m->data_qiov,
+ m->data_qiov_offset,
+ data_bytes)
+ : 0));
qemu_co_mutex_unlock(&s->lock);
/* First we read the existing data from both COW regions. We
@@ -905,7 +908,7 @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
if (start->nb_bytes) {
qemu_iovec_add(&qiov, start_buffer, start->nb_bytes);
}
- qemu_iovec_concat(&qiov, m->data_qiov, 0, data_bytes);
+ qemu_iovec_concat(&qiov, m->data_qiov, m->data_qiov_offset, data_bytes);
if (end->nb_bytes) {
qemu_iovec_add(&qiov, end_buffer, end->nb_bytes);
}
diff --git a/block/qcow2.c b/block/qcow2.c
index c389ef07b0..850117cfa2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2091,7 +2091,8 @@ fail:
/* Check if it's possible to merge a write request with the writing of
* the data from the COW regions */
static bool merge_cow(uint64_t offset, unsigned bytes,
- QEMUIOVector *hd_qiov, QCowL2Meta *l2meta)
+ QEMUIOVector *qiov, size_t qiov_offset,
+ QCowL2Meta *l2meta)
{
QCowL2Meta *m;
@@ -2120,11 +2121,12 @@ static bool merge_cow(uint64_t offset, unsigned bytes,
/* Make sure that adding both COW regions to the QEMUIOVector
* does not exceed IOV_MAX */
- if (hd_qiov->niov > IOV_MAX - 2) {
+ if (qemu_iovec_subvec_niov(qiov, qiov_offset, bytes) > IOV_MAX - 2) {
continue;
}
- m->data_qiov = hd_qiov;
+ m->data_qiov = qiov;
+ m->data_qiov_offset = qiov_offset;
return true;
}
@@ -2205,24 +2207,22 @@ static int handle_alloc_space(BlockDriverState *bs, QCowL2Meta *l2meta)
return 0;
}
-static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
- int flags)
+static coroutine_fn int qcow2_co_pwritev_part(
+ BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset, int flags)
{
BDRVQcow2State *s = bs->opaque;
int offset_in_cluster;
int ret;
unsigned int cur_bytes; /* number of sectors in current iteration */
uint64_t cluster_offset;
- QEMUIOVector hd_qiov;
+ QEMUIOVector encrypted_qiov;
uint64_t bytes_done = 0;
uint8_t *cluster_data = NULL;
QCowL2Meta *l2meta = NULL;
trace_qcow2_writev_start_req(qemu_coroutine_self(), offset, bytes);
- qemu_iovec_init(&hd_qiov, qiov->niov);
-
qemu_co_mutex_lock(&s->lock);
while (bytes != 0) {
@@ -2255,9 +2255,6 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
qemu_co_mutex_unlock(&s->lock);
- qemu_iovec_reset(&hd_qiov);
- qemu_iovec_concat(&hd_qiov, qiov, bytes_done, cur_bytes);
-
if (bs->encrypted) {
assert(s->crypto);
if (!cluster_data) {
@@ -2270,9 +2267,9 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
}
}
- assert(hd_qiov.size <=
- QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
- qemu_iovec_to_buf(&hd_qiov, 0, cluster_data, hd_qiov.size);
+ assert(cur_bytes <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
+ qemu_iovec_to_buf(qiov, qiov_offset + bytes_done,
+ cluster_data, cur_bytes);
if (qcow2_co_encrypt(bs, cluster_offset, offset,
cluster_data, cur_bytes) < 0) {
@@ -2280,8 +2277,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
goto out_unlocked;
}
- qemu_iovec_reset(&hd_qiov);
- qemu_iovec_add(&hd_qiov, cluster_data, cur_bytes);
+ qemu_iovec_init_buf(&encrypted_qiov, cluster_data, cur_bytes);
}
/* Try to efficiently initialize the physical space with zeroes */
@@ -2294,13 +2290,17 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
* writing of the guest data together with that of the COW regions.
* If it's not possible (or not necessary) then write the
* guest data now. */
- if (!merge_cow(offset, cur_bytes, &hd_qiov, l2meta)) {
+ if (!merge_cow(offset, cur_bytes,
+ bs->encrypted ? &encrypted_qiov : qiov,
+ bs->encrypted ? 0 : qiov_offset + bytes_done, l2meta))
+ {
BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
trace_qcow2_writev_data(qemu_coroutine_self(),
cluster_offset + offset_in_cluster);
- ret = bdrv_co_pwritev(s->data_file,
- cluster_offset + offset_in_cluster,
- cur_bytes, &hd_qiov, 0);
+ ret = bdrv_co_pwritev_part(
+ s->data_file, cluster_offset + offset_in_cluster, cur_bytes,
+ bs->encrypted ? &encrypted_qiov : qiov,
+ bs->encrypted ? 0 : qiov_offset + bytes_done, 0);
if (ret < 0) {
goto out_unlocked;
}
@@ -2329,7 +2329,6 @@ out_locked:
qemu_co_mutex_unlock(&s->lock);
- qemu_iovec_destroy(&hd_qiov);
qemu_vfree(cluster_data);
trace_qcow2_writev_done_req(qemu_coroutine_self(), ret);
@@ -3993,8 +3992,9 @@ fail:
/* XXX: put compressed sectors first, then all the cluster aligned
tables to avoid losing bytes in alignment */
static coroutine_fn int
-qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov)
+qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset)
{
BDRVQcow2State *s = bs->opaque;
int ret;
@@ -4031,7 +4031,7 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
/* Zero-pad last write if image size is not cluster aligned */
memset(buf + bytes, 0, s->cluster_size - bytes);
}
- qemu_iovec_to_buf(qiov, 0, buf, bytes);
+ qemu_iovec_to_buf(qiov, qiov_offset, buf, bytes);
out_buf = g_malloc(s->cluster_size);
@@ -4039,7 +4039,7 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
buf, s->cluster_size);
if (out_len == -ENOMEM) {
/* could not compress: write normal cluster */
- ret = qcow2_co_pwritev(bs, offset, bytes, qiov, 0);
+ ret = qcow2_co_pwritev_part(bs, offset, bytes, qiov, qiov_offset, 0);
if (ret < 0) {
goto fail;
}
@@ -4623,8 +4623,8 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
BDRVQcow2State *s = bs->opaque;
BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE);
- return bs->drv->bdrv_co_pwritev(bs, qcow2_vm_state_offset(s) + pos,
- qiov->size, qiov, 0);
+ return bs->drv->bdrv_co_pwritev_part(bs, qcow2_vm_state_offset(s) + pos,
+ qiov->size, qiov, 0, 0);
}
static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
@@ -5175,7 +5175,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_co_block_status = qcow2_co_block_status,
.bdrv_co_preadv_part = qcow2_co_preadv_part,
- .bdrv_co_pwritev = qcow2_co_pwritev,
+ .bdrv_co_pwritev_part = qcow2_co_pwritev_part,
.bdrv_co_flush_to_os = qcow2_co_flush_to_os,
.bdrv_co_pwrite_zeroes = qcow2_co_pwrite_zeroes,
@@ -5183,7 +5183,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_co_copy_range_from = qcow2_co_copy_range_from,
.bdrv_co_copy_range_to = qcow2_co_copy_range_to,
.bdrv_co_truncate = qcow2_co_truncate,
- .bdrv_co_pwritev_compressed = qcow2_co_pwritev_compressed,
+ .bdrv_co_pwritev_compressed_part = qcow2_co_pwritev_compressed_part,
.bdrv_make_empty = qcow2_make_empty,
.bdrv_snapshot_create = qcow2_snapshot_create,
diff --git a/util/iov.c b/util/iov.c
index d988a533ce..0ed75e764c 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -401,6 +401,16 @@ static struct iovec *qiov_slice(QEMUIOVector *qiov,
return iov;
}
+int qemu_iovec_subvec_niov(QEMUIOVector *qiov, size_t offset, size_t len)
+{
+ size_t head, tail;
+ int niov;
+
+ qiov_slice(qiov, offset, len, &head, &tail, &niov);
+
+ return niov;
+}
+
/*
* Compile new iovec, combining @head_buf buffer, sub-qiov of @mid_qiov,
* and @tail_buf buffer into new qiov.
--
2.18.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (11 preceding siblings ...)
2019-06-04 16:15 ` [Qemu-devel] [PATCH v2 12/12] block/qcow2: implement .bdrv_co_pwritev(_compressed)_part Vladimir Sementsov-Ogievskiy
@ 2019-06-04 17:48 ` no-reply
2019-06-28 8:43 ` Stefan Hajnoczi
` (2 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: no-reply @ 2019-06-04 17:48 UTC (permalink / raw)
To: vsementsov
Cc: fam, kwolf, vsementsov, qemu-block, qemu-devel, mreitz, stefanha,
den, jsnow
Patchew URL: https://patchew.org/QEMU/20190604161514.262241-1-vsementsov@virtuozzo.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
CC accel/tcg/trace.o
CC crypto/trace.o
CC authz/trace.o
/tmp/qemu-test/src/util/iov.c:428:9: error: variable 'mid_niov' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
if (mid_len) {
^~~~~~~
/tmp/qemu-test/src/util/iov.c:433:31: note: uninitialized use occurs here
The full log is available at
http://patchew.org/logs/20190604161514.262241-1-vsementsov@virtuozzo.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (12 preceding siblings ...)
2019-06-04 17:48 ` [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io no-reply
@ 2019-06-28 8:43 ` Stefan Hajnoczi
2019-07-25 8:28 ` Vladimir Sementsov-Ogievskiy
2019-07-29 15:24 ` Stefan Hajnoczi
2019-08-22 15:50 ` [Qemu-devel] " Stefan Hajnoczi
15 siblings, 1 reply; 27+ messages in thread
From: Stefan Hajnoczi @ 2019-06-28 8:43 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: fam, kwolf, qemu-block, qemu-devel, mreitz, stefanha, den, jsnow
[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]
On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> Here is new parameter qiov_offset for io path, to avoid
> a lot of places with same pattern of creating local_qiov or hd_qiov
> variables.
>
> These series also includes my
> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
> with some changes [described in 01 and 03 emails]
>
> Vladimir Sementsov-Ogievskiy (12):
> util/iov: introduce qemu_iovec_init_extended
> util/iov: improve qemu_iovec_is_zero
> block/io: refactor padding
> block: define .*_part io handlers in BlockDriver
> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
> block/io: bdrv_co_do_copy_on_readv: lazy allocation
> block/io: bdrv_aligned_preadv: use and support qiov_offset
> block/io: bdrv_aligned_pwritev: use and support qiov_offset
> block/io: introduce bdrv_co_p{read,write}v_part
> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
> block/qcow2: implement .bdrv_co_preadv_part
> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
>
> block/qcow2.h | 1 +
> include/block/block_int.h | 21 ++
> include/qemu/iov.h | 10 +-
> block/backup.c | 2 +-
> block/io.c | 532 ++++++++++++++++++++++----------------
> block/qcow2-cluster.c | 14 +-
> block/qcow2.c | 131 +++++-----
> qemu-img.c | 4 +-
> util/iov.c | 153 +++++++++--
> 9 files changed, 559 insertions(+), 309 deletions(-)
>
> --
> 2.18.0
>
>
I don't see a significant advantage after taking into account more
complex code (e.g. additional block driver interfaces) and the risk of
introducing new bugs. A measurable performance improvement would make
this refactoring more attractive. Still:
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-06-28 8:43 ` Stefan Hajnoczi
@ 2019-07-25 8:28 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-07-25 8:28 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: fam, kwolf, Denis Lunev, qemu-block, qemu-devel, mreitz, stefanha, jsnow
28.06.2019 11:43, Stefan Hajnoczi wrote:
> On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> Here is new parameter qiov_offset for io path, to avoid
>> a lot of places with same pattern of creating local_qiov or hd_qiov
>> variables.
>>
>> These series also includes my
>> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
>> with some changes [described in 01 and 03 emails]
>>
>> Vladimir Sementsov-Ogievskiy (12):
>> util/iov: introduce qemu_iovec_init_extended
>> util/iov: improve qemu_iovec_is_zero
>> block/io: refactor padding
>> block: define .*_part io handlers in BlockDriver
>> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
>> block/io: bdrv_co_do_copy_on_readv: lazy allocation
>> block/io: bdrv_aligned_preadv: use and support qiov_offset
>> block/io: bdrv_aligned_pwritev: use and support qiov_offset
>> block/io: introduce bdrv_co_p{read,write}v_part
>> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
>> block/qcow2: implement .bdrv_co_preadv_part
>> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
>>
>> block/qcow2.h | 1 +
>> include/block/block_int.h | 21 ++
>> include/qemu/iov.h | 10 +-
>> block/backup.c | 2 +-
>> block/io.c | 532 ++++++++++++++++++++++----------------
>> block/qcow2-cluster.c | 14 +-
>> block/qcow2.c | 131 +++++-----
>> qemu-img.c | 4 +-
>> util/iov.c | 153 +++++++++--
>> 9 files changed, 559 insertions(+), 309 deletions(-)
>>
>> --
>> 2.18.0
>>
>>
>
> I don't see a significant advantage after taking into account more
> complex code (e.g. additional block driver interfaces) and the risk of
> introducing new bugs. A measurable performance improvement would make
> this refactoring more attractive. Still:
>
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
>
Hmm, I understand your doubt. And I doubt that there will be some significant performance
gain.
What will you say if instead of adding new interfaces I just add qiov_offset parameter to
old ones, and add one boolean field to BlockDriver like .supports_qiov_offset, so, generic
code will use non-zero qiov_offset only for drivers supporting it? And, maybe, add corresponding
asserts to all not-supporting driver handlers?
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (13 preceding siblings ...)
2019-06-28 8:43 ` Stefan Hajnoczi
@ 2019-07-29 15:24 ` Stefan Hajnoczi
2019-07-29 15:34 ` Vladimir Sementsov-Ogievskiy
2019-08-15 11:12 ` Vladimir Sementsov-Ogievskiy
2019-08-22 15:50 ` [Qemu-devel] " Stefan Hajnoczi
15 siblings, 2 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2019-07-29 15:24 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: fam, kwolf, qemu-block, qemu-devel, mreitz, den, jsnow
[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]
On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> Here is new parameter qiov_offset for io path, to avoid
> a lot of places with same pattern of creating local_qiov or hd_qiov
> variables.
>
> These series also includes my
> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
> with some changes [described in 01 and 03 emails]
>
> Vladimir Sementsov-Ogievskiy (12):
> util/iov: introduce qemu_iovec_init_extended
> util/iov: improve qemu_iovec_is_zero
> block/io: refactor padding
> block: define .*_part io handlers in BlockDriver
> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
> block/io: bdrv_co_do_copy_on_readv: lazy allocation
> block/io: bdrv_aligned_preadv: use and support qiov_offset
> block/io: bdrv_aligned_pwritev: use and support qiov_offset
> block/io: introduce bdrv_co_p{read,write}v_part
> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
> block/qcow2: implement .bdrv_co_preadv_part
> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
>
> block/qcow2.h | 1 +
> include/block/block_int.h | 21 ++
> include/qemu/iov.h | 10 +-
> block/backup.c | 2 +-
> block/io.c | 532 ++++++++++++++++++++++----------------
> block/qcow2-cluster.c | 14 +-
> block/qcow2.c | 131 +++++-----
> qemu-img.c | 4 +-
> util/iov.c | 153 +++++++++--
> 9 files changed, 559 insertions(+), 309 deletions(-)
>
> --
> 2.18.0
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-07-29 15:24 ` Stefan Hajnoczi
@ 2019-07-29 15:34 ` Vladimir Sementsov-Ogievskiy
2019-08-15 11:12 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-07-29 15:34 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: fam, kwolf, Denis Lunev, qemu-block, qemu-devel, mreitz, jsnow
29.07.2019 18:24, Stefan Hajnoczi wrote:
> On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> Here is new parameter qiov_offset for io path, to avoid
>> a lot of places with same pattern of creating local_qiov or hd_qiov
>> variables.
>>
>> These series also includes my
>> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
>> with some changes [described in 01 and 03 emails]
>>
>> Vladimir Sementsov-Ogievskiy (12):
>> util/iov: introduce qemu_iovec_init_extended
>> util/iov: improve qemu_iovec_is_zero
>> block/io: refactor padding
>> block: define .*_part io handlers in BlockDriver
>> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
>> block/io: bdrv_co_do_copy_on_readv: lazy allocation
>> block/io: bdrv_aligned_preadv: use and support qiov_offset
>> block/io: bdrv_aligned_pwritev: use and support qiov_offset
>> block/io: introduce bdrv_co_p{read,write}v_part
>> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
>> block/qcow2: implement .bdrv_co_preadv_part
>> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
>>
>> block/qcow2.h | 1 +
>> include/block/block_int.h | 21 ++
>> include/qemu/iov.h | 10 +-
>> block/backup.c | 2 +-
>> block/io.c | 532 ++++++++++++++++++++++----------------
>> block/qcow2-cluster.c | 14 +-
>> block/qcow2.c | 131 +++++-----
>> qemu-img.c | 4 +-
>> util/iov.c | 153 +++++++++--
>> 9 files changed, 559 insertions(+), 309 deletions(-)
>>
>> --
>> 2.18.0
>
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block
>
> Stefan
>
Thanks! Than I can base my further work on parallelizing qcow2
read/write loops on it, great! (long ago v1 was
[PATCH 0/7] qcow2: async handling of fragmented io)
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-07-29 15:24 ` Stefan Hajnoczi
2019-07-29 15:34 ` Vladimir Sementsov-Ogievskiy
@ 2019-08-15 11:12 ` Vladimir Sementsov-Ogievskiy
2019-08-22 14:48 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
1 sibling, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-08-15 11:12 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: fam, kwolf, Denis Lunev, qemu-block, qemu-devel, mreitz, jsnow
29.07.2019 18:24, Stefan Hajnoczi wrote:
> On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> Here is new parameter qiov_offset for io path, to avoid
>> a lot of places with same pattern of creating local_qiov or hd_qiov
>> variables.
>>
>> These series also includes my
>> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
>> with some changes [described in 01 and 03 emails]
>>
>> Vladimir Sementsov-Ogievskiy (12):
>> util/iov: introduce qemu_iovec_init_extended
>> util/iov: improve qemu_iovec_is_zero
>> block/io: refactor padding
>> block: define .*_part io handlers in BlockDriver
>> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
>> block/io: bdrv_co_do_copy_on_readv: lazy allocation
>> block/io: bdrv_aligned_preadv: use and support qiov_offset
>> block/io: bdrv_aligned_pwritev: use and support qiov_offset
>> block/io: introduce bdrv_co_p{read,write}v_part
>> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
>> block/qcow2: implement .bdrv_co_preadv_part
>> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
>>
>> block/qcow2.h | 1 +
>> include/block/block_int.h | 21 ++
>> include/qemu/iov.h | 10 +-
>> block/backup.c | 2 +-
>> block/io.c | 532 ++++++++++++++++++++++----------------
>> block/qcow2-cluster.c | 14 +-
>> block/qcow2.c | 131 +++++-----
>> qemu-img.c | 4 +-
>> util/iov.c | 153 +++++++++--
>> 9 files changed, 559 insertions(+), 309 deletions(-)
>>
>> --
>> 2.18.0
>
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block
>
> Stefan
>
Could you please squash this into 01:
diff --git a/util/iov.c b/util/iov.c
index 0ed75e764c..5059e10431 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -422,7 +422,7 @@ void qemu_iovec_init_extended(
void *tail_buf, size_t tail_len)
{
size_t mid_head, mid_tail;
- int total_niov, mid_niov;
+ int total_niov, mid_niov = 0;
struct iovec *p, *mid_iov;
if (mid_len) {
--
Best regards,
Vladimir
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [Qemu-block] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-08-15 11:12 ` Vladimir Sementsov-Ogievskiy
@ 2019-08-22 14:48 ` Stefan Hajnoczi
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2019-08-22 14:48 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: fam, kwolf, Denis Lunev, qemu-block, qemu-devel, mreitz, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 2451 bytes --]
On Thu, Aug 15, 2019 at 11:12:35AM +0000, Vladimir Sementsov-Ogievskiy wrote:
> 29.07.2019 18:24, Stefan Hajnoczi wrote:
> > On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> >> Hi all!
> >>
> >> Here is new parameter qiov_offset for io path, to avoid
> >> a lot of places with same pattern of creating local_qiov or hd_qiov
> >> variables.
> >>
> >> These series also includes my
> >> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
> >> with some changes [described in 01 and 03 emails]
> >>
> >> Vladimir Sementsov-Ogievskiy (12):
> >> util/iov: introduce qemu_iovec_init_extended
> >> util/iov: improve qemu_iovec_is_zero
> >> block/io: refactor padding
> >> block: define .*_part io handlers in BlockDriver
> >> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
> >> block/io: bdrv_co_do_copy_on_readv: lazy allocation
> >> block/io: bdrv_aligned_preadv: use and support qiov_offset
> >> block/io: bdrv_aligned_pwritev: use and support qiov_offset
> >> block/io: introduce bdrv_co_p{read,write}v_part
> >> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
> >> block/qcow2: implement .bdrv_co_preadv_part
> >> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
> >>
> >> block/qcow2.h | 1 +
> >> include/block/block_int.h | 21 ++
> >> include/qemu/iov.h | 10 +-
> >> block/backup.c | 2 +-
> >> block/io.c | 532 ++++++++++++++++++++++----------------
> >> block/qcow2-cluster.c | 14 +-
> >> block/qcow2.c | 131 +++++-----
> >> qemu-img.c | 4 +-
> >> util/iov.c | 153 +++++++++--
> >> 9 files changed, 559 insertions(+), 309 deletions(-)
> >>
> >> --
> >> 2.18.0
> >
> > Thanks, applied to my block tree:
> > https://github.com/stefanha/qemu/commits/block
> >
> > Stefan
> >
>
> Could you please squash this into 01:
>
> diff --git a/util/iov.c b/util/iov.c
> index 0ed75e764c..5059e10431 100644
> --- a/util/iov.c
> +++ b/util/iov.c
> @@ -422,7 +422,7 @@ void qemu_iovec_init_extended(
> void *tail_buf, size_t tail_len)
> {
> size_t mid_head, mid_tail;
> - int total_niov, mid_niov;
> + int total_niov, mid_niov = 0;
> struct iovec *p, *mid_iov;
>
> if (mid_len) {
Done. Sending the pull request today.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-06-04 16:15 [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io Vladimir Sementsov-Ogievskiy
` (14 preceding siblings ...)
2019-07-29 15:24 ` Stefan Hajnoczi
@ 2019-08-22 15:50 ` Stefan Hajnoczi
2019-08-22 17:24 ` Vladimir Sementsov-Ogievskiy
15 siblings, 1 reply; 27+ messages in thread
From: Stefan Hajnoczi @ 2019-08-22 15:50 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: fam, kwolf, qemu-block, qemu-devel, mreitz, stefanha, den, jsnow
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> Here is new parameter qiov_offset for io path, to avoid
> a lot of places with same pattern of creating local_qiov or hd_qiov
> variables.
>
> These series also includes my
> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
> with some changes [described in 01 and 03 emails]
>
> Vladimir Sementsov-Ogievskiy (12):
> util/iov: introduce qemu_iovec_init_extended
> util/iov: improve qemu_iovec_is_zero
> block/io: refactor padding
> block: define .*_part io handlers in BlockDriver
> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
> block/io: bdrv_co_do_copy_on_readv: lazy allocation
> block/io: bdrv_aligned_preadv: use and support qiov_offset
> block/io: bdrv_aligned_pwritev: use and support qiov_offset
> block/io: introduce bdrv_co_p{read,write}v_part
> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
> block/qcow2: implement .bdrv_co_preadv_part
> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
>
> block/qcow2.h | 1 +
> include/block/block_int.h | 21 ++
> include/qemu/iov.h | 10 +-
> block/backup.c | 2 +-
> block/io.c | 532 ++++++++++++++++++++++----------------
> block/qcow2-cluster.c | 14 +-
> block/qcow2.c | 131 +++++-----
> qemu-img.c | 4 +-
> util/iov.c | 153 +++++++++--
> 9 files changed, 559 insertions(+), 309 deletions(-)
qemu-iotests 077 fails after I apply this series (including your
uninitialized variable fix). I'm afraid I can't include it in the block
pull request, sorry!
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-08-22 15:50 ` [Qemu-devel] " Stefan Hajnoczi
@ 2019-08-22 17:24 ` Vladimir Sementsov-Ogievskiy
2019-08-22 17:39 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-08-22 17:24 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: fam, kwolf, Denis Lunev, qemu-block, qemu-devel, mreitz, stefanha, jsnow
22.08.2019 18:50, Stefan Hajnoczi wrote:
> On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> Here is new parameter qiov_offset for io path, to avoid
>> a lot of places with same pattern of creating local_qiov or hd_qiov
>> variables.
>>
>> These series also includes my
>> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
>> with some changes [described in 01 and 03 emails]
>>
>> Vladimir Sementsov-Ogievskiy (12):
>> util/iov: introduce qemu_iovec_init_extended
>> util/iov: improve qemu_iovec_is_zero
>> block/io: refactor padding
>> block: define .*_part io handlers in BlockDriver
>> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
>> block/io: bdrv_co_do_copy_on_readv: lazy allocation
>> block/io: bdrv_aligned_preadv: use and support qiov_offset
>> block/io: bdrv_aligned_pwritev: use and support qiov_offset
>> block/io: introduce bdrv_co_p{read,write}v_part
>> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
>> block/qcow2: implement .bdrv_co_preadv_part
>> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
>>
>> block/qcow2.h | 1 +
>> include/block/block_int.h | 21 ++
>> include/qemu/iov.h | 10 +-
>> block/backup.c | 2 +-
>> block/io.c | 532 ++++++++++++++++++++++----------------
>> block/qcow2-cluster.c | 14 +-
>> block/qcow2.c | 131 +++++-----
>> qemu-img.c | 4 +-
>> util/iov.c | 153 +++++++++--
>> 9 files changed, 559 insertions(+), 309 deletions(-)
>
> qemu-iotests 077 fails after I apply this series (including your
> uninitialized variable fix). I'm afraid I can't include it in the block
> pull request, sorry!
>
> Stefan
>
Hmm, 77 don't work on master for me:
077 fail [20:23:57] [20:23:59] output mismatch (see 077.out.bad)
--- /work/src/qemu/up-block-drop-hd-qiov/tests/qemu-iotests/077.out 2019-04-22 15:06:56.162045432 +0300
+++ /work/src/qemu/up-block-drop-hd-qiov/tests/qemu-iotests/077.out.bad 2019-08-22 20:23:59.124122307 +0300
@@ -1,7 +1,15 @@
QA output created by 077
+==117186==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
+==117186==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffefc071000; bottom 0x7fad7277b000; size: 0x0051898f6000 (350200225792)
+False positive error reports may follow
+For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
== Some concurrent requests involving RMW ==
+==117197==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
+==117197==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffa4fcc000; bottom 0x7fa93a2c1000; size: 0x00566ad0b000 (371159248896)
+False positive error reports may follow
+For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
wrote XXX/XXX bytes at offset XXX
XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote XXX/XXX bytes at offset XXX
@@ -66,6 +74,10 @@
XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== Verify image content ==
+==117219==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
+==117219==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc722de000; bottom 0x7f0848232000; size: 0x00f42a0ac000 (1048677367808)
+False positive error reports may follow
+For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
read 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 512/512 bytes at offset 512
@@ -156,5 +168,9 @@
1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 2048/2048 bytes at offset 71680
2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+==117229==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
+==117229==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffa3342000; bottom 0x7fd85a275000; size: 0x0027490cd000 (168729300992)
+False positive error reports may follow
+For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
No errors were found on the image.
*** done
Failures: 077
Failed 1 of 1 tests
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-08-22 17:24 ` Vladimir Sementsov-Ogievskiy
@ 2019-08-22 17:39 ` Vladimir Sementsov-Ogievskiy
2019-08-22 17:59 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-08-22 17:39 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: fam, kwolf, Denis Lunev, qemu-block, qemu-devel, mreitz, stefanha, jsnow
22.08.2019 20:24, Vladimir Sementsov-Ogievskiy wrote:
> 22.08.2019 18:50, Stefan Hajnoczi wrote:
>> On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>> Hi all!
>>>
>>> Here is new parameter qiov_offset for io path, to avoid
>>> a lot of places with same pattern of creating local_qiov or hd_qiov
>>> variables.
>>>
>>> These series also includes my
>>> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
>>> with some changes [described in 01 and 03 emails]
>>>
>>> Vladimir Sementsov-Ogievskiy (12):
>>> util/iov: introduce qemu_iovec_init_extended
>>> util/iov: improve qemu_iovec_is_zero
>>> block/io: refactor padding
>>> block: define .*_part io handlers in BlockDriver
>>> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
>>> block/io: bdrv_co_do_copy_on_readv: lazy allocation
>>> block/io: bdrv_aligned_preadv: use and support qiov_offset
>>> block/io: bdrv_aligned_pwritev: use and support qiov_offset
>>> block/io: introduce bdrv_co_p{read,write}v_part
>>> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
>>> block/qcow2: implement .bdrv_co_preadv_part
>>> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
>>>
>>> block/qcow2.h | 1 +
>>> include/block/block_int.h | 21 ++
>>> include/qemu/iov.h | 10 +-
>>> block/backup.c | 2 +-
>>> block/io.c | 532 ++++++++++++++++++++++----------------
>>> block/qcow2-cluster.c | 14 +-
>>> block/qcow2.c | 131 +++++-----
>>> qemu-img.c | 4 +-
>>> util/iov.c | 153 +++++++++--
>>> 9 files changed, 559 insertions(+), 309 deletions(-)
>>
>> qemu-iotests 077 fails after I apply this series (including your
>> uninitialized variable fix). I'm afraid I can't include it in the block
>> pull request, sorry!
>>
>> Stefan
>>
>
> Hmm, 77 don't work on master for me:
> 077 fail [20:23:57] [20:23:59] output mismatch (see 077.out.bad)
> --- /work/src/qemu/up-block-drop-hd-qiov/tests/qemu-iotests/077.out 2019-04-22 15:06:56.162045432 +0300
> +++ /work/src/qemu/up-block-drop-hd-qiov/tests/qemu-iotests/077.out.bad 2019-08-22 20:23:59.124122307 +0300
> @@ -1,7 +1,15 @@
> QA output created by 077
> +==117186==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> +==117186==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffefc071000; bottom 0x7fad7277b000; size: 0x0051898f6000 (350200225792)
> +False positive error reports may follow
> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>
> == Some concurrent requests involving RMW ==
> +==117197==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> +==117197==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffa4fcc000; bottom 0x7fa93a2c1000; size: 0x00566ad0b000 (371159248896)
> +False positive error reports may follow
> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
> wrote XXX/XXX bytes at offset XXX
> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> wrote XXX/XXX bytes at offset XXX
> @@ -66,6 +74,10 @@
> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>
> == Verify image content ==
> +==117219==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> +==117219==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc722de000; bottom 0x7f0848232000; size: 0x00f42a0ac000 (1048677367808)
> +False positive error reports may follow
> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
> read 512/512 bytes at offset 0
> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> read 512/512 bytes at offset 512
> @@ -156,5 +168,9 @@
> 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> read 2048/2048 bytes at offset 71680
> 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==117229==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> +==117229==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffa3342000; bottom 0x7fd85a275000; size: 0x0027490cd000 (168729300992)
> +False positive error reports may follow
> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
> No errors were found on the image.
> *** done
> Failures: 077
> Failed 1 of 1 tests
>
>
>
But after "block/io: refactor padding" it hangs instead of this fail.. This is not good
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-08-22 17:39 ` Vladimir Sementsov-Ogievskiy
@ 2019-08-22 17:59 ` Vladimir Sementsov-Ogievskiy
2019-08-23 16:21 ` Stefan Hajnoczi
0 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-08-22 17:59 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: fam, kwolf, Denis Lunev, qemu-block, qemu-devel, mreitz, stefanha, jsnow
22.08.2019 20:39, Vladimir Sementsov-Ogievskiy wrote:
> 22.08.2019 20:24, Vladimir Sementsov-Ogievskiy wrote:
>> 22.08.2019 18:50, Stefan Hajnoczi wrote:
>>> On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>> Hi all!
>>>>
>>>> Here is new parameter qiov_offset for io path, to avoid
>>>> a lot of places with same pattern of creating local_qiov or hd_qiov
>>>> variables.
>>>>
>>>> These series also includes my
>>>> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
>>>> with some changes [described in 01 and 03 emails]
>>>>
>>>> Vladimir Sementsov-Ogievskiy (12):
>>>> util/iov: introduce qemu_iovec_init_extended
>>>> util/iov: improve qemu_iovec_is_zero
>>>> block/io: refactor padding
>>>> block: define .*_part io handlers in BlockDriver
>>>> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
>>>> block/io: bdrv_co_do_copy_on_readv: lazy allocation
>>>> block/io: bdrv_aligned_preadv: use and support qiov_offset
>>>> block/io: bdrv_aligned_pwritev: use and support qiov_offset
>>>> block/io: introduce bdrv_co_p{read,write}v_part
>>>> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
>>>> block/qcow2: implement .bdrv_co_preadv_part
>>>> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
>>>>
>>>> block/qcow2.h | 1 +
>>>> include/block/block_int.h | 21 ++
>>>> include/qemu/iov.h | 10 +-
>>>> block/backup.c | 2 +-
>>>> block/io.c | 532 ++++++++++++++++++++++----------------
>>>> block/qcow2-cluster.c | 14 +-
>>>> block/qcow2.c | 131 +++++-----
>>>> qemu-img.c | 4 +-
>>>> util/iov.c | 153 +++++++++--
>>>> 9 files changed, 559 insertions(+), 309 deletions(-)
>>>
>>> qemu-iotests 077 fails after I apply this series (including your
>>> uninitialized variable fix). I'm afraid I can't include it in the block
>>> pull request, sorry!
>>>
>>> Stefan
>>>
>>
>> Hmm, 77 don't work on master for me:
>> 077 fail [20:23:57] [20:23:59] output mismatch (see 077.out.bad)
>> --- /work/src/qemu/up-block-drop-hd-qiov/tests/qemu-iotests/077.out 2019-04-22 15:06:56.162045432 +0300
>> +++ /work/src/qemu/up-block-drop-hd-qiov/tests/qemu-iotests/077.out.bad 2019-08-22 20:23:59.124122307 +0300
>> @@ -1,7 +1,15 @@
>> QA output created by 077
>> +==117186==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>> +==117186==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffefc071000; bottom 0x7fad7277b000; size: 0x0051898f6000 (350200225792)
>> +False positive error reports may follow
>> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>>
>> == Some concurrent requests involving RMW ==
>> +==117197==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>> +==117197==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffa4fcc000; bottom 0x7fa93a2c1000; size: 0x00566ad0b000 (371159248896)
>> +False positive error reports may follow
>> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
>> wrote XXX/XXX bytes at offset XXX
>> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> wrote XXX/XXX bytes at offset XXX
>> @@ -66,6 +74,10 @@
>> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>
>> == Verify image content ==
>> +==117219==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>> +==117219==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc722de000; bottom 0x7f0848232000; size: 0x00f42a0ac000 (1048677367808)
>> +False positive error reports may follow
>> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
>> read 512/512 bytes at offset 0
>> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> read 512/512 bytes at offset 512
>> @@ -156,5 +168,9 @@
>> 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> read 2048/2048 bytes at offset 71680
>> 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +==117229==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>> +==117229==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffa3342000; bottom 0x7fd85a275000; size: 0x0027490cd000 (168729300992)
>> +False positive error reports may follow
>> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
>> No errors were found on the image.
>> *** done
>> Failures: 077
>> Failed 1 of 1 tests
>>
>>
>>
>
> But after "block/io: refactor padding" it hangs instead of this fail.. This is not good
>
>
Aha seems it's because 77 has "break pwritev_rmw_after_tail" which may not fire if we have merge_reads = true.
So, for me the following fix for 03 helps (77 fails, but not hangs, so same behavior as on master):
diff --git a/block/io.c b/block/io.c
index 6448f1c503..04e69400d8 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1493,13 +1493,23 @@ static int bdrv_padding_rmw_read(BdrvChild *child,
qemu_iovec_init_buf(&local_qiov, pad->buf, bytes);
- bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
+ if (pad->head) {
+ bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
+ }
+ if (pad->merge_reads && pad->tail) {
+ bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
+ }
ret = bdrv_aligned_preadv(child, req, req->overlap_offset, bytes,
align, &local_qiov, 0);
if (ret < 0) {
return ret;
}
- bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
+ if (pad->head) {
+ bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
+ }
+ if (pad->merge_reads && pad->tail) {
+ bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
+ }
Does it work for you?
--
Best regards,
Vladimir
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-08-22 17:59 ` Vladimir Sementsov-Ogievskiy
@ 2019-08-23 16:21 ` Stefan Hajnoczi
2019-08-24 10:15 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 27+ messages in thread
From: Stefan Hajnoczi @ 2019-08-23 16:21 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: fam, kwolf, Denis Lunev, qemu-block, Stefan Hajnoczi, qemu-devel,
mreitz, jsnow
[-- Attachment #1: Type: text/plain, Size: 6842 bytes --]
On Thu, Aug 22, 2019 at 05:59:43PM +0000, Vladimir Sementsov-Ogievskiy wrote:
> 22.08.2019 20:39, Vladimir Sementsov-Ogievskiy wrote:
> > 22.08.2019 20:24, Vladimir Sementsov-Ogievskiy wrote:
> >> 22.08.2019 18:50, Stefan Hajnoczi wrote:
> >>> On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> >>>> Hi all!
> >>>>
> >>>> Here is new parameter qiov_offset for io path, to avoid
> >>>> a lot of places with same pattern of creating local_qiov or hd_qiov
> >>>> variables.
> >>>>
> >>>> These series also includes my
> >>>> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
> >>>> with some changes [described in 01 and 03 emails]
> >>>>
> >>>> Vladimir Sementsov-Ogievskiy (12):
> >>>> util/iov: introduce qemu_iovec_init_extended
> >>>> util/iov: improve qemu_iovec_is_zero
> >>>> block/io: refactor padding
> >>>> block: define .*_part io handlers in BlockDriver
> >>>> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
> >>>> block/io: bdrv_co_do_copy_on_readv: lazy allocation
> >>>> block/io: bdrv_aligned_preadv: use and support qiov_offset
> >>>> block/io: bdrv_aligned_pwritev: use and support qiov_offset
> >>>> block/io: introduce bdrv_co_p{read,write}v_part
> >>>> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
> >>>> block/qcow2: implement .bdrv_co_preadv_part
> >>>> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
> >>>>
> >>>> block/qcow2.h | 1 +
> >>>> include/block/block_int.h | 21 ++
> >>>> include/qemu/iov.h | 10 +-
> >>>> block/backup.c | 2 +-
> >>>> block/io.c | 532 ++++++++++++++++++++++----------------
> >>>> block/qcow2-cluster.c | 14 +-
> >>>> block/qcow2.c | 131 +++++-----
> >>>> qemu-img.c | 4 +-
> >>>> util/iov.c | 153 +++++++++--
> >>>> 9 files changed, 559 insertions(+), 309 deletions(-)
> >>>
> >>> qemu-iotests 077 fails after I apply this series (including your
> >>> uninitialized variable fix). I'm afraid I can't include it in the block
> >>> pull request, sorry!
> >>>
> >>> Stefan
> >>>
> >>
> >> Hmm, 77 don't work on master for me:
> >> 077 fail [20:23:57] [20:23:59] output mismatch (see 077.out.bad)
> >> --- /work/src/qemu/up-block-drop-hd-qiov/tests/qemu-iotests/077.out 2019-04-22 15:06:56.162045432 +0300
> >> +++ /work/src/qemu/up-block-drop-hd-qiov/tests/qemu-iotests/077.out.bad 2019-08-22 20:23:59.124122307 +0300
> >> @@ -1,7 +1,15 @@
> >> QA output created by 077
> >> +==117186==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> >> +==117186==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffefc071000; bottom 0x7fad7277b000; size: 0x0051898f6000 (350200225792)
> >> +False positive error reports may follow
> >> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
> >> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
> >>
> >> == Some concurrent requests involving RMW ==
> >> +==117197==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> >> +==117197==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffa4fcc000; bottom 0x7fa93a2c1000; size: 0x00566ad0b000 (371159248896)
> >> +False positive error reports may follow
> >> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
> >> wrote XXX/XXX bytes at offset XXX
> >> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >> wrote XXX/XXX bytes at offset XXX
> >> @@ -66,6 +74,10 @@
> >> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >>
> >> == Verify image content ==
> >> +==117219==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> >> +==117219==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc722de000; bottom 0x7f0848232000; size: 0x00f42a0ac000 (1048677367808)
> >> +False positive error reports may follow
> >> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
> >> read 512/512 bytes at offset 0
> >> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >> read 512/512 bytes at offset 512
> >> @@ -156,5 +168,9 @@
> >> 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >> read 2048/2048 bytes at offset 71680
> >> 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >> +==117229==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> >> +==117229==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffa3342000; bottom 0x7fd85a275000; size: 0x0027490cd000 (168729300992)
> >> +False positive error reports may follow
> >> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
> >> No errors were found on the image.
> >> *** done
> >> Failures: 077
> >> Failed 1 of 1 tests
> >>
> >>
> >>
> >
> > But after "block/io: refactor padding" it hangs instead of this fail.. This is not good
> >
> >
>
> Aha seems it's because 77 has "break pwritev_rmw_after_tail" which may not fire if we have merge_reads = true.
>
> So, for me the following fix for 03 helps (77 fails, but not hangs, so same behavior as on master):
>
> diff --git a/block/io.c b/block/io.c
> index 6448f1c503..04e69400d8 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1493,13 +1493,23 @@ static int bdrv_padding_rmw_read(BdrvChild *child,
>
> qemu_iovec_init_buf(&local_qiov, pad->buf, bytes);
>
> - bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
> + if (pad->head) {
> + bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
> + }
> + if (pad->merge_reads && pad->tail) {
> + bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
> + }
> ret = bdrv_aligned_preadv(child, req, req->overlap_offset, bytes,
> align, &local_qiov, 0);
> if (ret < 0) {
> return ret;
> }
> - bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
> + if (pad->head) {
> + bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
> + }
> + if (pad->merge_reads && pad->tail) {
> + bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
> + }
>
>
> Does it work for you?
Yes, it works as expected now, thanks. I've squashed in your fix and
will send these patches in the next block pull request.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/12] block: qiov_offset parameter for io
2019-08-23 16:21 ` Stefan Hajnoczi
@ 2019-08-24 10:15 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-08-24 10:15 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: fam, kwolf, Denis Lunev, qemu-block, Stefan Hajnoczi, qemu-devel,
mreitz, jsnow
23.08.2019 19:21, Stefan Hajnoczi wrote:
> On Thu, Aug 22, 2019 at 05:59:43PM +0000, Vladimir Sementsov-Ogievskiy wrote:
>> 22.08.2019 20:39, Vladimir Sementsov-Ogievskiy wrote:
>>> 22.08.2019 20:24, Vladimir Sementsov-Ogievskiy wrote:
>>>> 22.08.2019 18:50, Stefan Hajnoczi wrote:
>>>>> On Tue, Jun 04, 2019 at 07:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>>>> Hi all!
>>>>>>
>>>>>> Here is new parameter qiov_offset for io path, to avoid
>>>>>> a lot of places with same pattern of creating local_qiov or hd_qiov
>>>>>> variables.
>>>>>>
>>>>>> These series also includes my
>>>>>> "[Qemu-devel] [PATCH 0/2] block/io: refactor padding"
>>>>>> with some changes [described in 01 and 03 emails]
>>>>>>
>>>>>> Vladimir Sementsov-Ogievskiy (12):
>>>>>> util/iov: introduce qemu_iovec_init_extended
>>>>>> util/iov: improve qemu_iovec_is_zero
>>>>>> block/io: refactor padding
>>>>>> block: define .*_part io handlers in BlockDriver
>>>>>> block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
>>>>>> block/io: bdrv_co_do_copy_on_readv: lazy allocation
>>>>>> block/io: bdrv_aligned_preadv: use and support qiov_offset
>>>>>> block/io: bdrv_aligned_pwritev: use and support qiov_offset
>>>>>> block/io: introduce bdrv_co_p{read,write}v_part
>>>>>> block/qcow2: refactor qcow2_co_preadv to use buffer-based io
>>>>>> block/qcow2: implement .bdrv_co_preadv_part
>>>>>> block/qcow2: implement .bdrv_co_pwritev(_compressed)_part
>>>>>>
>>>>>> block/qcow2.h | 1 +
>>>>>> include/block/block_int.h | 21 ++
>>>>>> include/qemu/iov.h | 10 +-
>>>>>> block/backup.c | 2 +-
>>>>>> block/io.c | 532 ++++++++++++++++++++++----------------
>>>>>> block/qcow2-cluster.c | 14 +-
>>>>>> block/qcow2.c | 131 +++++-----
>>>>>> qemu-img.c | 4 +-
>>>>>> util/iov.c | 153 +++++++++--
>>>>>> 9 files changed, 559 insertions(+), 309 deletions(-)
>>>>>
>>>>> qemu-iotests 077 fails after I apply this series (including your
>>>>> uninitialized variable fix). I'm afraid I can't include it in the block
>>>>> pull request, sorry!
>>>>>
>>>>> Stefan
>>>>>
>>>>
>>>> Hmm, 77 don't work on master for me:
>>>> 077 fail [20:23:57] [20:23:59] output mismatch (see 077.out.bad)
>>>> --- /work/src/qemu/up-block-drop-hd-qiov/tests/qemu-iotests/077.out 2019-04-22 15:06:56.162045432 +0300
>>>> +++ /work/src/qemu/up-block-drop-hd-qiov/tests/qemu-iotests/077.out.bad 2019-08-22 20:23:59.124122307 +0300
>>>> @@ -1,7 +1,15 @@
>>>> QA output created by 077
>>>> +==117186==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>>>> +==117186==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffefc071000; bottom 0x7fad7277b000; size: 0x0051898f6000 (350200225792)
>>>> +False positive error reports may follow
>>>> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
>>>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>>>>
>>>> == Some concurrent requests involving RMW ==
>>>> +==117197==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>>>> +==117197==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffa4fcc000; bottom 0x7fa93a2c1000; size: 0x00566ad0b000 (371159248896)
>>>> +False positive error reports may follow
>>>> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
>>>> wrote XXX/XXX bytes at offset XXX
>>>> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>>> wrote XXX/XXX bytes at offset XXX
>>>> @@ -66,6 +74,10 @@
>>>> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>>>
>>>> == Verify image content ==
>>>> +==117219==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>>>> +==117219==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc722de000; bottom 0x7f0848232000; size: 0x00f42a0ac000 (1048677367808)
>>>> +False positive error reports may follow
>>>> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
>>>> read 512/512 bytes at offset 0
>>>> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>>> read 512/512 bytes at offset 512
>>>> @@ -156,5 +168,9 @@
>>>> 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>>> read 2048/2048 bytes at offset 71680
>>>> 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>>> +==117229==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>>>> +==117229==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffa3342000; bottom 0x7fd85a275000; size: 0x0027490cd000 (168729300992)
>>>> +False positive error reports may follow
>>>> +For details see http://code.google.com/p/address-sanitizer/issues/detail?id=189
>>>> No errors were found on the image.
>>>> *** done
>>>> Failures: 077
>>>> Failed 1 of 1 tests
>>>>
>>>>
>>>>
>>>
>>> But after "block/io: refactor padding" it hangs instead of this fail.. This is not good
>>>
>>>
>>
>> Aha seems it's because 77 has "break pwritev_rmw_after_tail" which may not fire if we have merge_reads = true.
>>
>> So, for me the following fix for 03 helps (77 fails, but not hangs, so same behavior as on master):
>>
>> diff --git a/block/io.c b/block/io.c
>> index 6448f1c503..04e69400d8 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -1493,13 +1493,23 @@ static int bdrv_padding_rmw_read(BdrvChild *child,
>>
>> qemu_iovec_init_buf(&local_qiov, pad->buf, bytes);
>>
>> - bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
>> + if (pad->head) {
>> + bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
>> + }
>> + if (pad->merge_reads && pad->tail) {
>> + bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
>> + }
>> ret = bdrv_aligned_preadv(child, req, req->overlap_offset, bytes,
>> align, &local_qiov, 0);
>> if (ret < 0) {
>> return ret;
>> }
>> - bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
>> + if (pad->head) {
>> + bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
>> + }
>> + if (pad->merge_reads && pad->tail) {
>> + bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
>> + }
>>
>>
>> Does it work for you?
>
> Yes, it works as expected now, thanks. I've squashed in your fix and
> will send these patches in the next block pull request.
>
Thank you!
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 27+ messages in thread