[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PULL 05/24] block/io: fix copy_range
From: |
Kevin Wolf |
Subject: |
[Qemu-block] [PULL 05/24] block/io: fix copy_range |
Date: |
Tue, 10 Jul 2018 17:42:45 +0200 |
From: Vladimir Sementsov-Ogievskiy <address@hidden>
Here two things are fixed:
1. Architecture
On each recursion step, we go to the child of src or dst, only for one
of them. So, it's wrong to create tracked requests for both on each
step. It leads to tracked requests duplication.
2. Wait for serializing requests on write path independently of
BDRV_REQ_NO_SERIALISING
Before commit 9ded4a01149 "backup: Use copy offloading",
BDRV_REQ_NO_SERIALISING was used for only one case: read in
copy-on-write operation during backup. Also, the flag was handled only
on read path (in bdrv_co_preadv and bdrv_aligned_preadv).
After 9ded4a01149, flag is used for not waiting serializing operations
on backup target (in same case of copy-on-write operation). This
behavior change is unsubstantiated and potentially dangerous, let's
drop it and add additional asserts and documentation.
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: Fam Zheng <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
include/block/block.h | 12 ++++++++++++
block/io.c | 42 +++++++++++++++++++++++++++---------------
2 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 706ef009ad..f7ddff45b6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -50,6 +50,18 @@ typedef enum {
* opened with BDRV_O_UNMAP.
*/
BDRV_REQ_MAY_UNMAP = 0x4,
+
+ /*
+ * The BDRV_REQ_NO_SERIALISING flag is only valid for reads and means that
+ * we don't want wait_serialising_requests() during the read operation.
+ *
+ * This flag is used for backup copy-on-write operations, when we need to
+ * read old data before write (write notifier triggered). It is okay since
+ * we already waited for other serializing requests in the initiating write
+ * (see bdrv_aligned_pwritev), and it is necessary if the initiating write
+ * is already serializing (without the flag, the read would deadlock
+ * waiting for the serialising write to complete).
+ */
BDRV_REQ_NO_SERIALISING = 0x8,
BDRV_REQ_FUA = 0x10,
BDRV_REQ_WRITE_COMPRESSED = 0x20,
diff --git a/block/io.c b/block/io.c
index 4c0831149c..3a321d69d3 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1592,6 +1592,8 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild
*child,
max_transfer = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_transfer, INT_MAX),
align);
+ /* BDRV_REQ_NO_SERIALISING is only for read operation */
+ assert(!(flags & BDRV_REQ_NO_SERIALISING));
waited = wait_serialising_requests(req);
assert(!waited || !req->serialising);
assert(req->overlap_offset <= offset);
@@ -2916,7 +2918,7 @@ static int coroutine_fn
bdrv_co_copy_range_internal(BdrvChild *src,
BdrvRequestFlags flags,
bool recurse_src)
{
- BdrvTrackedRequest src_req, dst_req;
+ BdrvTrackedRequest req;
int ret;
if (!dst || !dst->bs) {
@@ -2943,32 +2945,42 @@ static int coroutine_fn
bdrv_co_copy_range_internal(BdrvChild *src,
|| src->bs->encrypted || dst->bs->encrypted) {
return -ENOTSUP;
}
- bdrv_inc_in_flight(src->bs);
- bdrv_inc_in_flight(dst->bs);
- tracked_request_begin(&src_req, src->bs, src_offset,
- bytes, BDRV_TRACKED_READ);
- tracked_request_begin(&dst_req, dst->bs, dst_offset,
- bytes, BDRV_TRACKED_WRITE);
- if (!(flags & BDRV_REQ_NO_SERIALISING)) {
- wait_serialising_requests(&src_req);
- wait_serialising_requests(&dst_req);
- }
if (recurse_src) {
+ bdrv_inc_in_flight(src->bs);
+ tracked_request_begin(&req, src->bs, src_offset, bytes,
+ BDRV_TRACKED_READ);
+
+ if (!(flags & BDRV_REQ_NO_SERIALISING)) {
+ wait_serialising_requests(&req);
+ }
+
ret = src->bs->drv->bdrv_co_copy_range_from(src->bs,
src, src_offset,
dst, dst_offset,
bytes, flags);
+
+ tracked_request_end(&req);
+ bdrv_dec_in_flight(src->bs);
} else {
+ bdrv_inc_in_flight(dst->bs);
+ tracked_request_begin(&req, dst->bs, dst_offset, bytes,
+ BDRV_TRACKED_WRITE);
+
+ /* BDRV_REQ_NO_SERIALISING is only for read operation,
+ * so we ignore it in flags.
+ */
+ wait_serialising_requests(&req);
+
ret = dst->bs->drv->bdrv_co_copy_range_to(dst->bs,
src, src_offset,
dst, dst_offset,
bytes, flags);
+
+ tracked_request_end(&req);
+ bdrv_dec_in_flight(dst->bs);
}
- tracked_request_end(&src_req);
- tracked_request_end(&dst_req);
- bdrv_dec_in_flight(src->bs);
- bdrv_dec_in_flight(dst->bs);
+
return ret;
}
--
2.13.6
- [Qemu-block] [PULL 00/24] Block layer patches, Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 03/24] block: Fix copy-on-read crash with partial final cluster, Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 01/24] block: Poll after drain on attaching a node, Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 02/24] test-bdrv-drain: Test bdrv_append() to drained node, Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 04/24] iotests: 222: Don't run with luks, Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 06/24] block: split flags in copy_range, Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 05/24] block/io: fix copy_range,
Kevin Wolf <=
- [Qemu-block] [PULL 07/24] block: add BDRV_REQ_SERIALISING flag, Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 11/24] Revert "block: Remove dead deprecation warning code", Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 08/24] block/backup: fix fleecing scheme: use serialized writes, Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 09/24] qapi/block-core.json: Add missing documentation for blklogwrites log-append option, Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 10/24] block/blklogwrites: Make sure the log sector size is not too small, Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 13/24] Revert "block: Remove deprecated -drive option addr", Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 15/24] block: Prefix file driver trace points with "file_", Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 12/24] Revert "block: Remove deprecated -drive option serial", Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 14/24] Revert "block: Remove deprecated -drive geometry options", Kevin Wolf, 2018/07/10
- [Qemu-block] [PULL 16/24] block: Add copy offloading trace points, Kevin Wolf, 2018/07/10