From 787e4a8500020695eb391e2f1cc4767ee071d441 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 6 Mar 2013 11:52:48 +0100 Subject: block: Add options QDict to bdrv_file_open() prototypes The new parameter is unused yet. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/nbd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'block/nbd.c') diff --git a/block/nbd.c b/block/nbd.c index a5812948d2..0473908996 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -376,7 +376,8 @@ static void nbd_teardown_connection(BlockDriverState *bs) closesocket(s->sock); } -static int nbd_open(BlockDriverState *bs, const char* filename, int flags) +static int nbd_open(BlockDriverState *bs, const char* filename, + QDict *options, int flags) { BDRVNBDState *s = bs->opaque; int result; -- cgit v1.2.3 From f17c90bed11a6e277614b5a5d16434004f24d572 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 15 Mar 2013 11:55:29 +0100 Subject: nbd: Keep hostname and port separate 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 Reviewed-by: Eric Blake --- block/nbd.c | 49 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 9 deletions(-) (limited to 'block/nbd.c') diff --git a/block/nbd.c b/block/nbd.c index 0473908996..ecbc892f19 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); } -- cgit v1.2.3 From f53a1febcd9d887149ac1429880a3f2fdb2c117f Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 7 Mar 2013 16:15:11 +0100 Subject: nbd: Accept -drive options for the network connection The existing parsers for the file name now parse everything into the bdrv_open() options QDict. Instead of using these parsers, you can now directly specify the options on the command line, like this: qemu-system-x86_64 -drive file=nbd:,file.port=1234,file.host=::1 Clearly the file=... part could use further improvement, but it's a start. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/nbd.c | 129 ++++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 77 insertions(+), 52 deletions(-) (limited to 'block/nbd.c') diff --git a/block/nbd.c b/block/nbd.c index ecbc892f19..5ed8502071 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -32,6 +32,8 @@ #include "block/block_int.h" #include "qemu/module.h" #include "qemu/sockets.h" +#include "qapi/qmp/qjson.h" +#include "qapi/qmp/qint.h" #include #include @@ -65,20 +67,19 @@ typedef struct BDRVNBDState { Coroutine *recv_coroutine[MAX_NBD_REQUESTS]; struct nbd_reply reply; - int is_unix; - char *unix_path; - - InetSocketAddress *inet_addr; + bool is_unix; + QemuOpts *socket_opts; char *export_name; /* An NBD server may export several devices */ } BDRVNBDState; -static int nbd_parse_uri(BDRVNBDState *s, const char *filename) +static int nbd_parse_uri(const char *filename, QDict *options) { URI *uri; const char *p; QueryParams *qp = NULL; int ret = 0; + bool is_unix; uri = uri_parse(filename); if (!uri) { @@ -87,11 +88,11 @@ static int nbd_parse_uri(BDRVNBDState *s, const char *filename) /* transport */ if (!strcmp(uri->scheme, "nbd")) { - s->is_unix = false; + is_unix = false; } else if (!strcmp(uri->scheme, "nbd+tcp")) { - s->is_unix = false; + is_unix = false; } else if (!strcmp(uri->scheme, "nbd+unix")) { - s->is_unix = true; + is_unix = true; } else { ret = -EINVAL; goto out; @@ -100,24 +101,26 @@ static int nbd_parse_uri(BDRVNBDState *s, const char *filename) p = uri->path ? uri->path : "/"; p += strspn(p, "/"); if (p[0]) { - s->export_name = g_strdup(p); + qdict_put(options, "export", qstring_from_str(p)); } qp = query_params_parse(uri->query); - if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) { + if (qp->n > 1 || (is_unix && !qp->n) || (!is_unix && qp->n)) { ret = -EINVAL; goto out; } - if (s->is_unix) { + if (is_unix) { /* nbd+unix:///export?socket=path */ if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) { ret = -EINVAL; goto out; } - s->unix_path = g_strdup(qp->p[0].value); + qdict_put(options, "path", qstring_from_str(qp->p[0].value)); } else { /* nbd[+tcp]://host:port/export */ + char *port_str; + if (!uri->server) { ret = -EINVAL; goto out; @@ -126,11 +129,10 @@ static int nbd_parse_uri(BDRVNBDState *s, const char *filename) uri->port = NBD_DEFAULT_PORT; } - s->inet_addr = g_new0(InetSocketAddress, 1); - *s->inet_addr = (InetSocketAddress) { - .host = g_strdup(uri->server), - .port = g_strdup_printf("%d", uri->port), - }; + port_str = g_strdup_printf("%d", uri->port); + qdict_put(options, "host", qstring_from_str(uri->server)); + qdict_put(options, "port", qstring_from_str(port_str)); + g_free(port_str); } out: @@ -141,17 +143,17 @@ out: return ret; } -static int nbd_config(BDRVNBDState *s, const char *filename) +static int nbd_parse_filename(const char *filename, QDict *options) { char *file; char *export_name; const char *host_spec; const char *unixpath; - int err = -EINVAL; + int ret = -EINVAL; Error *local_err = NULL; if (strstr(filename, "://")) { - return nbd_parse_uri(s, filename); + return nbd_parse_uri(filename, options); } file = g_strdup(filename); @@ -163,7 +165,8 @@ static int nbd_config(BDRVNBDState *s, const char *filename) } export_name[0] = 0; /* truncate 'file' */ export_name += strlen(EN_OPTSTR); - s->export_name = g_strdup(export_name); + + qdict_put(options, "export", qstring_from_str(export_name)); } /* extract the host_spec - fail if it's not nbd:... */ @@ -171,32 +174,65 @@ static int nbd_config(BDRVNBDState *s, const char *filename) goto out; } + if (!*host_spec) { + ret = 1; + goto out; + } + /* are we a UNIX or TCP socket? */ if (strstart(host_spec, "unix:", &unixpath)) { - s->is_unix = true; - s->unix_path = g_strdup(unixpath); + qdict_put(options, "path", qstring_from_str(unixpath)); } else { - s->is_unix = false; - s->inet_addr = inet_parse(host_spec, &local_err); + InetSocketAddress *addr = NULL; + + addr = inet_parse(host_spec, &local_err); if (local_err != NULL) { qerror_report_err(local_err); error_free(local_err); goto out; } - } - err = 0; + qdict_put(options, "host", qstring_from_str(addr->host)); + qdict_put(options, "port", qstring_from_str(addr->port)); + qapi_free_InetSocketAddress(addr); + } + ret = 1; out: g_free(file); - if (err != 0) { - g_free(s->export_name); - g_free(s->unix_path); - qapi_free_InetSocketAddress(s->inet_addr); + return ret; +} + +static int nbd_config(BDRVNBDState *s, QDict *options) +{ + Error *local_err = NULL; + + if (qdict_haskey(options, "path")) { + s->is_unix = true; + } else if (qdict_haskey(options, "host")) { + s->is_unix = false; + } else { + return -EINVAL; } - return err; + + s->socket_opts = qemu_opts_create_nofail(&socket_optslist); + + qemu_opts_absorb_qdict(s->socket_opts, options, &local_err); + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); + return -EINVAL; + } + + s->export_name = g_strdup(qdict_get_try_str(options, "export")); + if (s->export_name) { + qdict_del(options, "export"); + } + + return 0; } + static void nbd_coroutine_start(BDRVNBDState *s, struct nbd_request *request) { int i; @@ -343,24 +379,9 @@ static int nbd_establish_connection(BlockDriverState *bs) size_t blocksize; if (s->is_unix) { - sock = unix_socket_outgoing(s->unix_path); + sock = unix_socket_outgoing(qemu_opt_get(s->socket_opts, "path")); } else { - 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); + sock = tcp_socket_outgoing_opts(s->socket_opts); } /* Failed to establish connection */ @@ -416,7 +437,12 @@ static int nbd_open(BlockDriverState *bs, const char* filename, qemu_co_mutex_init(&s->free_sema); /* Pop the config into our state object. Exit if invalid. */ - result = nbd_config(s, filename); + result = nbd_parse_filename(filename, options); + if (result < 0) { + return result; + } + + result = nbd_config(s, options); if (result != 0) { return result; } @@ -580,8 +606,7 @@ static void nbd_close(BlockDriverState *bs) { BDRVNBDState *s = bs->opaque; g_free(s->export_name); - g_free(s->unix_path); - qapi_free_InetSocketAddress(s->inet_addr); + qemu_opts_del(s->socket_opts); nbd_teardown_connection(bs); } -- cgit v1.2.3 From 6963a30d82413bea36c7545137b090b284cc2b18 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 15 Mar 2013 18:47:22 +0100 Subject: block: Introduce .bdrv_parse_filename callback If a driver needs structured data and not just a string, it can provide a .bdrv_parse_filename callback now that parses the command line string into separate options. Keeping this separate from .bdrv_open_filename ensures that the preferred way of directly specifying the options always works as well if parsing the string works. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/nbd.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) (limited to 'block/nbd.c') diff --git a/block/nbd.c b/block/nbd.c index 5ed8502071..9858f060ca 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -143,17 +143,20 @@ out: return ret; } -static int nbd_parse_filename(const char *filename, QDict *options) +static void nbd_parse_filename(const char *filename, QDict *options, + Error **errp) { char *file; char *export_name; const char *host_spec; const char *unixpath; - int ret = -EINVAL; - Error *local_err = NULL; if (strstr(filename, "://")) { - return nbd_parse_uri(filename, options); + int ret = nbd_parse_uri(filename, options); + if (ret < 0) { + error_setg(errp, "No valid URL specified"); + } + return; } file = g_strdup(filename); @@ -171,11 +174,11 @@ static int nbd_parse_filename(const char *filename, QDict *options) /* extract the host_spec - fail if it's not nbd:... */ if (!strstart(file, "nbd:", &host_spec)) { + error_setg(errp, "File name string for NBD must start with 'nbd:'"); goto out; } if (!*host_spec) { - ret = 1; goto out; } @@ -185,10 +188,8 @@ static int nbd_parse_filename(const char *filename, QDict *options) } else { InetSocketAddress *addr = NULL; - addr = inet_parse(host_spec, &local_err); - if (local_err != NULL) { - qerror_report_err(local_err); - error_free(local_err); + addr = inet_parse(host_spec, errp); + if (error_is_set(errp)) { goto out; } @@ -197,10 +198,8 @@ static int nbd_parse_filename(const char *filename, QDict *options) qapi_free_InetSocketAddress(addr); } - ret = 1; out: g_free(file); - return ret; } static int nbd_config(BDRVNBDState *s, QDict *options) @@ -437,11 +436,6 @@ static int nbd_open(BlockDriverState *bs, const char* filename, qemu_co_mutex_init(&s->free_sema); /* Pop the config into our state object. Exit if invalid. */ - result = nbd_parse_filename(filename, options); - if (result < 0) { - return result; - } - result = nbd_config(s, options); if (result != 0) { return result; @@ -622,6 +616,7 @@ static BlockDriver bdrv_nbd = { .format_name = "nbd", .protocol_name = "nbd", .instance_size = sizeof(BDRVNBDState), + .bdrv_parse_filename = nbd_parse_filename, .bdrv_file_open = nbd_open, .bdrv_co_readv = nbd_co_readv, .bdrv_co_writev = nbd_co_writev, @@ -635,6 +630,7 @@ static BlockDriver bdrv_nbd_tcp = { .format_name = "nbd", .protocol_name = "nbd+tcp", .instance_size = sizeof(BDRVNBDState), + .bdrv_parse_filename = nbd_parse_filename, .bdrv_file_open = nbd_open, .bdrv_co_readv = nbd_co_readv, .bdrv_co_writev = nbd_co_writev, @@ -648,6 +644,7 @@ static BlockDriver bdrv_nbd_unix = { .format_name = "nbd", .protocol_name = "nbd+unix", .instance_size = sizeof(BDRVNBDState), + .bdrv_parse_filename = nbd_parse_filename, .bdrv_file_open = nbd_open, .bdrv_co_readv = nbd_co_readv, .bdrv_co_writev = nbd_co_writev, -- cgit v1.2.3 From bebbf7fa9c6235022ecd15f8f934d27e5ccab63a Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 18 Mar 2013 16:56:05 +0100 Subject: nbd: Use default port if only host is specified The URL method already takes care to apply the default port when none is specfied. Directly specifying driver-specific options required the port number until now. Allow leaving it out and apply the default. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/nbd.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'block/nbd.c') diff --git a/block/nbd.c b/block/nbd.c index 9858f060ca..67f1df298b 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -118,21 +118,18 @@ static int nbd_parse_uri(const char *filename, QDict *options) } qdict_put(options, "path", qstring_from_str(qp->p[0].value)); } else { - /* nbd[+tcp]://host:port/export */ - char *port_str; - + /* nbd[+tcp]://host[:port]/export */ if (!uri->server) { ret = -EINVAL; goto out; } - if (!uri->port) { - uri->port = NBD_DEFAULT_PORT; - } - port_str = g_strdup_printf("%d", uri->port); qdict_put(options, "host", qstring_from_str(uri->server)); - qdict_put(options, "port", qstring_from_str(port_str)); - g_free(port_str); + if (uri->port) { + char* port_str = g_strdup_printf("%d", uri->port); + qdict_put(options, "port", qstring_from_str(port_str)); + g_free(port_str); + } } out: @@ -223,6 +220,10 @@ static int nbd_config(BDRVNBDState *s, QDict *options) return -EINVAL; } + if (!qemu_opt_get(s->socket_opts, "port")) { + qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT); + } + s->export_name = g_strdup(qdict_get_try_str(options, "export")); if (s->export_name) { qdict_del(options, "export"); -- cgit v1.2.3 From 681e7ad024d80123a1ae8e35f86fb1a7f03b1bc9 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 20 Mar 2013 19:23:23 +0100 Subject: nbd: Check against invalid option combinations A file name may only specified if no host or socket path is specified. The latter two may not appear at the same time either. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/nbd.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'block/nbd.c') diff --git a/block/nbd.c b/block/nbd.c index 67f1df298b..3d711b2735 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -148,6 +148,15 @@ static void nbd_parse_filename(const char *filename, QDict *options, const char *host_spec; const char *unixpath; + if (qdict_haskey(options, "host") + || qdict_haskey(options, "port") + || qdict_haskey(options, "path")) + { + error_setg(errp, "host/port/path and a file name may not be specified " + "at the same time"); + return; + } + if (strstr(filename, "://")) { int ret = nbd_parse_uri(filename, options); if (ret < 0) { @@ -204,6 +213,11 @@ static int nbd_config(BDRVNBDState *s, QDict *options) Error *local_err = NULL; if (qdict_haskey(options, "path")) { + if (qdict_haskey(options, "host")) { + qerror_report(ERROR_CLASS_GENERIC_ERROR, "path and host may not " + "be used at the same time."); + return -EINVAL; + } s->is_unix = true; } else if (qdict_haskey(options, "host")) { s->is_unix = false; -- cgit v1.2.3