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

[Eric Blake]

On Tue, Jan 31, 2023 at 11:52:27AM +0000, Richard W.M. Jones wrote:
> 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

Eww. I didn't mean for that to happen, obviously.

> 
> > 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.

Do you want me to try to play with this as well (since I do like the
improved help message, if we can get it to work reliably), or would I
be duplicating efforts at this point?

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