[PATCH libnbd v2] python: Raise a custom exception containing error string and errno.
by Richard W.M. Jones
This kind of fixes the problems in v1. The exception still primarily
lives in the libnbdmod and you could still refer to it using
libnbdmod.Error (but don't do that). However when the exception is
printed it now appears as nbd.Error, and you can catch it also using
the same name.
Other problems:
- There is no "nice" interface to accessing the exception fields.
You have to use ex.args[0] and ex.args[1].
- The errno is represented as a number, which means it will be hard
to write portable Python code depending on the errno.
Rich.
4 years, 10 months
[libnbd PATCH] generator: Add support for namespace constants
by Martin Kletzander
This just defines the namespace, its contexts and related constants and the only
supported one is currently base:allocation. The names of the constants are not
very future-proof, but shorter than LIBNBD_META_NS_CONTEXT_BASE_ALLOCATION or
similar.
Currently the output looks like this:
/* "base" namespace */
/* "base" namespace contexts */
/* "base:allocation" context related constants */
Separated by two empty lines from unrelated parts of the header file above and
below.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
Notes:
Everything is up for a debate:
- the names might need to be different so that they do not clash with other
constants in the scope later on,
- the fact that "base" and "base:allocation" are even defined, which might be
useless, since listing contexts of a namespace is not exposed,
- whether this should live in a separate (still included in libnbd.h) file,
- and more...
generator/generator | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/generator/generator b/generator/generator
index 684fb44ac918..9d8d257fd2f1 100755
--- a/generator/generator
+++ b/generator/generator
@@ -2066,6 +2066,13 @@ let constants = [
"READ_ERROR", 3;
]
+let metadata_namespaces = [
+ "base", [ "allocation", [
+ "STATE_HOLE", 1 lsl 0;
+ "STATE_ZERO", 1 lsl 1;
+ ] ];
+]
+
(*----------------------------------------------------------------------*)
(* Helper functions. *)
@@ -2908,6 +2915,25 @@ let print_extern_and_define name args ret =
pr "#define LIBNBD_HAVE_NBD_%s 1\n" name_upper;
pr "\n"
+let print_ns_ctxt ns ns_upper ctxt consts =
+ let ctxt_upper = String.uppercase_ascii ctxt in
+ pr "#define LIBNBD_CONTEXT_%s_%s \"%s:%s\"\n"
+ ns_upper ctxt_upper ns ctxt;
+ pr "\n";
+ pr "/* \"%s:%s\" context related constants */\n" ns ctxt;
+ List.iter (fun (n, i) -> pr "#define LIBNBD_%-30s %d\n" n i) consts
+
+let print_ns ns ctxts =
+ let ns_upper = String.uppercase_ascii ns in
+ pr "/* \"%s\" namespace */\n" ns;
+ pr "#define LIBNBD_NAMESPACE_%s \"%s\"\n" ns_upper ns;
+ pr "\n";
+ pr "/* \"%s\" namespace contexts */\n" ns;
+ List.iter (
+ fun (ctxt, consts) -> print_ns_ctxt ns ns_upper ctxt consts
+ ) ctxts;
+ pr "\n"
+
let generate_include_libnbd_h () =
generate_header CStyle;
@@ -2944,6 +2970,10 @@ let generate_include_libnbd_h () =
fun (name, { args; ret }) -> print_extern_and_define name args ret
) handle_calls;
pr "\n";
+ List.iter (
+ fun (ns, ctxts) -> print_ns ns ctxts
+ ) metadata_namespaces;
+ pr "\n";
pr "#endif /* LIBNBD_H */\n"
let generate_lib_unlocked_h () =
--
2.22.0
4 years, 10 months
[PATCH libnbd] python: Raise a custom exception containing error string and errno.
by Richard W.M. Jones
I spent a good few hours this morning trying to make this work and
came up with the following patch. It's not quite right though.
The exception I've created exists in the libnbdmod module (ie. the
underlying C module that we use for the Python bindings). Ideally
we'd define and throw an exception from the normal nbd module, but I
couldn't work out how to do that.
Probably someone who knows a bit more about Python might find this
easy to do.
Rich.
4 years, 10 months
[nbdkit PATCH] nbd: Update for libnbd 0.1.5
by Eric Blake
libnbd 0.1.5 was released today, but is not yet packaged for
Fedora. I'd prefer to bump the minimum version in the pkg-config check
in configure.ac, as such, I won't be pushing this patch as-is, but
waiting until Fedora does have the newer build. But anyone playing
with a self-built libnbd library in the meantime needs this patch to
deal with the ABI change.
---
plugins/nbd/nbd.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index 68ea3dd0..818f94d6 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -783,9 +783,18 @@ nbdplug_flush (void *handle, uint32_t flags)
return nbdplug_reply (h, s);
}
+/* XXX: Hack this signature to compile to 0.1.3 (minimum configure
+ * version) as well as 0.1.5 (API change, with witness of the addition
+ * of pread_structured at the same release). Once configure requires a
+ * higher minimum version, the #if hack can be dropped.
+ */
static int
nbdplug_extent (void *opaque, const char *metacontext, uint64_t offset,
- uint32_t *entries, size_t nr_entries)
+ uint32_t *entries, size_t nr_entries
+#if LIBNBD_HAVE_NBD_PREAD_STRUCTURED
+ , int *error
+#endif
+ )
{
struct nbdkit_extents *extents = opaque;
--
2.20.1
4 years, 10 months
[libnbd PATCH] opt-go: Better decoding of known errors
by Eric Blake
I'm easily able to provoke NBD_REP_ERR_TLS_REQD (use nbd_set_tls(0) to
talk to a server that requires encryption) and NBD_REP_ERR_UNKNOWN
(forget to use nbd_set_export_name for qemu-nbd); it's nice to display
a useful error for these rather than "unknown reply from NBD_OPT_GO:
0x80000005" or similar. Other errors are less common, but as long as
we're decoding things, it doesn't hurt to decode everything in the
protocol.
---
I'm pushing this one now (I mentioned it on IRC earlier today).
generator/states-newstyle-opt-go.c | 32 +++++++++++++++++++++++++++---
lib/nbd-protocol.h | 22 +++++++++++---------
2 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c
index 91a85ef..e245c75 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -147,9 +147,35 @@
SET_NEXT_STATE (%^OPT_EXPORT_NAME.START);
return 0;
default:
- if (handle_reply_error (h) == 0)
- set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%" PRIx32,
- reply);
+ if (handle_reply_error (h) == 0) {
+ /* Decode expected known errors into a nicer string */
+ switch (reply) {
+ case NBD_REP_ERR_POLICY:
+ case NBD_REP_ERR_PLATFORM:
+ set_error (0, "handshake: server policy prevents NBD_OPT_GO");
+ break;
+ case NBD_REP_ERR_INVALID:
+ case NBD_REP_ERR_TOO_BIG:
+ set_error (EINVAL, "handshake: server rejected NBD_OPT_GO as invalid");
+ break;
+ case NBD_REP_ERR_TLS_REQD:
+ set_error (ENOTSUP, "handshake: server requires TLS encryption first");
+ break;
+ case NBD_REP_ERR_UNKNOWN:
+ set_error (ENOENT, "handshake: server has no export named '%s'",
+ h->export_name);
+ break;
+ case NBD_REP_ERR_SHUTDOWN:
+ set_error (ESHUTDOWN, "handshake: server is shutting down");
+ break;
+ case NBD_REP_ERR_BLOCK_SIZE_REQD:
+ set_error (EINVAL, "handshake: server requires specific block sizes");
+ break;
+ default:
+ set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%" PRIx32,
+ reply);
+ }
+ }
SET_NEXT_STATE (%.DEAD);
return -1;
}
diff --git a/lib/nbd-protocol.h b/lib/nbd-protocol.h
index 405af3e..3e3fb4e 100644
--- a/lib/nbd-protocol.h
+++ b/lib/nbd-protocol.h
@@ -123,15 +123,19 @@ struct nbd_fixed_new_option_reply {
#define NBD_REP_ERR(val) (0x80000000 | (val))
#define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000))
-#define NBD_REP_ACK 1
-#define NBD_REP_SERVER 2
-#define NBD_REP_INFO 3
-#define NBD_REP_META_CONTEXT 4
-#define NBD_REP_ERR_UNSUP NBD_REP_ERR (1)
-#define NBD_REP_ERR_POLICY NBD_REP_ERR (2)
-#define NBD_REP_ERR_INVALID NBD_REP_ERR (3)
-#define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4)
-#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5)
+#define NBD_REP_ACK 1
+#define NBD_REP_SERVER 2
+#define NBD_REP_INFO 3
+#define NBD_REP_META_CONTEXT 4
+#define NBD_REP_ERR_UNSUP NBD_REP_ERR (1)
+#define NBD_REP_ERR_POLICY NBD_REP_ERR (2)
+#define NBD_REP_ERR_INVALID NBD_REP_ERR (3)
+#define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4)
+#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5)
+#define NBD_REP_ERR_UNKNOWN NBD_REP_ERR (6)
+#define NBD_REP_ERR_SHUTDOWN NBD_REP_ERR (7)
+#define NBD_REP_ERR_BLOCK_SIZE_REQD NBD_REP_ERR (8)
+#define NBD_REP_ERR_TOO_BIG NBD_REP_ERR (9)
#define NBD_INFO_EXPORT 0
--
2.20.1
4 years, 10 months
[libnbd PATCH] maint: Use $(NULL) for all Makefile.am macro lists
by Eric Blake
This borrows from a trick in libvirt - by defining $(NULL) to expand
to an empty string, we can more consistently write multi-line macros
where all useful lines terminate with \, making it easier to
add/remove lines without worrying about whether \ needs to be touched
up on neighboring lines.
---
Looks big, but is fairly mechanical. I'm also doing a similar patch
for nbdkit, where it would have prevented the typo fixed in commit 268e7816.
Makefile.am | 6 ++--
common-rules.mk | 3 ++
docs/Makefile.am | 9 ++++--
examples/Makefile.am | 51 ++++++++++++++++++++----------
generator/Makefile.am | 6 ++--
include/Makefile.am | 3 +-
interop/Makefile.am | 24 +++++++++-----
lib/Makefile.am | 21 ++++++++----
ocaml/Makefile.am | 15 ++++++---
ocaml/examples/Makefile.am | 3 +-
sh/Makefile.am | 3 +-
tests/Makefile.am | 65 ++++++++++++++++++++++++--------------
12 files changed, 139 insertions(+), 70 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index d758378..6a0f4bd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -22,7 +22,8 @@ ACLOCAL_AMFLAGS = -I m4
EXTRA_DIST = \
.dir-locals.el \
.gitignore \
- html/pod.css
+ html/pod.css \
+ $(NULL)
SUBDIRS = \
generator \
@@ -36,7 +37,8 @@ SUBDIRS = \
sh \
ocaml \
ocaml/examples \
- interop
+ interop \
+ $(NULL)
noinst_SCRIPTS = run
diff --git a/common-rules.mk b/common-rules.mk
index 498d221..eb8c92e 100644
--- a/common-rules.mk
+++ b/common-rules.mk
@@ -18,6 +18,9 @@
# common-rules.mk is included in every Makefile.am.
# subdir-rules.mk is included only in subdirectories.
+# Convenient list terminator
+NULL =
+
CLEANFILES = *~
$(generator_built): $(top_builddir)/generator/stamp-generator
diff --git a/docs/Makefile.am b/docs/Makefile.am
index f002541..234fff1 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -18,17 +18,20 @@
include $(top_srcdir)/subdir-rules.mk
generator_built = \
- libnbd-api.pod
+ libnbd-api.pod \
+ $(NULL)
EXTRA_DIST = \
$(generator_built) \
- libnbd.pod
+ libnbd.pod \
+ $(NULL)
if HAVE_POD
man_MANS = \
libnbd.3 \
- libnbd-api.3
+ libnbd-api.3 \
+ $(NULL)
CLEANFILES += $(man_MANS)
libnbd.3: libnbd.pod
diff --git a/examples/Makefile.am b/examples/Makefile.am
index b933873..f0d03f1 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -23,42 +23,59 @@ noinst_PROGRAMS = \
batched-read-write \
simple-fetch-first-sector \
simple-reads-and-writes \
- threaded-reads-and-writes
+ threaded-reads-and-writes \
+ $(NULL)
simple_fetch_first_sector_SOURCES = \
- simple-fetch-first-sector.c
+ simple-fetch-first-sector.c \
+ $(NULL)
simple_fetch_first_sector_CPPFLAGS = \
- -I$(top_srcdir)/include
+ -I$(top_srcdir)/include \
+ $(NULL)
simple_fetch_first_sector_CFLAGS = \
- $(WARNINGS_CFLAGS)
+ $(WARNINGS_CFLAGS) \
+ $(NULL)
simple_fetch_first_sector_LDADD = \
- $(top_builddir)/lib/libnbd.la
+ $(top_builddir)/lib/libnbd.la \
+ $(NULL)
simple_reads_and_writes_SOURCES = \
- simple-reads-and-writes.c
+ simple-reads-and-writes.c \
+ $(NULL)
simple_reads_and_writes_CPPFLAGS = \
- -I$(top_srcdir)/include
+ -I$(top_srcdir)/include \
+ $(NULL)
simple_reads_and_writes_CFLAGS = \
- $(WARNINGS_CFLAGS)
+ $(WARNINGS_CFLAGS) \
+ $(NULL)
simple_reads_and_writes_LDADD = \
- $(top_builddir)/lib/libnbd.la
+ $(top_builddir)/lib/libnbd.la \
+ $(NULL)
threaded_reads_and_writes_SOURCES = \
- threaded-reads-and-writes.c
+ threaded-reads-and-writes.c \
+ $(NULL)
threaded_reads_and_writes_CPPFLAGS = \
- -I$(top_srcdir)/include
+ -I$(top_srcdir)/include \
+ $(NULL)
threaded_reads_and_writes_CFLAGS = \
$(WARNINGS_CFLAGS) \
- $(PTHREAD_CFLAGS)
+ $(PTHREAD_CFLAGS) \
+ $(NULL)
threaded_reads_and_writes_LDADD = \
$(top_builddir)/lib/libnbd.la \
- $(PTHREAD_LIBS)
+ $(PTHREAD_LIBS) \
+ $(NULL)
batched_read_write_SOURCES = \
- batched-read-write.c
+ batched-read-write.c \
+ $(NULL)
batched_read_write_CPPFLAGS = \
- -I$(top_srcdir)/include
+ -I$(top_srcdir)/include \
+ $(NULL)
batched_read_write_CFLAGS = \
- $(WARNINGS_CFLAGS)
+ $(WARNINGS_CFLAGS) \
+ $(NULL)
batched_read_write_LDADD = \
- $(top_builddir)/lib/libnbd.la
+ $(top_builddir)/lib/libnbd.la \
+ $(NULL)
diff --git a/generator/Makefile.am b/generator/Makefile.am
index 4f4e685..c20184a 100644
--- a/generator/Makefile.am
+++ b/generator/Makefile.am
@@ -35,11 +35,13 @@ states_code = \
states-oldstyle.c \
states-reply-simple.c \
states-reply-structured.c \
- states-reply.c
+ states-reply.c \
+ $(NULL)
EXTRA_DIST = \
generator \
- $(states_code)
+ $(states_code) \
+ $(NULL)
if HAVE_OCAML_TOPLEVEL
diff --git a/include/Makefile.am b/include/Makefile.am
index cbeb560..e786447 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -24,4 +24,5 @@ EXTRA_DIST = $(generator_built)
BUILT_SOURCES = $(generator_built)
include_HEADERS = \
- libnbd.h
+ libnbd.h \
+ $(NULL)
diff --git a/interop/Makefile.am b/interop/Makefile.am
index cf36e34..1d2d187 100644
--- a/interop/Makefile.am
+++ b/interop/Makefile.am
@@ -19,7 +19,8 @@ include $(top_srcdir)/subdir-rules.mk
EXTRA_DIST = \
dirty-bitmap.sh \
- structured-read.sh
+ structured-read.sh \
+ $(NULL)
TESTS_ENVIRONMENT = LIBNBD_DEBUG=1
LOG_COMPILER = $(top_builddir)/run
@@ -49,11 +50,13 @@ check_PROGRAMS += \
interop-qemu-nbd-tls-certs \
interop-qemu-nbd-tls-psk \
dirty-bitmap \
- structured-read
+ structured-read \
+ $(NULL)
TESTS += \
interop-qemu-nbd \
dirty-bitmap.sh \
- structured-read.sh
+ structured-read.sh \
+ $(NULL)
# tls tests assume the pre-existence of files created in ../tests/Makefile.am,
# so we can only run them under the same conditions used there
@@ -62,11 +65,13 @@ if HAVE_NBDKIT
if HAVE_GNUTLS
if HAVE_CERTTOOL
TESTS += \
- interop-qemu-nbd-tls-certs
+ interop-qemu-nbd-tls-certs \
+ $(NULL)
endif
if HAVE_PSKTOOL
TESTS += \
- interop-qemu-nbd-tls-psk
+ interop-qemu-nbd-tls-psk \
+ $(NULL)
endif
endif
endif
@@ -77,7 +82,8 @@ interop_qemu_nbd_CPPFLAGS = \
-DSERVE_OVER_TCP=1 \
-DSERVER=\"$(QEMU_NBD)\" \
-DSERVER_PARAMS='"-f", "raw", "-x", "/", "-p", port_str, tmpfile' \
- -DEXPORT_NAME='"/"'
+ -DEXPORT_NAME='"/"' \
+ $(NULL)
interop_qemu_nbd_CFLAGS = $(WARNINGS_CFLAGS)
interop_qemu_nbd_LDADD = $(top_builddir)/lib/libnbd.la
@@ -89,7 +95,8 @@ interop_qemu_nbd_tls_certs_CPPFLAGS = \
-DSERVER=\"$(QEMU_NBD)\" \
-DSERVER_PARAMS='"--object", "tls-creds-x509,id=tls0,endpoint=server,dir=$(abs_top_builddir)/tests/pki", "--tls-creds", "tls0", "-f", "raw", "-x", "/", "-p", port_str, tmpfile' \
-DEXPORT_NAME='"/"' \
- -DCERTS=1
+ -DCERTS=1 \
+ $(NULL)
interop_qemu_nbd_tls_certs_CFLAGS = $(WARNINGS_CFLAGS)
interop_qemu_nbd_tls_certs_LDADD = $(top_builddir)/lib/libnbd.la
@@ -101,7 +108,8 @@ interop_qemu_nbd_tls_psk_CPPFLAGS = \
-DSERVER=\"$(QEMU_NBD)\" \
-DSERVER_PARAMS='"--object", "tls-creds-psk,id=tls0,endpoint=server,dir=$(abs_top_builddir)/tests", "--tls-creds", "tls0", "-f", "raw", "-x", "/", "-p", port_str, tmpfile' \
-DEXPORT_NAME='"/"' \
- -DPSK=1
+ -DPSK=1 \
+ $(NULL)
interop_qemu_nbd_tls_psk_CFLAGS = $(WARNINGS_CFLAGS)
interop_qemu_nbd_tls_psk_LDADD = $(top_builddir)/lib/libnbd.la
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 72d2d0b..7939375 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -21,11 +21,13 @@ generator_built = \
api.c \
states.c \
states.h \
- unlocked.h
+ unlocked.h \
+ $(NULL)
EXTRA_DIST = \
$(generator_built) \
- libnbd.syms
+ libnbd.syms \
+ $(NULL)
lib_LTLIBRARIES = libnbd.la
@@ -51,22 +53,27 @@ libnbd_la_SOURCES = \
states.c \
states.h \
unlocked.h \
- utils.c
+ utils.c \
+ $(NULL)
libnbd_la_CPPFLAGS = \
-I$(top_srcdir)/include \
- -Dsysconfdir=\"$(sysconfdir)\"
+ -Dsysconfdir=\"$(sysconfdir)\" \
+ $(NULL)
libnbd_la_CFLAGS = \
$(WARNINGS_CFLAGS) \
$(PTHREAD_CFLAGS) \
$(GNUTLS_CFLAGS) \
- $(LIBXML2_CFLAGS)
+ $(LIBXML2_CFLAGS) \
+ $(NULL)
libnbd_la_LIBADD = \
$(GNUTLS_LIBS) \
- $(LIBXML2_LIBS)
+ $(LIBXML2_LIBS) \
+ $(NULL)
libnbd_la_LDFLAGS = \
$(PTHREAD_LIBS) \
-Wl,--version-script=$(srcdir)/libnbd.syms \
- -version-info 0:0:0
+ -version-info 0:0:0 \
+ $(NULL)
# pkg-config / pkgconf
diff --git a/ocaml/Makefile.am b/ocaml/Makefile.am
index 0d876bb..bae9e58 100644
--- a/ocaml/Makefile.am
+++ b/ocaml/Makefile.am
@@ -20,10 +20,12 @@ include $(top_srcdir)/subdir-rules.mk
generator_built = \
NBD.mli \
NBD.ml \
- nbd-c.c
+ nbd-c.c \
+ $(NULL)
EXTRA_DIST = \
- $(generator_built)
+ $(generator_built) \
+ $(NULL)
CLEANFILES += *.annot *.cmi *.cmo *.cmx *.o *.a *.so
@@ -71,18 +73,21 @@ endif
libnbdocaml_a_CPPFLAGS = \
-I$(top_builddir) -I$(OCAMLLIB) -I$(top_srcdir)/ocaml \
-I$(top_srcdir)/include \
- -DCAML_NAME_SPACE
+ -DCAML_NAME_SPACE \
+ $(NULL)
libnbdocaml_a_CFLAGS = \
$(WARNINGS_CFLAGS) \
- -fPIC
+ -fPIC \
+ $(NULL)
libnbdocaml_a_SOURCES = \
nbd-c.c \
nbd-c.h \
buffer.c \
handle.c \
- helpers.c
+ helpers.c \
+ $(NULL)
%.bc: %.cmo mlnbd.cma
$(top_builddir)/libtool -dlopen $(top_builddir)/lib/.libs/libnbd.la --mode=execute \
diff --git a/ocaml/examples/Makefile.am b/ocaml/examples/Makefile.am
index 6488f62..7867e14 100644
--- a/ocaml/examples/Makefile.am
+++ b/ocaml/examples/Makefile.am
@@ -19,7 +19,8 @@ include $(top_srcdir)/subdir-rules.mk
EXTRA_DIST = \
LICENSE-FOR-EXAMPLES \
- extents.ml
+ extents.ml \
+ $(NULL)
CLEANFILES += *.annot *.cmi *.cmo *.cmx *.o *.a *.so *.bc *.opt
diff --git a/sh/Makefile.am b/sh/Makefile.am
index 3a30ccc..505f751 100644
--- a/sh/Makefile.am
+++ b/sh/Makefile.am
@@ -20,7 +20,8 @@ include $(top_srcdir)/subdir-rules.mk
EXTRA_DIST = \
nbdsh.pod \
examples/LICENSE-FOR-EXAMPLES \
- examples/hexdump.sh
+ examples/hexdump.sh \
+ $(NULL)
if HAVE_PYTHON
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2a0acf0..149f890 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -23,7 +23,8 @@ CLEANFILES += \
connect-unix.sock \
connect-uri-tcp.pid \
connect-uri-unix.pid \
- connect-uri-unix.sock
+ connect-uri-unix.sock \
+ $(NULL)
EXTRA_DIST = \
aio-parallel.sh \
@@ -35,7 +36,8 @@ EXTRA_DIST = \
make-pki.sh \
meta-base-allocation.sh \
synch-parallel.sh \
- synch-parallel-tls.sh
+ synch-parallel-tls.sh \
+ $(NULL)
if HAVE_NBDKIT
@@ -66,9 +68,10 @@ check_PROGRAMS = \
aio-parallel \
aio-parallel-load \
synch-parallel \
- meta-base-allocation
-# can-cache-flag
-# can-not-cache-flag
+ meta-base-allocation \
+# can-cache-flag \
+# can-not-cache-flag \
+ $(NULL)
# Make sure that $srcdir is available to tests.
# Enable debug in all tests.
@@ -101,9 +104,10 @@ TESTS = \
aio-parallel.sh \
aio-parallel-load.sh \
synch-parallel.sh \
- meta-base-allocation
-# can-cache-flag
-# can-not-cache-flag
+ meta-base-allocation \
+# can-cache-flag \
+# can-not-cache-flag \
+ $(NULL)
errors_SOURCES = errors.c
errors_CPPFLAGS = -I$(top_srcdir)/include
@@ -132,55 +136,64 @@ read_write_flag_LDADD = $(top_builddir)/lib/libnbd.la
can_flush_flag_SOURCES = eflags.c
can_flush_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=can_flush
+ -I$(top_srcdir)/include -Dflag=can_flush \
+ $(NULL)
can_flush_flag_CFLAGS = $(WARNINGS_CFLAGS)
can_flush_flag_LDADD = $(top_builddir)/lib/libnbd.la
can_not_flush_flag_SOURCES = eflags.c
can_not_flush_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=can_flush -Dvalue=false
+ -I$(top_srcdir)/include -Dflag=can_flush -Dvalue=false \
+ $(NULL)
can_not_flush_flag_CFLAGS = $(WARNINGS_CFLAGS)
can_not_flush_flag_LDADD = $(top_builddir)/lib/libnbd.la
can_fua_flag_SOURCES = eflags.c
can_fua_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=can_fua -Dvalue=native
+ -I$(top_srcdir)/include -Dflag=can_fua -Dvalue=native \
+ $(NULL)
can_fua_flag_CFLAGS = $(WARNINGS_CFLAGS)
can_fua_flag_LDADD = $(top_builddir)/lib/libnbd.la
can_not_fua_flag_SOURCES = eflags.c
can_not_fua_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=can_fua -Dvalue=none
+ -I$(top_srcdir)/include -Dflag=can_fua -Dvalue=none \
+ $(NULL)
can_not_fua_flag_CFLAGS = $(WARNINGS_CFLAGS)
can_not_fua_flag_LDADD = $(top_builddir)/lib/libnbd.la
is_rotational_flag_SOURCES = eflags.c
is_rotational_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=is_rotational
+ -I$(top_srcdir)/include -Dflag=is_rotational \
+ $(NULL)
is_rotational_flag_CFLAGS = $(WARNINGS_CFLAGS)
is_rotational_flag_LDADD = $(top_builddir)/lib/libnbd.la
is_not_rotational_flag_SOURCES = eflags.c
is_not_rotational_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=is_rotational -Dvalue=false
+ -I$(top_srcdir)/include -Dflag=is_rotational -Dvalue=false \
+ $(NULL)
is_not_rotational_flag_CFLAGS = $(WARNINGS_CFLAGS)
is_not_rotational_flag_LDADD = $(top_builddir)/lib/libnbd.la
can_trim_flag_SOURCES = eflags.c
can_trim_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=can_trim
+ -I$(top_srcdir)/include -Dflag=can_trim \
+ $(NULL)
can_trim_flag_CFLAGS = $(WARNINGS_CFLAGS)
can_trim_flag_LDADD = $(top_builddir)/lib/libnbd.la
can_not_trim_flag_SOURCES = eflags.c
can_not_trim_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=can_trim -Dvalue=false
+ -I$(top_srcdir)/include -Dflag=can_trim -Dvalue=false \
+ $(NULL)
can_not_trim_flag_CFLAGS = $(WARNINGS_CFLAGS)
can_not_trim_flag_LDADD = $(top_builddir)/lib/libnbd.la
can_zero_flag_SOURCES = eflags.c
can_zero_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=can_zero
+ -I$(top_srcdir)/include -Dflag=can_zero \
+ $(NULL)
can_zero_flag_CFLAGS = $(WARNINGS_CFLAGS)
can_zero_flag_LDADD = $(top_builddir)/lib/libnbd.la
# Note: can't test not-zero case because nbdkit emulates zero if the
@@ -188,26 +201,30 @@ can_zero_flag_LDADD = $(top_builddir)/lib/libnbd.la
can_multi_conn_flag_SOURCES = eflags.c
can_multi_conn_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=can_multi_conn
+ -I$(top_srcdir)/include -Dflag=can_multi_conn \
+ $(NULL)
can_multi_conn_flag_CFLAGS = $(WARNINGS_CFLAGS)
can_multi_conn_flag_LDADD = $(top_builddir)/lib/libnbd.la
can_not_multi_conn_flag_SOURCES = eflags.c
can_not_multi_conn_flag_CPPFLAGS = \
- -I$(top_srcdir)/include -Dflag=can_multi_conn -Dvalue=false
+ -I$(top_srcdir)/include -Dflag=can_multi_conn -Dvalue=false \
+ $(NULL)
can_not_multi_conn_flag_CFLAGS = $(WARNINGS_CFLAGS)
can_not_multi_conn_flag_LDADD = $(top_builddir)/lib/libnbd.la
# Waiting for nbdkit to add support for can_cache.
#can_cache_flag_SOURCES = eflags.c
#can_cache_flag_CPPFLAGS = \
-# -I$(top_srcdir)/include -Dflag=can_cache -Dvalue=native
+# -I$(top_srcdir)/include -Dflag=can_cache -Dvalue=native \
+# $(NULL)
#can_cache_flag_CFLAGS = $(WARNINGS_CFLAGS)
#can_cache_flag_LDADD = $(top_builddir)/lib/libnbd.la
#
#can_not_cache_flag_SOURCES = eflags.c
#can_not_cache_flag_CPPFLAGS = \
-# -I$(top_srcdir)/include -Dflag=can_cache -Dvalue=none
+# -I$(top_srcdir)/include -Dflag=can_cache -Dvalue=none \
+# $(NULL)
#can_not_cache_flag_CFLAGS = $(WARNINGS_CFLAGS)
#can_not_cache_flag_LDADD = $(top_builddir)/lib/libnbd.la
@@ -283,12 +300,14 @@ check_PROGRAMS += \
connect-tls-psk \
aio-parallel-tls \
aio-parallel-load-tls \
- synch-parallel-tls
+ synch-parallel-tls \
+ $(NULL)
TESTS += \
connect-tls-psk \
aio-parallel-tls.sh \
aio-parallel-load-tls.sh \
- synch-parallel-tls.sh
+ synch-parallel-tls.sh \
+ $(NULL)
check_DATA += keys.psk
connect_tls_psk_SOURCES = connect-tls.c
--
2.20.1
4 years, 10 months
[libnbd PATCH] python: Fix bindings for Path parameters
by Eric Blake
Our use of PyUnicode_FSConverter was wrong - the result is a PyObject*
rather than a char* (where dereferencing then calling free() on that
pointer as char* has catastrophic effects).
With this patch, I was able to set up a qemu-nbd encrypted server over
a Unix socket (using a pending patch on the qemu list), coupled with
a python connection to that socket:
$ ~/qemu/qemu-nbd -r -k /tmp/nbdsock --object \
tls-creds-psk,id=tls0,endpoint=server,dir=/home/eblake/libnbd/tests \
--tls-creds tls0 -f raw -x / tmpfile
$ ./run nbdsh
nbd> h.set_tls_psk_file('tests/keys.psk')
nbd> h.set_tls(2)
nbd> h.set_export_name('/')
nbd> h.connect_unix('/tmp/nbdsock')
instead of getting a segfault.
---
generator/generator | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/generator/generator b/generator/generator
index c29460c..fa12232 100755
--- a/generator/generator
+++ b/generator/generator
@@ -3608,7 +3608,9 @@ let print_python_binding name { args; ret } =
| Mutable arg ->
pr " PyObject *%s;\n" (List.hd (name_of_arg arg))
| Opaque _ -> ()
- | Path n -> pr " char *%s = NULL;\n" n
+ | Path n ->
+ pr " PyObject *py_%s = NULL;\n" n;
+ pr " char *%s = NULL;\n" n
| SockAddrAndLen (n, _) ->
pr " /* XXX Complicated - Python uses a tuple of different\n";
pr " * lengths for the different socket types.\n";
@@ -3699,7 +3701,7 @@ let print_python_binding name { args; ret } =
| Int64 n -> pr ", &%s" n
| Mutable arg -> pr ", &%s" (List.hd (name_of_arg arg))
| Opaque n -> pr ", &%s_data->data" (find_callback n)
- | Path n -> pr ", PyUnicode_FSConverter, &%s" n
+ | Path n -> pr ", PyUnicode_FSConverter, &py_%s" n
| SockAddrAndLen (n, _) -> pr ", &%s" n
| String n -> pr ", &%s" n
| StringList n -> pr ", &py_%s" n
@@ -3750,7 +3752,9 @@ let print_python_binding name { args; ret } =
| Mutable _ ->
pr " abort (); /* Mutable for normal Python parameters not impl */\n"
| Opaque n -> ()
- | Path _ -> ()
+ | Path n ->
+ pr " %s = PyBytes_AS_STRING (py_%s);\n" n n;
+ pr " assert (%s != NULL);\n" n
| SockAddrAndLen _ ->
pr " abort (); /* XXX SockAddrAndLen not implemented */\n";
| String _ -> ()
@@ -3860,7 +3864,8 @@ let print_python_binding name { args; ret } =
| Int64 _ -> ()
| Mutable _ -> ()
| Opaque _ -> ()
- | Path n -> pr " free (%s);\n" n
+ | Path n ->
+ pr " Py_XDECREF (py_%s);\n" n
| SockAddrAndLen _ -> ()
| String n -> ()
| StringList n -> pr " nbd_internal_py_free_string_list (%s);\n" n
--
2.20.1
4 years, 10 months
[libnbd PATCH] block-status: Make callback usage consistent with pread_structured
by Eric Blake
Now that we have a way to pass Mutable(Int "error") to callbacks,
that's a much nicer way for language bindings to use than relying on
the value of errno after returning -1. Update the semantics for
block status to match what pread_structured does, getting errno out
of the equation, and changing things to call the callback for a
second context even after an earlier callback failure sets the error.
This is an API/ABI break to existing clients, but that's okay because
we have not yet declared stable API. (If you need to compile against
libnbd 0.4 and 0.5 simultaneously, you can use
#ifdef LIBNBD_HAVE_NBD_PREAD_STRUCTURED
as a witness for the new signature rather than the old).
---
This applies on top of my changes to let pread_structured use Mutable.
I'll go ahead and push it, so we can cut a release soon.
generator/generator | 17 +++++----
generator/states-reply-structured.c | 56 ++++++++++++++---------------
interop/dirty-bitmap.c | 15 ++++----
lib/internal.h | 2 +-
ocaml/examples/extents.ml | 3 +-
python/t/460-block-status.py | 3 +-
tests/meta-base-allocation.c | 9 ++---
7 files changed, 54 insertions(+), 51 deletions(-)
diff --git a/generator/generator b/generator/generator
index ec09836..c29460c 100755
--- a/generator/generator
+++ b/generator/generator
@@ -1527,7 +1527,8 @@ punching a hole.";
Callback ("extent", [Opaque "data"; String "metacontext";
UInt64 "offset";
ArrayAndLen (UInt32 "entries",
- "nr_entries")]);
+ "nr_entries");
+ Mutable (Int "error")]);
Flags "flags" ];
ret = RErr;
permitted_states = [ Connected ];
@@ -1548,10 +1549,11 @@ supported by the server (see C<nbd_can_meta_context>) this call
returns information about extents by calling back to the
C<extent> function. The callback cannot call C<nbd_*> APIs on the
same handle since it holds the handle lock and will
-cause a deadlock. If the callback returns C<-1>, any remaining
-contexts will be ignored and the overall block status command
-will fail with the same value of C<errno> left after the
-failing callback.
+cause a deadlock. If the callback returns C<-1>, and no earlier
+error has been detected, then the overall block status command
+will fail with any non-zero value stored into the callback's
+C<error> parameter (with a default of C<EPROTO>); but any further
+contexts will still invoke the callback.
The C<extent> function is called once per type of metadata available,
with the C<data> passed to this function. The C<metacontext>
@@ -1564,6 +1566,8 @@ NBD protocol document in the section about
C<NBD_REPLY_TYPE_BLOCK_STATUS> describes the meaning of this array;
for contexts known to libnbd, B<E<lt>libnbd.hE<gt>> contains constants
beginning with C<LIBNBD_STATE_> that may help decipher the values.
+On entry to the callback, the C<error> parameter contains the errno
+value of any previously detected error.
It is possible for the extent function to be called
more times than you expect (if the server is buggy),
@@ -1812,7 +1816,8 @@ in C<nbd_zero>.";
CallbackPersist ("extent", [Opaque "data"; String "metacontext";
UInt64 "offset";
ArrayAndLen (UInt32 "entries",
- "nr_entries")]);
+ "nr_entries");
+ Mutable (Int "error")]);
Flags "flags" ];
ret = RInt64;
permitted_states = [ Connected ];
diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c
index fa11dd6..91c6215 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -478,37 +478,33 @@
assert (h->bs_entries);
assert (length >= 12);
- if (cmd->error)
+ /* Need to byte-swap the entries returned, but apart from that we
+ * don't validate them.
+ */
+ for (i = 0; i < length/4; ++i)
+ h->bs_entries[i] = be32toh (h->bs_entries[i]);
+
+ /* Look up the context ID. */
+ context_id = h->bs_entries[0];
+ for (meta_context = h->meta_contexts;
+ meta_context;
+ meta_context = meta_context->next)
+ if (context_id == meta_context->context_id)
+ break;
+
+ if (meta_context) {
+ /* Call the caller's extent function. */
+ int error = cmd->error;
+
+ if (cmd->cb.fn.extent (cmd->cb.opaque, meta_context->name, cmd->offset,
+ &h->bs_entries[1], (length-4) / 4, &error) == -1)
+ if (cmd->error == 0)
+ cmd->error = error ? error : EPROTO;
+ }
+ else
/* Emit a debug message, but ignore it. */
- debug (h, "ignoring meta context ID %" PRIu32 " after callback failure",
- be32toh (h->bs_entries[0]));
- else {
- /* Need to byte-swap the entries returned, but apart from that we
- * don't validate them.
- */
- for (i = 0; i < length/4; ++i)
- h->bs_entries[i] = be32toh (h->bs_entries[i]);
-
- /* Look up the context ID. */
- context_id = h->bs_entries[0];
- for (meta_context = h->meta_contexts;
- meta_context;
- meta_context = meta_context->next)
- if (context_id == meta_context->context_id)
- break;
-
- if (meta_context) {
- /* Call the caller's extent function. */
- errno = 0;
- if (cmd->cb.fn.extent (cmd->cb.opaque, meta_context->name, cmd->offset,
- &h->bs_entries[1], (length-4) / 4) == -1)
- cmd->error = errno ? errno : EPROTO;
- }
- else
- /* Emit a debug message, but ignore it. */
- debug (h, "server sent unexpected meta context ID %" PRIu32,
- context_id);
- }
+ debug (h, "server sent unexpected meta context ID %" PRIu32,
+ context_id);
SET_NEXT_STATE(%FINISH);
}
diff --git a/interop/dirty-bitmap.c b/interop/dirty-bitmap.c
index 329fbea..a19fb64 100644
--- a/interop/dirty-bitmap.c
+++ b/interop/dirty-bitmap.c
@@ -41,7 +41,7 @@ struct data {
static int
cb (void *opaque, const char *metacontext, uint64_t offset,
- uint32_t *entries, size_t len)
+ uint32_t *entries, size_t len, int *error)
{
struct data *data = opaque;
@@ -50,6 +50,7 @@ cb (void *opaque, const char *metacontext, uint64_t offset,
* the fact that qemu-nbd is compliant.
*/
assert (offset == 0);
+ assert (!*error || (data->fail && data->count == 1 && *error == EPROTO));
assert (data->count-- > 0); /* [qemu-nbd] */
if (strcmp (metacontext, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0) {
@@ -91,7 +92,8 @@ cb (void *opaque, const char *metacontext, uint64_t offset,
}
if (data->fail) {
- errno = EPROTO; /* Something NBD servers can't send */
+ /* Something NBD servers can't send */
+ *error = data->count == 1 ? EPROTO : ECONNREFUSED;
return -1;
}
return 0;
@@ -147,17 +149,14 @@ main (int argc, char *argv[])
}
assert (data.seen_base && data.seen_dirty);
- /* Trigger a failed callback, to prove connection stays up. No way
- * to know which context will respond first, but we can check that
- * the other one did not get visited.
- */
- data = (struct data) { .count = 1, .fail = true, };
+ /* Trigger a failed callback, to prove connection stays up. */
+ data = (struct data) { .count = 2, .fail = true, };
if (nbd_block_status (nbd, exportsize, 0, &data, cb, 0) != -1) {
fprintf (stderr, "unexpected block status success\n");
exit (EXIT_FAILURE);
}
assert (nbd_get_errno () == EPROTO && nbd_aio_is_ready (nbd));
- assert (data.seen_base ^ data.seen_dirty);
+ assert (data.seen_base && data.seen_dirty);
if (nbd_pread (nbd, &c, 1, 0, 0) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
diff --git a/lib/internal.h b/lib/internal.h
index f827957..88ad703 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -232,7 +232,7 @@ struct socket {
};
typedef int (*extent_fn) (void *data, const char *metacontext, uint64_t offset,
- uint32_t *entries, size_t nr_entries);
+ uint32_t *entries, size_t nr_entries, int *error);
typedef int (*read_fn) (void *data, const void *buf, size_t count,
uint64_t offset, int *error, int status);
diff --git a/ocaml/examples/extents.ml b/ocaml/examples/extents.ml
index b49f98e..6681446 100644
--- a/ocaml/examples/extents.ml
+++ b/ocaml/examples/extents.ml
@@ -16,7 +16,8 @@ let () =
(* Read the extents and print them. *)
let size = NBD.get_size nbd in
NBD.block_status nbd size 0_L () (
- fun () meta _ entries ->
+ fun () meta _ entries err ->
+ printf "err=%d\n" !err;
if meta = "base:allocation" then (
printf "index\tlength\tflags\n";
for i = 0 to Array.length entries / 2 - 1 do
diff --git a/python/t/460-block-status.py b/python/t/460-block-status.py
index eeaa7b2..7e4ba2c 100644
--- a/python/t/460-block-status.py
+++ b/python/t/460-block-status.py
@@ -27,9 +27,10 @@ h.connect_command (["nbdkit", "-s", "--exit-with-parent", "-v",
"sh", script])
entries = []
-def f (data, metacontext, offset, e):
+def f (data, metacontext, offset, e, err):
global entries
assert data == 42
+ assert err.value == 0
if metacontext != "base:allocation":
return
entries = e
diff --git a/tests/meta-base-allocation.c b/tests/meta-base-allocation.c
index a9ddbd0..8ae215e 100644
--- a/tests/meta-base-allocation.c
+++ b/tests/meta-base-allocation.c
@@ -30,7 +30,7 @@
static int check_extent (void *data, const char *metacontext,
uint64_t offset,
- uint32_t *entries, size_t nr_entries);
+ uint32_t *entries, size_t nr_entries, int *error);
int
main (int argc, char *argv[])
@@ -128,15 +128,16 @@ main (int argc, char *argv[])
static int
check_extent (void *data, const char *metacontext,
uint64_t offset,
- uint32_t *entries, size_t nr_entries)
+ uint32_t *entries, size_t nr_entries, int *error)
{
size_t i;
int id = * (int *)data;
printf ("extent: id=%d, metacontext=%s, offset=%" PRIu64 ", "
- "nr_entries=%zu\n",
- id, metacontext, offset, nr_entries);
+ "nr_entries=%zu, error=%d\n",
+ id, metacontext, offset, nr_entries, *error);
+ assert (*error == 0);
if (strcmp (metacontext, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0) {
for (i = 0; i < nr_entries; i += 2) {
printf ("\t%zu\tlength=%" PRIu32 ", status=%" PRIu32 "\n",
--
2.20.1
4 years, 10 months
[libnbd PATCH] tests: Add test for abrupt server death
by Eric Blake
It's worth testing that our transition to the DEAD state works as
expected, by intentionally killing a server.
This test also makes it possible to test what happens when pending
commands are stranded, so that an upcoming patch to add notifiers can
show the difference it makes.
The test was surprisingly hard to write. For starters, sending SIGINT
to nbdkit was sometimes enough to kill the process (if it hadn't yet
read the NBD_CMD_READ, and therefore did try to wait for any
outstanding requests before quitting), but often it did not (because
nbdkit was stuck waiting for pthread_join()). Then there's the race
that nbd_poll() can sometimes get lucky enough to catch a POLLHUP in
REPLY.START where recv() returning 0 transitions things to CLOSED, but
more often catches a POLLERR and transitions to DEAD. Then there was
the hour I spent scratching my head why kill didn't seem to get rid of
the child process even though the poll() was definitely seeing the fd
closing, until I remembered that kill(pid,0) to zombie processes
succeeds until you wait() or alter SIGCHLD to specifically prevent
zombies. And debugging the now-fixed uninitialized variable in
nbd_unlocked_poll added to the mix.
I'm not 100% sure the test is portable to non-Linux - I guess we'll
eventually find out when someone worries about a BSD port of libnbd.
---
Applies on top of my nbd_poll return value semantic change.
I'm open to bike-shedding on the test name.
.gitignore | 1 +
tests/Makefile.am | 7 +++
tests/disconnect.c | 143 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 151 insertions(+)
create mode 100644 tests/disconnect.c
diff --git a/.gitignore b/.gitignore
index ea496ac..2b8092c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -109,6 +109,7 @@ Makefile.in
/tests/connect-unix
/tests/connect-uri-tcp
/tests/connect-uri-unix
+/tests/disconnect
/tests/errors
/tests/functions.sh
/tests/get-size
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0314d91..bbd7330 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -43,6 +43,7 @@ check_DATA =
check_PROGRAMS = \
errors \
+ disconnect \
get-size \
read-only-flag \
read-write-flag \
@@ -77,6 +78,7 @@ LOG_COMPILER = $(top_builddir)/run
TESTS = \
errors \
+ disconnect \
get-size \
read-only-flag \
read-write-flag \
@@ -108,6 +110,11 @@ errors_CPPFLAGS = -I$(top_srcdir)/include
errors_CFLAGS = $(WARNINGS_CFLAGS)
errors_LDADD = $(top_builddir)/lib/libnbd.la
+disconnect_SOURCES = disconnect.c
+disconnect_CPPFLAGS = -I$(top_srcdir)/include
+disconnect_CFLAGS = $(WARNINGS_CFLAGS)
+disconnect_LDADD = $(top_builddir)/lib/libnbd.la
+
get_size_SOURCES = get-size.c
get_size_CPPFLAGS = -I$(top_srcdir)/include
get_size_CFLAGS = $(WARNINGS_CFLAGS)
diff --git a/tests/disconnect.c b/tests/disconnect.c
new file mode 100644
index 0000000..677d44b
--- /dev/null
+++ b/tests/disconnect.c
@@ -0,0 +1,143 @@
+/* NBD client library in userspace
+ * Copyright (C) 2013-2019 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Deliberately disconnect while stranding commands, to check their status.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <signal.h>
+
+#include <libnbd.h>
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ char buf[512];
+ int64_t handle;
+ char pidfile[] = "/tmp/libnbd-test-disconnectXXXXXX";
+ int fd;
+ pid_t pid;
+ int r;
+ int delay = 0;
+ const char *cmd[] = { "nbdkit", "--pidfile", pidfile, "-s",
+ "--exit-with-parent", "--filter=delay", "memory",
+ "size=1m", "delay-reads=5", NULL };
+
+ /* We're going to kill the child, but don't want to wait for a zombie */
+ if (signal (SIGCHLD, SIG_IGN) == SIG_ERR) {
+ fprintf (stderr, "%s: signal: %s\n", argv[0], strerror (errno));
+ exit (EXIT_FAILURE);
+ }
+
+ fd = mkstemp (pidfile);
+ if (fd < 0) {
+ fprintf (stderr, "%s: mkstemp: %s\n", argv[0], strerror (errno));
+ exit (EXIT_FAILURE);
+ }
+
+ nbd = nbd_create ();
+ if (nbd == NULL) {
+ fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
+ goto fail;
+ }
+
+ /* Connect to a slow server. */
+ if (nbd_connect_command (nbd, (char **) cmd) == -1) {
+ fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
+ goto fail;
+ }
+ if (read (fd, buf, sizeof buf) == -1) {
+ fprintf (stderr, "%s: read: %s\n", argv[0], strerror (errno));
+ goto fail;
+ }
+ pid = atoi (buf);
+ if (pid <= 0) {
+ fprintf (stderr, "%s: unable to parse server's pid\n", argv[0]);
+ goto fail;
+ }
+
+ /* Issue a read that should not complete yet. */
+ if ((handle = nbd_aio_pread (nbd, buf, sizeof buf, 0, 0)) == -1) {
+ fprintf (stderr, "%s: test failed: nbd_aio_pread\n", argv[0]);
+ goto fail;
+ }
+ if (nbd_aio_peek_command_completed (nbd) != 0) {
+ fprintf (stderr, "%s: test failed: nbd_aio_peek_command_completed\n",
+ argv[0]);
+ goto fail;
+ }
+ if (nbd_aio_command_completed (nbd, handle) != 0) {
+ fprintf (stderr, "%s: test failed: nbd_aio_command_completed\n", argv[0]);
+ goto fail;
+ }
+
+ /* Kill the server forcefully (SIGINT is not always strong enough,
+ * as nbdkit waits for pending transactions to finish before
+ * actually exiting), although it's a race whether our signal
+ * arrives while nbdkit has a pending transaction.
+ */
+ if (kill (pid, SIGKILL) == -1) {
+ fprintf (stderr, "%s: kill: %s\n", argv[0], strerror (errno));
+ goto fail;
+ }
+ /* Wait up to 10 seconds, 100 ms at a time */
+ while (kill (pid, 0) == 0) {
+ if (delay++ > 100) {
+ fprintf (stderr, "%s: kill taking too long\n", argv[0]);
+ goto fail;
+ }
+ usleep (100 * 1000);
+ }
+
+ /* This part is somewhat racy if we don't wait for the process death
+ * above - depending on load and timing, nbd_poll may succeed or
+ * fail, and we may transition to either CLOSED (the state machine
+ * saw a clean EOF) or DEAD (the state machine saw a stranded
+ * transaction or POLLERR).
+ */
+ while ((r = nbd_poll (nbd, 1000)) == 1)
+ if (nbd_aio_is_dead (nbd) || nbd_aio_is_closed (nbd))
+ break;
+ if (!(nbd_aio_is_dead (nbd) || nbd_aio_is_closed (nbd))) {
+ fprintf (stderr, "%s: test failed: server death not detected\n", argv[0]);
+ goto fail;
+ }
+
+ /* Proof that the read was stranded */
+ if (nbd_aio_peek_command_completed (nbd) != 0) {
+ fprintf (stderr, "%s: test failed: nbd_aio_peek_command_completed\n",
+ argv[0]);
+ goto fail;
+ }
+ if (nbd_aio_command_completed (nbd, handle) != 0) {
+ fprintf (stderr, "%s: test failed: nbd_aio_command_completed\n", argv[0]);
+ goto fail;
+ }
+
+ nbd_close (nbd);
+ exit (EXIT_SUCCESS);
+
+ fail:
+ unlink (pidfile);
+ exit (EXIT_FAILURE);
+}
--
2.20.1
4 years, 10 months
[libnbd PATCH 0/2] socket handling cleanups
by Eric Blake
While working on a new test of what happens when the server goes away
while commands are in flight, I managed to hit a race where I hit
death from SIGPIPE instead of a clean transition to the DEAD state. I
also found myself wanting to use nbd_poll from the test, but with a
way to distinguish between the state machine progressing vs. hanging.
Eric Blake (2):
socket: Avoid SIGPIPE where possible
poll: Improve our interface
generator/generator | 18 +++++++++++++-----
lib/poll.c | 13 ++++++++++---
lib/socket.c | 7 +++++++
3 files changed, 30 insertions(+), 8 deletions(-)
--
2.20.1
4 years, 10 months
