search for: nbd_close

...handle in one thread while another thread is locked causes >> undefined behavior (as closing does not obtain a lock, and can cause >> use-after-free or other nasty problems to the thread that does own the >> lock). However, it is not sensible to try and obtain a lock in >> nbd_close, as POSIX says that it is also undefined for any other >> thread to wait on a mutex that has already been destroyed. Therefore, >> we don't need to change our code, but merely remind users that >> nbd_close is not safe until all other uses of the handle have ceased. >>...

...hread while another thread is locked causes >>> undefined behavior (as closing does not obtain a lock, and can cause >>> use-after-free or other nasty problems to the thread that does own the >>> lock).  However, it is not sensible to try and obtain a lock in >>> nbd_close, as POSIX says that it is also undefined for any other >>> thread to wait on a mutex that has already been destroyed.  Therefore, >>> we don't need to change our code, but merely remind users that >>> nbd_close is not safe until all other uses of the handle have cease...

Closing the handle in one thread while another thread is locked causes undefined behavior (as closing does not obtain a lock, and can cause use-after-free or other nasty problems to the thread that does own the lock). However, it is not sensible to try and obtain a lock in nbd_close, as POSIX says that it is also undefined for any other thread to wait on a mutex that has already been destroyed. Therefore, we don't need to change our code, but merely remind users that nbd_close is not safe until all other uses of the handle have ceased. --- generator/generator | 3 ++- 1...

...> Closing the handle in one thread while another thread is locked causes > undefined behavior (as closing does not obtain a lock, and can cause > use-after-free or other nasty problems to the thread that does own the > lock). However, it is not sensible to try and obtain a lock in > nbd_close, as POSIX says that it is also undefined for any other > thread to wait on a mutex that has already been destroyed. Therefore, > we don't need to change our code, but merely remind users that > nbd_close is not safe until all other uses of the handle have ceased. > --- > >...

This is a simple patch which stops nbd_close from waiting too long for a server subprocess to shut down. I wanted to send SIGHUP because the server will be able to catch it and do a clean shutdown if that is required. Is another signal better? Is it right to send a signal here? Rich.

...->cb.fn_user_data, > + NULL, 0, 0, 0, NULL); > + if (cmd->cb.callback) > + cmd->cb.callback (LIBNBD_CALLBACK_FREE, cmd->cb.user_data, > + 0, NULL); > + > free (cmd); nbd_aio_command_completed is skipped when a user calls nbd_close. While we've documented that nbd_close loses the exit status of any unretired command (different from the fact that completion callbacks run on transition to DEAD but the handle is still around), it is still probably worth tweaking nbd_close's use of free_cmd_list() to still call FREE on a...

...0m10.499s user 0m0.065s sys 0m0.023s With this patch the elapsed time is near instantaneous. --- lib/handle.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/handle.c b/lib/handle.c index 1fe4467..111cfc5 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -124,8 +124,10 @@ nbd_close (struct nbd_handle *h) freeaddrinfo (h->result); if (h->sock) h->sock->ops->close (h->sock); - if (h->pid >= 0) /* XXX kill it? */ + if (h->pid >= 0) { + kill (h->pid, SIGHUP); waitpid (h->pid, NULL, 0); + } free (h->export_name);...

One API change, some small documentation changes.

...is patch the elapsed time is near instantaneous. > --- > lib/handle.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/handle.c b/lib/handle.c > index 1fe4467..111cfc5 100644 > --- a/lib/handle.c > +++ b/lib/handle.c > @@ -124,8 +124,10 @@ nbd_close (struct nbd_handle *h) > freeaddrinfo (h->result); > if (h->sock) > h->sock->ops->close (h->sock); Note that this patch is trying to speed up a slow waitpid() issued _after_ we call h->sock->ops->close(). You'd think that after we close() our e...

...ity.pod create mode 100644 SECURITY diff --git a/docs/Makefile.am b/docs/Makefile.am index df58586..4c99b5d 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -38,6 +38,7 @@ generator_built = \ EXTRA_DIST = \ $(generator_built) \ libnbd.pod \ + libnbd-security.pod \ nbd_create.pod \ nbd_close.3 \ nbd_get_error.3 \ @@ -48,6 +49,7 @@ if HAVE_POD man_MANS = \ libnbd.3 \ + libnbd-security.1 \ nbd_create.3 \ nbd_close.3 \ nbd_get_error.3 \ @@ -73,4 +75,9 @@ libnbd.3: libnbd.pod $(top_builddir)/podwrapper.pl \ --html $(top_builddir)/html/$@.html \ $< +libnbd-securi...

...tor index 7aad57c..92a1aae 100755 --- a/generator/generator +++ b/generator/generator @@ -3055,7 +3055,6 @@ let generate_lib_libnbd_syms () = pr "{\n"; pr " global:\n"; - pr " nbd_add_close_callback;\n"; pr " nbd_create;\n"; pr " nbd_close;\n"; pr " nbd_get_errno;\n"; @@ -3255,11 +3254,6 @@ let generate_include_libnbd_h () = pr "extern int nbd_get_errno (void);\n"; pr "#define LIBNBD_HAVE_NBD_GET_ERRNO 1\n"; pr "\n"; - pr "extern int nbd_add_close_callback (struct nbd_...

