qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, pkrempa@redhat.com, qemu-block@nongnu.org,
	armbru@redhat.com, rjones@redhat.com, vsementsov@virtuozzo.com,
	stefanha@redhat.com, Max Reitz <mreitz@redhat.com>
Subject: [PATCH v6 03/11] nbd: Utilize QAPI_CLONE for type conversion
Date: Tue, 27 Oct 2020 00:05:48 -0500	[thread overview]
Message-ID: <20201027050556.269064-4-eblake@redhat.com> (raw)
In-Reply-To: <20201027050556.269064-1-eblake@redhat.com>

Rather than open-coding the translation from the deprecated
NbdServerAddOptions type to the preferred BlockExportOptionsNbd, it's
better to utilize QAPI_CLONE_MEMBERS.  This solves a couple of issues:
first, if we do any more refactoring of the base type (which an
upcoming patch plans to do), we don't have to revisit the open-coding.
Second, our assignment to arg->name is fishy: the generated QAPI code
for qapi_free_NbdServerAddOptions does not visit arg->name if
arg->has_name is false, but if it DID visit it, we would have
introduced a double-free situation when arg is finally freed.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 blockdev-nbd.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 8174023e5c47..cee9134b12eb 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -14,6 +14,8 @@
 #include "sysemu/block-backend.h"
 #include "hw/block/block.h"
 #include "qapi/error.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/qapi-visit-block-export.h"
 #include "qapi/qapi-commands-block-export.h"
 #include "block/nbd.h"
 #include "io/channel-socket.h"
@@ -195,7 +197,8 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
      * the device name as a default here for compatibility.
      */
     if (!arg->has_name) {
-        arg->name = arg->device;
+        arg->has_name = true;
+        arg->name = g_strdup(arg->device);
     }

     export_opts = g_new(BlockExportOptions, 1);
@@ -205,15 +208,9 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
         .node_name              = g_strdup(bdrv_get_node_name(bs)),
         .has_writable           = arg->has_writable,
         .writable               = arg->writable,
-        .u.nbd = {
-            .has_name           = true,
-            .name               = g_strdup(arg->name),
-            .has_description    = arg->has_description,
-            .description        = g_strdup(arg->description),
-            .has_bitmap         = arg->has_bitmap,
-            .bitmap             = g_strdup(arg->bitmap),
-        },
     };
+    QAPI_CLONE_MEMBERS(BlockExportOptionsNbd, &export_opts->u.nbd,
+                       qapi_NbdServerAddOptions_base(arg));

     /*
      * nbd-server-add doesn't complain when a read-only device should be
-- 
2.29.0



  parent reply	other threads:[~2020-10-27  5:11 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-27  5:05 [PATCH v6 00/11] Exposing backing-chain allocation over NBD Eric Blake
2020-10-27  5:05 ` [PATCH v6 01/11] block: Simplify QAPI_LIST_ADD Eric Blake
2020-10-27 10:06   ` Vladimir Sementsov-Ogievskiy
2020-10-27 12:58   ` Markus Armbruster
2020-10-27  5:05 ` [PATCH v6 02/11] qapi: Make QAPI_LIST_ADD() public Eric Blake
2020-10-27  5:05 ` Eric Blake [this message]
2020-10-27  5:05 ` [PATCH v6 04/11] nbd: Update qapi to support exporting multiple bitmaps Eric Blake
2020-10-27 10:29   ` Vladimir Sementsov-Ogievskiy
2020-10-27 12:37   ` Peter Krempa
2020-10-27  5:05 ` [PATCH v6 05/11] nbd: Simplify qemu bitmap context name Eric Blake
2020-10-27  5:05 ` [PATCH v6 06/11] nbd: Refactor counting of metadata contexts Eric Blake
2020-10-27 10:33   ` Vladimir Sementsov-Ogievskiy
2020-10-27  5:05 ` [PATCH v6 07/11] nbd: Allow export of multiple bitmaps for one device Eric Blake
2020-10-27  5:05 ` [PATCH v6 08/11] block: Return depth level during bdrv_is_allocated_above Eric Blake
2020-10-27 12:05   ` Vladimir Sementsov-Ogievskiy
2020-10-27  5:05 ` [PATCH v6 09/11] nbd: Add new qemu:allocation-depth metadata context Eric Blake
2020-10-27 10:53   ` Vladimir Sementsov-Ogievskiy
2020-10-27  5:05 ` [PATCH v6 10/11] nbd: Add 'qemu-nbd -A' to expose allocation depth Eric Blake
2020-10-27 11:03   ` Vladimir Sementsov-Ogievskiy
2020-10-27  5:05 ` [PATCH v6 11/11] qapi: Use QAPI_LIST_ADD() where possible Eric Blake
2020-10-27  5:53   ` Thomas Huth
2020-10-27  6:39   ` David Gibson
2020-10-27 10:09   ` Markus Armbruster
2020-10-27 12:28     ` Eric Blake
2020-10-27 15:36       ` Markus Armbruster
2020-10-27 18:44         ` Eric Blake
2020-10-27 11:26   ` Dr. David Alan Gilbert
2020-10-27 13:42   ` Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201027050556.269064-4-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pkrempa@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).