search for: public_st

Split h->state into: - h->public_state = the state on entry to the locked region This is also the atomicly, publicly visible state. - h->state = the real current state of the handle When we leave the locked region we update h->public_state with h->state, so that from outside the lock the handle appears to move atomical...

v1 was here: https://www.redhat.com/archives/libguestfs/2019-June/thread.html#00055 v2 was here: https://www.redhat.com/archives/libguestfs/2019-June/thread.html#00067 v3: - Fix atomicly -> atomically in commit message. - Fix a comment. - Fix TOCTTOU: There is now an inline function generated called <name>_is_permitted_state, and this is called twice, first outside the

Split h->state into: - h->public_state = the state on entry to the locked region This is also the atomically, publicly visible state. - h->state = the real current state of the handle When we leave the locked region we update h->public_state with h->state, so that from outside the lock the handle appears to move atomic...

..."; - print_trace_leave ret; - pr "\n" - ); + pr "\n"; if !need_out_label then pr " out:\n"; + if may_set_error then ( + print_trace_leave ret; + pr "\n" + ); if is_locked then ( pr " if (h->public_state != get_next_state (h))\n"; pr " h->public_state = get_next_state (h);\n"; -- 2.28.0

...ALLBACK (%s_callback);\n" cbname + | _ -> () + ) args; + List.iter ( + function + | OClosure { cbname } -> + pr " FREE_CALLBACK (%s_callback);\n" cbname + | OFlags _ -> () + ) optargs; if is_locked then ( pr " if (h->public_state != get_next_state (h))\n"; pr " h->public_state = get_next_state (h);\n"; diff --git a/lib/debug.c b/lib/debug.c index 1b503d9..b598ad3 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -54,6 +54,7 @@ nbd_unlocked_set_debug_callback (struct nbd_handle *h, nbd_unlocked_...

...*debug_fn) (unsigned, void *, const char *, const char *); @@ -84,9 +83,6 @@ struct nbd_handle { debug_fn debug_fn; void *debug_data; - /* Linked list of close callbacks. */ - struct close_callback *close_callbacks; - /* State machine. * * The actual current state is ‘state’. ‘public_state’ is updated @@ -226,12 +222,6 @@ struct meta_context { uint32_t context_id; /* Context ID negotiated with the server. */ }; -struct close_callback { - struct close_callback *next; /* Linked list. */ - nbd_close_callback cb; /* Function. */ - void *user_data;...

...b/internal.h index b2637bd..788a62e 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -148,6 +148,9 @@ struct nbd_handle { bool debug; nbd_debug_callback debug_callback; + /* How strict to be. */ + unsigned strict; + /* State machine. * * The actual current state is ‘state’. ‘public_state’ is updated diff --git a/generator/API.ml b/generator/API.ml index 962b787..8811d3c 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -169,7 +169,13 @@ let handshake_flags = { flags = [ "FIXED_NEWSTYLE", 1 lsl 0; "NO_ZEROES", 1 lsl 1; - ] + ] +} +l...

We have been inconsistent on how much we reject client-side without even consulting the server, vs. how much we depend on the server to detect failure (even if our request can be deemed undefined per NBD protocol). I'd like to change it so that by default, we reject as much as we can client-side for less traffic, but where the user can also change things on the fly for server-side integration

As promised in my earlier thread on libnbd completion callback question. Eric Blake (2): generator: Refactor handling of closures in unlocked functions generator: Free closures on failure docs/libnbd.pod | 2 +- generator/C.ml | 48 +++++++++++------ generator/C.mli | 1 + lib/debug.c | 7 +-- lib/opt.c | 31 ++++++-----

...644 --- a/lib/handle.c +++ b/lib/handle.c @@ -75,7 +75,7 @@ nbd_create (void) s = getenv ("LIBNBD_DEBUG"); h->debug = s && strcmp (s, "1") == 0; - h->strict = LIBNBD_STRICT_COMMANDS; + h->strict = LIBNBD_STRICT_COMMANDS | LIBNBD_STRICT_BOUNDS; h->public_state = STATE_START; h->state = STATE_START; diff --git a/lib/rw.c b/lib/rw.c index b5c1698..adfb7ac 100644 --- a/lib/rw.c +++ b/lib/rw.c @@ -278,6 +278,18 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf, } } + if (h->strict & LIBNBD_STRICT_BOUNDS) { + if (count ==...

...b/internal.h index 96699b5..2a5147f 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -148,6 +148,9 @@ struct nbd_handle { bool debug; nbd_debug_callback debug_callback; + /* How strict to be. */ + uint32_t strict; + /* State machine. * * The actual current state is ‘state’. ‘public_state’ is updated diff --git a/generator/API.ml b/generator/API.ml index b212e95..aa970e6 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -102,6 +102,13 @@ and link = | ExternalLink of string * int | URLLink of string +let strict_call_description = "\n +By default, libnbd will reject a...

...10 @@ struct nbd_handle { nbd_debug_callback debug_callback; void *debug_data; + /* Free callbacks kept in pointer order. */ + struct free_callback *free_callbacks; + size_t nr_free_callbacks, alloc_free_callbacks; + /* State machine. * * The actual current state is ‘state’. ‘public_state’ is updated @@ -220,6 +225,12 @@ struct nbd_handle { bool disconnect_request; /* True if we've queued NBD_CMD_DISC */ }; +struct free_callback { + void *ptr; + nbd_free_callback cb; + void *user_data; +}; + struct meta_context { struct meta_context *next; /* Linked list....

...te into: > > - h->state = the state on entry to the locked region > > - h->next_state = the current state and what the "publicly visible" > state will become when we leave the locked region The rest of this thread discusses potential other names, such as h->public_state for the state visible to unlocked code and h->state for internal while still locked; I trust that once you pick a naming scheme you like, you can redo this patch accordingly. I'll go ahead and review the changes as spelled here to at least see if I can spot any problems. > > Some...

I need to think about this patch series a bit more, but it does at least pass the tests. Rich.

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

I think I've addressed everything that was raised in review. Some of the highlights: - Callbacks should be freed reliably along all exit paths. - There's a simple test of closure lifetimes. - I've tried to use VALID|FREE in all the places where I'm confident that it's safe and correct to do. There may be more places. Note this is an optimization and shouldn't

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.

v1 was here: https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00231 The changes address everything that Eric picked up in his review of the first two patches. I have also added two more patches (4 and 5) which respectively fix docs and change int status -> unsigned status, as discussed. Passes make, check, check-valgrind. Rich.

This implements most of what I wrote here: https://www.redhat.com/archives/libguestfs/2019-July/msg00213.html