Libguestfs - Sep 2020 - Re: [libnbd PATCH] nbdsh: Add --opt-mode command line option

Similar to --base-allocation, this is a common enough configuration
that cannot be performed with -c when using --uri, but which makes
life easier in scripting.  And like --base-allocation, I'm starting
with only a long option spelling, rather than burning a short option
letter.
---
 sh/nbdsh.pod       | 21 +++++++++++++++++++--
 python/nbdsh.py    |  6 ++++++
 sh/test-context.sh | 32 +++++++++++++++++++++++++++++++-
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/sh/nbdsh.pod b/sh/nbdsh.pod
index 46c0f48..c1f1672 100644
--- a/sh/nbdsh.pod
+++ b/sh/nbdsh.pod
@@ -60,8 +60,10 @@ Display brief command line help and exit.

 Request the use of the "base:allocation" meta context, which is the
 most common context used with L<nbd_block_status(3)>.  This is
-equivalent to calling S<C<h.set_meta_context
-(nbd.CONTEXT_BASE_ALLOCATION)>> in the shell prior to connecting.
+equivalent to calling
+S<C<h.set_meta_context(nbd.CONTEXT_BASE_ALLOCATION)>> in the shell
+prior to connecting, and works even when combined with C<--uri> (while
+attempting the same with C<-c> would be too late).

 =item B<-c> 'COMMAND ...'

@@ -70,6 +72,15 @@ equivalent to calling S<C<h.set_meta_context
 Instead of starting an interactive shell, run a command.  This option
 can be specified multiple times in order to run multiple commands.

+=item B<--opt-mode>
+
+Request that option mode be enabled, which gives fine-grained control
+over option negotiation after initially contacting the server but
+prior to actually using the export.  This is equivalent to calling
+S<C<h.set_opt_mode(True)>> in the shell prior to connecting, and
works
+even when combined with C<--uri> (while attempting the same with
C<-c>
+would be too late).
+
 =item B<-c ->

 =item B<--command ->
@@ -85,6 +96,12 @@ Read standard input and execute it as a command.
 Connect to the given L<NBD
URI|https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md>.
 This is equivalent to the S<C<h.connect_uri(URI)>> command in the
shell.

+Note that the connection is created prior to processing any C<-c>
+commands, which prevents the use of configuration commands such as
+S<C<h.add_meta_context(>NAMEC<)>> from the command line when
mixed
+with this option.  The options C<--opt-mode> and
C<--base-allocation>
+can be used to make this situation easier to manage.
+
 =item B<-V>

 =item B<--version>
diff --git a/python/nbdsh.py b/python/nbdsh.py
index 083dfee..4a1b5f3 100644
--- a/python/nbdsh.py
+++ b/python/nbdsh.py
@@ -38,6 +38,10 @@ def shell():
                         help='request the "base:allocation" meta
context')
     long_options.append("--base-allocation")

+    parser.add_argument('--opt-mode', action='store_true',
+                        help='request opt mode during connection')
+    long_options.append("--opt-mode")
+
     parser.add_argument('-u', '--uri', help="connect to
NBD URI")
     short_options.append("-u")
     long_options.append("--uri")
@@ -89,6 +93,8 @@ help(nbd)                          # Display documentation

     if args.base_allocation:
         h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
+    if args.opt_mode:
+        h.set_opt_mode(True)
     if args.uri is not None:
         try:
             h.connect_uri(args.uri)
diff --git a/sh/test-context.sh b/sh/test-context.sh
index fab1aad..5fb5cad 100755
--- a/sh/test-context.sh
+++ b/sh/test-context.sh
@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 # nbd client library in userspace
-# Copyright (C) 2019 Red Hat Inc.
+# Copyright (C) 2019-2020 Red Hat Inc.
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -31,6 +31,19 @@ if test "x$output" != xFalse; then
     fail=1
 fi

+# Use of -c to request context is too late with -u
+output=$(nbdkit -U - null --run 'nbdsh -c "
+try:
+    h.add_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
+    assert(False)
+except nbd.Error:
+    print(\"okay\")
+" -u "nbd+unix://?socket=$unixsocket"')
+if test "x$output" != xokay; then
+    echo "$0: unexpected output: $output"
+    fail=1
+fi
+
 # With --base-allocation (and a server that supports it), meta context works.
 output=$(nbdkit -U - null --run 'nbdsh \
     --base-allocation --uri "nbd+unix://?socket=$unixsocket" \
@@ -62,4 +75,21 @@ else
     echo "$0: nbdkit lacks --no-sr"
 fi

+# Test interaction with opt mode
+output=$(nbdkit -U - null --run 'nbdsh \
+    -u "nbd+unix://?socket=$unixsocket" --opt-mode --base-allocation
\
+    -c "
+try:
+    h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION)
+    assert(False)
+except nbd.Error:
+    pass
+" \
+    -c "h.opt_go()" \
+    -c "print(h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION))"')
+if test "x$output" != xTrue; then
+    echo "$0: unexpected output: $output"
+    fail=1
+fi
+
 exit $fail
-- 
2.28.0

Yeah I was wondering when this was coming, and "--full-info" too :-). 
ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On 9/28/20 9:09 AM, Richard W.M. Jones wrote:
> 
> Yeah I was wondering when this was coming, and "--full-info" too
:-).  ACK.
Notice that you can do:

nbdsh --opt-mode -u "$uri" \
   -c 'h.set_full_info(True)' \
   -c 'h.opt_go()' \
   ...

without issues.  In fact, now that we have --opt-mode, there is less 
reason to need --base-allocation as an option (although I don't plan on 
removing it, for back-compat reasons).  In short, our previous 
limitation was the number of config items that had to be set 
pre-connection, but now with opt mode, we can now configure those things 
things in opt mode.  So I'm less inclined to add --full-info.

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