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

[Richard W.M. Jones]

On Tue, Jan 31, 2023 at 01:33:25PM +0200, Nir Soffer wrote:
> On Tue, Jan 31, 2023 at 12:34 AM Richard W.M. Jones <rjones at redhat.com> wrote:
> >
> > 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.
> 
> Can you share the error when it breaks?

$ rpm -qf /usr/bin/nbdsh
python3-libnbd-1.15.9-2.fc38.x86_64

$ nbdsh 

Welcome to nbdsh, the shell for interacting with
Network Block Device (NBD) servers.

The ‘nbd’ module has already been imported and there
is an open NBD handle called ‘h’ in state 'START'.

h.connect_tcp("remote", "10809")   # Connect to a remote server.
h.get_size()                       # Get size of the remote disk.
buf = h.pread(512, 0)              # Read the first sector.
exit() or Ctrl-D                   # Quit the shell
help(nbd)                          # Display documentation

nbd> print(h)
Traceback (most recent call last):
  File "/usr/lib64/python3.11/code.py", line 90, in runcode
    exec(code, self.locals)
  File "<console>", line 1, in <module>
NameError: name 'h' is not defined

> I'm not sure what is the issue, but usually if you have a global
> variable created only in
> some flows, adding:
> 
>     thing = None
> 
> At the start of the module makes sure that the name exists later,
> regardless of the flow
> taken. Code can take the right action based on:
> 
>     if thing is None:
>         ...

Good point, I was trying 'undef h' instead.  'h' not being present in
the dictionary at all vs 'h = None' are slightly different, although I
suppose it doesn't matter in this particular case.

The other point is that 'h' (when defined) is not a global.  The patch
assumes it is a global, but then uses it in some places as if it is a
local.

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