June 2019 - Libguestfs - libguestfs List Archives

[PATCH libnbd v2] python: Raise a custom exception containing error string and errno.
by Richard W.M. Jones 28 Jun '19

28 Jun '19

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.

2 3

[libnbd PATCH] generator: Add support for namespace constants
by Martin Kletzander 28 Jun '19

28 Jun '19

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

3 4

[PATCH libnbd] python: Raise a custom exception containing error string and errno.
by Richard W.M. Jones 28 Jun '19

28 Jun '19

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.

1 1

[nbdkit PATCH] nbd: Update for libnbd 0.1.5
by Eric Blake 28 Jun '19

28 Jun '19

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

2 1

[libnbd PATCH] opt-go: Better decoding of known errors
by Eric Blake 28 Jun '19

28 Jun '19

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

1 0

[libnbd PATCH] maint: Use $(NULL) for all Makefile.am macro lists
by Eric Blake 27 Jun '19

27 Jun '19

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

2 1

[libnbd PATCH] python: Fix bindings for Path parameters
by Eric Blake 27 Jun '19

27 Jun '19

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

2 1

[libnbd PATCH] block-status: Make callback usage consistent with pread_structured
by Eric Blake 27 Jun '19

27 Jun '19

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

2 1

[libnbd PATCH] tests: Add test for abrupt server death
by Eric Blake 27 Jun '19

27 Jun '19

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

2 2

[libnbd PATCH 0/2] socket handling cleanups
by Eric Blake 27 Jun '19

27 Jun '19

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

2 4