Re: [Libguestfs] [libnbd PATCH 0/2] Tighten URI parser

On 6/26/19 4:05 AM, Martin Kletzander wrote: > On Tue, Jun 25, 2019 at 09:09:58PM -0500, Eric Blake wrote: >> I'm not sure whether we want to go with just the first patch (reject >> nbd:unix:/path but still accept nbd:/path), or squash the two in order >> to go with the second (reject both abbreviated forms, and require >> scheme://...).  Either way, though, nbdkit -U - --run '$nbd' will now >> error out rather than inadvertently connect over TCP to >> localhost:10809 instead of the intended Unix connection (and in the >> meantime, you want to use --run '$unixsocket', or maybe we should >> teach nbdkit to support --run '$uri'). >> > > Oh, I get it now. So "nbd:unix:/path" is a valid URL without authority > where the > path is "unix:/path".  Well, TIL =) > It is nicely visible here: > >  https://tools.ietf.org/html/rfc3986#section-3 > > These patches are fine from my standpoint, although I must say thinking > about > the above as defined in the RFC makes me re-evaluate my feelings about it. > Having URI without authority just use a local file (socket) makes sense > now. Except that our URI proposal is stating that we want: nbd+unix:///exportname?socket=/path/to/sock Using a mere nbd+unix:/path/to/sock would be wrong, as it would treat '/path/to/sock' as the export name, not the socket name. > Even having the unix: prefix to show that it is a UNIX socket seems > sensible. > At least unless you realize that having an actual URI it makes sense to > have > that as a part of the scheme, as is customary. > > What I am trying to say is that maybe just the first patch is enough.  When > people want to just write, for example nbd:/tmp/my_nbd.sock by hand, it is > easier than typing the whole nbd+unix:?socket=/tmp/my_nbd.sock.  On the > other > hand, how often do you need to write it manually... My worry is that nbd:/path/to/sock is wrong for use as a Unix socket connection, because it leaves no place for the export name (even if the export name is ''). It is too confusing if 'nbd:export' sometimes treats the path part as the export name, and sometimes as the socket name, so I'd rather that we ALWAYS treat the path part as the export name. > > Anyway, I don't really have strong feelings about this, so whatever you > choose > is fine as long as nbd:unix:/path does not try connecting to localhost over > network =) > >> Eric Blake (2): >>  uri: Reject nbd:unix:/path/to/socket as invalid URI >>  uri: Reject nbd:unix:/path/to/socket as invalid URI > > The second one should say "Reject nbd:/path/to/socket as invalid URL", I > guess. My actual plan is to squash it into a single patch; I just posted as two patches in case we only wanted to go halfway (the first patch). But given this conversation, you've convinced me that it is more confusing to allow anything not of the form nbd://, even if it is valid as a URI. (That is, we are explicitly declaring that when using the nbd: or nbd+unix: scheme, we have additional restrictions that make only a subset of valid URIs usable in our scheme).

-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org