[v2,17/31] block/export: Add blk_exp_close_all(_type)

Commit Message

Kevin Wolf Sept. 24, 2020, 3:27 p.m. UTC

This adds a function to shut down all block exports, and another one to
shut down the block exports of a single type. The latter is used for now
when stopping the NBD server. As soon as we implement support for
multiple NBD servers, we'll need a per-server list of exports and it
will be replaced by a function using that.

As a side effect, the BlockExport layer has a list tracking all existing
exports now. closed_exports loses its only user and can go away.

Signed-off-by: Kevin Wolf <[e-mail]>
Reviewed-by: Max Reitz <[e-mail]>
---
 include/block/export.h | 15 ++++++++
 include/block/nbd.h    |  2 -
 block.c                |  2 +-
 block/export/export.c  | 86 +++++++++++++++++++++++++++++++++++++++++-
 blockdev-nbd.c         |  2 +-
 nbd/server.c           | 34 +++--------------
 qemu-nbd.c             |  2 +-
 7 files changed, 107 insertions(+), 36 deletions(-)

Patch

diff --git a/include/block/export.h b/include/block/export.h
index cf9b1c9dad..6fffcb5651 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -15,6 +15,7 @@ 
 #define BLOCK_EXPORT_H
 
 #include "qapi/qapi-types-block-export.h"
+#include "qemu/queue.h"
 
 typedef struct BlockExport BlockExport;
 
@@ -36,6 +37,14 @@  typedef struct BlockExportDriver {
      * references have been dropped.
      */
     void (*delete)(BlockExport *);
+
+    /*
+     * Start to disconnect all clients and drop other references held
+     * internally by the export driver. When the function returns, there may
+     * still be active references while the export is in the process of
+     * shutting down.
+     */
+    void (*request_shutdown)(BlockExport *);
 } BlockExportDriver;
 
 struct BlockExport {
@@ -50,10 +59,16 @@  struct BlockExport {
 
     /* The AioContext whose lock protects this BlockExport object. */
     AioContext *ctx;
+
+    /* List entry for block_exports */
+    QLIST_ENTRY(BlockExport) next;
 };
 
 BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp);
 void blk_exp_ref(BlockExport *exp);
 void blk_exp_unref(BlockExport *exp);
+void blk_exp_request_shutdown(BlockExport *exp);
+void blk_exp_close_all(void);
+void blk_exp_close_all_type(BlockExportType type);
 
 #endif
diff --git a/include/block/nbd.h b/include/block/nbd.h
index fc2c153d5b..0b9f3e5d4e 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -337,12 +337,10 @@  int nbd_export_new(BlockExport *blk_exp, BlockDriverState *bs,
                    const char *bitmap, bool readonly, bool shared,
                    bool writethrough, Error **errp);
 void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
-void nbd_export_close(NBDExport *exp);
 void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
 
 AioContext *nbd_export_aio_context(NBDExport *exp);
 NBDExport *nbd_export_find(const char *name);
-void nbd_export_close_all(void);
 
 void nbd_client_new(QIOChannelSocket *sioc,
                     QCryptoTLSCreds *tlscreds,
diff --git a/block.c b/block.c
index 11ab55f80b..47a9d91fb4 100644
--- a/block.c
+++ b/block.c
@@ -4462,7 +4462,7 @@  static void bdrv_close(BlockDriverState *bs)
 void bdrv_close_all(void)
 {
     assert(job_next(NULL) == NULL);
-    nbd_export_close_all();
+    blk_exp_close_all();
 
     /* Drop references from requests still in flight, such as canceled block
      * jobs whose AIO context has not been polled yet */
diff --git a/block/export/export.c b/block/export/export.c
index 6b2b29078b..e94a68c183 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -24,6 +24,10 @@  static const BlockExportDriver *blk_exp_drivers[] = {
     &blk_exp_nbd,
 };
 
+/* Only accessed from the main thread */
+static QLIST_HEAD(, BlockExport) block_exports =
+    QLIST_HEAD_INITIALIZER(block_exports);
+
 static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
 {
     int i;
@@ -61,6 +65,7 @@  BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
         return NULL;
     }
 
+    QLIST_INSERT_HEAD(&block_exports, exp, next);
     return exp;
 }
 
@@ -71,14 +76,91 @@  void blk_exp_ref(BlockExport *exp)
     exp->refcount++;
 }
 
