On 28/03/2024 15.59, Daniel P. Berrangé wrote:
On Thu, Mar 28, 2024 at 09:54:49AM -0500, Eric Blake wrote:
On Thu, Mar 28, 2024 at 03:06:03PM +0100, Thomas Huth wrote:
Since version 2.66, glib has useful URI parsing functions, too.
Use those instead of the QEMU-internal ones to be finally able
to get rid of the latter. The g_uri_get_host() also takes care
of removing the square brackets from IPv6 addresses, so we can
drop that part of the QEMU code now, too.
Signed-off-by: Thomas Huth <th...@redhat.com>
---
block/nbd.c | 66 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 38 insertions(+), 28 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index ef05f7cdfd..95b507f872 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -31,7 +31,6 @@
#include "qemu/osdep.h"
#include "trace.h"
-#include "qemu/uri.h"
#include "qemu/option.h"
#include "qemu/cutils.h"
#include "qemu/main-loop.h"
@@ -1514,30 +1513,34 @@ static void nbd_client_close(BlockDriverState *bs)
static int nbd_parse_uri(const char *filename, QDict *options)
{
- URI *uri;
+ GUri *uri;
Is it worth using 'g_autoptr(GUri) uri = NULL;' here, to simplify cleanup later?
Sounds like a good idea, I'll give it a try!
const char *p;
- QueryParams *qp = NULL;
+ GHashTable *qp = NULL;
Presumably would be easier if qp is also auto-free.
+ int qp_n;
int ret = 0;
bool is_unix;
+ const char *uri_scheme, *uri_query, *uri_server;
+ int uri_port;
- uri = uri_parse(filename);
+ uri = g_uri_parse(filename, G_URI_FLAGS_NONE, NULL);
The glib API is fairly close to what we have in qemu, making this a
nice switchover.
/* nbd[+tcp]://host[:port]/export */
- if (!uri->server) {
+ if (!uri_server) {
ret = -EINVAL;
goto out;
}
- /* strip braces from literal IPv6 address */
- if (uri->server[0] == '[') {
- host = qstring_from_substr(uri->server, 1,
- strlen(uri->server) - 1);
- } else {
- host = qstring_from_str(uri->server);
- }
-
qdict_put_str(options, "server.type", "inet");
- qdict_put(options, "server.host", host);
+ qdict_put_str(options, "server.host", uri_server);
- port_str = g_strdup_printf("%d", uri->port ?: NBD_DEFAULT_PORT);
+ port_str = g_strdup_printf("%d", uri_port != -1 ? uri_port
+ : NBD_DEFAULT_PORT);
qdict_put_str(options, "server.port", port_str);
If a user requests nbd://hostname:0/export, this now sets server.port
to "0" instead of "10809". Is that an intentional change? No one
actually passes an explicit ":0" port on purpose, but we do have to
worry about malicious URIs.
Passing '0' will cause the kernel to allocate a random free
port, so that is potentially introducing new semantics ?
Ok, so passing a 0 does not really make sense, since QEMU needs to know the
exact port. I'll change the check from "uri_port != -1" to "uri_port > 0" in
the next version.
Thomas
