4:59 a.m.
Callback functions now return an int instead of a void. This allows
in some cases for the callback to indicate that there was an error.
This is a small change to the API:
For nbd_set_debug_callback the signature has changed, but the return
value is ignored.
For nbd_block_status and nbd_aio_block_status the extent function can
return an error, which causes the block status command to return an
error. Unfortunately this causes us to set the state to dead,
although with more effort we could recover from this. Because of this
behaviour I didn't document what happens in the error case as we may
want to change it in future.
For Python and OCaml bindings, raising any kind of exception from the
callback is equivalent to returning -1 from the C callback.
---
generator/generator | 36 +++++++++++++++++++++--------
generator/states-reply-structured.c | 12 +++++++---
interop/dirty-bitmap.c | 4 +++-
lib/debug.c | 3 ++-
lib/internal.h | 4 ++--
tests/meta-base-allocation.c | 10 ++++----
6 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/generator/generator b/generator/generator
index 4c895e8..ff6075d 100755
--- a/generator/generator
+++ b/generator/generator
@@ -830,7 +830,7 @@ and arg =
written by the function *)
| BytesPersistIn of string * string (* same as above, but buffer persists *)
| BytesPersistOut of string * string
-| Callback of string * arg list (* callback function returning void *)
+| Callback of string * arg list (* callback function returning int *)
| CallbackPersist of string * arg list (* as above, but callback persists *)
| Flags of string (* NBD_CMD_FLAG_* flags *)
| Int of string (* small int *)
@@ -2680,7 +2680,7 @@ let rec print_c_arg_list ?(handle = false) args =
| BytesPersistOut (n, len) -> pr "void *%s, size_t %s" n len
| Callback (n, args)
| CallbackPersist (n, args) ->
- pr "void (*%s) " n; print_c_arg_list args
+ pr "int (*%s) " n; print_c_arg_list args
| Flags n -> pr "uint32_t %s" n
| Int n -> pr "int %s" n
| Int64 n -> pr "int64_t %s" n
@@ -3146,11 +3146,12 @@ let print_python_binding name { args; ret } =
pr " PyObject *data;\n";
pr "};\n";
pr "\n";
- pr "static void\n";
+ pr "static int\n";
pr "%s_%s_wrapper " name cb_name;
print_c_arg_list args;
pr "\n";
pr "{\n";
+ pr " int ret;\n";
pr " PyGILState_STATE py_save = PyGILState_UNLOCKED;\n";
pr " PyObject *py_args, *py_ret;\n";
List.iter (
@@ -3213,10 +3214,14 @@ let print_python_binding name { args; ret } =
pr "\n";
pr " Py_DECREF (py_args);\n";
pr "\n";
- pr " if (py_ret != NULL)\n";
+ pr " if (py_ret != NULL) {\n";
+ pr " ret = 0;\n";
pr " Py_DECREF (py_ret); /* return value is discarded */\n";
- pr " else\n";
+ pr " }\n";
+ pr " else {\n";
+ pr " ret = -1;\n";
pr " PyErr_PrintEx (0); /* print exception */\n";
+ pr " };\n";
pr "\n";
List.iter (
function
@@ -3232,6 +3237,7 @@ let print_python_binding name { args; ret } =
| Flags _ | Int _ | Int64 _ | Path _ | SockAddrAndLen _ | StringList _
| UInt _ | UInt32 _ -> assert false
) args;
+ pr " return ret;\n";
pr "}\n";
pr "\n"
| _ -> ()
@@ -3917,7 +3923,7 @@ let print_ocaml_binding (name, { args; ret }) =
| UInt _ | UInt32 _ -> assert false
) args in
- pr "static void\n";
+ pr "static int\n";
pr "%s_%s_wrapper_locked " name cb_name;
print_c_arg_list args;
pr "\n";
@@ -3957,24 +3963,34 @@ let print_ocaml_binding (name, { args; ret }) =
pr " rv = caml_callbackN_exn (fnv, %d, args);\n"
(List.length argnames);
- pr " if (Is_exception_result (rv))\n";
+ pr " if (Is_exception_result (rv)) {\n";
+ pr " /* XXX This is not really an error as callbacks can return\n";
+ pr " * an error indication. But perhaps we should direct this\n";
+ pr " * to a more suitable place or formalize what exception.\n";
+ pr " * means error versus unexpected failure.\n";
+ pr " */\n";
pr " fprintf (stderr,\n";
pr " \"libnbd: uncaught OCaml exception:
%%s\",\n";
pr " caml_format_exception (Extract_exception (rv)));\n";
+ pr " CAMLreturnT (int, -1);\n";
+ pr " }\n";
pr "\n";
- pr " CAMLreturn0;\n";
+ pr " CAMLreturnT (int, 0);\n";
pr "}\n";
pr "\n";
- pr "static void\n";
+ pr "static int\n";
pr "%s_%s_wrapper " name cb_name;
print_c_arg_list args;
pr "\n";
pr "{\n";
+ pr " int ret;\n";
+ pr "\n";
pr " caml_leave_blocking_section ();\n";
let c_argnames = List.flatten (List.map name_of_arg args) in
- pr " %s_%s_wrapper_locked (%s);\n" name cb_name
+ pr " ret = %s_%s_wrapper_locked (%s);\n" name cb_name
(String.concat ", " c_argnames);
pr " caml_enter_blocking_section ();\n";
+ pr " return ret;\n";
pr "}\n"
| _ -> ()
) args;
diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c
index e6c1a8a..c835713 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -369,10 +369,16 @@
if (context_id == meta_context->context_id)
break;
- if (meta_context)
+ if (meta_context) {
/* Call the caller's extent function. */
- cmd->extent_fn (cmd->data, meta_context->name, cmd->offset,
- &h->bs_entries[1], (length-4) / 4);
+ if (cmd->extent_fn (cmd->data, meta_context->name, cmd->offset,
+ &h->bs_entries[1], (length-4) / 4) == -1) {
+ SET_NEXT_STATE (%.DEAD); /* XXX We should be able to recover. */
+ if (errno == 0) errno = EPROTO;
+ set_error (errno, "extent function failed");
+ return -1;
+ }
+ }
else
/* Emit a debug message, but ignore it. */
debug (h, "server sent unexpected meta context ID %" PRIu32,
diff --git a/interop/dirty-bitmap.c b/interop/dirty-bitmap.c
index b3a89d0..8d34173 100644
--- a/interop/dirty-bitmap.c
+++ b/interop/dirty-bitmap.c
@@ -32,7 +32,7 @@ static const char *base_allocation = "base:allocation";
static int calls; /* Track which contexts passed through callback */
-static void
+static int
cb (void *data, const char *metacontext, uint64_t offset,
uint32_t *entries, size_t len)
{
@@ -71,6 +71,8 @@ cb (void *data, const char *metacontext, uint64_t offset,
fprintf (stderr, "unexpected context %s\n", metacontext);
exit (EXIT_FAILURE);
}
+
+ return 0;
}
int
diff --git a/lib/debug.c b/lib/debug.c
index 8e2ff76..02d49f7 100644
--- a/lib/debug.c
+++ b/lib/debug.c
@@ -41,7 +41,7 @@ nbd_unlocked_get_debug (struct nbd_handle *h)
int
nbd_unlocked_set_debug_callback (struct nbd_handle *h,
void *data,
- void (*debug_fn) (void *, const char *, const char *))
+ int (*debug_fn) (void *, const char *, const char *))
{
h->debug_fn = debug_fn;
h->debug_data = data;
@@ -75,6 +75,7 @@ nbd_internal_debug (struct nbd_handle *h, const char *fs, ...)
goto out;
if (h->debug_fn)
+ /* ignore return value */
h->debug_fn (h->debug_data, context, msg);
else
fprintf (stderr, "libnbd: debug: %s: %s\n", context ? :
"unknown", msg);
diff --git a/lib/internal.h b/lib/internal.h
index c8e5094..c1a57ac 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -75,7 +75,7 @@ struct nbd_handle {
/* For debugging. */
bool debug;
void *debug_data;
- void (*debug_fn) (void *, const char *, const char *);
+ int (*debug_fn) (void *, const char *, const char *);
/* Linked list of close callbacks. */
struct close_callback *close_callbacks;
@@ -212,7 +212,7 @@ struct socket {
const struct socket_ops *ops;
};
-typedef void (*extent_fn) (void *data, const char *metacontext, uint64_t offset, uint32_t
*entries, size_t nr_entries);
+typedef int (*extent_fn) (void *data, const char *metacontext, uint64_t offset, uint32_t
*entries, size_t nr_entries);
struct command_in_flight {
struct command_in_flight *next;
diff --git a/tests/meta-base-allocation.c b/tests/meta-base-allocation.c
index 71f2afc..b00aa50 100644
--- a/tests/meta-base-allocation.c
+++ b/tests/meta-base-allocation.c
@@ -28,9 +28,9 @@
#include <libnbd.h>
-static void check_extent (void *data, const char *metacontext,
- uint64_t offset,
- uint32_t *entries, size_t nr_entries);
+static int check_extent (void *data, const char *metacontext,
+ uint64_t offset,
+ uint32_t *entries, size_t nr_entries);
int
main (int argc, char *argv[])
@@ -125,7 +125,7 @@ main (int argc, char *argv[])
exit (EXIT_SUCCESS);
}
-static void
+static int
check_extent (void *data, const char *metacontext,
uint64_t offset,
uint32_t *entries, size_t nr_entries)
@@ -173,4 +173,6 @@ check_extent (void *data, const char *metacontext,
else
fprintf (stderr, "warning: ignored unexpected meta context %s\n",
metacontext);
+
+ return 0;
}
--
2.21.0