[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v3 1/4] block: disallow BDRV_REQ_NO
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v3 1/4] block: disallow BDRV_REQ_NO_SERIALISING for write |
Date: |
Fri, 6 Jul 2018 16:32:57 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 07/05/2018 02:46 AM, Vladimir Sementsov-Ogievskiy wrote:
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.
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
include/block/block.h | 13 +++++++++++++
block/io.c | 7 ++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
Commenting only on the grammar:
diff --git a/include/block/block.h b/include/block/block.h
index e5c7759a0c..a06a4d27de 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -50,6 +50,19 @@ typedef enum {
* opened with BDRV_O_UNMAP.
*/
BDRV_REQ_MAY_UNMAP = 0x4,
+
+ /* The BDRV_REQ_NO_SERIALISING means that we don't want to
+ * wait_serialising_requests(), when reading.
Either:
/* BDRV_REQ_NO_SERALISING means that...
or
/* The BDRV_REQ_NO_SERIALISING flag means that...
s/want to/want/
+ *
+ * This flag is used for backup copy on write operation, when we need to
+ * read old data before write (write notifier triggered). It is ok, due to
+ * we already waited for serializing requests in initiative write (see
+ * bdrv_aligned_pwritev), and it is necessary for the case when initiative
+ * write is serializing itself (we'll dead lock waiting it).
It is okay since we already waited for other serializing requests in the
initiating write (see bdrv_aligned_pwritev), and it is necessary since
the initiating write is already serializing (without the flag, the read
would deadlock waiting for the write to complete).
+ *
+ * The described case is the only usage for the flag for now, so, it is
+ * supported only for read operation and restricted for write.
This last sentence is rather wordy; I'm fine with just:
The flag is only valid during read operations.
+ */
BDRV_REQ_NO_SERIALISING = 0x8,
BDRV_REQ_FUA = 0x10,
BDRV_REQ_WRITE_COMPRESSED = 0x20,
We're inconsistent on which flags we document; it might be nice to have
a comment for each of them. But not necessarily this patch's problem.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- [Qemu-block] [PATCH v3 0/4] fix image fleecing, Vladimir Sementsov-Ogievskiy, 2018/07/05
- [Qemu-block] [PATCH v3 4/4] block/backup: fix fleecing scheme: use serialized writes, Vladimir Sementsov-Ogievskiy, 2018/07/05
- [Qemu-block] [PATCH v3 3/4] block: add BDRV_REQ_SERIALISING flag, Vladimir Sementsov-Ogievskiy, 2018/07/05
- [Qemu-block] [PATCH v3 2/4] block: split flags in copy_range, Vladimir Sementsov-Ogievskiy, 2018/07/05
- Re: [Qemu-block] [PATCH v3 0/4] fix image fleecing, Fam Zheng, 2018/07/06