Re: [PATCH for-9.1 6/9] block/nbd: Use URI parsing code from glib
Adjusting cc list to add upstream NBD and drop developers unrelated to
this part of the qemu series...
On Thu, Mar 28, 2024 at 02:13:42PM +0000, Richard W.M. Jones 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.
> >
> >
> > if (is_unix) {
> > /* nbd+unix:///export?socket=path */
> > - if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) {
> > + const char *uri_socket = g_hash_table_lookup(qp, "socket");
> > + if (uri_server || uri_port != -1 || !uri_socket) {
> > ret = -EINVAL;
> > goto out;
> > }
The spec for NBD URIs is at:
https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md
Should any of this spec mention case-insensitive concerns, such as
whether 'NBD://' may be equivalent to 'nbd://', and whether
'nbd+unix:///?socket=x' is equivalent to 'nbd+unix:///?Socket=x'?
Right now, I think that most implementations of NBD servers and
clients happen to use case-sensitive parsing; but glib provides the
option to do case-insensitive query parsing.
If I read https://docs.gtk.org/glib/type_func.Uri.parse_params.html
correctly, passing G_URI_PARAMS_CASE_INSENSITIVE (which you did not
do) would mean that 'nbd+unix:///?socket=ignore&Socket=/for/real'
would result in this g_hash_table_lookup finding only "Socket", not
"socket". Maybe it is worth an explicit addition to the NBD URI spec
to mention that we intend to be case-sensitive (in the parts where it
can be; I'm not sure if the schema part must be handled
case-insensitively without re-reading the RFCs), and therefore that
'Socket=' does NOT have the same meaning as 'socket='.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
Reply to: