Re: [Qemu-devel] [PATCH v2 20/22] qemu-nbd: Add --list option

2018-12-15 Thread Richard W.M. Jones
On Sat, Dec 15, 2018 at 07:53:22AM -0600, Eric Blake wrote:
> @@ -557,13 +660,14 @@ int main(int argc, char **argv)
>  Error *local_err = NULL;
>  BlockdevDetectZeroesOptions detect_zeroes = 
> BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
>  QDict *options = NULL;
> -const char *export_name = ""; /* Default export name */
> +const char *export_name = NULL;

You might want to comment that export_name is now a tri-state value,
with NULL meaning that it's not set by the user.


Nevertheless the patch looks good, and the addition of the example
(including host name setting) resolves my previous problem with the
new functionality.  Therefore:

Reviewed-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html



[Qemu-devel] [PATCH v2 20/22] qemu-nbd: Add --list option

2018-12-15 Thread Eric Blake
We want to be able to detect whether a given qemu NBD server is
exposing the right export(s) and dirty bitmaps, at least for
regression testing.  We could use 'nbd-client -l' from the upstream
NBD project to list exports, but it's annoying to rely on
out-of-tree binaries; furthermore, nbd-client doesn't necessarily
know about all of the qemu NBD extensions.  Thus, it is time to add
a new mode to qemu-nbd that merely sniffs all possible information
from the server during handshake phase, then disconnects and dumps
the information.

This patch actually implements --list/-L, while reusing other
options such as --tls-creds for now designating how to connect
as the client (rather than their non-list usage of how to operate
as the server).

I debated about adding this functionality to something akin to
'qemu-img info' - but that tool does not readily lend itself
to connecting to an arbitrary NBD server without also tying to
a specific export (I may, however, still add ImageInfoSpecificNBD
for reporting the bitmaps available when connecting to a single
export).  And, while it may feel a bit odd that normally
qemu-nbd is a server but 'qemu-nbd -L' is a client, we are not
really making the qemu-nbd binary that much larger, because
'qemu-nbd -c' has to operate as both server and client
simultaneously across two threads when feeding the kernel module
for /dev/nbdN access.

Sample output:
$ qemu-nbd -L
exports available: 1
 export: ''
  size:  65536
  flags: 0x4ed ( flush fua trim zeroes df cache )
  min block: 512
  opt block: 4096
  max block: 33554432
  available meta contexts: 1
   base:allocation

Note that the output only lists sizes if the server sent
NBD_FLAG_HAS_FLAGS, because a newstyle server does not give
the size otherwise.  It has the side effect that for really
old servers that did not send any flags, the size is not
output even though it was available.  However, I'm not too
concerned about that - oldstyle servers are (rightfully)
getting less common to encounter (qemu 3.0 was the last
version where we even serve it), and most existing servers
that still even offer oldstyle negotiation (such as nbdkit)
still send flags (since that was added to the NBD protocol
in 2007 to permit read-only connections).

Not done here, but maybe worth future experiments: capture
the meat of NBDExportInfo into a QAPI struct, and use the
generated QAPI pretty-printers instead of hand-rolling our
output loop.  It would also permit us to add a JSON output
mode for machine parsing.

Signed-off-by: Eric Blake 

---
v2: commit message improvements [Vladimir]
wording tweak to --help output [Vladimir]
update man page documentation
---
 qemu-nbd.texi |  30 --
 qemu-nbd.c| 155 +-
 2 files changed, 166 insertions(+), 19 deletions(-)

diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 0e24c2801ee..1e168c10850 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -2,6 +2,8 @@
 @c man begin SYNOPSIS
 @command{qemu-nbd} [OPTION]... @var{filename}

+@command{qemu-nbd} @option{-L} [OPTION]...
+
 @command{qemu-nbd} @option{-d} @var{dev}
 @c man end
 @end example
@@ -10,8 +12,9 @@

 Provide access to various QEMU NBD features.  Most commonly used to
 export a QEMU disk image using the NBD protocol as a server, but can
-also be used (on Linux) to manage kernel bindings of a /dev/nbdX
-block device to a QEMU server.
+also be used as a client to query properties of a remote server, or
+(on Linux) to manage kernel bindings of a /dev/nbdX block device to a
+QEMU server.

 @c man end

@@ -28,13 +31,15 @@ See the @code{qemu(1)} manual page for full details of the 
properties
 supported. The common object types that it makes sense to define are the
 @code{secret} object, which is used to supply passwords and/or encryption
 keys, and the @code{tls-creds} object, which is used to supply TLS
-credentials for the qemu-nbd server.
+credentials for the qemu-nbd server or client.
 @item -p, --port=@var{port}
-The TCP port to listen on (default @samp{10809}).
+The TCP port to listen on as a server, or connect to as a client
+(default @samp{10809}).
 @item -o, --offset=@var{offset}
 The offset into the image.
 @item -b, --bind=@var{iface}
-The interface to bind to (default @samp{0.0.0.0}).
+The interface to bind to as a server, or connect to as a client
+(default @samp{0.0.0.0}).
 @item -k, --socket=@var{path}
 Use a unix socket with path @var{path}.
 @item --image-opts
@@ -88,10 +93,14 @@ Set the NBD volume export name (default of a zero-length 
string).
 @item -D, --description=@var{description}
 Set the NBD volume export description, as a human-readable
 string.
+@item -L, --list
+Connect as a client and list all details about the exports exposed by
+a remote NBD server.
 @item --tls-creds=ID
 Enable mandatory TLS encryption for the server by setting the ID
 of the TLS credentials object previously created with the --object
-option.
+option; or provide the credentials needed for connecting as a client