block/export: Add 'id' option to block-export-add

We'll need an id to identify block exports in monitor commands. This
adds one.

Note that this is different from the 'name' option in the NBD server,
which is the externally visible export name. While block export ids need
to be unique in the whole process, export names must be unique only for
the same server. Different export types or (potentially in the future)
multiple NBD servers can have the same export name externally, but still
need different block export ids internally.

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

diff --git a/block/export/export.c b/block/export/export.c
index e94a68c1838013eca81f0e49a222ab8aeb4054f0..7a4a78449a02038639f43be534f69c6105e86699 100644 (file)
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -19,6 +19,7 @@
 #include "block/nbd.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-block-export.h"
+#include "qemu/id.h"
 
 static const BlockExportDriver *blk_exp_drivers[] = {
     &blk_exp_nbd,
@@ -28,6 +29,19 @@ 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 *exp;
+
+    QLIST_FOREACH(exp, &block_exports, next) {
+        if (strcmp(id, exp->id) == 0) {
+            return exp;
+        }
+    }
+
+    return NULL;
+}
+
 static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
 {
     int i;
@@ -46,6 +60,15 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
     BlockExport *exp;
     int ret;
 
+    if (!id_wellformed(export->id)) {
+        error_setg(errp, "Invalid block export id");
+        return NULL;
+    }
+    if (blk_exp_find(export->id)) {
+        error_setg(errp, "Block export id '%s' is already in use", export->id);
+        return NULL;
+    }
+
     drv = blk_exp_find_driver(export->type);
     if (!drv) {
         error_setg(errp, "No driver found for the requested export type");
@@ -57,10 +80,12 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
     *exp = (BlockExport) {
         .drv        = drv,
         .refcount   = 1,
+        .id         = g_strdup(export->id),
     };
 
     ret = drv->create(exp, export, errp);
     if (ret < 0) {
+        g_free(exp->id);
         g_free(exp);
         return NULL;
     }
@@ -87,6 +112,7 @@ static void blk_exp_delete_bh(void *opaque)
     assert(exp->refcount == 0);
     QLIST_REMOVE(exp, next);
     exp->drv->delete(exp);
+    g_free(exp->id);
     g_free(exp);
 
     aio_context_release(aio_context);

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index f927264777fcfe55fa316a7d6e1513fcd8f6ead4..814554dd90e11af858fbaf628a4d8a1529648226 100644 (file)
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -269,6 +269,7 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
     export_opts = g_new(BlockExportOptions, 1);
     *export_opts = (BlockExportOptions) {
         .type                   = BLOCK_EXPORT_TYPE_NBD,
+        .id                     = g_strdup(arg->name),
         .node_name              = g_strdup(bdrv_get_node_name(bs)),
         .u.nbd = {
             .has_name           = true,

diff --git a/include/block/export.h b/include/block/export.h
index 6fffcb5651d846cbeeebecd95dbf0ecc89ef4798..cdc6e161eadaf28c3f0114b78c13db41858f085c 100644 (file)
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -50,6 +50,9 @@ typedef struct BlockExportDriver {
 struct BlockExport {
     const BlockExportDriver *drv;
 
+    /* Unique identifier for the export */
+    char *id;
+
     /*
      * Reference count for this block export. This includes strong references
      * both from the owner (qemu-nbd or the monitor) and clients connected to

diff --git a/qapi/block-export.json b/qapi/block-export.json
index 1091c97f6fe12ce08969f1aea33195addc61e232..658bdf05e162deb51660fef874d03f7cedfa3bd5 100644 (file)
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -105,6 +105,8 @@
 #
 # Export a block node to QEMU's embedded NBD server.
 #
+# The export name will be used as the id for the resulting block export.
+#
 # Returns: error if the server is not running, or export with the same name
 #          already exists.
 #
@@ -182,6 +184,8 @@
 # Describes a block export, i.e. how single node should be exported on an
 # external interface.
 #
+# @id: A unique identifier for the block export (across all export types)
+#
 # @node-name: The node name of the block node to be exported (since: 5.2)
 #
 # @writethrough: If true, caches are flushed after every write request to the
@@ -192,6 +196,7 @@
 ##
 { 'union': 'BlockExportOptions',
   'base': { 'type': 'BlockExportType',
+            'id': 'str',
             'node-name': 'str',
             '*writethrough': 'bool' },
   'discriminator': 'type',

diff --git a/qemu-nbd.c b/qemu-nbd.c
index cfa5b78b1af703a5ed5cd3c53db231e6da0ee44f..66cb8f91b14886ca1258f2e673722a5cd13d9f72 100644 (file)
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1064,6 +1064,7 @@ int main(int argc, char **argv)
     export_opts = g_new(BlockExportOptions, 1);
     *export_opts = (BlockExportOptions) {
         .type               = BLOCK_EXPORT_TYPE_NBD,
+        .id                 = g_strdup("qemu-nbd-export"),
         .node_name          = g_strdup(bdrv_get_node_name(bs)),
         .has_writethrough   = true,
         .writethrough       = writethrough,

diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 0fcab6ed2d1e72049b92a83e07fa6616a3c074fa..e6157ff518290e217a17ef2f3c8b8dec9e31939a 100644 (file)
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -92,7 +92,7 @@ static void help(void)
 "  --chardev <options>    configure a character device backend\n"
 "                         (see the qemu(1) man page for possible options)\n"
 "\n"
-"  --export [type=]nbd,device=<node-name>[,name=<export-name>]\n"
+"  --export [type=]nbd,device=<node-name>,id=<id>,[,name=<export-name>]\n"
 "           [,writable=on|off][,bitmap=<name>]\n"
 "                         export the specified block node over NBD\n"
 "                         (requires --nbd-server)\n"

diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
index e1eaaedb55b3f6538c40c845afba9ec57c64b002..31ce9e6fe0bce75515adef597c16a9e80f5b2119 100644 (file)
--- a/tests/qemu-iotests/223.out
+++ b/tests/qemu-iotests/223.out
@@ -45,7 +45,7 @@ exports available: 0
 {"execute":"nbd-server-add", "arguments":{"device":"nosuch"}}
 {"error": {"class": "GenericError", "desc": "Cannot find device=nosuch nor node_name=nosuch"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n"}}
-{"error": {"class": "GenericError", "desc": "NBD server already has export named 'n'"}}
+{"error": {"class": "GenericError", "desc": "Block export id 'n' is already in use"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b2"}}
 {"error": {"class": "GenericError", "desc": "Enabled bitmap 'b2' incompatible with readonly export"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b3"}}
@@ -126,7 +126,7 @@ exports available: 0
 {"execute":"nbd-server-add", "arguments":{"device":"nosuch"}}
 {"error": {"class": "GenericError", "desc": "Cannot find device=nosuch nor node_name=nosuch"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n"}}
-{"error": {"class": "GenericError", "desc": "NBD server already has export named 'n'"}}
+{"error": {"class": "GenericError", "desc": "Block export id 'n' is already in use"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b2"}}
 {"error": {"class": "GenericError", "desc": "Enabled bitmap 'b2' incompatible with readonly export"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b3"}}