Libguestfs - Jun 2019 - Re: [nbdkit PATCH] captive: Support $uri in --run

The existing --run '$nbd' outputs an older form that differs between
libguestfs and qemu, and which is not always a valid URI.  For
historical compatibility, we probably can't change that; but we can
instead add a new '$uri' that outputs a valid URI.  Note that the
libguestfs '$nbd' TCP form is already a valid URI, but that libguestfs
still needs to be taught about the nbd+unix:// scheme, so for there,
you are still better off using '$nbd'; but for qemu, using
'$uri'
already works since qemu v1.3.0 in 2012.

Note that the NBD project has not actually yet posted a link for the
preferred URI syntax; once Rich is happy with his work on that
project, we can touch this up to link to that page.

Reported-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 docs/nbdkit-captive.pod |  8 +++++++-
 server/captive.c        | 14 +++++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod
index 6f69cca5..59df6690 100644
--- a/docs/nbdkit-captive.pod
+++ b/docs/nbdkit-captive.pod
@@ -42,9 +42,15 @@ The following shell variables are available in the
I<--run> argument:

 =over 4

+=item C<$uri>
+
+A URI that refers to the nbdkit port or socket in the preferred form
+documented by the NBD project.
+
 =item C<$nbd>

-A URL that refers to the nbdkit port or socket.
+An older URL that refers to the nbdkit port or socket in a manner
+specific to certain tools.

 Note there is some magic here, since qemu and guestfish URLs have a
 different format, so nbdkit tries to guess which you are running.  If
diff --git a/server/captive.c b/server/captive.c
index 6971af2e..c5274f65 100644
--- a/server/captive.c
+++ b/server/captive.c
@@ -66,7 +66,19 @@ run_command (void)
     exit (EXIT_FAILURE);
   }

-  /* Construct $nbd "URL".  Unfortunately guestfish and qemu take
+  /* Construct $uri. */
+  fprintf (fp, "uri=");
+  if (port) {
+    fprintf (fp, "nbd://localhost:");
+    shell_quote (port, fp);
+  }
+  else if (unixsocket) {
+    fprintf (fp, "nbd+unix://\\?socket=");
+    shell_quote (unixsocket, fp);
+  }
+  fprintf (fp, "\n");
+
+  /* Construct older $nbd "URL".  Unfortunately guestfish and qemu
take
    * different syntax, so try to guess which one we need.
    */
   fprintf (fp, "nbd=");
-- 
2.20.1

On Tue, Jun 25, 2019 at 09:35:11PM -0500, Eric Blake
wrote:
>The existing --run '$nbd' outputs an older form that differs between
>libguestfs and qemu, and which is not always a valid URI.  For
>historical compatibility, we probably can't change that; but we can
>instead add a new '$uri' that outputs a valid URI.  Note that the
>libguestfs '$nbd' TCP form is already a valid URI, but that
libguestfs
>still needs to be taught about the nbd+unix:// scheme, so for there,
>you are still better off using '$nbd'; but for qemu, using
'$uri'
>already works since qemu v1.3.0 in 2012.
>
>Note that the NBD project has not actually yet posted a link for the
>preferred URI syntax; once Rich is happy with his work on that
>project, we can touch this up to link to that page.
>
>Reported-by: Martin Kletzander <mkletzan@redhat.com>
>Signed-off-by: Eric Blake <eblake@redhat.com>
>---
> docs/nbdkit-captive.pod |  8 +++++++-
> server/captive.c        | 14 +++++++++++++-
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
Looks good to me, thanks.  I haven't even though of this solution =)

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
>diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod
>index 6f69cca5..59df6690 100644
>--- a/docs/nbdkit-captive.pod
>+++ b/docs/nbdkit-captive.pod
>@@ -42,9 +42,15 @@ The following shell variables are available in the
I<--run> argument:
>
> =over 4
>
>+=item C<$uri>
>+
>+A URI that refers to the nbdkit port or socket in the preferred form
>+documented by the NBD project.
>+
> =item C<$nbd>
>
>-A URL that refers to the nbdkit port or socket.
>+An older URL that refers to the nbdkit port or socket in a manner
>+specific to certain tools.
>
> Note there is some magic here, since qemu and guestfish URLs have a
> different format, so nbdkit tries to guess which you are running.  If
>diff --git a/server/captive.c b/server/captive.c
>index 6971af2e..c5274f65 100644
>--- a/server/captive.c
>+++ b/server/captive.c
>@@ -66,7 +66,19 @@ run_command (void)
>     exit (EXIT_FAILURE);
>   }
>
>-  /* Construct $nbd "URL".  Unfortunately guestfish and qemu take
>+  /* Construct $uri. */
>+  fprintf (fp, "uri=");
>+  if (port) {
>+    fprintf (fp, "nbd://localhost:");
>+    shell_quote (port, fp);
>+  }
>+  else if (unixsocket) {
>+    fprintf (fp, "nbd+unix://\\?socket=");
>+    shell_quote (unixsocket, fp);
>+  }
>+  fprintf (fp, "\n");
>+
>+  /* Construct older $nbd "URL".  Unfortunately guestfish and
qemu take
>    * different syntax, so try to guess which one we need.
>    */
>   fprintf (fp, "nbd=");
>-- 
>2.20.1
>

