Libguestfs - Feb 2023 - no way to force-close NBD handle in nbdsh

On Fri, Feb 24, 2023 at 04:41:20PM -0600, Eric Blake
wrote:
> I'm out of time this weekend, but while trying to write a test for an
> nbdkit bug fix (a nasty assertion failure when the client disconnects
> uncleanly without NBD_CMD_DISC, which is what nbdcopy does if it gets
> an EIO read response from the server), I realized it is extremely hard
> to trigger this using nbdsh, but easy to do in C or the Go bindings.
> 
> In the Go bindings, we intentionally coded things so that the Go
> structure knows if its underlying C pointer is live or not; it exposes
> a way to force early closure, then the bindings return a
> closed_handle_error() if early closure has happened.  We probably need
> to support something similar in the Python bindings.
> 
> As a short-term hack, I tried directly calling h.__del__() - this is
> not a good idea, as our current __del__ implementation is not designed
> to be called twice, and when the later garbage collection calls it
> again, we segfault trying to free invalid memory.
This is a bug :-(
> But less drastic
> measures, such as:
> 
> import gc
> del h
> h = None
> gc.collect()
> 
> were still insufficient to trigger a normal __del__ of h, because I
> can't figure out how to force the garbage collector to see that I want
> to close the handle but keep python open.
> 
> In the end, I did get a working test written (by just quit()ing python
> instead of trying to keep it alive).  See my next email for the nbdkit
> patch that spawned my request here.
I think we want an explicit h.close method, just like we already have
in OCaml and golang bindings.  This also implies a "handle is closed"
error/exception which is raised when you call another method on the
handle after it is closed.

I thought that this was how the Python bindings already worked, but
I've checked and it's as you say - there's no method to explicitly
close the handle.

Such a change is backwards compatible with existing clients (since you
can still let the handle go out of scope and have it closed
automatically).

I can have a go at adding this since I unexpectedly have some free
time this weekend.

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 Sat, Feb 25, 2023 at 09:57:41AM +0000, Richard W.M. Jones
wrote:
> I can have a go at adding this since I unexpectedly have some free
> time this weekend.
https://gitlab.com/nbdkit/libnbd/-/commit/b7f85610ac19085a1631439fbcb7dd241f1e9ebf
https://gitlab.com/nbdkit/libnbd/-/commit/71757015ea15815b5ff1c04ef482152b089eb305
https://gitlab.com/nbdkit/libnbd/-/commit/1aee8525f134f7c7f490da1e5b2622d7b2ece2e9

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