Richard W.M. Jones
2019-Jul-16 09:51 UTC
[Libguestfs] [PATCH libnbd] generator: Define new Closure type
** INCOMPLETE ** This is the generator change as discussed on the list already. The Python and OCaml bindings are not yet done. It passes all [C only] tests and valgrind. Note that nbd_add_close_callback is inconsistent with other closure types because it passes the user_data parameter after the function. (This is not caused by the current patch, it was already inconsistent). We decided that nbd_add_close_callback should be manually generated and not automatically generated because it should only be called from C, or perhaps more accurately it is only _needed_ from C (to support cleanup of the non-C bindings), but I don't think there's any reason not to automatically generate it. If we did generate it, then it would be an API break because these two parameters would get swapped around. Rich.
Richard W.M. Jones
2019-Jul-16 09:51 UTC
[Libguestfs] [PATCH libnbd] generator: Define new Closure type instead of callbacks.
A Closure is a list of (usually one, but can be more) closures. In C
there is also a singe ‘void *user_data’ parameter which is passed by
the caller into the function and through as the first parameter of
each callback invocation.
By grouping the previously separate Opaque and Callback* parameters
together we can avoid the awkward situation where we have to scan
through the argument list to try to match them up.
Because this is a closure, in non-C languages we can simply map these
to a closure and drop the opaque parameter entirely. It is not needed
since languages with proper closures can capture local state in the
closure.
Unlike the previous code it is no longer possible to mix persistent
and non-persistent callbacks in the same API call. This was not used
before and is unlikely to be useful.
For the C API there is no API or ABI change (the only change is the
naming of the opaque pointer which is not part of the API). For the
non-C languages the opaque parameter is no longer required as
discussed above.
Partly based on Eric Blake's earlier work here:
https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00160
---
Makefile.am | 10 +-
generator/generator | 173 ++++++++++++++++------------
generator/states-reply-simple.c | 2 +-
generator/states-reply-structured.c | 12 +-
generator/states-reply.c | 2 +-
generator/states.c | 3 +-
lib/handle.c | 7 +-
lib/internal.h | 11 +-
lib/rw.c | 56 ++++-----
9 files changed, 155 insertions(+), 121 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 98a0284..1a322f7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -34,13 +34,15 @@ SUBDIRS = \
examples \
valgrind \
tests \
- python \
- sh \
- ocaml \
- ocaml/examples \
interop \
$(NULL)
+# python \
+# sh \
+# ocaml \
+# ocaml/examples \
+#
+
noinst_SCRIPTS = run
# Check no files are missing from EXTRA_DIST rules, and that all
diff --git a/generator/generator b/generator/generator
index caa6353..afd11d0 100755
--- a/generator/generator
+++ b/generator/generator
@@ -849,13 +849,14 @@ 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 int *)
-| CallbackPersist of string * arg list (* as above, but callback persists *)
+| Closure of bool * closure list (* void *opaque + one or more closures
+ flag if true means callbacks persist
+ in the handle, false means they only
+ exist during the function call *)
| Flags of string (* NBD_CMD_FLAG_* flags *)
| Int of string (* small int *)
| Int64 of string (* 64 bit signed int *)
| Mutable of arg (* mutable argument, eg. int* *)
-| Opaque of string (* opaque object, void* in C *)
| Path of string (* filename or path *)
| SockAddrAndLen of string * string (* struct sockaddr * + socklen_t *)
| String of string (* string *)
@@ -863,6 +864,10 @@ and arg | UInt of string (* small unsigned int
*)
| UInt32 of string (* 32 bit unsigned int *)
| UInt64 of string (* 64 bit unsigned int *)
+and closure = {
+ cbname : string; (* name of callback function *)
+ cbargs : arg list; (* all closures return int for now *)
+}
and ret | RBool (* return a boolean, or error *)
| RConstString (* return a const string, NULL for error *)
@@ -915,9 +920,9 @@ Return the state of the debug flag on this handle.";
"set_debug_callback", {
default_call with
- args = [ Opaque "data";
- CallbackPersist ("debug_fn", [Opaque "data";
- String "context"; String
"msg"]) ];
+ args = [ Closure (true,
+ [{ cbname="debug_fn";
+ cbargs=[String "context"; String
"msg"] }]) ];
ret = RErr;
shortdesc = "set the debug callback";
longdesc = "\
@@ -1345,10 +1350,11 @@ protocol extensions).";
"pread_structured", {
default_call with
args = [ BytesOut ("buf", "count"); UInt64
"offset";
- Opaque "data";
- Callback ("chunk", [ Opaque "data"; BytesIn
("subbuf", "count");
+ Closure (false,
+ [{ cbname="chunk";
+ cbargs=[ BytesIn ("subbuf",
"count");
UInt64 "offset"; Int
"status";
- Mutable (Int "error"); ]);
+ Mutable (Int "error")] }]);
Flags "flags" ];
ret = RErr;
permitted_states = [ Connected ];
@@ -1534,12 +1540,13 @@ punching a hole.";
"block_status", {
default_call with
args = [ UInt64 "count"; UInt64 "offset";
- Opaque "data";
- Callback ("extent", [Opaque "data"; String
"metacontext";
- UInt64 "offset";
- ArrayAndLen (UInt32 "entries",
- "nr_entries");
- Mutable (Int "error")]);
+ Closure (false,
+ [ {cbname="extent";
+ cbargs=[String "metacontext";
+ UInt64 "offset";
+ ArrayAndLen (UInt32 "entries",
+ "nr_entries");
+ Mutable (Int "error")]} ]);
Flags "flags" ];
ret = RErr;
permitted_states = [ Connected ];
@@ -1717,9 +1724,9 @@ C<nbd_pread>.";
"aio_pread_callback", {
default_call with
args = [ BytesPersistOut ("buf", "count"); UInt64
"offset";
- Opaque "data";
- CallbackPersist ("callback", [ Opaque "data";
Int64 "cookie";
- Mutable (Int "error") ]);
+ Closure (true,
+ [ {cbname="callback";
+ cbargs=[Int64 "cookie"; Mutable (Int
"error")] } ]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
@@ -1747,12 +1754,12 @@ cause a deadlock.";
"aio_pread_structured", {
default_call with
args = [ BytesPersistOut ("buf", "count"); UInt64
"offset";
- Opaque "data";
- CallbackPersist ("chunk", [ Opaque "data";
- BytesIn ("subbuf",
"count");
- UInt64 "offset";
- Int "status";
- Mutable (Int "error"); ]);
+ Closure (true,
+ [ {cbname="chunk";
+ cbargs=[BytesIn ("subbuf",
"count");
+ UInt64 "offset";
+ Int "status";
+ Mutable (Int "error");]} ]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
@@ -1769,14 +1776,15 @@ documented in C<nbd_pread_structured>.";
"aio_pread_structured_callback", {
default_call with
args = [ BytesPersistOut ("buf", "count"); UInt64
"offset";
- Opaque "data";
- CallbackPersist ("chunk", [ Opaque "data";
- BytesIn ("subbuf",
"count");
- UInt64 "offset";
- Int "status";
- Mutable (Int "error"); ]);
- CallbackPersist ("callback", [ Opaque "data";
Int64 "cookie";
- Mutable (Int "error") ]);
+ Closure (true,
+ [ {cbname="chunk";
+ cbargs=[BytesIn ("subbuf",
"count");
+ UInt64 "offset";
+ Int "status";
+ Mutable (Int "error"); ]};
+ {cbname="callback";
+ cbargs=[Int64 "cookie";
+ Mutable (Int "error"); ]} ]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
@@ -1819,9 +1827,9 @@ C<nbd_pwrite>.";
"aio_pwrite_callback", {
default_call with
args = [ BytesPersistIn ("buf", "count"); UInt64
"offset";
- Opaque "data";
- CallbackPersist ("callback", [ Opaque "data";
Int64 "cookie";
- Mutable (Int "error") ]);
+ Closure (true,
+ [ {cbname="callback";
+ cbargs=[Int64 "cookie"; Mutable (Int
"error")]} ]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
@@ -1884,9 +1892,9 @@ Parameters behave as documented in
C<nbd_flush>.";
"aio_flush_callback", {
default_call with
- args = [ Opaque "data";
- CallbackPersist ("callback", [ Opaque "data";
Int64 "cookie";
- Mutable (Int "error") ]);
+ args = [ Closure (true,
+ [ {cbname="callback";
+ cbargs=[Int64 "cookie"; Mutable (Int
"error")]} ]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
@@ -1927,9 +1935,9 @@ Parameters behave as documented in
C<nbd_trim>.";
"aio_trim_callback", {
default_call with
args = [ UInt64 "count"; UInt64 "offset";
- Opaque "data";
- CallbackPersist ("callback", [ Opaque "data";
Int64 "cookie";
- Mutable (Int "error") ]);
+ Closure (true,
+ [ {cbname="callback";
+ cbargs=[Int64 "cookie"; Mutable (Int
"error")]} ]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
@@ -1970,9 +1978,9 @@ Parameters behave as documented in
C<nbd_cache>.";
"aio_cache_callback", {
default_call with
args = [ UInt64 "count"; UInt64 "offset";
- Opaque "data";
- CallbackPersist ("callback", [ Opaque "data";
Int64 "cookie";
- Mutable (Int "error") ]);
+ Closure (true,
+ [ {cbname="callback";
+ cbargs=[Int64 "cookie"; Mutable (Int
"error")]} ]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
@@ -2013,9 +2021,9 @@ Parameters behave as documented in
C<nbd_zero>.";
"aio_zero_callback", {
default_call with
args = [ UInt64 "count"; UInt64 "offset";
- Opaque "data";
- CallbackPersist ("callback", [ Opaque "data";
Int64 "cookie";
- Mutable (Int "error") ]);
+ Closure (true,
+ [ {cbname="callback";
+ cbargs=[Int64 "cookie"; Mutable (Int
"error")]} ]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
@@ -2042,12 +2050,12 @@ cause a deadlock.";
"aio_block_status", {
default_call with
args = [ UInt64 "count"; UInt64 "offset";
- Opaque "data";
- CallbackPersist ("extent", [Opaque "data";
String "metacontext";
- UInt64 "offset";
- ArrayAndLen (UInt32
"entries",
- "nr_entries");
- Mutable (Int "error")]);
+ Closure (true,
+ [ {cbname="extent";
+ cbargs=[String "metacontext"; UInt64
"offset";
+ ArrayAndLen (UInt32 "entries",
+ "nr_entries");
+ Mutable (Int "error")] } ]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
@@ -2063,14 +2071,14 @@ Parameters behave as documented in
C<nbd_block_status>.";
"aio_block_status_callback", {
default_call with
args = [ UInt64 "count"; UInt64 "offset";
- Opaque "data";
- CallbackPersist ("extent", [Opaque "data";
String "metacontext";
- UInt64 "offset";
- ArrayAndLen (UInt32
"entries",
- "nr_entries");
- Mutable (Int "error") ]);
- CallbackPersist ("callback", [ Opaque "data";
Int64 "cookie";
- Mutable (Int "error") ]);
+ Closure (true,
+ [ {cbname="extent";
+ cbargs=[String "metacontext"; UInt64
"offset";
+ ArrayAndLen (UInt32 "entries",
+ "nr_entries");
+ Mutable (Int "error")]};
+ {cbname="callback";
+ cbargs=[Int64 "cookie"; Mutable (Int
"error")]} ]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
@@ -3143,19 +3151,19 @@ let generate_lib_libnbd_syms () pr " local:
*;\n";
pr "};\n"
-let rec name_of_arg = function
-| ArrayAndLen (arg, n) -> name_of_arg arg @ [n]
+let rec c_name_of_arg = function
+| ArrayAndLen (arg, n) -> c_name_of_arg arg @ [n]
| Bool n -> [n]
| BytesIn (n, len) -> [n; len]
| BytesOut (n, len) -> [n; len]
| BytesPersistIn (n, len) -> [n; len]
| BytesPersistOut (n, len) -> [n; len]
-| Callback (n, _) | CallbackPersist (n, _) -> [n]
+| Closure (_, closures) ->
+ "user_data" :: List.map (fun { cbname } -> cbname) closures
| Flags n -> [n]
| Int n -> [n]
| Int64 n -> [n]
-| Opaque n -> [n]
-| Mutable arg -> name_of_arg arg
+| Mutable arg -> c_name_of_arg arg
| Path n -> [n]
| SockAddrAndLen (n, len) -> [n; len]
| String n -> [n]
@@ -3164,13 +3172,18 @@ let rec name_of_arg = function
| UInt32 n -> [n]
| UInt64 n -> [n]
-let rec print_c_arg_list ?(handle = false) args +let rec print_c_arg_list
?(handle = false) ?(user_data = false) args pr "(";
let comma = ref false in
if handle then (
comma := true;
pr "struct nbd_handle *h";
);
+ if user_data then (
+ if !comma then pr ", ";
+ comma := true;
+ pr "void *user_data";
+ );
List.iter (
fun arg ->
if !comma then pr ", ";
@@ -3183,15 +3196,18 @@ let rec print_c_arg_list ?(handle = false) args |
BytesPersistIn (n, len) -> pr "const void *%s, size_t %s" n len
| BytesOut (n, len)
| BytesPersistOut (n, len) -> pr "void *%s, size_t %s" n len
- | Callback (n, args)
- | CallbackPersist (n, args) ->
- pr "int (*%s) " n; print_c_arg_list args
+ | Closure (_, cls) ->
+ pr "void *user_data";
+ List.iter (
+ fun { cbname; cbargs } ->
+ pr ", int (*%s) " cbname;
+ print_c_arg_list ~user_data:true cbargs
+ ) cls
| Flags n -> pr "uint32_t %s" n
| Int n -> pr "int %s" n
| Int64 n -> pr "int64_t %s" n
| Mutable (Int n) -> pr "int *%s" n
| Mutable arg -> assert false
- | Opaque n -> pr "void *%s" n
| Path n
| String n -> pr "const char *%s" n
| StringList n -> pr "char **%s" n
@@ -3258,7 +3274,7 @@ let generate_include_libnbd_h () pr "\n";
pr "struct nbd_handle;\n";
pr "\n";
- pr "typedef void (*nbd_close_callback) (void *data);\n";
+ pr "typedef void (*nbd_close_callback) (void *user_data);\n";
pr "\n";
List.iter (fun (n, i) -> pr "#define LIBNBD_%-30s %d\n" n i)
constants;
pr "\n";
@@ -3275,7 +3291,8 @@ let generate_include_libnbd_h () pr "#define
LIBNBD_HAVE_NBD_GET_ERRNO 1\n";
pr "\n";
pr "extern int nbd_add_close_callback (struct nbd_handle *h,\n";
- pr " nbd_close_callback cb, void
*data);\n";
+ pr " nbd_close_callback cb,\n";
+ pr " void *user_data);\n";
pr "#define LIBNBD_HAVE_NBD_ADD_CLOSE_CALLBACK 1\n";
pr "\n";
List.iter (
@@ -3386,7 +3403,7 @@ let generate_lib_api_c () pr " }\n"
);
pr " ret = nbd_unlocked_%s (h" name;
- let argnames = List.flatten (List.map name_of_arg args) in
+ let argnames = List.flatten (List.map c_name_of_arg args) in
List.iter (pr ", %s") argnames;
pr ");\n";
if permitted_states <> [] then
@@ -3560,9 +3577,9 @@ are called is not defined. This API is only available
from C and is designed to help when writing bindings to libnbd
from other programming languages.
- typedef void (*nbd_close_callback) (void *data);
+ typedef void (*nbd_close_callback) (void *user_data);
int nbd_add_close_callback (struct nbd_handle *nbd,
- nbd_close_callback cb, void *data);
+ nbd_close_callback cb, void *user_data);
";
@@ -3587,6 +3604,7 @@ Richard W.M. Jones
Copyright (C) 2019 Red Hat Inc.
"
+(*
(*----------------------------------------------------------------------*)
(* Python bindings. *)
@@ -4938,6 +4956,7 @@ let generate_ocaml_nbd_c () pr "\n";
List.iter print_ocaml_binding handle_calls
+*)
(*----------------------------------------------------------------------*)
@@ -4950,6 +4969,7 @@ let () output_to "lib/unlocked.h"
generate_lib_unlocked_h;
output_to "lib/api.c" generate_lib_api_c;
output_to "docs/libnbd-api.pod" generate_docs_libnbd_api_pod;
+(*
output_to "python/methods.h" generate_python_methods_h;
output_to "python/libnbdmod.c" generate_python_libnbdmod_c;
output_to "python/methods.c" generate_python_methods_c;
@@ -4957,3 +4977,4 @@ let () output_to "ocaml/NBD.mli"
generate_ocaml_nbd_mli;
output_to "ocaml/NBD.ml" generate_ocaml_nbd_ml;
output_to "ocaml/nbd-c.c" generate_ocaml_nbd_c;
+ *)
diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c
index ba26f8c..72a401b 100644
--- a/generator/states-reply-simple.c
+++ b/generator/states-reply-simple.c
@@ -64,7 +64,7 @@
int error = 0;
assert (cmd->error == 0);
- if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data, cmd->count,
+ if (cmd->cb.fn.read (cmd->cb.user_data, cmd->data,
cmd->count,
cmd->offset, LIBNBD_READ_DATA, &error) == -1)
cmd->error = error ? error : EPROTO;
}
diff --git a/generator/states-reply-structured.c
b/generator/states-reply-structured.c
index 5c4d2f2..3168d1b 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -298,7 +298,8 @@
* current error rather than any earlier one. If the callback fails
* without setting errno, then use the server's error below.
*/
- if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data + (offset -
cmd->offset),
+ if (cmd->cb.fn.read (cmd->cb.user_data,
+ cmd->data + (offset - cmd->offset),
0, offset, LIBNBD_READ_ERROR, &scratch) == -1)
if (cmd->error == 0)
cmd->error = scratch;
@@ -384,7 +385,8 @@
if (cmd->cb.fn.read) {
int error = cmd->error;
- if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data + (offset -
cmd->offset),
+ if (cmd->cb.fn.read (cmd->cb.user_data,
+ cmd->data + (offset - cmd->offset),
length - sizeof offset, offset,
LIBNBD_READ_DATA, &error) == -1)
if (cmd->error == 0)
@@ -445,7 +447,8 @@
if (cmd->cb.fn.read) {
int error = cmd->error;
- if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data + offset, length,
+ if (cmd->cb.fn.read (cmd->cb.user_data,
+ cmd->data + offset, length,
cmd->offset + offset,
LIBNBD_READ_HOLE, &error) == -1)
if (cmd->error == 0)
@@ -496,7 +499,8 @@
/* Call the caller's extent function. */
int error = cmd->error;
- if (cmd->cb.fn.extent (cmd->cb.opaque, meta_context->name,
cmd->offset,
+ if (cmd->cb.fn.extent (cmd->cb.user_data,
+ meta_context->name, cmd->offset,
&h->bs_entries[1], (length-4) / 4,
&error) == -1)
if (cmd->error == 0)
cmd->error = error ? error : EPROTO;
diff --git a/generator/states-reply.c b/generator/states-reply.c
index 742fc1a..ebdbdc1 100644
--- a/generator/states-reply.c
+++ b/generator/states-reply.c
@@ -184,7 +184,7 @@ save_reply_state (struct nbd_handle *h)
if (cmd->cb.callback) {
int error = cmd->error;
- if (cmd->cb.callback (cmd->cb.opaque, cookie, &error) == -1
&& error)
+ if (cmd->cb.callback (cmd->cb.user_data, cookie, &error) == -1
&& error)
cmd->error = error;
}
diff --git a/generator/states.c b/generator/states.c
index 992f833..93ad4e5 100644
--- a/generator/states.c
+++ b/generator/states.c
@@ -124,7 +124,8 @@ void abort_commands (struct nbd_handle *h,
int error = cmd->error ? cmd->error : ENOTCONN;
assert (cmd->type != NBD_CMD_DISC);
- if (cmd->cb.callback (cmd->cb.opaque, cmd->cookie, &error)
== -1 && error)
+ if (cmd->cb.callback (cmd->cb.user_data, cmd->cookie,
+ &error) == -1 && error)
cmd->error = error;
}
if (cmd->error == 0)
diff --git a/lib/handle.c b/lib/handle.c
index cbe7e8a..5003227 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -98,7 +98,7 @@ nbd_close (struct nbd_handle *h)
for (cc = h->close_callbacks; cc != NULL; cc = cc_next) {
cc_next = cc->next;
- cc->cb (cc->data);
+ cc->cb (cc->user_data);
free (cc);
}
@@ -202,7 +202,8 @@ nbd_unlocked_add_meta_context (struct nbd_handle *h, const
char *name)
* programming languages.
*/
int
-nbd_add_close_callback (struct nbd_handle *h, nbd_close_callback cb, void
*data)
+nbd_add_close_callback (struct nbd_handle *h,
+ nbd_close_callback cb, void *user_data)
{
int ret;
struct close_callback *cc;
@@ -216,7 +217,7 @@ nbd_add_close_callback (struct nbd_handle *h,
nbd_close_callback cb, void *data)
}
cc->next = h->close_callbacks;
cc->cb = cb;
- cc->data = data;
+ cc->user_data = user_data;
h->close_callbacks = cc;
ret = 0;
diff --git a/lib/internal.h b/lib/internal.h
index 44272b9..347bf69 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -212,7 +212,7 @@ struct meta_context {
struct close_callback {
struct close_callback *next; /* Linked list. */
nbd_close_callback cb; /* Function. */
- void *data; /* Data. */
+ void *user_data; /* Data. */
};
struct socket_ops {
@@ -241,14 +241,15 @@ struct socket {
const struct socket_ops *ops;
};
-typedef int (*extent_fn) (void *data, const char *metacontext, uint64_t offset,
+typedef int (*extent_fn) (void *user_data,
+ const char *metacontext, uint64_t offset,
uint32_t *entries, size_t nr_entries, int *error);
-typedef int (*read_fn) (void *data, const void *buf, size_t count,
+typedef int (*read_fn) (void *user_data, const void *buf, size_t count,
uint64_t offset, int status, int *error);
-typedef int (*callback_fn) (void *data, int64_t cookie, int *error);
+typedef int (*callback_fn) (void *user_data, int64_t cookie, int *error);
struct command_cb {
- void *opaque;
+ void *user_data;
union {
extent_fn extent;
read_fn read;
diff --git a/lib/rw.c b/lib/rw.c
index a878fea..f2fe4e0 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -60,12 +60,12 @@ nbd_unlocked_pread (struct nbd_handle *h, void *buf,
int
nbd_unlocked_pread_structured (struct nbd_handle *h, void *buf,
size_t count, uint64_t offset,
- void *opaque, read_fn read, uint32_t flags)
+ void *user_data, read_fn read, uint32_t flags)
{
int64_t ch;
ch = nbd_unlocked_aio_pread_structured (h, buf, count, offset,
- opaque, read, flags);
+ user_data, read, flags);
if (ch == -1)
return -1;
@@ -145,11 +145,12 @@ nbd_unlocked_zero (struct nbd_handle *h,
int
nbd_unlocked_block_status (struct nbd_handle *h,
uint64_t count, uint64_t offset,
- void *data, extent_fn extent, uint32_t flags)
+ void *user_data, extent_fn extent, uint32_t flags)
{
int64_t ch;
- ch = nbd_unlocked_aio_block_status (h, count, offset, data, extent, flags);
+ ch = nbd_unlocked_aio_block_status (h, count, offset, user_data, extent,
+ flags);
if (ch == -1)
return -1;
@@ -159,8 +160,8 @@ nbd_unlocked_block_status (struct nbd_handle *h,
int64_t
nbd_internal_command_common (struct nbd_handle *h,
uint16_t flags, uint16_t type,
- uint64_t offset, uint64_t count, void *data,
- struct command_cb *cb)
+ uint64_t offset, uint64_t count,
+ void *data, struct command_cb *cb)
{
struct command_in_flight *cmd, *prev_cmd;
@@ -256,10 +257,10 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf,
int64_t
nbd_unlocked_aio_pread_callback (struct nbd_handle *h, void *buf,
size_t count, uint64_t offset,
- void *opaque, callback_fn callback,
+ void *user_data, callback_fn callback,
uint32_t flags)
{
- struct command_cb cb = { .opaque = opaque, .callback = callback, };
+ struct command_cb cb = { .user_data = user_data, .callback = callback, };
/* We could silently accept flag DF, but it really only makes sense
* with callbacks, because otherwise there is no observable change
@@ -277,20 +278,22 @@ nbd_unlocked_aio_pread_callback (struct nbd_handle *h,
void *buf,
int64_t
nbd_unlocked_aio_pread_structured (struct nbd_handle *h, void *buf,
size_t count, uint64_t offset,
- void *opaque, read_fn read, uint32_t flags)
+ void *user_data, read_fn read,
+ uint32_t flags)
{
return nbd_unlocked_aio_pread_structured_callback (h, buf, count, offset,
- opaque, read, NULL,
flags);
+ user_data, read, NULL,
+ flags);
}
int64_t
nbd_unlocked_aio_pread_structured_callback (struct nbd_handle *h, void *buf,
size_t count, uint64_t offset,
- void *opaque, read_fn read,
+ void *user_data, read_fn read,
callback_fn callback,
uint32_t flags)
{
- struct command_cb cb = { .opaque = opaque, .fn.read = read,
+ struct command_cb cb = { .user_data = user_data, .fn.read = read,
.callback = callback, };
if ((flags & ~LIBNBD_CMD_FLAG_DF) != 0) {
@@ -320,10 +323,10 @@ nbd_unlocked_aio_pwrite (struct nbd_handle *h, const void
*buf,
int64_t
nbd_unlocked_aio_pwrite_callback (struct nbd_handle *h, const void *buf,
size_t count, uint64_t offset,
- void *opaque, callback_fn callback,
+ void *user_data, callback_fn callback,
uint32_t flags)
{
- struct command_cb cb = { .opaque = opaque, .callback = callback, };
+ struct command_cb cb = { .user_data = user_data, .callback = callback, };
if (nbd_unlocked_read_only (h) == 1) {
set_error (EINVAL, "server does not support write operations");
@@ -352,10 +355,10 @@ nbd_unlocked_aio_flush (struct nbd_handle *h, uint32_t
flags)
}
int64_t
-nbd_unlocked_aio_flush_callback (struct nbd_handle *h, void *opaque,
+nbd_unlocked_aio_flush_callback (struct nbd_handle *h, void *user_data,
callback_fn callback, uint32_t flags)
{
- struct command_cb cb = { .opaque = opaque, .callback = callback, };
+ struct command_cb cb = { .user_data = user_data, .callback = callback, };
if (nbd_unlocked_can_flush (h) != 1) {
set_error (EINVAL, "server does not support flush operations");
@@ -382,10 +385,10 @@ nbd_unlocked_aio_trim (struct nbd_handle *h,
int64_t
nbd_unlocked_aio_trim_callback (struct nbd_handle *h,
uint64_t count, uint64_t offset,
- void *opaque, callback_fn callback,
+ void *user_data, callback_fn callback,
uint32_t flags)
{
- struct command_cb cb = { .opaque = opaque, .callback = callback, };
+ struct command_cb cb = { .user_data = user_data, .callback = callback, };
if (nbd_unlocked_read_only (h) == 1) {
set_error (EINVAL, "server does not support write operations");
@@ -422,10 +425,10 @@ nbd_unlocked_aio_cache (struct nbd_handle *h,
int64_t
nbd_unlocked_aio_cache_callback (struct nbd_handle *h,
uint64_t count, uint64_t offset,
- void *opaque, callback_fn callback,
+ void *user_data, callback_fn callback,
uint32_t flags)
{
- struct command_cb cb = { .opaque = opaque, .callback = callback, };
+ struct command_cb cb = { .user_data = user_data, .callback = callback, };
/* Actually according to the NBD protocol document, servers do exist
* that support NBD_CMD_CACHE but don't advertise the
@@ -456,10 +459,10 @@ nbd_unlocked_aio_zero (struct nbd_handle *h,
int64_t
nbd_unlocked_aio_zero_callback (struct nbd_handle *h,
uint64_t count, uint64_t offset,
- void *opaque, callback_fn callback,
+ void *user_data, callback_fn callback,
uint32_t flags)
{
- struct command_cb cb = { .opaque = opaque, .callback = callback, };
+ struct command_cb cb = { .user_data = user_data, .callback = callback, };
if (nbd_unlocked_read_only (h) == 1) {
set_error (EINVAL, "server does not support write operations");
@@ -489,20 +492,21 @@ nbd_unlocked_aio_zero_callback (struct nbd_handle *h,
int64_t
nbd_unlocked_aio_block_status (struct nbd_handle *h,
uint64_t count, uint64_t offset,
- void *data, extent_fn extent,
+ void *user_data, extent_fn extent,
uint32_t flags)
{
- return nbd_unlocked_aio_block_status_callback (h, count, offset, data,
extent,
+ return nbd_unlocked_aio_block_status_callback (h, count, offset,
+ user_data, extent,
NULL, flags);
}
int64_t
nbd_unlocked_aio_block_status_callback (struct nbd_handle *h,
uint64_t count, uint64_t offset,
- void *data, extent_fn extent,
+ void *user_data, extent_fn extent,
callback_fn callback, uint32_t flags)
{
- struct command_cb cb = { .opaque = data, .fn.extent = extent,
+ struct command_cb cb = { .user_data = user_data, .fn.extent = extent,
.callback = callback, };
if (!h->structured_replies) {
--
2.22.0
Eric Blake
2019-Jul-16 13:22 UTC
Re: [Libguestfs] [PATCH libnbd] generator: Define new Closure type
On 7/16/19 4:51 AM, Richard W.M. Jones wrote:> > Note that nbd_add_close_callback is inconsistent with other closure > types because it passes the user_data parameter after the function. > (This is not caused by the current patch, it was already > inconsistent). We decided that nbd_add_close_callback should be > manually generated and not automatically generated because it should > only be called from C, or perhaps more accurately it is only _needed_ > from C (to support cleanup of the non-C bindings), but I don't think > there's any reason not to automatically generate it. If we did > generate it, then it would be an API break because these two > parameters would get swapped around.I don't mind a separate patch to switch the argument order to make it consistent, whether or not we decide to generate it (we've already broken API since the last release due to s/notify/callback/, and we've already had other API breaks that swapped argument order of int *exit for consistency, so getting things consistent now is worthwhile). As for whether to generate it (so that other languages can register a close callback), I'm on the fence. But in the long run, having our initial stable API with close callbacks being C only, and a later release that adds language callbacks because it is generated, should be upwards compatible. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Possibly Parallel Threads
- [PATCH libnbd v2] generator: Define new Closure type
- [PATCH libnbd v2 0/5] lib: Implement closure lifetimes.
- [PATCH libnbd 0/3] Implement closure lifetimes.
- [PATCH libnbd v3 0/2] lib: Implement closure lifetimes.
- Re: [PATCH libnbd v2] generator: Define new Closure type instead of callbacks.