[RFC,17/22] block/export: Add blk_exp_close_all(_type)

Commit Message

Kevin Wolf Aug. 13, 2020, 4:29 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]>
---
 include/block/export.h |  8 +++++++
 include/block/nbd.h    |  2 --
 block.c                |  2 +-
 block/export/export.c  | 52 ++++++++++++++++++++++++++++++++++++++++++
 blockdev-nbd.c         |  2 +-
 nbd/server.c           | 34 ++++-----------------------
 qemu-nbd.c             |  2 +-
 7 files changed, 68 insertions(+), 34 deletions(-)

Comments

Max Reitz Aug. 18, 2020, 3 p.m. UTC | #1

On 13.08.20 18:29, Kevin Wolf wrote:
> 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]>
> ---
>  include/block/export.h |  8 +++++++
>  include/block/nbd.h    |  2 --
>  block.c                |  2 +-
>  block/export/export.c  | 52 ++++++++++++++++++++++++++++++++++++++++++
>  blockdev-nbd.c         |  2 +-
>  nbd/server.c           | 34 ++++-----------------------
>  qemu-nbd.c             |  2 +-
>  7 files changed, 68 insertions(+), 34 deletions(-)

Reviewed-by: Max Reitz <[e-mail]>

> diff --git a/block/export/export.c b/block/export/export.c
> index 9de108cbc1..675db9a8b9 100644
> --- a/block/export/export.c
> +++ b/block/export/export.c

[...]

> +/* type == BLOCK_EXPORT_TYPE__MAX for all types */
> +void blk_exp_close_all_type(BlockExportType type)
> +{
> +    BlockExport *exp, *next;
> +
> +    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);

What’s interesting about this is that I saw from the header file that
you added both this and the type-specific function and wondered “Why not
just pass __MAX to close_all_type() to close all?”  And then I thought
“Because that would be stupid as an external interface”.

So I see you had the same thinking.

Patch

diff --git a/include/block/export.h b/include/block/export.h
index 597fc58245..1698b68f09 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;
 
@@ -23,6 +24,7 @@  typedef struct BlockExportDriver {
     size_t instance_size;
     int (*create)(BlockExport *, BlockExportOptions *, Error **);
     void (*delete)(BlockExport *);
+    void (*request_shutdown)(BlockExport *);
 } BlockExportDriver;
 
 struct BlockExport {
@@ -40,6 +42,9 @@  struct BlockExport {
      * BlockExportDriver callbacks.
      */
     AioContext *ctx;
+
+    /* List entry for block_exports */
+    QLIST_ENTRY(BlockExport) next;
 };
 
 extern const BlockExportDriver blk_exp_nbd;
@@ -47,5 +52,8 @@  extern const BlockExportDriver blk_exp_nbd;
 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 602536feb2..91a9d4f96d 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -335,12 +335,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 d9ac0e07eb..357c72846e 100644
--- a/block.c
+++ b/block.c
@@ -4424,7 +4424,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 9de108cbc1..675db9a8b9 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -24,6 +24,9 @@  static const BlockExportDriver* blk_exp_drivers[] = {
     &blk_exp_nbd,
 };
 
+static QLIST_HEAD(, BlockExport) block_exports =
+    QLIST_HEAD_INITIALIZER(block_exports);
+
 static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
 {
     int i;
@@ -60,6 +63,7 @@  BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
         return NULL;
     }
 
+    QLIST_INSERT_HEAD(&block_exports, exp, next);
     return exp;
 }
 
@@ -73,11 +77,59 @@  void blk_exp_unref(BlockExport *exp)
 {
     assert(exp->refcount > 0);
     if (--exp->refcount == 0) {
+        QLIST_REMOVE(exp, next);
         exp->drv->delete(exp);
         g_free(exp);
+        aio_wait_kick();
     }
 }
 
+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);
+}
+
+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;
+
+    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)
 {
     blk_exp_add(export, errp);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index f97deba424..1c7aa874ee 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -277,7 +277,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 6a58297557..7e2976b81d 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 f31868708c..939a08902a 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1110,7 +1110,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);