search for: free_cmd_list

...nbd_internal_retire_and_free_command (h, cmd); /* If the command was successful, return true. */ if (error == 0) diff --git a/lib/handle.c b/lib/handle.c index ae0d196..0f50e38 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -32,13 +32,13 @@ #include "internal.h" static void -free_cmd_list (struct command *list) +free_cmd_list (struct nbd_handle *h, struct command *list) { struct command *cmd, *cmd_next; for (cmd = list; cmd != NULL; cmd = cmd_next) { cmd_next = cmd->next; - nbd_internal_retire_and_free_command (cmd); + nbd_internal_retire_and_free_command (h,...

...et_activation or nbd_connect_command. At the moment these functions move the handle to the DEAD state if they fail, but that's not really necessary in all cases. > >--- a/lib/handle.c > >+++ b/lib/handle.c > >@@ -129,6 +129,16 @@ nbd_close (struct nbd_handle *h) > > free_cmd_list (h->cmds_in_flight); > > free_cmd_list (h->cmds_done); > > nbd_internal_free_string_list (h->argv); > >+ if (h->sa_sockpath) { > >+ if (h->pid > 0) > >+ kill (h->pid, SIGTERM); > >+ unlink (h->sa_sockpath); > >+...

...allback) * + (&h->free_callbacks[h->nr_free_callbacks] - (free_cb+1))); + h->nr_free_callbacks--; + } +} diff --git a/lib/handle.c b/lib/handle.c index 054c8a7..ae0d196 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -126,6 +126,12 @@ nbd_close (struct nbd_handle *h) free_cmd_list (h->cmds_to_issue); free_cmd_list (h->cmds_in_flight); free_cmd_list (h->cmds_done); + + /* Any remaining free callbacks indicate an error. */ + if (h->nr_free_callbacks != 0) + abort (); + free (h->free_callbacks); + nbd_internal_free_string_list (h->argv); free...

