search for: set_callback_to_nul

...e = 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_clear_debug_callback (h); h->debug_callback = *debug_callback; + SET_CALLBACK_TO_NULL (*debug_callback); return 0; } diff --git a/lib/opt.c b/lib/opt.c index 003ecf8..6ea8326 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -156,6 +156,7 @@ nbd_unlocked_opt_list (struct nbd_handle *h, nbd_list_callback *list) if (nbd_unlocked_aio_opt_list (h, &l, &c) == -1) return -1;...

...+ set_error (count_err, "request out of bounds"); + return -1; + } + } + switch (type) { /* Commands which send or receive data are limited to MAX_REQUEST_SIZE. */ case NBD_CMD_READ: @@ -282,7 +296,7 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf, SET_CALLBACK_TO_NULL (*completion); return nbd_internal_command_common (h, flags, NBD_CMD_READ, offset, count, - buf, &cb); + EINVAL, buf, &cb); } int64_t @@ -306,7 +320,7 @@ nbd_unlocked_aio_pread_structured (struct nbd_handle *h,...

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 ++++++-----

...d { }; /* Test if a callback is "null" or not, and set it to null. */ -#define CALLBACK_IS_NULL(cb) ((cb).callback == NULL) +#define CALLBACK_IS_NULL(cb) ((cb).callback == NULL && (cb).free == NULL) #define CALLBACK_IS_NOT_NULL(cb) (! CALLBACK_IS_NULL ((cb))) -#define SET_CALLBACK_TO_NULL(cb) ((cb).callback = NULL) +#define SET_CALLBACK_TO_NULL(cb) ((cb).callback = NULL, (cb).free = NULL) /* Call a callback. */ -#define CALL_CALLBACK(cb, ...) \ - (cb).callback ((cb).user_data, ##__VA_ARGS__) +#define CALL_CALLBACK(cb, ...) \ + ((cb).cal...

...k_status to accept NULL for the callback; which is probably not a good idea. In short, there are some points in the code that care only whether .callback is NULL, and others that care whether both pointers are NULL. > #define CALLBACK_IS_NOT_NULL(cb) (! CALLBACK_IS_NULL ((cb))) > -#define SET_CALLBACK_TO_NULL(cb) ((cb).callback = NULL) > +#define SET_CALLBACK_TO_NULL(cb) ((cb).callback = NULL, (cb).free = NULL) > > /* Call a callback. */ > -#define CALL_CALLBACK(cb, ...) \ > - (cb).callback ((cb).user_data, ##__VA_ARGS__) > +#define CALL_CALLBACK(cb, ...)...

This is a combination of these two earlier series: https://www.redhat.com/archives/libguestfs/2019-August/msg00235.html https://www.redhat.com/archives/libguestfs/2019-August/msg00240.html plus changes to allow .callback = NULL / .free != NULL, and to reduce the complexity of freeing callbacks. Although it's rather long there's nothing complex here. We might consider squashing some

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

...if (id == -1) return -1; h->disconnect_request = true; diff --git a/lib/rw.c b/lib/rw.c index f49fe25..e3e80ad 100644 --- a/lib/rw.c +++ b/lib/rw.c @@ -281,7 +281,7 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf, struct command_cb cb = { .completion = *completion }; SET_CALLBACK_TO_NULL (*completion); - return nbd_internal_command_common (h, 0, NBD_CMD_READ, offset, count, + return nbd_internal_command_common (h, flags, NBD_CMD_READ, offset, count, buf, &cb); } @@ -350,7 +350,7 @@ nbd_unlocked_aio_flush (struct nbd_handle *h, }...

...nternal.h @@ -273,6 +273,11 @@ struct command { uint32_t error; /* Local errno value */ }; +/* Test if a callback is "null" or not, and set it to null. */ +#define CALLBACK_IS_NULL(cb) ((cb).callback == NULL) +#define CALLBACK_IS_NOT_NULL(cb) (! CALLBACK_IS_NULL ((cb))) +#define SET_CALLBACK_TO_NULL(cb) ((cb).callback = NULL) + /* Call a callback. */ #define CALL_CALLBACK(cb, ...) \ (cb).callback ((cb).user_data, ##__VA_ARGS__) @@ -286,9 +291,9 @@ struct command { #define FREE_CALLBACK(cb) \ do {...

...helper s = { .context = *context }; + nbd_context_callback l = { .callback = context_visitor, .user_data = &s }; + nbd_completion_callback c = { .callback = context_complete, .user_data = &s }; + + if (nbd_unlocked_aio_opt_list_meta_context (h, &l, &c) == -1) + return -1; + + SET_CALLBACK_TO_NULL (*context); + if (wait_for_option (h) == -1) + return -1; + if (s.err) { + set_error (s.err, "server replied with error to list meta context request"); + return -1; + } + return s.count; +} + /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */ int nbd_unlocke...