ThreadSanitizer [1] pointed out that in the nbd plugin, nbd_close() can attempt close() in the main thread while the worker thread is still attempting to start a read(). Normally, if the read() loses the race, it will get a harmless EBADF that exits the worker thread (which is what we want, as we are closing the connection anyway); but if another connection happ...

...garding the memory deallocation of the opaque pointer. Maybe it is guaranteed that: a) for `aio_*` functions the callback will be called only once and b) all the other functions (currently only `set_debug_callback`) need to be either deregistered or the data can be deregistered only after `nbd_close()` was called? I can't really see that handled in the bindings and I wanted to know that in order to finish (or actually just move a little bit) with the Rust bindings. Thanks, Martin

...plete nbdkit: debug: using thread model: parallel nbdkit: debug: bound to unix socket /tmp/tmp.sHet2Luahj nbdkit: debug: forked into background (new pid = 422301) nbdkit: debug: written pidfile /home/rjones/d/nbdkit/tests/full.pid libnbd: debug: nbd1: nbd_create: opening handle libnbd: debug: nbd1: nbd_close: closing handle libnbd: debug: nbd2: nbd_create: opening handle libnbd: debug: nbd2: nbd_close: closing handle libnbd: debug: nbd3: nbd_create: opening handle libnbd: debug: nbd3: nbd_close: closing handle libnbd: debug: nbd4: nbd_create: opening handle libnbd: debug: nbd4: nbd_close: closing handl...

...(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); > + if (s == -1) { > + SET_NEXT_STATE (%.DEAD); > + set_error (errno, "socket"); > + return 0; > + } If we fail here or later, do/should we try to clean up the /tmp/libnbdXXX directory created earlier? /me reads ahead - nbd_close tries to address it Still, if we fail at this point, h->sa_sockpath is set but not yet created [1] > + > + addr.sun_family = AF_UNIX; > + memcpy (addr.sun_path, h->sa_sockpath, strlen (h->sa_sockpath) + 1); > + if (bind (s, (struct sockaddr *) &addr, sizeof addr) == -...

...ity.pod create mode 100644 SECURITY diff --git a/docs/Makefile.am b/docs/Makefile.am index df58586..4c99b5d 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -38,6 +38,7 @@ generator_built = \ EXTRA_DIST = \ $(generator_built) \ libnbd.pod \ + libnbd-security.pod \ nbd_create.pod \ nbd_close.3 \ nbd_get_error.3 \ @@ -48,6 +49,7 @@ if HAVE_POD man_MANS = \ libnbd.3 \ + libnbd-security.1 \ nbd_create.3 \ nbd_close.3 \ nbd_get_error.3 \ @@ -73,4 +75,9 @@ libnbd.3: libnbd.pod $(top_builddir)/podwrapper.pl \ --html $(top_builddir)/html/$@.html \ $< +libnbd-securi...

...r *, const char *), > void *data) > { > + if (h->debug_fn) > + /* ignore return value */ > + h->debug_fn (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); This frees the first debug callback when installing a second; but where does nbd_close free the second? Also, do we allow C code to pass NULL to uninstall a handler? Should other languages be able to clear out a callback? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

Very much a work in progress as there are still many tests using qemu-io which are candidates for conversion. You'll notice at the end of test-full.sh that the new test has some duplicated code which looks as if it ought to be refactored into a Python function. When I tried to do that, I got loads of strange Python problems which may indicate bugs in nbdsh itself or problems with my

...0644 --- a/lib/handle.c +++ b/lib/handle.c @@ -22,6 +22,8 @@ #include <stdlib.h> #include <unistd.h> #include <errno.h> +#include <signal.h> +#include <assert.h> #include <netdb.h> #include <sys/types.h> #include <sys/wait.h> @@ -124,7 +126,7 @@ nbd_close (struct nbd_handle *h) freeaddrinfo (h->result); if (h->sock) h->sock->ops->close (h->sock); - if (h->pid >= 0) /* XXX kill it? */ + if (h->pid > 0) waitpid (h->pid, NULL, 0); free (h->export_name); @@ -215,6 +217,30 @@ nbd_unlocked_get_v...

...: + abort_option (h); abort_commands (h, &h->cmds_to_issue); abort_commands (h, &h->cmds_in_flight); h->in_flight = 0; diff --git a/lib/handle.c b/lib/handle.c index c308461..7987d59 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -112,8 +112,6 @@ nbd_create (void) void nbd_close (struct nbd_handle *h) { - size_t i; - nbd_internal_set_error_context ("nbd_close"); if (h == NULL) @@ -126,13 +124,7 @@ nbd_close (struct nbd_handle *h) free (h->bs_entries); nbd_internal_reset_size_and_flags (h); - for (i = 0; i < h->nr_exports; ++i) { - fre...