...y set? > + h->argv = copy; > + > + return nbd_internal_run (h, cmd_connect_sa); > +} > diff --git a/lib/handle.c b/lib/handle.c > index 2af25fe..a7f2c79 100644 > --- a/lib/handle.c > +++ b/lib/handle.c > @@ -129,6 +129,16 @@ nbd_close (struct nbd_handle *h) > free_cmd_list (h->cmds_in_flight); > free_cmd_list (h->cmds_done); > nbd_internal_free_string_list (h->argv); > + if (h->sa_sockpath) { > + if (h->pid > 0) > + kill (h->pid, SIGTERM); > + unlink (h->sa_sockpath); > + free (h->sa_sockpath); &g...

...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 any pending callbacks on commands stranded by abrupt close to allow the user to still avoid memory leaks. > +++ b/lib/debug.c > @@ -40,9 +40,11 @@ nbd_unlocked_get_debug (struct nbd_handle *h) > > int > nbd_unlocked_set_debug_callback (struct nbd_handle *...

As proposed here: https://www.redhat.com/archives/libguestfs/2019-August/msg00130.html I didn't actually read Eric's replies to that yet because I've been concentrating on writing these patches all day. Anyway here they are and I'll look at what Eric said about the proposal next. Rich.

...ta_context *m, *m_next; + size_t i; nbd_internal_set_error_context ("nbd_close"); @@ -130,6 +131,9 @@ nbd_close (struct nbd_handle *h) free (m->name); free (m); } + for (i = 0; i < h->nr_exports; ++i) + free (h->exports[i]); + free (h->exports); free_cmd_list (h->cmds_to_issue); free_cmd_list (h->cmds_in_flight); free_cmd_list (h->cmds_done); @@ -226,6 +230,52 @@ nbd_unlocked_get_export_name (struct nbd_handle *h) return copy; } +int +nbd_unlocked_set_list_exports (struct nbd_handle *h, bool list) +{ + h->list_exports = true; +...

If the user calls nbd_kill_subprocess, we shouldn't kill the process again when we close the handle (since the process has likely gone and we might be killing a different process). --- lib/handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/handle.c b/lib/handle.c index 2af25fe..5ad818e 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -315,6 +315,8 @@

v1 was posted here: https://www.redhat.com/archives/libguestfs/2019-September/thread.html#00337 v2: - Drop the first patch. - Hopefully fix the multiple issues with fork-safety and general behaviour on error paths. Note this requires execvpe for which there seems to be no equivalent on FreeBSD, except some kind of tedious path parsing (but can we assign to environ?) Rich.

...>argv) + nbd_internal_free_string_list (h->argv); + h->argv = copy; + + return nbd_internal_run (h, cmd_connect_sa); +} diff --git a/lib/handle.c b/lib/handle.c index 5ad818e..18cc8d0 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -129,6 +129,16 @@ nbd_close (struct nbd_handle *h) free_cmd_list (h->cmds_in_flight); free_cmd_list (h->cmds_done); nbd_internal_free_string_list (h->argv); + if (h->sa_sockpath) { + if (h->pid > 0) + kill (h->pid, SIGTERM); + unlink (h->sa_sockpath); + free (h->sa_sockpath); + } + if (h->sa_tmpdir) { + rmd...

An incremental improvement on top of listing exports. I still think it's worth experimenting with revisiting how our API for list mode should actually work [1] (so that we can reuse a single connection for both grabbing the list and finally using NBD_OPT_GO), but this change was easier to whip together while still thinking about that. [1]

Proposal for new APIs to list exports. The general shape of the API can probably best be seen from the examples/list-exports.c example. Rich.

...ries); nbd_internal_reset_size_and_flags (h); - for (i = 0; i < h->nr_exports; ++i) { - free (h->exports[i].name); - free (h->exports[i].description); - } - free (h->exports); - free (h->canonical_name); - free (h->description); + nbd_internal_free_option (h); free_cmd_list (h->cmds_to_issue); free_cmd_list (h->cmds_in_flight); free_cmd_list (h->cmds_done); @@ -288,60 +280,6 @@ nbd_unlocked_get_export_description (struct nbd_handle *h) return r; } -int -nbd_unlocked_get_nr_list_exports (struct nbd_handle *h) -{ - if (!h->exports) { - set_err...

...>argv) + nbd_internal_free_string_list (h->argv); + h->argv = copy; + + return nbd_internal_run (h, cmd_connect_sa); +} diff --git a/lib/handle.c b/lib/handle.c index 2af25fe..a7f2c79 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -129,6 +129,16 @@ nbd_close (struct nbd_handle *h) free_cmd_list (h->cmds_in_flight); free_cmd_list (h->cmds_done); nbd_internal_free_string_list (h->argv); + if (h->sa_sockpath) { + if (h->pid > 0) + kill (h->pid, SIGTERM); + unlink (h->sa_sockpath); + free (h->sa_sockpath); + } + if (h->sa_tmpdir) { + rmd...

...ut: > + unlink (tmpfile); > + > + exit (r == 0 ? EXIT_SUCCESS : EXIT_FAILURE); > +} > diff --git a/lib/connect.c b/lib/connect.c > index f98bcdb..c1cbef7 100644 > --- a/lib/connect.c > +++ b/lib/handle.c > @@ -129,6 +129,16 @@ nbd_close (struct nbd_handle *h) > free_cmd_list (h->cmds_in_flight); > free_cmd_list (h->cmds_done); > nbd_internal_free_string_list (h->argv); > + if (h->sa_sockpath) { > + if (h->pid > 0) > + kill (h->pid, SIGTERM); Are we sure that SIGTERM Is always going to be sufficient? Or do we need...

...ries); nbd_internal_reset_size_and_flags (h); - for (i = 0; i < h->nr_exports; ++i) { - free (h->exports[i].name); - free (h->exports[i].description); - } - free (h->exports); - free (h->canonical_name); - free (h->description); + nbd_internal_free_option (h); free_cmd_list (h->cmds_to_issue); free_cmd_list (h->cmds_in_flight); free_cmd_list (h->cmds_done); @@ -288,60 +280,6 @@ nbd_unlocked_get_export_description (struct nbd_handle *h) return r; } -int -nbd_unlocked_get_nr_list_exports (struct nbd_handle *h) -{ - if (!h->exports) { - set_err...

This isn't quite finished because not all of the tests or examples have been updated, but it demonstrates an idea: Should we forget about the concept of having multiple connections managed under a single handle? In this patch there is a single ‘struct nbd_handle *’ which manages a single state machine and connection (and therefore no nbd_connection). To connect to a multi-conn server you must

This adds coalescing of adjacent extents of the same type, as mentioned by Eric Blake in the commit message here: https://github.com/libguestfs/libnbd/commit/46072f6611f80245846a445766da071e457b00cd The patch series is rather long because it detours through adding the <vector.h> library from nbdkit into libnbd and replacing ad hoc uses of realloc, char ** etc in various places. Rich.

This is my proposal for fixing the documentation to match practice (namely, that completion.callback is not invoked in the cases where the aio call itself reports errors); we could instead try to go the other direction and tweak the generator to guarantee that both completion.callback and completion.free are reached no matter what, but that felt more invasive to me. Eric Blake (2): api: Tighten

This is a subset of my v2 posting, but limited to just the NBD_OPT_LIST handling. The biggest change since v2 is the addition of added unit testing in all four language bindings (C, python, ocaml, golang). The tests require nbdkit built from git on PATH, and may not be entirely idiomatic, but I at least validated that they catch issues (for example, adding an exit statement near the end of the