diff mbox

[v1,1/2] nbd: convert to use the QAPI SocketAddress object

Message ID 1442411543-28513-2-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé Sept. 16, 2015, 1:52 p.m. UTC 
The nbd block driver currently uses a QemuOpts object
when setting up sockets. Switch it over to use the
QAPI SocketAddress object instead.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/nbd.c | 69 +++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 37 insertions(+), 32 deletions(-)

Comments

Eric Blake Sept. 16, 2015, 2:02 p.m. UTC | #1 
On 09/16/2015 07:52 AM, Daniel P. Berrange wrote:
> The nbd block driver currently uses a QemuOpts object
> when setting up sockets. Switch it over to use the
> QAPI SocketAddress object instead.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  block/nbd.c | 69 +++++++++++++++++++++++++++++++++----------------------------
>  1 file changed, 37 insertions(+), 32 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

Wen - this patch will help in your efforts to expose NBD via
blockdev-add QMP.

> -    if (!qemu_opt_get(s->socket_opts, "port")) {
> -        qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT,
> -                            &error_abort);
> -    }
> +    s->client.is_unix = saddr->kind == SOCKET_ADDRESS_KIND_UNIX;
>  

Minor conflict with my pending patch to rename the C representation of
qapi unions to spell things with 'type':
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02598.html
but as that needs to be rebased once Markus' introspection lands, you
don't have to wait for it.
Paolo Bonzini Sept. 16, 2015, 2:21 p.m. UTC | #2 
On 16/09/2015 15:52, Daniel P. Berrange wrote:
> +    saddr = nbd_config(s, options, &export, &local_err);

You can use errp directly here.  Otherwise looks fine.

Paolo

> +    if (!saddr) {
>          error_propagate(errp, local_err);
>          return -EINVAL;
>      }
diff mbox

Patch

diff --git a/block/nbd.c b/block/nbd.c
index 2176186..b3ba54a 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -43,7 +43,6 @@ 
 
 typedef struct BDRVNBDState {
     NbdClientSession client;
-    QemuOpts *socket_opts;
 } BDRVNBDState;
 
 static int nbd_parse_uri(const char *filename, QDict *options)
@@ -190,10 +189,10 @@  out:
     g_free(file);
 }
 
-static void nbd_config(BDRVNBDState *s, QDict *options, char **export,
-                       Error **errp)
+static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char **export,
+                                 Error **errp)
 {
-    Error *local_err = NULL;
+    SocketAddress *saddr;
 
     if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) {
         if (qdict_haskey(options, "path")) {
@@ -201,28 +200,37 @@  static void nbd_config(BDRVNBDState *s, QDict *options, char **export,
         } else {
             error_setg(errp, "one of path and host must be specified.");
         }
-        return;
+        return NULL;
     }
 
-    s->client.is_unix = qdict_haskey(options, "path");
-    s->socket_opts = qemu_opts_create(&socket_optslist, NULL, 0,
-                                      &error_abort);
+    saddr = g_new0(SocketAddress, 1);
 
-    qemu_opts_absorb_qdict(s->socket_opts, options, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
+    if (qdict_haskey(options, "path")) {
+        saddr->kind = SOCKET_ADDRESS_KIND_UNIX;
+        saddr->q_unix = g_new0(UnixSocketAddress, 1);
+        saddr->q_unix->path = g_strdup(qdict_get_str(options, "path"));
+        qdict_del(options, "path");
+    } else {
+        saddr->kind = SOCKET_ADDRESS_KIND_INET;
+        saddr->inet = g_new0(InetSocketAddress, 1);
+        saddr->inet->host = g_strdup(qdict_get_str(options, "host"));
+        if (!qdict_get_try_str(options, "port")) {
+            saddr->inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT);
+        } else {
+            saddr->inet->port = g_strdup(qdict_get_str(options, "port"));
+        }
+        qdict_del(options, "host");
+        qdict_del(options, "port");
     }
 
-    if (!qemu_opt_get(s->socket_opts, "port")) {
-        qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT,
-                            &error_abort);
-    }
+    s->client.is_unix = saddr->kind == SOCKET_ADDRESS_KIND_UNIX;
 
     *export = g_strdup(qdict_get_try_str(options, "export"));
     if (*export) {
         qdict_del(options, "export");
     }
+
+    return saddr;
 }
 
 NbdClientSession *nbd_get_client_session(BlockDriverState *bs)
@@ -231,26 +239,24 @@  NbdClientSession *nbd_get_client_session(BlockDriverState *bs)
     return &s->client;
 }
 
-static int nbd_establish_connection(BlockDriverState *bs, Error **errp)
+static int nbd_establish_connection(BlockDriverState *bs,
+                                    SocketAddress *saddr,
+                                    Error **errp)
 {
     BDRVNBDState *s = bs->opaque;
     int sock;
 
-    if (s->client.is_unix) {
-        sock = unix_connect_opts(s->socket_opts, errp, NULL, NULL);
-    } else {
-        sock = inet_connect_opts(s->socket_opts, errp, NULL, NULL);
-        if (sock >= 0) {
-            socket_set_nodelay(sock);
-        }
-    }
+    sock = socket_connect(saddr, errp, NULL, NULL);
 
-    /* Failed to establish connection */
     if (sock < 0) {
         logout("Failed to establish connection to NBD server\n");
         return -EIO;
     }
 
+    if (!s->client.is_unix) {
+        socket_set_nodelay(sock);
+    }
+
     return sock;
 }
 
@@ -261,10 +267,11 @@  static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
     char *export = NULL;
     int result, sock;
     Error *local_err = NULL;
+    SocketAddress *saddr;
 
     /* Pop the config into our state object. Exit if invalid. */
-    nbd_config(s, options, &export, &local_err);
-    if (local_err) {
+    saddr = nbd_config(s, options, &export, &local_err);
+    if (!saddr) {
         error_propagate(errp, local_err);
         return -EINVAL;
     }
@@ -272,7 +279,8 @@  static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
     /* establish TCP connection, return error if it fails
      * TODO: Configurable retry-until-timeout behaviour.
      */
-    sock = nbd_establish_connection(bs, errp);
+    sock = nbd_establish_connection(bs, saddr, errp);
+    qapi_free_SocketAddress(saddr);
     if (sock < 0) {
         g_free(export);
         return sock;
@@ -315,9 +323,6 @@  static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
 
 static void nbd_close(BlockDriverState *bs)
 {
-    BDRVNBDState *s = bs->opaque;
-
-    qemu_opts_del(s->socket_opts);
     nbd_client_close(bs);
 }