On 22.12.21 18:40, Vladimir Sementsov-Ogievskiy wrote:
Split intersecting-requests functionality out of block-copy to be
reused in copy-before-write filter.
Note: while being here, fix tiny typo in MAINTAINERS.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
include/block/reqlist.h | 67 +++++++++++++++++++++++
block/block-copy.c | 116 +++++++++++++---------------------------
block/reqlist.c | 76 ++++++++++++++++++++++++++
MAINTAINERS | 4 +-
block/meson.build | 1 +
5 files changed, 184 insertions(+), 80 deletions(-)
create mode 100644 include/block/reqlist.h
create mode 100644 block/reqlist.c
Looks good to me, this split makes sense.
I have just minor comments (50 % about pre-existing things) below.
diff --git a/include/block/reqlist.h b/include/block/reqlist.h
new file mode 100644
index 0000000000..b904d80216
--- /dev/null
+++ b/include/block/reqlist.h
@@ -0,0 +1,67 @@
+/*
+ * reqlist API
+ *
+ * Copyright (C) 2013 Proxmox Server Solutions
+ * Copyright (c) 2021 Virtuozzo International GmbH.
+ *
+ * Authors:
+ * Dietmar Maurer (diet...@proxmox.com)
+ * Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef REQLIST_H
+#define REQLIST_H
+
+#include "qemu/coroutine.h"
+
+/*
+ * The API is not thread-safe and shouldn't be. The struct is public to be part
+ * of other structures and protected by third-party locks, see
+ * block/block-copy.c for example.
+ */
+
+typedef struct BlockReq {
+ int64_t offset;
+ int64_t bytes;
+
+ CoQueue wait_queue; /* coroutines blocked on this req */
+ QLIST_ENTRY(BlockReq) list;
+} BlockReq;
+
+typedef QLIST_HEAD(, BlockReq) BlockReqList;
+
+/*
+ * Initialize new request and add it to the list. Caller should be sure that
I’d say s/should/must/, because that is guarded by an assertion.
+ * there are no conflicting requests in the list.
+ */
+void reqlist_init_req(BlockReqList *reqs, BlockReq *req, int64_t offset,
+ int64_t bytes);
+/* Search for request in the list intersecting with @offset/@bytes area. */
+BlockReq *reqlist_find_conflict(BlockReqList *reqs, int64_t offset,
+ int64_t bytes);
+
+/*
+ * If there are no intersecting requests return false. Otherwise, wait for the
+ * first found intersecting request to finish and return true.
+ *
+ * @lock is passed to qemu_co_queue_wait()
+ * False return value proves that lock was NOT released.
I’d say “was released at no point” instead, because when first reading
this I understood it to mean that lock simply is locked when this
function returns, and so I thought that this implies that when `true` is
returned, the lock is released (and remains released).
+ */
+bool coroutine_fn reqlist_wait_one(BlockReqList *reqs, int64_t offset,
+ int64_t bytes, CoMutex *lock);
+
+/*
+ * Shrink request and wake all waiting coroutines (may be some of them are not
s/may be/maybe/
+ * intersecting with shrunk request).
+ */
+void coroutine_fn reqlist_shrink_req(BlockReq *req, int64_t new_bytes);
+
+/*
+ * Remove request and wake all waiting coroutines. Do not release any memory.
+ */
+void coroutine_fn reqlist_remove_req(BlockReq *req);
+
+#endif /* REQLIST_H */
diff --git a/block/reqlist.c b/block/reqlist.c
new file mode 100644
index 0000000000..5e320ba649
--- /dev/null
+++ b/block/reqlist.c
@@ -0,0 +1,76 @@
[...]
+BlockReq *reqlist_find_conflict(BlockReqList *reqs, int64_t offset,
+ int64_t bytes)
+{
+ BlockReq *r;
+
+ QLIST_FOREACH(r, reqs, list) {
+ if (offset + bytes > r->offset && offset < r->offset + r->bytes) {
(Late, I know, the old code was exactly this, but:) Why not use
ranges_overlap()?
+ return r;
+ }
+ }
+
+ return NULL;
+}
