Patchwork [06/14] nbd: Keep hostname and port separate

login
register
mail settings
Submitter Kevin Wolf
Date March 22, 2013, 5:41 p.m.
Message ID <1363974083-28440-7-git-send-email-kwolf@redhat.com>
Download mbox | patch
Permalink /patch/230225/
State New
Headers show

Comments

Kevin Wolf - March 22, 2013, 5:41 p.m.
The NBD block supports an URL syntax, for which a URL parser returns
separate hostname and port fields. It also supports the traditional qemu
syntax encoded in a filename. Until now, after parsing the URL to get
each piece of information, a new string is built to be fed to socket
functions.

Instead of building a string in the URL case that is immediately parsed
again, parse the string in both cases and use the QemuOpts interface to
qemu-sockets.c.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/nbd.c            | 49 ++++++++++++++++++++++++++++++++++++++++---------
 include/block/nbd.h    |  2 ++
 include/qemu/sockets.h |  1 +
 nbd.c                  | 12 ++++++++++++
 util/qemu-sockets.c    |  6 +++---
 5 files changed, 58 insertions(+), 12 deletions(-)
Paolo Bonzini - March 22, 2013, 10:46 p.m.
Il 22/03/2013 18:41, Kevin Wolf ha scritto:
> +        QemuOpts *opts = qemu_opts_create_nofail(&socket_optslist);
> +
> +        qemu_opt_set(opts, "host", s->inet_addr->host);
> +        qemu_opt_set(opts, "port", s->inet_addr->port);
> +        if (s->inet_addr->has_to) {
> +            qemu_opt_set_number(opts, "to", s->inet_addr->to);
> +        }
> +        if (s->inet_addr->has_ipv4) {
> +            qemu_opt_set_number(opts, "ipv4", s->inet_addr->ipv4);
> +        }
> +        if (s->inet_addr->has_ipv6) {
> +            qemu_opt_set_number(opts, "ipv6", s->inet_addr->ipv6);
> +        }
> +
> +        sock = tcp_socket_outgoing_opts(opts);

Sorry for the late review... You're basically reinventing socket_connect
here.  Would like to clean it up or shall I do it?

Paolo
Kevin Wolf - March 25, 2013, 9:09 a.m.
Am 22.03.2013 um 23:46 hat Paolo Bonzini geschrieben:
> Il 22/03/2013 18:41, Kevin Wolf ha scritto:
> > +        QemuOpts *opts = qemu_opts_create_nofail(&socket_optslist);
> > +
> > +        qemu_opt_set(opts, "host", s->inet_addr->host);
> > +        qemu_opt_set(opts, "port", s->inet_addr->port);
> > +        if (s->inet_addr->has_to) {
> > +            qemu_opt_set_number(opts, "to", s->inet_addr->to);
> > +        }
> > +        if (s->inet_addr->has_ipv4) {
> > +            qemu_opt_set_number(opts, "ipv4", s->inet_addr->ipv4);
> > +        }
> > +        if (s->inet_addr->has_ipv6) {
> > +            qemu_opt_set_number(opts, "ipv6", s->inet_addr->ipv6);
> > +        }
> > +
> > +        sock = tcp_socket_outgoing_opts(opts);
> 
> Sorry for the late review... You're basically reinventing socket_connect
> here.  Would like to clean it up or shall I do it?

It's probably best if you change whatever you like to have changed.
FWIW, this specific code doesn't exist any more at the end of the
series.

Kevin

Patch

diff --git a/block/nbd.c b/block/nbd.c
index 0473908..ecbc892 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -66,7 +66,10 @@  typedef struct BDRVNBDState {
     struct nbd_reply reply;
 
     int is_unix;
-    char *host_spec;
+    char *unix_path;
+
+    InetSocketAddress *inet_addr;
+
     char *export_name; /* An NBD server may export several devices */
 } BDRVNBDState;
 
@@ -112,7 +115,7 @@  static int nbd_parse_uri(BDRVNBDState *s, const char *filename)
             ret = -EINVAL;
             goto out;
         }
