Libguestfs - Sep 2016 - Re: [PATCH] fish: drop leading '/' in nbd paths (RHBZ#1379585)

When parsing the URI, drop the leading '/' from the path also when the
protocol is 'nbd': in this case, the path represents the export name,
which does not need the '/' coming from the URI format.

Improve the coverage for nbd in test-add-uri.sh, adding a couple of
tests, and adjusting the result of an existing one.
---
 fish/test-add-uri.sh | 8 +++++++-
 fish/uri.c           | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fish/test-add-uri.sh b/fish/test-add-uri.sh
index 07b68c4..0264829 100755
--- a/fish/test-add-uri.sh
+++ b/fish/test-add-uri.sh
@@ -49,14 +49,20 @@ grep -sq 'add_drive "disk"
"protocol:gluster" "server:tcp:example.com"' test-add
 $VG guestfish -x -a nbd://example.com </dev/null >test-add-uri.out
2>&1
 grep -sq 'add_drive "" "protocol:nbd"
"server:tcp:example.com"' test-add-uri.out || fail
 
+$VG guestfish -x -a nbd://example.com/export </dev/null >test-add-uri.out
2>&1
+grep -sq 'add_drive "export" "protocol:nbd"
"server:tcp:example.com"' test-add-uri.out || fail
+
 $VG guestfish -x -a nbd://example.com:3000 </dev/null >test-add-uri.out
2>&1
 grep -sq 'add_drive "" "protocol:nbd"
"server:tcp:example.com:3000"' test-add-uri.out || fail
 
+$VG guestfish -x -a nbd://example.com:3000/export </dev/null
>test-add-uri.out 2>&1
+grep -sq 'add_drive "export" "protocol:nbd"
"server:tcp:example.com:3000"' test-add-uri.out || fail
+
 $VG guestfish -x -a 'nbd://?socket=/sk' </dev/null
>test-add-uri.out 2>&1
 grep -sq 'add_drive "" "protocol:nbd"
"server:unix:/sk"' test-add-uri.out || fail
 
 $VG guestfish -x -a 'nbd:///export?socket=/sk' </dev/null
>test-add-uri.out 2>&1
-grep -sq 'add_drive "/export" "protocol:nbd"
"server:unix:/sk"' test-add-uri.out || fail
+grep -sq 'add_drive "export" "protocol:nbd"
"server:unix:/sk"' test-add-uri.out || fail
 
 # rbd
 $VG guestfish -x -a rbd://example.com:6789/pool/disk </dev/null
>test-add-uri.out 2>&1
diff --git a/fish/uri.c b/fish/uri.c
index ae30bce..4ae34af 100644
--- a/fish/uri.c
+++ b/fish/uri.c
@@ -194,6 +194,7 @@ parse (const char *arg, char **path_ret, char
**protocol_ret,
   if (path && path[0] == '/' &&
       (STREQ (uri->scheme, "gluster") ||
        STREQ (uri->scheme, "iscsi") ||
+       STREQ (uri->scheme, "nbd") ||
        STREQ (uri->scheme, "rbd") ||
        STREQ (uri->scheme, "sheepdog")))
     path++;
-- 
2.7.4

On Tue, Sep 27, 2016 at 11:20:07AM +0200, Pino Toscano
wrote:
> When parsing the URI, drop the leading '/' from the path also when
the
> protocol is 'nbd': in this case, the path represents the export
name,
> which does not need the '/' coming from the URI format.
> 
> Improve the coverage for nbd in test-add-uri.sh, adding a couple of
> tests, and adjusting the result of an existing one.
The trouble with this is that exportnames generally do start
with a '/'.

Can one specify an exportname if it begins with '/'?

Rich.
>  fish/test-add-uri.sh | 8 +++++++-
>  fish/uri.c           | 1 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fish/test-add-uri.sh b/fish/test-add-uri.sh
> index 07b68c4..0264829 100755
> --- a/fish/test-add-uri.sh
> +++ b/fish/test-add-uri.sh
> @@ -49,14 +49,20 @@ grep -sq 'add_drive "disk"
"protocol:gluster" "server:tcp:example.com"' test-add
>  $VG guestfish -x -a nbd://example.com </dev/null >test-add-uri.out
2>&1
>  grep -sq 'add_drive "" "protocol:nbd"
"server:tcp:example.com"' test-add-uri.out || fail
>  
> +$VG guestfish -x -a nbd://example.com/export </dev/null
>test-add-uri.out 2>&1
> +grep -sq 'add_drive "export" "protocol:nbd"
"server:tcp:example.com"' test-add-uri.out || fail
> +
>  $VG guestfish -x -a nbd://example.com:3000 </dev/null
>test-add-uri.out 2>&1
>  grep -sq 'add_drive "" "protocol:nbd"
"server:tcp:example.com:3000"' test-add-uri.out || fail
>  
> +$VG guestfish -x -a nbd://example.com:3000/export </dev/null
>test-add-uri.out 2>&1
> +grep -sq 'add_drive "export" "protocol:nbd"
"server:tcp:example.com:3000"' test-add-uri.out || fail
> +
>  $VG guestfish -x -a 'nbd://?socket=/sk' </dev/null
>test-add-uri.out 2>&1
>  grep -sq 'add_drive "" "protocol:nbd"
"server:unix:/sk"' test-add-uri.out || fail
>  
>  $VG guestfish -x -a 'nbd:///export?socket=/sk' </dev/null
>test-add-uri.out 2>&1
> -grep -sq 'add_drive "/export" "protocol:nbd"
"server:unix:/sk"' test-add-uri.out || fail
> +grep -sq 'add_drive "export" "protocol:nbd"
"server:unix:/sk"' test-add-uri.out || fail
>  
>  # rbd
>  $VG guestfish -x -a rbd://example.com:6789/pool/disk </dev/null
>test-add-uri.out 2>&1
> diff --git a/fish/uri.c b/fish/uri.c
> index ae30bce..4ae34af 100644
> --- a/fish/uri.c
> +++ b/fish/uri.c
> @@ -194,6 +194,7 @@ parse (const char *arg, char **path_ret, char
**protocol_ret,
>    if (path && path[0] == '/' &&
>        (STREQ (uri->scheme, "gluster") ||
>         STREQ (uri->scheme, "iscsi") ||
> +       STREQ (uri->scheme, "nbd") ||
>         STREQ (uri->scheme, "rbd") ||
>         STREQ (uri->scheme, "sheepdog")))
>      path++;
> -- 
> 2.7.4
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Tuesday, 27 September 2016 11:08:34 CEST Richard W.M. Jones
wrote:
> On Tue, Sep 27, 2016 at 11:20:07AM +0200, Pino Toscano wrote:
> > When parsing the URI, drop the leading '/' from the path also
when the
> > protocol is 'nbd': in this case, the path represents the
export name,
> > which does not need the '/' coming from the URI format.
> > 
> > Improve the coverage for nbd in test-add-uri.sh, adding a couple of
> > tests, and adjusting the result of an existing one.
> 
> The trouble with this is that exportnames generally do start
> with a '/'.
This seems what nbd-server requires, although the protocol documentation
does not put any limitation to that.  Indeed, neither qemu-nbd nor
nbdkit seem to do any validation on it (thus accepting even names not
starting with '/'.
> Can one specify an exportname if it begins with '/'?
When using the API directly, 'add-drive "/name" ..' will still
work,
while nbd://host[:port]//name (with two slashes) as URI will do the job.
It's a bit ugly, I agree, but I don't see a way to make both work with
the URI syntax.

-- 
Pino Toscano