[Top][All Lists]

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

Re: [PATCH v4 02/25] include/block/block: split header into I/O and glob

From: Emanuele Giuseppe Esposito
Subject: Re: [PATCH v4 02/25] include/block/block: split header into I/O and global state API
Date: Tue, 16 Nov 2021 15:00:21 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0

On 12/11/2021 13:25, Hanna Reitz wrote:
On 25.10.21 12:17, Emanuele Giuseppe Esposito wrote:
block.h currently contains a mix of functions:
some of them run under the BQL and modify the block layer graph,
others are instead thread-safe and perform I/O in iothreads.
It is not easy to understand which function is part of which
group (I/O vs GS), and this patch aims to clarify it.

The "GS" functions need the BQL, and often use
aio_context_acquire/release and/or drain to be sure they
can modify the graph safely.
The I/O function are instead thread safe, and can run in
any AioContext.

By splitting the header in two files, block-io.h
and block-global-state.h we have a clearer view on what
needs what kind of protection. block-common.h
contains common structures shared by both headers.

block.h is left there for legacy and to avoid changing
all includes in all c files that use the block APIs.

Assertions are added in the next patch.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
  block.c                            |   3 +
  block/meson.build                  |   7 +-
  include/block/block-common.h       | 389 +++++++++++++
  include/block/block-global-state.h | 286 ++++++++++
  include/block/block-io.h           | 306 ++++++++++
  include/block/block.h              | 878 +----------------------------
  6 files changed, 1012 insertions(+), 857 deletions(-)
  create mode 100644 include/block/block-common.h
  create mode 100644 include/block/block-global-state.h
  create mode 100644 include/block/block-io.h


diff --git a/include/block/block-common.h b/include/block/block-common.h
new file mode 100644
index 0000000000..4f1fd8de21
--- /dev/null
+++ b/include/block/block-common.h


+#define BLKDBG_EVENT(child, evt) \
+    do { \
+        if (child) { \
+            bdrv_debug_event(child->bs, evt); \
+        } \
+    } while (0)

This is defined twice, once here, and...

This is unnecessary, as bdrv_debug_event is in block-io.c
Will remove that.

diff --git a/include/block/block-io.h b/include/block/block-io.h
new file mode 100644
index 0000000000..9af4609ccb
--- /dev/null
+++ b/include/block/block-io.h


+#define BLKDBG_EVENT(child, evt) \
+    do { \
+        if (child) { \
+            bdrv_debug_event(child->bs, evt); \
+        } \
+    } while (0)

...once here.


+ * bdrv_drained_begin:
+ *
+ * Begin a quiesced section for exclusive access to the BDS, by disabling + * external request sources including NBD server and device model. Note that + * this doesn't block timers or coroutines from submitting more requests, which
+ * means block_job_pause is still necessary.

Where does this sentence come from?  I can’t see it in master or in the lines removed from block.h:

This is another mistake, I copied the old header. This sentence was removed by this patch (sent by me) and then merged in master:

Will fix this and double check all headers so that the comments are updated (but there shouldn't be any other mistakes).

Thank you,

reply via email to

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