Libguestfs - Jan 2023 - [libnbd PATCH v2 3/3] nbdsh: Improve --help and initial banner contents.

Document all options in --help output.  If -n is not in use, then
enhance the banner to print the current state of h, and further tailor
the advice given on useful next steps to take to mention opt_go when
using --opt-mode.
---
 python/nbdsh.py | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/python/nbdsh.py b/python/nbdsh.py
index 0919c9ec..872b94fc 100644
--- a/python/nbdsh.py
+++ b/python/nbdsh.py
@@ -86,11 +86,13 @@ def __call__(self, parser, namespace, values,
option_string=None):
     parser.add_argument('--connect', dest='uri',
action=SnippetAction,
                         help=argparse.SUPPRESS)

-    parser.add_argument('-v', '--verbose',
action='store_true')
+    parser.add_argument('-v', '--verbose',
action='store_true',
+                        help="enable verbose debugging")
     short_options.append("-v")
     long_options.append("--verbose")

-    parser.add_argument('-V', '--version',
action='store_true')
+    parser.add_argument('-V', '--version',
action='store_true',
+                        help="display version information")
     short_options.append("-V")
     long_options.append("--version")

@@ -127,7 +129,10 @@ def __call__(self, parser, namespace, values,
option_string=None):
         os.environ["LIBNBD_DEBUG"] = "1"

     # Create the handle.
-    if not args.n:
+    if args.n:
+        pass
+    else:
+        global h
         h = nbd.NBD()
         h.set_handle_name("nbdsh")

@@ -165,21 +170,35 @@ def line(x): lines.append(x)
     def blank(): line("")
     def example(ex, desc): line("%-34s # %s" % (ex, desc))

+    connect_hint = False
+    go_hint = False
     blank()
     line("Welcome to nbdsh, the shell for interacting with")
     line("Network Block Device (NBD) servers.")
     blank()
-    if not args.n:
-        line("The ???nbd??? module has already been imported and
there")
-        line("is an open NBD handle called ???h???.")
-        blank()
-    else:
+    if args.n:
         line("The ???nbd??? module has already been imported.")
         blank()
         example("h = nbd.NBD()", "Create a new handle.")
-    if False:  # args.uri is None:
+        connect_hint = True
+    else:
+        global h
+        state = h.connection_state()
+        state = state[:state.find(':')]
+        line("The ???nbd??? module has already been imported and
there")
+        line("is an open NBD handle called ???h??? in state
'%s'." % state)
+        blank()
+        if h.aio_is_created():
+            connect_hint = True
+            if h.get_opt_mode():
+                go_hint = True
+        elif h.aio_is_negotiating():
+            go_hint = True
+    if connect_hint:
         example('h.connect_tcp("remote", "10809")',
                 "Connect to a remote server.")
+    if go_hint:
+        example("h.opt_go()", "Finish option negotiation")
     example("h.get_size()", "Get size of the remote disk.")
     example("buf = h.pread(512, 0)", "Read the first
sector.")
     example("exit() or Ctrl-D", "Quit the shell")
-- 
2.38.1

On Fri, Nov 4, 2022 at 11:18 PM Eric Blake <eblake at redhat.com> wrote:
[...]
> @@ -127,7 +129,10 @@ def __call__(self, parser, namespace, values,
> option_string=None):
>          os.environ["LIBNBD_DEBUG"] = "1"
>
>      # Create the handle.
> -    if not args.n:
> +    if args.n:
> +        pass
> +    else:
>
Why add useless branch?

> +        global h
>          h = nbd.NBD()
>          h.set_handle_name("nbdsh")
>
> @@ -165,21 +170,35 @@ def line(x): lines.append(x)
>      def blank(): line("")
>      def example(ex, desc): line("%-34s # %s" % (ex, desc))
>
> +    connect_hint = False
> +    go_hint = False
>      blank()
>      line("Welcome to nbdsh, the shell for interacting with")
>      line("Network Block Device (NBD) servers.")
>      blank()
> -    if not args.n:
> -        line("The ?nbd? module has already been imported and
there")
> -        line("is an open NBD handle called ?h?.")
> -        blank()
> -    else:
> +    if args.n:
>          line("The ?nbd? module has already been imported.")
>          blank()
>          example("h = nbd.NBD()", "Create a new
handle.")
> -    if False:  # args.uri is None:
>
Good that this was removed, but it will be better to remove in the previous
patch.

Nir
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20221105/b6a250aa/attachment.htm>

For the series:

Acked-by: Richard W.M. Jones <rjones at redhat.com>

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 Fri, Nov 04, 2022 at 04:18:31PM -0500, Eric Blake
wrote:
> Document all options in --help output.  If -n is not in use, then
> enhance the banner to print the current state of h, and further tailor
> the advice given on useful next steps to take to mention opt_go when
> using --opt-mode.
I had to partially revert this patch (reverting most of it) because it
unfortunately breaks the implicit handle creation :-(

https://gitlab.com/nbdkit/libnbd/-/commit/5a02c7d2cc6a201f9e5531c0c20c2f3c22b805a2

I'm not actually sure how to do this correctly in Python.  I made
several attempts, but I don't think Python is very good about having a
variable which is only defined on some paths -- maybe it's not
possible at all.

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