...rt the DF flag"); - return -1; + if (h->strict & LIBNBD_STRICT_COMMANDS) { + if ((flags & LIBNBD_CMD_FLAG_DF) != 0 && + nbd_unlocked_can_df (h) != 1) { + set_error (EINVAL, "server does not support the DF flag"); + return -1; + } } SET_CALLBACK_TO_NULL (*chunk); @@ -315,15 +317,17 @@ nbd_unlocked_aio_pwrite (struct nbd_handle *h, const void *buf, { struct command_cb cb = { .completion = *completion }; - if (nbd_unlocked_is_read_only (h) == 1) { - set_error (EPERM, "server does not support write operations"); - return -1; -...

...t;disconnect_request = true; > diff --git a/lib/rw.c b/lib/rw.c > index f49fe25..e3e80ad 100644 > --- a/lib/rw.c > +++ b/lib/rw.c > @@ -281,7 +281,7 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf, > struct command_cb cb = { .completion = *completion }; > > SET_CALLBACK_TO_NULL (*completion); > - return nbd_internal_command_common (h, 0, NBD_CMD_READ, offset, count, > + return nbd_internal_command_common (h, flags, NBD_CMD_READ, offset, count, > buf, &cb); > } > > @@ -350,7 +350,7 @@ nbd_unlocked_aio_flush...

...DF, but it really only makes sense - * with callbacks, because otherwise there is no observable change - * except that the server may fail where it would otherwise succeed. - */ - if (flags != 0) { - set_error (EINVAL, "invalid flag: %" PRIu32, flags); - return -1; - } - SET_CALLBACK_TO_NULL (*completion); return nbd_internal_command_common (h, 0, NBD_CMD_READ, offset, count, buf, &cb); @@ -304,11 +295,6 @@ nbd_unlocked_aio_pread_structured (struct nbd_handle *h, void *buf, struct command_cb cb = { .fn.chunk = *chunk,...

I'm posting this now, as I'm at the end of a workday and I got things working for manual experimentation. Still to do: - write interop tests for qemu-nbd and nbdkit (including my proposed patch addition of qemu-nbd -A to show qemu:allocation-depth) - figure out if we can make 'nbdinfo --map' use the new API to automatically select all contexts advertised by the server Eric Blake

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

...that uint32_t is unsigned int. Not true of all generic C platforms, but certainly true for the POSIX-like platforms we target (anyone that defines uint32_t as 'unsigned long' on a platform with 32-bit longs is unusual, but even then we should still be okay). > > > > > SET_CALLBACK_TO_NULL (*completion); > > return nbd_internal_command_common (h, flags, NBD_CMD_WRITE, offset, count, > > diff --git a/tests/Makefile.am b/tests/Makefile.am > > index 3a93251e..8b839bf5 100644 > > --- a/tests/Makefile.am > > +++ b/tests/Makefile.am > > @@ -232,6 +23...

In v2: ack'ed preliminary patches have been pushed, and I've added a lot of testsuite coverage as well as putting the new API to use in nbdinfo. Eric Blake (2): api: Add nbd_opt_list_meta_context info: List available meta-contexts lib/internal.h | 1 + generator/API.ml | 84 ++++++++-