On Tue, Jun 25, 2019 at 09:35:11PM -0500, Eric Blake
wrote:
> The existing --run '$nbd' outputs an older form that differs
between
> libguestfs and qemu, and which is not always a valid URI.  For
> historical compatibility, we probably can't change that; but we can
> instead add a new '$uri' that outputs a valid URI.  Note that the
> libguestfs '$nbd' TCP form is already a valid URI, but that
libguestfs
> still needs to be taught about the nbd+unix:// scheme, so for there,
> you are still better off using '$nbd'; but for qemu, using
'$uri'
> already works since qemu v1.3.0 in 2012.
> 
> Note that the NBD project has not actually yet posted a link for the
> preferred URI syntax; once Rich is happy with his work on that
> project, we can touch this up to link to that page.
> 
> Reported-by: Martin Kletzander <mkletzan@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  docs/nbdkit-captive.pod |  8 +++++++-
>  server/captive.c        | 14 +++++++++++++-
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod
> index 6f69cca5..59df6690 100644
> --- a/docs/nbdkit-captive.pod
> +++ b/docs/nbdkit-captive.pod
> @@ -42,9 +42,15 @@ The following shell variables are available in the
I<--run> argument:
> 
>  =over 4
> 
> +=item C<$uri>
> +
> +A URI that refers to the nbdkit port or socket in the preferred form
> +documented by the NBD project.
> +
>  =item C<$nbd>
> 
> -A URL that refers to the nbdkit port or socket.
> +An older URL that refers to the nbdkit port or socket in a manner
> +specific to certain tools.
> 
>  Note there is some magic here, since qemu and guestfish URLs have a
>  different format, so nbdkit tries to guess which you are running.  If
> diff --git a/server/captive.c b/server/captive.c
> index 6971af2e..c5274f65 100644
> --- a/server/captive.c
> +++ b/server/captive.c
> @@ -66,7 +66,19 @@ run_command (void)
>      exit (EXIT_FAILURE);
>    }
> 
> -  /* Construct $nbd "URL".  Unfortunately guestfish and qemu
take
> +  /* Construct $uri. */
> +  fprintf (fp, "uri=");
> +  if (port) {
> +    fprintf (fp, "nbd://localhost:");
> +    shell_quote (port, fp);
> +  }
> +  else if (unixsocket) {
> +    fprintf (fp, "nbd+unix://\\?socket=");
> +    shell_quote (unixsocket, fp);
> +  }
> +  fprintf (fp, "\n");
> +
> +  /* Construct older $nbd "URL".  Unfortunately guestfish and
qemu take
>     * different syntax, so try to guess which one we need.
>     */
>    fprintf (fp, "nbd=");
Looks reasonable, although I guess we may one day find we need more
quoting of unixsocket (both shell quoting and URI quoting, urrgh).
Anyway:

ACK

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On 6/26/19 3:56 AM, Richard W.M. Jones wrote:
> On Tue, Jun 25, 2019 at 09:35:11PM -0500, Eric Blake wrote:
>> The existing --run '$nbd' outputs an older form that differs
between
>> libguestfs and qemu, and which is not always a valid URI.  For
>> historical compatibility, we probably can't change that; but we can
>> instead add a new '$uri' that outputs a valid URI.  Note that
the
>> libguestfs '$nbd' TCP form is already a valid URI, but that
libguestfs
>> still needs to be taught about the nbd+unix:// scheme, so for there,
>> you are still better off using '$nbd'; but for qemu, using
'$uri'
>> already works since qemu v1.3.0 in 2012.
>>
>> Note that the NBD project has not actually yet posted a link for the
>> preferred URI syntax; once Rich is happy with his work on that
>> project, we can touch this up to link to that page.
>>
>> Reported-by: Martin Kletzander <mkletzan@redhat.com>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>> +  fprintf (fp, "uri=");
>> +  if (port) {
>> +    fprintf (fp, "nbd://localhost:");
>> +    shell_quote (port, fp);
>> +  }
>> +  else if (unixsocket) {
>> +    fprintf (fp, "nbd+unix://\\?socket=");
>> +    shell_quote (unixsocket, fp);
>> +  }
>> +  fprintf (fp, "\n");
>> +
>> +  /* Construct older $nbd "URL".  Unfortunately guestfish
and qemu take
>>     * different syntax, so try to guess which one we need.
>>     */
>>    fprintf (fp, "nbd=");
> 
> Looks reasonable, although I guess we may one day find we need more
> quoting of unixsocket (both shell quoting and URI quoting, urrgh).
Oh, good point. Normally, our generated unixsocket name (assuming
$TMPDIR is sane) is valid as a URI query string without any % encoding,
or any shell quoting; but if $TMPDIR contains unusual characters (for
example, #), we need to URI-encode (rather than shell-quote).
Fortunately, any URI-encoded string needs no further shell quoting
(since any character that needs shell quoting also ends up getting
%-encoded for URI-encoding).
> Anyway:
> 
> ACK
> 
> Rich.
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org