-        s->host_spec = g_strdup(qp->p[0].value);
+        s->unix_path = g_strdup(qp->p[0].value);
     } else {
         /* nbd[+tcp]://host:port/export */
         if (!uri->server) {
@@ -122,7 +125,12 @@  static int nbd_parse_uri(BDRVNBDState *s, const char *filename)
         if (!uri->port) {
             uri->port = NBD_DEFAULT_PORT;
         }
-        s->host_spec = g_strdup_printf("%s:%d", uri->server, uri->port);
+
+        s->inet_addr = g_new0(InetSocketAddress, 1);
+        *s->inet_addr = (InetSocketAddress) {
+            .host   = g_strdup(uri->server),
+            .port   = g_strdup_printf("%d", uri->port),
+        };
     }
 
 out:
@@ -140,6 +148,7 @@  static int nbd_config(BDRVNBDState *s, const char *filename)
     const char *host_spec;
     const char *unixpath;
     int err = -EINVAL;
+    Error *local_err = NULL;
 
     if (strstr(filename, "://")) {
         return nbd_parse_uri(s, filename);
@@ -165,10 +174,15 @@  static int nbd_config(BDRVNBDState *s, const char *filename)
     /* are we a UNIX or TCP socket? */
     if (strstart(host_spec, "unix:", &unixpath)) {
         s->is_unix = true;
-        s->host_spec = g_strdup(unixpath);
+        s->unix_path = g_strdup(unixpath);
     } else {
         s->is_unix = false;
-        s->host_spec = g_strdup(host_spec);
+        s->inet_addr = inet_parse(host_spec, &local_err);
+        if (local_err != NULL) {
+            qerror_report_err(local_err);
+            error_free(local_err);
+            goto out;
+        }
     }
 
     err = 0;
@@ -177,7 +191,8 @@  out:
     g_free(file);
     if (err != 0) {
         g_free(s->export_name);
-        g_free(s->host_spec);
+        g_free(s->unix_path);
+        qapi_free_InetSocketAddress(s->inet_addr);
     }
     return err;
 }
@@ -328,9 +343,24 @@  static int nbd_establish_connection(BlockDriverState *bs)
     size_t blocksize;
 
     if (s->is_unix) {
-        sock = unix_socket_outgoing(s->host_spec);
+        sock = unix_socket_outgoing(s->unix_path);
     } else {
-        sock = tcp_socket_outgoing_spec(s->host_spec);
+        QemuOpts *opts = qemu_opts_create_nofail(&socket_optslist);
+
+        qemu_opt_set(opts, "host", s->inet_addr->host);
+        qemu_opt_set(opts, "port", s->inet_addr->port);
+        if (s->inet_addr->has_to) {
+            qemu_opt_set_number(opts, "to", s->inet_addr->to);
+        }
+        if (s->inet_addr->has_ipv4) {
+            qemu_opt_set_number(opts, "ipv4", s->inet_addr->ipv4);
+        }
+        if (s->inet_addr->has_ipv6) {
+            qemu_opt_set_number(opts, "ipv6", s->inet_addr->ipv6);
+        }
+
+        sock = tcp_socket_outgoing_opts(opts);
+        qemu_opts_del(opts);
     }
 
     /* Failed to establish connection */
@@ -550,7 +580,8 @@  static void nbd_close(BlockDriverState *bs)
 {
     BDRVNBDState *s = bs->opaque;
     g_free(s->export_name);
-    g_free(s->host_spec);
+    g_free(s->unix_path);
+    qapi_free_InetSocketAddress(s->inet_addr);
 
     nbd_teardown_connection(bs);
 }
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 344f05b..9b52d50 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -22,6 +22,7 @@ 
 #include <sys/types.h>
 
 #include "qemu-common.h"
+#include "qemu/option.h"
 
 struct nbd_request {
     uint32_t magic;
@@ -64,6 +65,7 @@  int tcp_socket_outgoing(const char *address, uint16_t port);
 int tcp_socket_incoming(const char *address, uint16_t port);
 int tcp_socket_outgoing_spec(const char *address_and_port);
 int tcp_socket_incoming_spec(const char *address_and_port);
+int tcp_socket_outgoing_opts(QemuOpts *opts);
 int unix_socket_outgoing(const char *path);
 int unix_socket_incoming(const char *path);
 
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 21846f9..d225f6d 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -47,6 +47,7 @@  int recv_all(int fd, void *buf, int len1, bool single_read);
  */
 typedef void NonBlockingConnectHandler(int fd, void *opaque);
 
+InetSocketAddress *inet_parse(const char *str, Error **errp);
 int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
 int inet_listen(const char *str, char *ostr, int olen,
                 int socktype, int port_offset, Error **errp);
diff --git a/nbd.c b/nbd.c
index 0698a02..97879ca 100644
--- a/nbd.c
+++ b/nbd.c
@@ -218,6 +218,18 @@  int tcp_socket_outgoing_spec(const char *address_and_port)
     return fd;
 }
 
+int tcp_socket_outgoing_opts(QemuOpts *opts)
+{
+    Error *local_err = NULL;
+    int fd = inet_connect_opts(opts, &local_err, NULL, NULL);
+    if (local_err != NULL) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+
+    return fd;
+}
+
 int tcp_socket_incoming(const char *address, uint16_t port)
 {
     char address_and_port[128];
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 39717d0..dc7524d 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -485,7 +485,7 @@  err:
 }
 
 /* compatibility wrapper */
-static InetSocketAddress *inet_parse(const char *str, Error **errp)
+InetSocketAddress *inet_parse(const char *str, Error **errp)
 {
     InetSocketAddress *addr;
     const char *optstr, *h;
@@ -555,7 +555,7 @@  fail:
     return NULL;
 }
 
-static void inet_addr_to_opts(QemuOpts *opts, InetSocketAddress *addr)
+static void inet_addr_to_opts(QemuOpts *opts, const InetSocketAddress *addr)
 {
     bool ipv4 = addr->ipv4 || !addr->has_ipv4;
     bool ipv6 = addr->ipv6 || !addr->has_ipv6;
@@ -622,7 +622,7 @@  int inet_connect(const char *str, Error **errp)
 
     addr = inet_parse(str, errp);
     if (addr != NULL) {
-        opts = qemu_opts_create_nofail(&dummy_opts);
+        opts = qemu_opts_create_nofail(&socket_optslist);
         inet_addr_to_opts(opts, addr);
         qapi_free_InetSocketAddress(addr);
         sock = inet_connect_opts(opts, errp, NULL, NULL);