diff mbox series

[RFC,18/22] block/export: Add 'id' option to block-export-add

Message ID 20200813162935.210070-19-kwolf@redhat.com
State New, archived
Headers show
Series block/export: Add infrastructure and QAPI for block exports | expand

Commit Message

Kevin Wolf Aug. 13, 2020, 4:29 p.m. UTC
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>
---
 qapi/block-export.json |  3 +++
 include/block/export.h |  3 +++
 block/export/export.c  | 27 +++++++++++++++++++++++++++
 qemu-nbd.c             |  1 +
 4 files changed, 34 insertions(+)

Comments

Max Reitz Aug. 18, 2020, 3:08 p.m. UTC | #1
On 13.08.20 18:29, Kevin Wolf wrote:
> 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>
> ---
>  qapi/block-export.json |  3 +++
>  include/block/export.h |  3 +++
>  block/export/export.c  | 27 +++++++++++++++++++++++++++
>  qemu-nbd.c             |  1 +
>  4 files changed, 34 insertions(+)

Looks good, just one thing:

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

[...]

> @@ -144,6 +170,7 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
>      BlockExportOptions *export_opts = g_new(BlockExportOptions, 1);
>      *export_opts = (BlockExportOptions) {
>          .type                   = BLOCK_EXPORT_TYPE_NBD,
> +        .id                     = g_strdup(arg->name ?: arg->device),

Maybe this behavior should be documented for nbd-server-add?

>          .device                 = g_strdup(arg->device),
>          .u.nbd = {
>              .has_name           = arg->has_name,
diff mbox series

Patch

diff --git a/qapi/block-export.json b/qapi/block-export.json
index d68f3bf87e..0d0db9ca1b 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -179,6 +179,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)
+#
 # @device: The device name or node name of the node to be exported
 #
 # @writethrough: If true, caches are flushed after every write request to the
@@ -189,6 +191,7 @@ 
 ##
 { 'union': 'BlockExportOptions',
   'base': { 'type': 'BlockExportType',
+            'id': 'str',
             'device': 'str',
             '*writethrough': 'bool' },
   'discriminator': 'type',
diff --git a/include/block/export.h b/include/block/export.h
index 1698b68f09..43229857b0 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -30,6 +30,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/block/export/export.c b/block/export/export.c
index 675db9a8b9..72f1fab975 100644
--- 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,
@@ -27,6 +28,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;
@@ -45,6 +59,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");
@@ -55,10 +78,12 @@  BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
     *exp = (BlockExport) {
         .drv        = &blk_exp_nbd,
         .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;
     }
@@ -79,6 +104,7 @@  void blk_exp_unref(BlockExport *exp)
     if (--exp->refcount == 0) {
         QLIST_REMOVE(exp, next);
         exp->drv->delete(exp);
+        g_free(exp->id);
         g_free(exp);
         aio_wait_kick();
     }
@@ -144,6 +170,7 @@  void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
     BlockExportOptions *export_opts = g_new(BlockExportOptions, 1);
     *export_opts = (BlockExportOptions) {
         .type                   = BLOCK_EXPORT_TYPE_NBD,
+        .id                     = g_strdup(arg->name ?: arg->device),
         .device                 = g_strdup(arg->device),
         .u.nbd = {
             .has_name           = arg->has_name,
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 939a08902a..c6fc0581c1 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1055,6 +1055,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"),
         .device             = g_strdup(bdrv_get_node_name(bs)),
         .has_writethrough   = true,
         .writethrough       = writethrough,