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

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

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