[RFC,10/22] nbd: Remove NBDExport.close callback

Commit Message

Kevin Wolf Aug. 13, 2020, 4:29 p.m. UTC

The export close callback is unused by the built-in NBD server. qemu-nbd
uses it only during shutdown to wait for the unrefed export to actually
go away. It can just use nbd_export_close_all() instead and do without
the callback.

This removes the close callback from nbd_export_new() and makes both
callers of it more similar.

Signed-off-by: Kevin Wolf <[e-mail]>
---
 include/block/nbd.h |  3 +--
 blockdev-nbd.c      |  2 +-
 nbd/server.c        |  9 +--------
 qemu-nbd.c          | 14 ++++----------
 4 files changed, 7 insertions(+), 21 deletions(-)

Comments

Max Reitz Aug. 17, 2020, 2:02 p.m. UTC | #1

On 13.08.20 18:29, Kevin Wolf wrote:
> The export close callback is unused by the built-in NBD server. qemu-nbd
> uses it only during shutdown to wait for the unrefed export to actually
> go away. It can just use nbd_export_close_all() instead and do without
> the callback.
> 
> This removes the close callback from nbd_export_new() and makes both
> callers of it more similar.
> 
> Signed-off-by: Kevin Wolf <[e-mail]>
> ---
>  include/block/nbd.h |  3 +--
>  blockdev-nbd.c      |  2 +-
>  nbd/server.c        |  9 +--------
>  qemu-nbd.c          | 14 ++++----------
>  4 files changed, 7 insertions(+), 21 deletions(-)

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

Eric Blake Aug. 19, 2020, 8:17 p.m. UTC | #2

On 8/13/20 11:29 AM, Kevin Wolf wrote:
> The export close callback is unused by the built-in NBD server. qemu-nbd
> uses it only during shutdown to wait for the unrefed export to actually
> go away. It can just use nbd_export_close_all() instead and do without
> the callback.
> 
> This removes the close callback from nbd_export_new() and makes both
> callers of it more similar.
> 
> Signed-off-by: Kevin Wolf <[e-mail]>
> ---

Reviewed-by: Eric Blake <[e-mail]>

Patch

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 6fc1f05ef4..50e1a46075 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -332,8 +332,7 @@  BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp);
 NBDExport *nbd_export_new(BlockDriverState *bs,
                           const char *name, const char *desc,
                           const char *bitmap, bool readonly, bool shared,
-                          void (*close)(NBDExport *), bool writethrough,
-                          Error **errp);
+                          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);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 17417c1b6b..d5b084acc2 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -218,7 +218,7 @@  BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
 
     exp = nbd_export_new(bs, arg->name, arg->description, arg->bitmap,
                          !arg->writable, !arg->writable,
-                         NULL, exp_args->writethrough, errp);
+                         exp_args->writethrough, errp);
     if (!exp) {
         goto out;
     }
diff --git a/nbd/server.c b/nbd/server.c
index 0b84fd30e2..eadc5b9804 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -84,7 +84,6 @@  struct NBDRequestData {
 struct NBDExport {
     BlockExport common;
     int refcount;
-    void (*close)(NBDExport *exp);
 
     BlockBackend *blk;
     char *name;
@@ -1520,8 +1519,7 @@  void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk)
 NBDExport *nbd_export_new(BlockDriverState *bs,
                           const char *name, const char *desc,
                           const char *bitmap, bool readonly, bool shared,
-                          void (*close)(NBDExport *), bool writethrough,
-                          Error **errp)
+                          bool writethrough, Error **errp)
 {
     AioContext *ctx;
     BlockBackend *blk;
@@ -1625,7 +1623,6 @@  NBDExport *nbd_export_new(BlockDriverState *bs,
         assert(strlen(exp->export_bitmap_context) < NBD_MAX_STRING_SIZE);
     }
 
-    exp->close = close;
     exp->ctx = ctx;
     blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
 
@@ -1723,10 +1720,6 @@  void nbd_export_put(NBDExport *exp)
         assert(exp->name == NULL);
         assert(exp->description == NULL);
 
-        if (exp->close) {
-            exp->close(exp);
-        }
-
         if (exp->blk) {
             if (exp->eject_notifier_blk) {
                 notifier_remove(&exp->eject_notifier);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index e348d5d6d8..48aa8a9d46 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -70,7 +70,7 @@  static int verbose;
 static char *srcpath;
 static SocketAddress *saddr;
 static int persistent = 0;
-static enum { RUNNING, TERMINATE, TERMINATING, TERMINATED } state;
+static enum { RUNNING, TERMINATE, TERMINATED } state;
 static int shared = 1;
 static int nb_fds;
 static QIONetListener *server;
@@ -331,12 +331,6 @@  static int nbd_can_accept(void)
     return state == RUNNING && nb_fds < shared;
 }
 
-static void nbd_export_closed(NBDExport *export)
-{
-    assert(state == TERMINATING);
-    state = TERMINATED;
-}
-
 static void nbd_update_server_watch(void);
 
 static void nbd_client_closed(NBDClient *client, bool negotiated)
@@ -1058,7 +1052,7 @@  int main(int argc, char **argv)
 
     export = nbd_export_new(bs, export_name,
                             export_description, bitmap, readonly, shared > 1,
-                            nbd_export_closed, writethrough, &error_fatal);
+                            writethrough, &error_fatal);
 
     if (device) {
 #if HAVE_NBD_DEVICE
@@ -1098,10 +1092,10 @@  int main(int argc, char **argv)
     do {
         main_loop_wait(false);
         if (state == TERMINATE) {
-            state = TERMINATING;
-            nbd_export_close(export);
             nbd_export_put(export);
+            nbd_export_close_all();
             export = NULL;
+            state = TERMINATED;
         }
     } while (state != TERMINATED);