Re: [PATCH 09/13] nbd: add GUri-based URI parsing version

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Jul 09, 2020 at 11:42:30PM +0400, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  block/nbd.c        | 86 +++++++++++++++++++++++++++++++++++-----------
>  util/Makefile.objs |  2 +-
>  2 files changed, 66 insertions(+), 22 deletions(-)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index faadcab442b..fdc4a53a98f 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -31,7 +31,10 @@
>  #include "qemu/osdep.h"
>  
>  #include "trace.h"
> +#ifndef HAVE_GLIB_GURI
>  #include "qemu/uri.h"
> +#endif
> +#include "qemu/error-report.h"
>  #include "qemu/option.h"
>  #include "qemu/cutils.h"
>  #include "qemu/main-loop.h"
> @@ -1513,71 +1516,112 @@ static int nbd_client_connect(BlockDriverState *bs, Error **errp)
>  /*
>   * Parse nbd_open options
>   */
> -
>  static int nbd_parse_uri(const char *filename, QDict *options)
>  {
> +    const char *p, *scheme, *server, *socket = NULL;
> +    int port;
> +    bool is_unix;
> +
> +#ifdef HAVE_GLIB_GURI
> +    g_autoptr(GUri) uri = NULL;
> +    g_autoptr(GHashTable) params = NULL;
> +    g_autoptr(GError) err = NULL;
> +
> +    uri = g_uri_parse(filename, G_URI_FLAGS_ENCODED_QUERY, &err);
> +    if (!uri) {
> +        error_report("Failed to parse NBD URI: %s", err->message);
> +        return -EINVAL;
> +    }
> +
> +    p = g_uri_get_path(uri);
> +    scheme = g_uri_get_scheme(uri);
> +    server = g_uri_get_host(uri);
> +    port = g_uri_get_port(uri);

I would have expected this code to fail to compile as we're setting
GLIB_VERSION_MAX_ALLOWED == GLIB_VERSION_2_48  and GUri is tagged
as newer than that.

In any case, having this conditonal code in all callers is definitely
not a desirable approach. If we want to use it, then I think we need to
pull a copy of GUri into QEMU and expose it via glib-compat.h, so that
callers can use it unconditionally.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|