[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 3/4] block: add BDRV_REQ_SERIALISING flag

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v3 3/4] block: add BDRV_REQ_SERIALISING flag
Date: Fri, 6 Jul 2018 16:52:32 -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:
Serialized writes should be used in copy-on-write of backup(sync=none)
for image fleecing scheme.

We need to change an assert in bdrv_aligned_pwritev, added in
28de2dcd88de. The assert may fail now, because call to
wait_serialising_requests here may become first call to it for this
request with serializing flag set. It occurs if the request is aligned
(otherwise, we should already set serializing flag before calling
bdrv_aligned_pwritev and correspondingly waited for all intersecting
requests). However, for aligned requests, we should not care about
outdating of previously read data, as there no such data. Therefore,
let's just update an assert to not care about aligned requests.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
  include/block/block.h | 15 ++++++++++++++-
  block/io.c            | 26 +++++++++++++++++++++++++-
  2 files changed, 39 insertions(+), 2 deletions(-)

Again just grammar suggestions:

diff --git a/include/block/block.h b/include/block/block.h
index 478ebc6c6c..fded1b7657 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -71,8 +71,21 @@ typedef enum {
       * content. */
+ /* BDRV_REQ_SERIALISING forces request serializing. Only for writes.

After comparison with patch 1/4, I'd suggest:

BDRV_REQ_SERIALISING forces request serialisation for writes.

+     * to serialize writes to target in backup process, when source is in
+     * backing chain of target (image fleecing scheme is example) to avoid a
+     * possibility for a client, reading from target during backup to read
+     * updated data from source in case of unhappy race of client-read and
+     * backup-cow-write.

It is used to ensure that writes to the backing file of a backup process target cannot race with a read of the backup target that defers to the backing file.

+     *
+     * Note, that BDRV_REQ_SERIALISING is _not_ opposite in meaning to
+     * BDRV_REQ_NO_SERIALISING. May be, better name for the latter is
+     * _DO_NOT_WAIT_FOR_SERIALISING, but it is too long.

The longer rationale on naming might fit better in the commit comment; I'd probably drop this paragraph from the code, But if you keep it, the second sentence reads better as:

A more descriptive name for the latter might be _DO_NOT_WAIT_FOR_SERIALISING, except that is too long.

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]