+/* Runs in the main thread */
+static void blk_exp_delete_bh(void *opaque)
+{
+    BlockExport *exp = opaque;
+    AioContext *aio_context = exp->ctx;
+
+    aio_context_acquire(aio_context);
+
+    assert(exp->refcount == 0);
+    QLIST_REMOVE(exp, next);
+    exp->drv->delete(exp);
+    g_free(exp);
+
+    aio_context_release(aio_context);
+}
+
 /* Callers must hold exp->ctx lock */
 void blk_exp_unref(BlockExport *exp)
 {
     assert(exp->refcount > 0);
     if (--exp->refcount == 0) {
-        exp->drv->delete(exp);
-        g_free(exp);
+        /* Touch the block_exports list only in the main thread */
+        aio_bh_schedule_oneshot(qemu_get_aio_context(), blk_exp_delete_bh,
+                                exp);
+    }
+}
+
+/*
+ * Drops the user reference to the export and requests that all client
+ * connections and other internally held references start to shut down. When
+ * the function returns, there may still be active references while the export
+ * is in the process of shutting down.
+ *
+ * Acquires exp->ctx internally. Callers must *not* hold the lock.
+ */
+void blk_exp_request_shutdown(BlockExport *exp)
+{
+    AioContext *aio_context = exp->ctx;
+
+    aio_context_acquire(aio_context);
+    exp->drv->request_shutdown(exp);
+    aio_context_release(aio_context);
+}
+
+/*
+ * Returns whether a block export of the given type exists.
+ * type == BLOCK_EXPORT_TYPE__MAX checks for an export of any type.
+ */
+static bool blk_exp_has_type(BlockExportType type)
+{
+    BlockExport *exp;
+
+    if (type == BLOCK_EXPORT_TYPE__MAX) {
+        return !QLIST_EMPTY(&block_exports);
+    }
+
+    QLIST_FOREACH(exp, &block_exports, next) {
+        if (exp->drv->type == type) {
+            return true;
+        }
     }
+
+    return false;
+}
+
+/* type == BLOCK_EXPORT_TYPE__MAX for all types */
+void blk_exp_close_all_type(BlockExportType type)
+{
+    BlockExport *exp, *next;
+
+    assert(in_aio_context_home_thread(qemu_get_aio_context()));
+
+    QLIST_FOREACH_SAFE(exp, &block_exports, next, next) {
+        if (type != BLOCK_EXPORT_TYPE__MAX && exp->drv->type != type) {
+            continue;
+        }
+        blk_exp_request_shutdown(exp);
+    }
+
+    AIO_WAIT_WHILE(NULL, blk_exp_has_type(type));
+}
+
+void blk_exp_close_all(void)
+{
+    blk_exp_close_all_type(BLOCK_EXPORT_TYPE__MAX);
 }
 
 void qmp_block_export_add(BlockExportOptions *export, Error **errp)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index b34f159888..f927264777 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -345,7 +345,7 @@  void qmp_nbd_server_stop(Error **errp)
         return;
     }
 
-    nbd_export_close_all();
+    blk_exp_close_all_type(BLOCK_EXPORT_TYPE_NBD);
 
     nbd_server_free(nbd_server);
     nbd_server = NULL;
diff --git a/nbd/server.c b/nbd/server.c
index f31d8bbb60..32147e4871 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -100,8 +100,6 @@  struct NBDExport {
 };
 
 static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
-static QTAILQ_HEAD(, NBDExport) closed_exports =
-        QTAILQ_HEAD_INITIALIZER(closed_exports);
 
 /* NBDExportMetaContexts represents a list of contexts to be exported,
  * as selected by NBD_OPT_SET_META_CONTEXT. Also used for
@@ -1494,12 +1492,8 @@  static void blk_aio_detach(void *opaque)
 static void nbd_eject_notifier(Notifier *n, void *data)
 {
     NBDExport *exp = container_of(n, NBDExport, eject_notifier);
-    AioContext *aio_context;
 
-    aio_context = exp->common.ctx;
-    aio_context_acquire(aio_context);
-    nbd_export_close(exp);
-    aio_context_release(aio_context);
+    blk_exp_request_shutdown(&exp->common);
 }
 
 void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk)
@@ -1652,8 +1646,9 @@  nbd_export_aio_context(NBDExport *exp)
     return exp->common.ctx;
 }
 
-void nbd_export_close(NBDExport *exp)
+static void nbd_export_request_shutdown(BlockExport *blk_exp)
 {
+    NBDExport *exp = container_of(blk_exp, NBDExport, common);
     NBDClient *client, *next;
 
     blk_exp_ref(&exp->common);
@@ -1672,7 +1667,6 @@  void nbd_export_close(NBDExport *exp)
         g_free(exp->name);
         exp->name = NULL;
         QTAILQ_REMOVE(&exports, exp, next);
-        QTAILQ_INSERT_TAIL(&closed_exports, exp, next);
     }
     blk_exp_unref(&exp->common);
 }
@@ -1681,7 +1675,7 @@  void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp)
 {
     ERRP_GUARD();
     if (mode == NBD_SERVER_REMOVE_MODE_HARD || QTAILQ_EMPTY(&exp->clients)) {
-        nbd_export_close(exp);
+        nbd_export_request_shutdown(&exp->common);
         return;
     }
 
@@ -1716,9 +1710,6 @@  static void nbd_export_delete(BlockExport *blk_exp)
         bdrv_dirty_bitmap_set_busy(exp->export_bitmap, false);
         g_free(exp->export_bitmap_context);
     }
-
-    QTAILQ_REMOVE(&closed_exports, exp, next);
-    aio_wait_kick();
 }
 
 const BlockExportDriver blk_exp_nbd = {
@@ -1726,24 +1717,9 @@  const BlockExportDriver blk_exp_nbd = {
     .instance_size      = sizeof(NBDExport),
     .create             = nbd_export_create,
     .delete             = nbd_export_delete,
+    .request_shutdown   = nbd_export_request_shutdown,
 };
 
-void nbd_export_close_all(void)
-{
-    NBDExport *exp, *next;
-    AioContext *aio_context;
-
-    QTAILQ_FOREACH_SAFE(exp, &exports, next, next) {
-        aio_context = exp->common.ctx;
-        aio_context_acquire(aio_context);
-        nbd_export_close(exp);
-        aio_context_release(aio_context);
-    }
-
-    AIO_WAIT_WHILE(NULL, !(QTAILQ_EMPTY(&exports) &&
-                           QTAILQ_EMPTY(&closed_exports)));
-}
-
 static int coroutine_fn nbd_co_send_iov(NBDClient *client, struct iovec *iov,
                                         unsigned niov, Error **errp)
 {
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 23c2b7da8e..c64f83618b 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1119,7 +1119,7 @@  int main(int argc, char **argv)
     do {
         main_loop_wait(false);
         if (state == TERMINATE) {
-            nbd_export_close_all();
+            blk_exp_close_all();
             state = TERMINATED;
         }
     } while (state != TERMINATED);