similar to: libnbd completion callback question

On 9/5/20 7:47 AM, Eric Blake wrote: > I noticed while reading the code that we have a documentation hole that > may cause memory leaks for clients that are unaware, in relation to > completion callbacks. > > The situation arises as follows: for all commands with a completion > callback, I checked that the code has clean semantics: either > nbd_aio_FOO() returns -1 and we

On 9/5/20 8:56 AM, Eric Blake wrote: > Thinking about it more, it is probably easiest to just declare that we > guarantee the cleanup callback is called regardless of failure mode, for > all closures, and that we merely had a memory leak in earlier libnbd > releases.  Yes, this means that we have a risk of older apps hitting a > double free if they cleaned up after a cookie of

On 7/16/19 10:39 AM, Eric Blake wrote: > On 7/16/19 10:04 AM, Richard W.M. Jones wrote: >> The API changes from: >> >> int nbd_add_close_callback (struct nbd_handle *h, >> nbd_close_callback cb, >> void *user_data); >> >> to: >> >> int nbd_add_close_callback (struct nbd_handle

On Sat, Jun 29, 2019 at 08:28:28AM -0500, Eric Blake wrote: > As mentioned in the previous patch, there are situations where an aio > client wants instant notification when a given command is complete, > rather than having to maintain a separate data structure to track all > in-flight commands and then iterate over that structure to learn which > commands are complete. It's

The 0.9.8 release breaks API, requiring a number of changes: - Use symbolic constants instead of magic numbers/open-coded strings (well, the string for "base:allocation" was present before this libnbd bump) - Change callbacks to drop the valid_flag parameter - Add _is to nbd_read_only call - Drop the _callback suffix on nbd_aio_FOO calls - Add a struct for managing callback/user_data

We want to default to requesting structured replies, whether or not that request will be honored (it's essential for efficient sparse file reads and the DF flag for structured pread, as well as for meta context support even if we do not request a default meta context). However, for integration testing, it can be nice to easily request a client that does not request structured replies. We can

v1 was here: https://listman.redhat.com/archives/libguestfs/2022-October/030216.html Since then, I've incorporated changes based on Rich's feedback: swap order of patches 2 and 3 less change in patch 1 (including no unsafe eval(%s) for --uri) in patch 2, include -c in list of snippets to store, and use dict of lambdas to map back to the desired action Eric Blake (3): nbdsh:

On 7/15/19 6:06 AM, Richard W.M. Jones wrote: > On Sat, Jun 29, 2019 at 08:28:28AM -0500, Eric Blake wrote: >> As mentioned in the previous patch, there are situations where an aio >> client wants instant notification when a given command is complete, >> rather than having to maintain a separate data structure to track all >> in-flight commands and then iterate over that

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 doesn't include Eric's new APIs, but if you push those then I can rebase this one on top. Rich.

On 8/13/19 5:36 PM, Richard W.M. Jones wrote: > The definition of functions that take a callback is changed so that > the callback and user_data are combined into a single structure, eg: > > int64_t nbd_aio_pread (struct nbd_handle *h, > void *buf, size_t count, uint64_t offset, > - int (*completion_callback) (/*..*/), void *user_data, > +

On 8/15/19 4:56 AM, Richard W.M. Jones wrote: > Previously the .free function of a callback was not called if the > .callback field was NULL, because the callback as a whole would be > considered to be "null". > > This change allows you to register callbacks where the .callback field > is NULL, but the .free field is != NULL, meaning that the callback is > freed

We decided to not request the "base:allocation" context by default (if a client wants to use block_status on a different context, then they'd have to get any default request out of the way); however, block status is useless without at least one meta context. This adds a convenience knob for requesting that, and has the nice benefit of working with the --connect command line option

In v2: - now based on my proposal to add LIBNBD_SHUTDOWN_IMMEDIATE - four flags instead of two: STRICT_FLAGS is new (patch 4), and STRICT_BOUNDS is separate from STRICT_ZERO_SIZE (patch 5) - various refactorings for more shared code and less duplication Eric Blake (5): api: Add xxx_MASK constant for each Flags type generator: Refactor filtering of accepted OFlags api: Add

As discussed in this thread, the parameter is an invitation to write code with race conditions: https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00309 --- docs/libnbd.pod | 6 +- examples/glib-main-loop.c | 10 ++-- examples/strict-structured-reads.c | 2 +- generator/generator | 57

On 7/30/19 10:36 AM, Richard W.M. Jones wrote: > As discussed in this thread, the parameter is an invitation to write > code with race conditions: > > https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00309 > --- > +++ b/generator/generator > @@ -1729,7 +1729,7 @@ C<nbd_pread>."; > default_call with > args = [ BytesPersistOut

The API changes from: int nbd_add_close_callback (struct nbd_handle *h, nbd_close_callback cb, void *user_data); to: int nbd_add_close_callback (struct nbd_handle *h, void *user_data, nbd_close_callback cb); The second way is consistent with how other callbacks work

Well, the golang changes (patch 1 and 2/3 of v1) were already committed, all that was left was the OCaml changes. I'm a lot happier with how things turned out with an UNKNOWN constructor in the OCaml variants. Eric Blake (3): tests: Enhance coverage of enum/flag range checking ocaml: Support unknown values for Enum/Flags ocaml: Typesafe returns for REnum/RFlags generator/OCaml.ml

This largish series adds several new features to the generator. Enum maps to enumerated types (like enum in C). The only current use for this is replacing the nbd_set_tls (nbd, 0/1/2) parameter with LIBNBD_TLS_DISABLE, LIBNBD_TLS_ALLOW, LIBNBD_TLS_REQUIRE (and natural equivalents in other programming languages). Flags maps to any uint32_t bitmask. It is basically a non-optional, generalized

On 8/10/19 8:02 AM, Richard W.M. Jones wrote: > Add an extra parameter to nbd_connect_uri to control what URIs are > permitted, in case the caller wants to pass in user-controlled URIs > but have some control over who/what/how the connection happens. For > example: > > nbd_connect_uri (nbd, "nbd://localhost", LIBNBD_CONNECT_URI_REQUIRE_TLS) > => error: URI