block/export: Add block-export-del

Implement a new QMP command block-export-del and make nbd-server-remove
a wrapper around it.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200924152717.287415-21-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

block/export/export.c

@@ -29,7 +29,7 @@ static const BlockExportDriver *blk_exp_drivers[] = {
 static QLIST_HEAD(, BlockExport) block_exports =
     QLIST_HEAD_INITIALIZER(block_exports);
 
-static BlockExport *blk_exp_find(const char *id)
+BlockExport *blk_exp_find(const char *id)
 {
     BlockExport *exp;
 
@@ -143,12 +143,23 @@ void blk_exp_request_shutdown(BlockExport *exp)
     AioContext *aio_context = exp->ctx;
 
     aio_context_acquire(aio_context);
+
+    /*
+     * If the user doesn't own the export any more, it is already shutting
+     * down. We must not call .request_shutdown and decrease the refcount a
+     * second time.
+     */
+    if (!exp->user_owned) {
+        goto out;
+    }
+
     exp->drv->request_shutdown(exp);
 
     assert(exp->user_owned);
     exp->user_owned = false;
     blk_exp_unref(exp);
 
+out:
     aio_context_release(aio_context);
 }
 
@@ -199,3 +210,33 @@ void qmp_block_export_add(BlockExportOptions *export, Error **errp)
 {
     blk_exp_add(export, errp);
 }
+
+void qmp_block_export_del(const char *id,
+                          bool has_mode, BlockExportRemoveMode mode,
+                          Error **errp)
+{
+    ERRP_GUARD();
+    BlockExport *exp;
+
+    exp = blk_exp_find(id);
+    if (exp == NULL) {
+        error_setg(errp, "Export '%s' is not found", id);
+        return;
+    }
+    if (!exp->user_owned) {
+        error_setg(errp, "Export '%s' is already shutting down", id);
+        return;
+    }
+
+    if (!has_mode) {
+        mode = BLOCK_EXPORT_REMOVE_MODE_SAFE;
+    }
+    if (mode == BLOCK_EXPORT_REMOVE_MODE_SAFE && exp->refcount > 1) {
+        error_setg(errp, "export '%s' still in use", exp->id);
+        error_append_hint(errp, "Use mode='hard' to force client "
+                          "disconnect\n");
+        return;
+    }
+
+    blk_exp_request_shutdown(exp);
+}

block/monitor/block-hmp-cmds.c

@@ -476,8 +476,8 @@ void hmp_nbd_server_remove(Monitor *mon, const QDict *qdict)
     bool force = qdict_get_try_bool(qdict, "force", false);
     Error *err = NULL;
 
-    /* Rely on NBD_SERVER_REMOVE_MODE_SAFE being the default */
+    /* Rely on BLOCK_EXPORT_REMOVE_MODE_SAFE being the default */
-    qmp_nbd_server_remove(name, force, NBD_SERVER_REMOVE_MODE_HARD, &err);
+    qmp_nbd_server_remove(name, force, BLOCK_EXPORT_REMOVE_MODE_HARD, &err);
     hmp_handle_error(mon, err);
 }
 

blockdev-nbd.c

@@ -307,31 +307,18 @@ fail:
 }
 
 void qmp_nbd_server_remove(const char *name,
-                           bool has_mode, NbdServerRemoveMode mode,
+                           bool has_mode, BlockExportRemoveMode mode,
                            Error **errp)
 {
-    NBDExport *exp;
+    BlockExport *exp;
-    AioContext *aio_context;
 
-    if (!nbd_server) {
+    exp = blk_exp_find(name);
-        error_setg(errp, "NBD server not running");
+    if (exp && exp->drv->type != BLOCK_EXPORT_TYPE_NBD) {
+        error_setg(errp, "Block export '%s' is not an NBD export", name);
         return;
     }
 
-    exp = nbd_export_find(name);
+    qmp_block_export_del(name, has_mode, mode, errp);
-    if (exp == NULL) {
-        error_setg(errp, "Export '%s' is not found", name);
-        return;
-    }
-
-    if (!has_mode) {
-        mode = NBD_SERVER_REMOVE_MODE_SAFE;
-    }
-
-    aio_context = nbd_export_aio_context(exp);
-    aio_context_acquire(aio_context);
-    nbd_export_remove(exp, mode, errp);
-    aio_context_release(aio_context);
 }
 
 void qmp_nbd_server_stop(Error **errp)

include/block/export.h

@@ -76,6 +76,7 @@ struct BlockExport {
 };
 
 BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp);
+BlockExport *blk_exp_find(const char *id);
 void blk_exp_ref(BlockExport *exp);
 void blk_exp_unref(BlockExport *exp);
 void blk_exp_request_shutdown(BlockExport *exp);

include/block/nbd.h

@@ -337,7 +337,6 @@ 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_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
 
 AioContext *nbd_export_aio_context(NBDExport *exp);
 NBDExport *nbd_export_find(const char *name);

nbd/server.c

@@ -1669,20 +1669,6 @@ static void nbd_export_request_shutdown(BlockExport *blk_exp)
     blk_exp_unref(&exp->common);
 }
 
-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_request_shutdown(&exp->common);
-        return;
-    }
-
-    assert(mode == NBD_SERVER_REMOVE_MODE_SAFE);
-
-    error_setg(errp, "export '%s' still in use", exp->name);
-    error_append_hint(errp, "Use mode='hard' to force client disconnect\n");
-}
-
 static void nbd_export_delete(BlockExport *blk_exp)
 {
     NBDExport *exp = container_of(blk_exp, NBDExport, common);

qapi/block-export.json

@@ -116,9 +116,9 @@
   'data': 'NbdServerAddOptions', 'boxed': true }
 
 ##
-# @NbdServerRemoveMode:
+# @BlockExportRemoveMode:
 #
-# Mode for removing an NBD export.
+# Mode for removing a block export.
 #
 # @safe: Remove export if there are no existing connections, fail otherwise.
 #
@@ -134,16 +134,16 @@
 #
 # Since: 2.12
 ##
-{'enum': 'NbdServerRemoveMode', 'data': ['safe', 'hard']}
+{'enum': 'BlockExportRemoveMode', 'data': ['safe', 'hard']}
 
 ##
 # @nbd-server-remove:
 #
 # Remove NBD export by name.
 #
-# @name: Export name.
+# @name: Block export id.
 #
-# @mode: Mode of command operation. See @NbdServerRemoveMode description.
+# @mode: Mode of command operation. See @BlockExportRemoveMode description.
 #        Default is 'safe'.
 #
 # Returns: error if
@@ -154,7 +154,7 @@
 # Since: 2.12
 ##
 { 'command': 'nbd-server-remove',
-  'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
+  'data': {'name': 'str', '*mode': 'BlockExportRemoveMode'} }
 
 ##
 # @nbd-server-stop:
@@ -213,3 +213,23 @@
 ##
 { 'command': 'block-export-add',
   'data': 'BlockExportOptions', 'boxed': true }
+
+##
+# @block-export-del:
+#
+# Request to remove a block export. This drops the user's reference to the
+# export, but the export may still stay around after this command returns until
+# the shutdown of the export has completed.
+#
+# @id: Block export id.
+#
+# @mode: Mode of command operation. See @BlockExportRemoveMode description.
+#        Default is 'safe'.
+#
+# Returns: Error if the export is not found or @mode is 'safe' and the export
+#          is still in use (e.g. by existing client connections)
+#
+# Since: 5.2
+##
+{ 'command': 'block-export-del',
+  'data': { 'id': 'str', '*mode': 'BlockExportRemoveMode' } }