[Qemu-devel] [PATCH 4/5] nbd: Merge nbd_export_bitmap into nbd_export_new

[Eric Blake <eblake@redhat.com>]

We only have one caller that wants to export a bitmap name,
which it does right after creation of the export. But there is
still a brief window of time where an NBD client could see the
export but not the dirty bitmap, which a robust client would
have to interpret as meaning the entire image should be treated
as dirty.  Better is to eliminate the window entirely, by
inlining nbd_export_bitmap() into nbd_export_new(), and refusing
to create the bitmap in the first place if the requested bitmap
can't be located.

This also moves the logic for determining the default bitmap
export name out of blockdev-nbd into nbd_export_new().

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/block/nbd.h |  4 +--
 blockdev-nbd.c      | 13 +-------
 nbd/server.c        | 76 +++++++++++++++++++++------------------------
 qemu-nbd.c          |  2 +-
 4 files changed, 38 insertions(+), 57 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 2f9a2aeb73c..e598305ecda 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -296,6 +296,7 @@ typedef struct NBDClient NBDClient;

 NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
                           const char *name, const char *description,
+                          const char *bitmap, const char *bitmap_name,
                           uint16_t nbdflags, void (*close)(NBDExport *),
                           bool writethrough, BlockBackend *on_eject_blk,
                           Error **errp);
@@ -319,9 +320,6 @@ void nbd_client_put(NBDClient *client);
 void nbd_server_start(SocketAddress *addr, const char *tls_creds,
                       Error **errp);

-void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
-                       const char *bitmap_export_name, Error **errp);
-
 /* nbd_read
  * Reads @size bytes from @ioc. Returns 0 on success.
  */
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index faecde3b6e1..d895ee4adf5 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -188,7 +188,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
         writable = false;
     }

-    exp = nbd_export_new(bs, 0, -1, name, NULL,
+    exp = nbd_export_new(bs, 0, -1, name, NULL, bitmap, bitmap_export_name,
                          writable ? 0 : NBD_FLAG_READ_ONLY,
                          NULL, false, on_eject_blk, errp);
     if (!exp) {
@@ -199,17 +199,6 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
      * our only way of accessing it is through nbd_export_find(), so we can drop
      * the strong reference that is @exp. */
     nbd_export_put(exp);
-
-    if (has_bitmap) {
-        Error *err = NULL;
-        nbd_export_bitmap(exp, bitmap,
-                          has_bitmap_export_name ? bitmap_export_name : bitmap,
-                          &err);
-        if (err) {
-            error_propagate(errp, err);
-            nbd_export_remove(exp, NBD_SERVER_REMOVE_MODE_HARD, NULL);
-        }
-    }
 }

 void qmp_nbd_server_remove(const char *name,
diff --git a/nbd/server.c b/nbd/server.c
index 9cb305aa1bf..556da68ac26 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1457,6 +1457,7 @@ static void nbd_eject_notifier(Notifier *n, void *data)

 NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
                           const char *name, const char *description,
+                          const char *bitmap, const char *bitmap_name,
                           uint16_t nbdflags, void (*close)(NBDExport *),
                           bool writethrough, BlockBackend *on_eject_blk,
                           Error **errp)
@@ -1507,6 +1508,40 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
     }
     exp->size -= exp->size % BDRV_SECTOR_SIZE;

+    if (bitmap) {
+        BdrvDirtyBitmap *bm = NULL;
+        BlockDriverState *bs = blk_bs(blk);
+
+        while (true) {
+            bm = bdrv_find_dirty_bitmap(bs, bitmap);
+            if (bm != NULL || bs->backing == NULL) {
+                break;
+            }
+
+            bs = bs->backing->bs;
+        }
+
+        if (bm == NULL) {
+            error_setg(errp, "Bitmap '%s' is not found", bitmap);
+            goto fail;
+        }
+
+        if (bdrv_dirty_bitmap_enabled(bm)) {
+            error_setg(errp, "Bitmap '%s' is enabled", bitmap);
+            goto fail;
+        }
+
+        if (bdrv_dirty_bitmap_user_locked(bm)) {
+            error_setg(errp, "Bitmap '%s' is in use", bitmap);
+            goto fail;
+        }
+
+        bdrv_dirty_bitmap_set_qmp_locked(bm, true);
+        exp->export_bitmap = bm;
+        exp->export_bitmap_context = g_strdup_printf("qemu:dirty-bitmap:%s",
+                                                     bitmap_name ?: bitmap);
+    }
+
     exp->close = close;
     exp->ctx = blk_get_aio_context(blk);
     blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
@@ -2417,44 +2452,3 @@ void nbd_client_new(QIOChannelSocket *sioc,
     co = qemu_coroutine_create(nbd_co_client_start, client);
     qemu_coroutine_enter(co);
 }
-
-void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
-                       const char *bitmap_export_name, Error **errp)
-{
-    BdrvDirtyBitmap *bm = NULL;
-    BlockDriverState *bs = blk_bs(exp->blk);
-
-    if (exp->export_bitmap) {
-        error_setg(errp, "Export bitmap is already set");
-        return;
-    }
-
-    while (true) {
-        bm = bdrv_find_dirty_bitmap(bs, bitmap);
-        if (bm != NULL || bs->backing == NULL) {
-            break;
-        }
-
-        bs = bs->backing->bs;
-    }
-
-    if (bm == NULL) {
-        error_setg(errp, "Bitmap '%s' is not found", bitmap);
-        return;
-    }
-
-    if (bdrv_dirty_bitmap_enabled(bm)) {
-        error_setg(errp, "Bitmap '%s' is enabled", bitmap);
-        return;
-    }
-
-    if (bdrv_dirty_bitmap_user_locked(bm)) {
-        error_setg(errp, "Bitmap '%s' is in use", bitmap);
-        return;
-    }
-
-    bdrv_dirty_bitmap_set_qmp_locked(bm, true);
-    exp->export_bitmap = bm;
-    exp->export_bitmap_context =
-            g_strdup_printf("qemu:dirty-bitmap:%s", bitmap_export_name);
-}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 696bd78a2e2..a6cc0f2553e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1015,7 +1015,7 @@ int main(int argc, char **argv)
         }
     }

-    exp = nbd_export_new(bs, dev_offset, fd_size, export_name,
+    exp = nbd_export_new(bs, dev_offset, fd_size, export_name, NULL, NULL,
                          export_description, nbdflags, nbd_export_closed,
                          writethrough, NULL, &error_fatal);

-- 
2.20.1