[nbdkit PATCH 0/2] Reduce network overhead with corking
by Eric Blake
Slightly RFC, as I need more time to investigate why Unix sockets
appeared to degrade with this patch. But as TCP sockets (over loopback
to localhost) and TLS sessions (regardless of underlying Unix or TCP)
both showed improvements, this looks like a worthwhile series.
Eric Blake (2):
server: Add support for corking
server: Cork around grouped transmission send()s
server/internal.h | 3 +++
server/connections.c | 27 +++++++++++++++++++++++
server/crypto.c | 19 ++++++++++++++++
server/protocol.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 101 insertions(+)
--
2.20.1
4 years, 11 months
The way of implementing structs of Rust bindings
by Hiroyuki Katsura
Hi, I'm now implementing generators of Rust, and I faced a problem.
In order for the wrapper to 'understand' the struct passed from C API, it
is required to incorporate the definition of the struct in C into Rust
code.
I have two approaches.
1. Create raw struct(#[repr(C)]), which has the equivalent memory mapping
to C struct and access through this struct in Rust
2. Use bindgen to create ffi struct from guestfs.h
Each of them has advantages and disadvantages.
# 1st approach
This is highly dependent on the implementation of API because it is subject
to the memory mapping of API struct. When the way struct is structured
changes in API, you have to modify Rust bindings. In order to avoid this
situation, you can define the same struct in C and Rust and then access the
API struct in C and then Rust accesses the data through this struct. This
means that
```
API:
struct guestfs_A:
- field: x FString
C:
struct RawA {
char* x
};
struct RawA to_RawA(struct guestfs_A* src) {
struct RawA x = {src->x};
return x;
}
Rust:
#repr(C)
use std::ffi::CStr;
use std::str;
#[repr(C)]
struct RawA {
x: *const i8
}
enum guestfs_A {} // opaque struct
extern {
fn to_RawA( src: *const guestfs_A ) -> RawA;
}
struct A {
x: String
}
impl A {
fn new(src: *const guestfs_A) -> A {
let dst = unsafe {to_RawA(src)};
let c_str = unsafe { CStr::from_ptr(dst.x) };
let s = c_str.to_str().unwrap().to_string();
A{ x: s }
}
}
```
This is a little verbose and inefficient.
# 2nd approach
The above is easily done by 'bindgen', which automatically generates rust
ffi bindings to C library. By using this, API struct is automatically
generated. However, it requires a new dependency: libclang.
# Question
Which of these approaches is more preferable?
Regards,
Hiroyuki Katsura
4 years, 11 months
[libnbd PATCH] api: Add nbd_supports_tls
by Eric Blake
This is slightly redundant with just trying nbd_set_tls(nbd, 2) then
checking for failure; however, this function does not set errors and
looks more similar to nbd_supports_uri.
---
This is borderline enough that I figured I'd post it to check if we want it.
generator/generator | 45 ++++++++++++++++++++++++++++++++++++++-------
interop/interop.c | 4 ++++
lib/handle.c | 12 ++++++++++++
3 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/generator/generator b/generator/generator
index ea6eea4..d21e786 100755
--- a/generator/generator
+++ b/generator/generator
@@ -971,7 +971,9 @@ the path to the certificates directory (C<nbd_set_tls_certificates>),
the username (C<nbd_set_tls_username>) and/or
the Pre-Shared Keys (PSK) file (C<nbd_set_tls_psk_file>). For now,
when using C<nbd_connect_uri>, any URI query parameters related to
-TLS are not handled automatically.
+TLS are not handled automatically. Setting the level higher than
+zero will fail if libnbd was not compiled against gnutls; you can
+test whether this is the case with C<nbd_supports_tls>.
For more information see L<libnbd(3)/ENCRYPTION AND AUTHENTICATION>.";
};
@@ -995,7 +997,11 @@ set and TLS is used then a compiled in default is used.
For root this is C</etc/pki/libnbd/>. For non-root this is
C<$HOME/.pki/libnbd> and C<$HOME/.config/pki/libnbd>. If
none of these directories can be found then the system
-trusted CAs are used.";
+trusted CAs are used.
+
+This function may be called regardless of whether TLS is
+supported, but will have no effect unless C<nbd_set_tls>
+is also used to request or require TLS.";
};
(* Can't implement this because we need a way to return string that
@@ -1018,7 +1024,11 @@ Get the current TLS directory. See C<nbd_set_tls_certificates>.";
Set this flag to control whether libnbd will verify the identity
of the server from the server's certificate and the certificate
authority. This defaults to true when connecting to TCP servers
-using TLS certificate authentication, and false otherwise.";
+using TLS certificate authentication, and false otherwise.
+
+This function may be called regardless of whether TLS is
+supported, but will have no effect unless C<nbd_set_tls>
+is also used to request or require TLS.";
};
"get_tls_verify_peer", {
@@ -1037,7 +1047,11 @@ Get the verify peer flag.";
longdesc = "\
Set the TLS client username. This is used
if authenticating with PSK over TLS is enabled.
-If not set then the local username is used.";
+If not set then the local username is used.
+
+This function may be called regardless of whether TLS is
+supported, but will have no effect unless C<nbd_set_tls>
+is also used to request or require TLS.";
};
"get_tls_username", {
@@ -1057,7 +1071,11 @@ Get the current TLS username. See C<nbd_set_tls_username>.";
Set the TLS Pre-Shared Keys (PSK) filename. This is used
if trying to authenticate to the server using with a pre-shared
key. There is no default so if this is not set then PSK
-authentication cannot be used to connect to the server.";
+authentication cannot be used to connect to the server.
+
+This function may be called regardless of whether TLS is
+supported, but will have no effect unless C<nbd_set_tls>
+is also used to request or require TLS.";
};
(* Can't implement this because we need a way to return string that
@@ -1112,7 +1130,9 @@ C<nbd_connect_tcp> or C<nbd_connect_unix>. This call returns when
the connection has been made.
This call will fail if libnbd was not compiled with libxml2; you can
-test whether this is the case with C<nbd_supports_uri>.";
+test whether this is the case with C<nbd_supports_uri>. Support for
+URIs that require TLS will fail if libnbd was not compiled with
+gnutls; you can test whether this is the case with C<nbd_supports_tls>.";
};
"connect_unix", {
@@ -1497,7 +1517,9 @@ and completed the NBD handshake by calling C<nbd_aio_is_ready>,
on the connection.
This call will fail if libnbd was not compiled with libxml2; you can
-test whether this is the case with C<nbd_supports_uri>.";
+test whether this is the case with C<nbd_supports_uri>. Support for
+URIs that require TLS will fail if libnbd was not compiled with
+gnutls; you can test whether this is the case with C<nbd_supports_tls>.";
};
"aio_connect_unix", {
@@ -1876,6 +1898,15 @@ The release number is incremented for each release along a particular
branch.";
};
+ "supports_tls", {
+ default_call with
+ args = []; ret = RBool; is_locked = false; may_set_error = false;
+ shortdesc = "return true if libnbd was compiled with support for TLS";
+ longdesc = "\
+Returns true if libnbd was compiled with gnutls which is required
+to support TLS encryption, or false if not. See C<nbd_set_tls>.";
+ };
+
"supports_uri", {
default_call with
args = []; ret = RBool; is_locked = false; may_set_error = false;
diff --git a/interop/interop.c b/interop/interop.c
index 24f79cc..5d129a0 100644
--- a/interop/interop.c
+++ b/interop/interop.c
@@ -71,6 +71,10 @@ main (int argc, char *argv[])
/* Require TLS on the handle and fail if not available or if the
* handshake fails.
*/
+ if (nbd_supports_tls (nbd) != 1) {
+ fprintf (stderr, "skip: compiled without TLS supports\n");
+ exit (77);
+ }
if (nbd_set_tls (nbd, 2) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
diff --git a/lib/handle.c b/lib/handle.c
index cc311ba..e40b274 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -227,6 +227,18 @@ nbd_unlocked_get_version (struct nbd_handle *h)
return PACKAGE_VERSION;
}
+/* NB: is_locked = false, may_set_error = false. */
+int
+nbd_unlocked_supports_tls (struct nbd_handle *h)
+{
+#ifdef HAVE_GNUTLS
+ return 1;
+#else
+ return 0;
+#endif
+}
+
+/* NB: is_locked = false, may_set_error = false. */
int
nbd_unlocked_supports_uri (struct nbd_handle *h)
{
--
2.20.1
4 years, 11 months
Re: [Libguestfs] [libguestfs/libguestfs] OpenStack support (#38)
by Richard W.M. Jones
On Wed, Jun 05, 2019 at 04:13:37PM +0000, kedorlaomer wrote:
> According to the manpage,
>
> > Virt-p2v converts a physical machine to run virtualized on KVM,
> > managed by libvirt, OpenStack, oVirt, Red Hat Virtualisation
> > (RHV), or one of the other targets supported by virt-v2v(1).
>
> In fact, `openstack` is one of the targets of `virt-v2v`. According
> to the file `p2v/conversion.c`, function `generate_wrapper_script`,
> the output mode of `virt-v2v` can be set to `glance`. The manpage of
> `virt-v2v` claims
>
> > only -o openstack should be used normally.
>
> But even if `-o glance` was appropriate, there's no provision for
> setting additional command line flags such as `-oo …` in order to
> set up the server id, authentication and other conversion options.
>
> Does the conversion to OpenStack work in any setup? Has it
> reportedly ever worked? What needs to be done to get it working?
Yes, the -o openstack mode does work and is used routinely.
However virt-p2v needs additional work in order to support -o
openstack, currently you can only convert to a flat image and then
import that.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
4 years, 11 months
[PATCH libnbd v2] lib: Atomically update h->state when leaving the locked region.
by Richard W.M. Jones
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
atomically from its previous state to the final state without going
through any intermediate states.
Some calls to ‘get_state’ become calls to ‘get_next_state’ if the need
the real state. Others which need to see the publicly visible state
are changed to ‘get_public_state’.
All calls to ‘set_state’ become ‘set_next_state’ because that is the
real state that gets updated.
The purpose of this patch is to make it easier to reason about the
state in lockless code.
---
generator/generator | 29 ++++++++++++++++-------------
lib/connect.c | 10 +++++-----
lib/disconnect.c | 8 ++++----
lib/handle.c | 2 +-
lib/internal.h | 17 ++++++++++++++---
lib/is-state.c | 28 ++++++++++++++++------------
lib/rw.c | 4 ++--
7 files changed, 58 insertions(+), 40 deletions(-)
diff --git a/generator/generator b/generator/generator
index 5faa897..246eaed 100755
--- a/generator/generator
+++ b/generator/generator
@@ -812,8 +812,8 @@ type call = {
(* List of permitted states for making this call. [[]] = Any state. *)
permitted_states : permitted_state list;
(* Most functions must take a lock. The only known exception is
- * for a function which {b only} reads from the atomic [h->state]
- * field and does nothing else with the handle.
+ * for a function which {b only} reads from the atomic
+ * [get_public_state] field and does nothing else with the handle.
*)
is_locked : bool;
(* Most functions can call set_error. For functions which are
@@ -2418,11 +2418,11 @@ let generate_lib_states_c () =
pr " enum state next_state = %s;\n" state_enum;
pr "\n";
pr " r = _enter_%s (h, &next_state, blocked);\n" state_enum;
- pr " if (get_state (h) != next_state) {\n";
+ pr " if (get_next_state (h) != next_state) {\n";
pr " debug (h, \"transition: %%s -> %%s\",\n";
pr " \"%s\",\n" display_name;
pr " nbd_internal_state_short_string (next_state));\n";
- pr " set_state (h, next_state);\n";
+ pr " set_next_state (h, next_state);\n";
pr " }\n";
pr " return r;\n";
pr "}\n";
@@ -2437,7 +2437,7 @@ let generate_lib_states_c () =
pr " bool blocked;\n";
pr "\n";
pr " /* Validate and handle the external event. */\n";
- pr " switch (get_state (h))\n";
+ pr " switch (get_next_state (h))\n";
pr " {\n";
List.iter (
fun ({ parsed = { display_name; state_enum; events } } as state) ->
@@ -2449,7 +2449,7 @@ let generate_lib_states_c () =
fun (e, next_state) ->
pr " case %s:\n" (c_string_of_external_event e);
if state != next_state then (
- pr " set_state (h, %s);\n" next_state.parsed.state_enum;
+ pr " set_next_state (h, %s);\n" next_state.parsed.state_enum;
pr " debug (h, \"event %%s: %%s -> %%s\",\n";
pr " \"%s\", \"%s\", \"%s\");\n"
(string_of_external_event e)
@@ -2465,7 +2465,7 @@ let generate_lib_states_c () =
pr " }\n";
pr "\n";
pr " set_error (EINVAL, \"external event %%d is invalid in state %%s\",\n";
- pr " ev, nbd_internal_state_short_string (get_state (h)));\n";
+ pr " ev, nbd_internal_state_short_string (get_next_state (h)));\n";
pr " return -1;\n";
pr "\n";
pr " ok:\n";
@@ -2473,7 +2473,7 @@ let generate_lib_states_c () =
pr " blocked = true;\n";
pr "\n";
pr " /* Run a single step. */\n";
- pr " switch (get_state (h))\n";
+ pr " switch (get_next_state (h))\n";
pr " {\n";
List.iter (
fun { parsed = { state_enum } } ->
@@ -2499,7 +2499,7 @@ let generate_lib_states_c () =
pr "{\n";
pr " int r = 0;\n";
pr "\n";
- pr " switch (get_state (h))\n";
+ pr " switch (get_next_state (h))\n";
pr " {\n";
List.iter (
fun ({ parsed = { state_enum; events } }) ->
@@ -2545,7 +2545,7 @@ let generate_lib_states_c () =
pr "const char *\n";
pr "nbd_unlocked_connection_state (struct nbd_handle *h)\n";
pr "{\n";
- pr " switch (get_state (h))\n";
+ pr " switch (get_next_state (h))\n";
pr " {\n";
List.iter (
fun ({ comment; parsed = { display_name; state_enum } }) ->
@@ -2844,7 +2844,7 @@ let generate_lib_api_c () =
pr " /* We can check the state outside the handle lock because the\n";
pr " * the state is atomic.\n";
pr " */\n";
- pr " enum state state = get_state (h);\n";
+ pr " enum state state = get_public_state (h);\n";
let tests =
List.map (
function
@@ -2869,8 +2869,11 @@ let generate_lib_api_c () =
let argnames = List.flatten (List.map name_of_arg args) in
List.iter (pr ", %s") argnames;
pr ");\n";
- if is_locked then
- pr " pthread_mutex_unlock (&h->lock);\n";
+ if is_locked then (
+ pr " if (h->public_state != get_next_state (h))\n";
+ pr " h->public_state = get_next_state (h);\n";
+ pr " pthread_mutex_unlock (&h->lock);\n"
+ );
pr " return ret;\n";
pr "}\n";
pr "\n";
diff --git a/lib/connect.c b/lib/connect.c
index b889f80..4e3141f 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -38,16 +38,16 @@
static int
error_unless_ready (struct nbd_handle *h)
{
- if (nbd_internal_is_state_ready (get_state (h)))
+ if (nbd_internal_is_state_ready (get_next_state (h)))
return 0;
/* Why did it fail? */
- if (nbd_internal_is_state_closed (get_state (h))) {
+ if (nbd_internal_is_state_closed (get_next_state (h))) {
set_error (0, "connection is closed");
return -1;
}
- if (nbd_internal_is_state_dead (get_state (h)))
+ if (nbd_internal_is_state_dead (get_next_state (h)))
/* Don't set the error here, keep the error set when
* the connection died.
*/
@@ -55,14 +55,14 @@ error_unless_ready (struct nbd_handle *h)
/* Should probably never happen. */
set_error (0, "connection in an unexpected state (%s)",
- nbd_internal_state_short_string (get_state (h)));
+ nbd_internal_state_short_string (get_next_state (h)));
return -1;
}
static int
wait_until_connected (struct nbd_handle *h)
{
- while (nbd_internal_is_state_connecting (get_state (h))) {
+ while (nbd_internal_is_state_connecting (get_next_state (h))) {
if (nbd_unlocked_poll (h, -1) == -1)
return -1;
}
diff --git a/lib/disconnect.c b/lib/disconnect.c
index 423edaf..95e9a37 100644
--- a/lib/disconnect.c
+++ b/lib/disconnect.c
@@ -29,14 +29,14 @@
int
nbd_unlocked_shutdown (struct nbd_handle *h)
{
- if (nbd_internal_is_state_ready (get_state (h)) ||
- nbd_internal_is_state_processing (get_state (h))) {
+ if (nbd_internal_is_state_ready (get_next_state (h)) ||
+ nbd_internal_is_state_processing (get_next_state (h))) {
if (nbd_unlocked_aio_disconnect (h, 0) == -1)
return -1;
}
- while (!nbd_internal_is_state_closed (get_state (h)) &&
- !nbd_internal_is_state_dead (get_state (h))) {
+ while (!nbd_internal_is_state_closed (get_next_state (h)) &&
+ !nbd_internal_is_state_dead (get_next_state (h))) {
if (nbd_unlocked_poll (h, -1) == -1)
return -1;
}
diff --git a/lib/handle.c b/lib/handle.c
index cc311ba..4167b3e 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -57,7 +57,7 @@ nbd_create (void)
s = getenv ("LIBNBD_DEBUG");
h->debug = s && strcmp (s, "1") == 0;
- h->state = STATE_START;
+ h->state = h->public_state = STATE_START;
h->pid = -1;
h->export_name = strdup ("");
diff --git a/lib/internal.h b/lib/internal.h
index 5b6152b..ce4bf5b 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -80,7 +80,17 @@ struct nbd_handle {
/* Linked list of close callbacks. */
struct close_callback *close_callbacks;
- _Atomic enum state state; /* State machine. */
+ /* State machine.
+ *
+ * The actual current state is ‘state’. ‘public_state’ is updated
+ * before we release the lock.
+ *
+ * Note don't access these fields directly, use the SET_NEXT_STATE
+ * macro in generator/states* code, or the set_next_state,
+ * get_next_state and get_public_state macros in regular code.
+ */
+ _Atomic enum state public_state;
+ enum state state;
bool structured_replies; /* If we negotiated NBD_OPT_STRUCTURED_REPLY */
@@ -291,8 +301,9 @@ extern const char *nbd_internal_state_short_string (enum state state);
extern enum state_group nbd_internal_state_group (enum state state);
extern enum state_group nbd_internal_state_group_parent (enum state_group group);
-#define set_state(h,next_state) ((h)->state) = (next_state)
-#define get_state(h) ((h)->state)
+#define set_next_state(h,next_state) ((h)->state) = (next_state)
+#define get_next_state(h) ((h)->state)
+#define get_public_state(h) ((h)->public_state)
/* utils.c */
extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp);
diff --git a/lib/is-state.c b/lib/is-state.c
index c941ab4..b2c20df 100644
--- a/lib/is-state.c
+++ b/lib/is-state.c
@@ -98,44 +98,48 @@ nbd_internal_is_state_closed (enum state state)
return state == STATE_CLOSED;
}
-/* NB: is_locked = false, may_set_error = false. */
+/* The nbd_unlocked_aio_is_* calls are the public APIs
+ * for reading the state of the handle.
+ *
+ * They all have: is_locked = false, may_set_error = false.
+ *
+ * They all read the public state, not the real state. Therefore you
+ * SHOULD NOT call these functions from elsewhere in the library (use
+ * nbd_internal_is_* instead).
+ */
+
int
nbd_unlocked_aio_is_created (struct nbd_handle *h)
{
- return nbd_internal_is_state_created (get_state (h));
+ return nbd_internal_is_state_created (get_public_state (h));
}
-/* NB: is_locked = false, may_set_error = false. */
int
nbd_unlocked_aio_is_connecting (struct nbd_handle *h)
{
- return nbd_internal_is_state_connecting (get_state (h));
+ return nbd_internal_is_state_connecting (get_public_state (h));
}
-/* NB: is_locked = false, may_set_error = false. */
int
nbd_unlocked_aio_is_ready (struct nbd_handle *h)
{
- return nbd_internal_is_state_ready (get_state (h));
+ return nbd_internal_is_state_ready (get_public_state (h));
}
-/* NB: is_locked = false, may_set_error = false. */
int
nbd_unlocked_aio_is_processing (struct nbd_handle *h)
{
- return nbd_internal_is_state_processing (get_state (h));
+ return nbd_internal_is_state_processing (get_public_state (h));
}
-/* NB: is_locked = false, may_set_error = false. */
int
nbd_unlocked_aio_is_dead (struct nbd_handle *h)
{
- return nbd_internal_is_state_dead (get_state (h));
+ return nbd_internal_is_state_dead (get_public_state (h));
}
-/* NB: is_locked = false, may_set_error = false. */
int
nbd_unlocked_aio_is_closed (struct nbd_handle *h)
{
- return nbd_internal_is_state_closed (get_state (h));
+ return nbd_internal_is_state_closed (get_public_state (h));
}
diff --git a/lib/rw.c b/lib/rw.c
index b38d95b..ad9c8a0 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -201,7 +201,7 @@ nbd_internal_command_common (struct nbd_handle *h,
* be handled automatically on a future cycle around to READY.
*/
if (h->cmds_to_issue != NULL) {
- assert (nbd_internal_is_state_processing (get_state (h)));
+ assert (nbd_internal_is_state_processing (get_next_state (h)));
prev_cmd = h->cmds_to_issue;
while (prev_cmd->next)
prev_cmd = prev_cmd->next;
@@ -209,7 +209,7 @@ nbd_internal_command_common (struct nbd_handle *h,
}
else {
h->cmds_to_issue = cmd;
- if (nbd_internal_is_state_ready (get_state (h)) &&
+ if (nbd_internal_is_state_ready (get_next_state (h)) &&
nbd_internal_run (h, cmd_issue) == -1)
return -1;
}
--
2.21.0
4 years, 11 months
[libnbd PATCH 0/2] Better handling of failed block_status callback
by Eric Blake
Rather than moving the connection to DEAD, we can just ignore further
contexts to the existing command handle, and fail the overall command
with the same errno as the failed callback.
Eric Blake (2):
states: Track cmd->error as errno, not wire value
api: Recover from block status callback failure
generator/generator | 5 ++-
generator/states-reply-simple.c | 2 +-
generator/states-reply-structured.c | 61 ++++++++++++++-------------
interop/dirty-bitmap.c | 65 +++++++++++++++++++++++------
lib/aio.c | 1 -
lib/internal.h | 2 +-
6 files changed, 91 insertions(+), 45 deletions(-)
--
2.20.1
4 years, 11 months
[PATCH libnbd] generator: Fix race condition when validating h->state.
by Richard W.M. Jones
The code for (eg) nbd_aio_pread starts by checking this outside
the lock:
if (!(nbd_aio_is_ready (h) || nbd_aio_is_processing (h))) {
However there can still be a race condition even though h->state is
atomic:
Thread A Thread B
(in a call that holds h->lock) (calling nbd_aio_pread)
--------------------------------------------------------------------
h->state is processing
checks nbd_aio_is_ready
(it's false)
h->state is moved to READY
checks nbd_aio_is_processing
(it's false)
validation check fails
However the state was valid so the validation check should have
succeeded.
Fixes commit e63a11736930c381a79a8cc2d03844cfff5db3ef.
Thanks: Eric Blake for identifying the race condition above.
---
generator/generator | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/generator/generator b/generator/generator
index ea6f739..8068762 100755
--- a/generator/generator
+++ b/generator/generator
@@ -2820,10 +2820,9 @@ let generate_lib_api_c () =
pr " nbd_internal_reset_error (\"nbd_%s\");\n" name;
pr "\n"
);
+ if is_locked then
+ pr " pthread_mutex_lock (&h->lock);\n";
if permitted_states <> [] then (
- pr " /* We can check the state outside the handle lock because the\n";
- pr " * the state is atomic.\n";
- pr " */\n";
let tests =
List.map (
function
@@ -2842,8 +2841,6 @@ let generate_lib_api_c () =
pr " }\n";
pr "\n"
);
- if is_locked then
- pr " pthread_mutex_lock (&h->lock);\n";
pr " ret = nbd_unlocked_%s (h" name;
let argnames = List.flatten (List.map name_of_arg args) in
List.iter (pr ", %s") argnames;
--
2.21.0
4 years, 11 months
[libnbd PATCH] generator: Add #define witnesses for all API
by Eric Blake
Make it easier for C libraries to consume arbitrary versions of
libnbd, by giving a probe for which functions the current version of
the library exports.
---
I'm fuzzy enough on OCaml that I'll get review for this, although I
like the resulting libnbd.h.
generator/generator | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/generator/generator b/generator/generator
index db7c10f..7d0ea3f 100755
--- a/generator/generator
+++ b/generator/generator
@@ -2712,6 +2712,12 @@ let print_extern name args ret =
print_call name args ret;
pr ";\n"
+let print_extern_and_define name args ret =
+ let name_upper = String.uppercase_ascii name in
+ print_extern name args ret;
+ pr "#define LIBNBD_HAVE_NBD_%s 1\n" name_upper;
+ pr "\n"
+
let generate_include_libnbd_h () =
generate_header CStyle;
@@ -2729,14 +2735,23 @@ let generate_include_libnbd_h () =
List.iter (fun (n, i) -> pr "#define LIBNBD_%-30s %d\n" n i) constants;
pr "\n";
pr "extern struct nbd_handle *nbd_create (void);\n";
+ pr "#define LIBNBD_HAVE_NBD_CREATE 1\n";
+ pr "\n";
pr "extern void nbd_close (struct nbd_handle *h);\n";
+ pr "#define LIBNBD_HAVE_NBD_CLOSE 1\n";
+ pr "\n";
pr "extern const char *nbd_get_error (void);\n";
+ pr "#define LIBNBD_HAVE_NBD_GET_ERROR 1\n";
+ pr "\n";
pr "extern int nbd_get_errno (void);\n";
+ 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 "#define LIBNBD_HAVE_NBD_ADD_CLOSE_CALLBACK 1\n";
pr "\n";
List.iter (
- fun (name, { args; ret }) -> print_extern name args ret
+ fun (name, { args; ret }) -> print_extern_and_define name args ret
) handle_calls;
pr "\n";
pr "#endif /* LIBNBD_H */\n"
@@ -2933,6 +2948,13 @@ in detail. If you want an overview of using the API, or to see
how to call the API from other programming languages, start
with libnbd(3).
+For the sake of conditional compilation across a range of libnbd
+versions, where a client may take advantage of newer API when present
+but gracefully continue to compile even when it is not, all functions
+declared in B<E<lt>libnbd.hE<gt>> have a corresponding witness macro
+with prefix C<LIBNBD_HAVE_>. For example, C<nbd_create> has a
+counterpart macro C<LIBNBD_HAVE_NBD_CREATE> defined to C<1>.
+
=head1 CREATE, GET AND CLOSE HANDLES
struct nbd_handle *nbd;
--
2.20.1
4 years, 11 months
[PATCH libnbd] api: nbd_get_version, nbd_supports_uri and nbd_get_package_name.
by Richard W.M. Jones
nbd_get_version returns the library version as a string.
nbd_supports_uri returns whether or not the library was compiled with
NBD URI support (ie. with libxml2).
nbd_get_package_name is fairly useless as it always returns the string
"libnbd", however it replaces a function that was written for the
Python bindings.
These take a handle parameter but don't need to use it. Changing the
generator to support a whole new class of API calls which don't need a
handle is a massive pain.
---
generator/generator | 51 +++++++++++++++++++++++++++++++++++++++++----
lib/handle.c | 22 +++++++++++++++++++
python/handle.c | 22 -------------------
3 files changed, 69 insertions(+), 26 deletions(-)
diff --git a/generator/generator b/generator/generator
index 54dc6cc..5e052df 100755
--- a/generator/generator
+++ b/generator/generator
@@ -1823,6 +1823,49 @@ can be used for debugging or troubleshooting, but you should not
rely on the state of connections since it may change in future
versions.";
};
+
+ "get_package_name", {
+ default_call with
+ args = []; ret = RConstString; is_locked = false; may_set_error = false;
+ shortdesc = "return the name of the library";
+ longdesc = "\
+Returns the name of the library, always C<\"libnbd\"> unless
+the library was modified with another name at compile time.";
+ };
+
+ "get_version", {
+ default_call with
+ args = []; ret = RConstString; is_locked = false; may_set_error = false;
+ shortdesc = "return a descriptive string for the state of the connection";
+ longdesc = "\
+Return the version of libnbd. This is returned as a string
+in the form C<\"major.minor.release\"> where each of major, minor
+and release is a small positive integer. For example C<\"1.0.3\">.
+
+The major number is C<0> for the early experimental versions of
+libnbd where we still had an unstable API, or C<1> for the versions
+of libnbd with a long-term stable API and ABI.
+
+The minor number is even (C<0>, C<2>, etc) for stable releases,
+and odd (C<1>, C<3>, etc) for development versions. Note that
+new APIs added in a development version remain experimental
+and subject to change in that branch until they appear in a stable
+release.
+
+The release number is increments for each release along a particular
+branch.";
+ };
+
+ "supports_uri", {
+ default_call with
+ args = []; ret = RBool; is_locked = false; may_set_error = false;
+ shortdesc = "return true if libnbd was compiled with support for NBD URIs";
+ longdesc = "\
+Returns true if libnbd was compiled with libxml2 which is required
+to support NBD URIs, or false if not. See C<nbd_connect_uri> and
+C<nbd_aio_connect_uri>.";
+ };
+
]
(* Constants, flags, etc. *)
@@ -3007,7 +3050,7 @@ get_handle (PyObject *obj)
fun name ->
pr "extern PyObject *nbd_internal_py_%s (PyObject *self, PyObject *args);\n"
name;
- ) ([ "create"; "close"; "get_package_name"; "get_package_version";
+ ) ([ "create"; "close";
"alloc_aio_buffer"; "aio_buffer_from_bytearray";
"aio_buffer_to_bytearray" ] @ List.map fst handle_calls);
@@ -3035,7 +3078,7 @@ let generate_python_libnbdmod_c () =
fun name ->
pr " { (char *) \"%s\", nbd_internal_py_%s, METH_VARARGS, NULL },\n"
name name;
- ) ([ "create"; "close"; "get_package_name"; "get_package_version";
+ ) ([ "create"; "close";
"alloc_aio_buffer"; "aio_buffer_from_bytearray";
"aio_buffer_to_bytearray" ] @ List.map fst handle_calls);
pr " { NULL, NULL, 0, NULL }\n";
@@ -3607,8 +3650,8 @@ class NBD (object):
(* For nbdsh. *)
pr "\
-package_name = libnbdmod.get_package_name ()
-__version__ = libnbdmod.get_package_version ()
+package_name = NBD().get_package_name()
+__version__ = NBD().get_version()
if __name__ == \"__main__\":
import argparse
diff --git a/lib/handle.c b/lib/handle.c
index a42b8dc..cc311ba 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -214,3 +214,25 @@ nbd_add_close_callback (struct nbd_handle *h, nbd_close_callback cb, void *data)
pthread_mutex_unlock (&h->lock);
return ret;
}
+
+const char *
+nbd_unlocked_get_package_name (struct nbd_handle *h)
+{
+ return PACKAGE_NAME;
+}
+
+const char *
+nbd_unlocked_get_version (struct nbd_handle *h)
+{
+ return PACKAGE_VERSION;
+}
+
+int
+nbd_unlocked_supports_uri (struct nbd_handle *h)
+{
+#ifdef HAVE_LIBXML2
+ return 1;
+#else
+ return 0;
+#endif
+}
diff --git a/python/handle.c b/python/handle.c
index 7b5e139..7cff41a 100644
--- a/python/handle.c
+++ b/python/handle.c
@@ -89,28 +89,6 @@ free_aio_buffer (PyObject *capsule)
free (buf);
}
-PyObject *
-nbd_internal_py_get_package_name (PyObject *self, PyObject *args)
-{
- static char name[] = PACKAGE_NAME;
-
- if (!PyArg_ParseTuple (args, (char *) ":nbd_internal_py_get_package_name"))
- return NULL;
-
- return PyUnicode_FromStringAndSize (name, strlen (name));
-}
-
-PyObject *
-nbd_internal_py_get_package_version (PyObject *self, PyObject *args)
-{
- static char version[] = PACKAGE_VERSION;
-
- if (!PyArg_ParseTuple (args, (char *) ":nbd_internal_py_get_package_version"))
- return NULL;
-
- return PyUnicode_FromStringAndSize (version, strlen (version));
-}
-
/* Allocate a persistent buffer used for nbd_aio_pread and
* nbd_aio_pwrite.
*/
--
2.21.0
4 years, 11 months