June 2019 - Libguestfs - libguestfs List Archives

[libnbd PATCH 0/2] Tighten URI parser
by Eric Blake 27 Jun '19

27 Jun '19

I'm not sure whether we want to go with just the first patch (reject nbd:unix:/path but still accept nbd:/path), or squash the two in order to go with the second (reject both abbreviated forms, and require scheme://...). Either way, though, nbdkit -U - --run '$nbd' will now error out rather than inadvertently connect over TCP to localhost:10809 instead of the intended Unix connection (and in the meantime, you want to use --run '$unixsocket', or maybe we should teach nbdkit to support --run '$uri'). Eric Blake (2): uri: Reject nbd:unix:/path/to/socket as invalid URI uri: Reject nbd:unix:/path/to/socket as invalid URI lib/connect.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) -- 2.20.1

3 6

[libnbd PATCH] pread_structured: Change callback type to use Mutable error
by Eric Blake 27 Jun '19

27 Jun '19

Take advantage of the previous patch to make it easier for language bindings to affect the exact error they want on failure, rather than requiring them to influence errno. Update the python test to tickle the changed bindings, and to prove that we can at least trigger an exception, although we are still lacking bindings for python code to access the last NBD exception and error code gracefully (maybe we should be returning a specific subclass of Exception that contains the last error information). --- Applies on top of my fixes to Rich's proposal for a Mutable(Int) callback parameter in the generator. I validated that: except Exception as e: print(e) in 405-pread-structured.py is sufficient to produce output resembling: nbd_pread_structured: read: command failed: Protocol error Traceback (most recent call last): File "./t/405-pread-structured.py", line 48, in <module> buf = h.pread_structured (512, 0, 43, f, nbd.CMD_FLAG_DF) File "/home/eblake/libnbd/python/nbd.py", line 517, in pread_structured return libnbdmod.pread_structured (self._o, count, offset, data, chunk, flag s) RuntimeError: nbd_pread_structured: read: command failed: Protocol error where assigning to error.value in the callback affected the strerror reference in the RuntimeError; but we really should be thinking about returning a subclass of RuntimeError for programmatic access from python rather than just scraping the generic error string. generator/generator | 28 ++++++++++++++++------------ generator/states-reply-simple.c | 7 ++++--- generator/states-reply-structured.c | 21 ++++++++++++--------- interop/structured-read.c | 6 +++--- lib/internal.h | 2 +- python/t/405-pread-structured.py | 12 +++++++++++- tests/oldstyle.c | 8 ++++---- 7 files changed, 51 insertions(+), 33 deletions(-) diff --git a/generator/generator b/generator/generator index a9a23f4..37fa0fc 100755 --- a/generator/generator +++ b/generator/generator @@ -1330,7 +1330,7 @@ protocol extensions)."; args = [ BytesOut ("buf", "count"); UInt64 "offset"; Opaque "data"; Callback ("chunk", [ Opaque "data"; BytesIn ("subbuf", "count"); - UInt64 "offset"; Int "error"; + UInt64 "offset"; Mutable (Int "error"); Int "status" ]); Flags "flags" ]; ret = RErr; @@ -1346,8 +1346,9 @@ additional sanity checking on the server's reply. 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>, and no earlier error has been detected, then the overall read command will -fail with the same value of C<errno> left after the failing callback; -but any further chunks will still invoke the callback. +fail with any non-zero value stored into the callback's C<error> +parameter (with a default of C<EPROTO>); but any further chunks will +still invoke the callback. The C<chunk> function is called once per chunk of data received. The C<subbuf> and C<count> parameters represent the subset of the original @@ -1356,25 +1357,27 @@ C<subbuf> always points within the original C<buf>; but this guarantee may not extend to other language bindings). The C<offset> parameter represents the absolute offset at which C<subbuf> begins within the image (note that this is not the relative offset of C<subbuf> within -the original buffer C<buf>). The C<error> parameter is controlled by -the C<status> parameter, which is one of +the original buffer C<buf>). Changes to C<error> on output are ignored +unless the callback fails. The input meaning of the C<error> parameter +is controlled by the C<status> parameter, which is one of =over 4 =item C<LIBNBD_READ_DATA> = 1 -C<subbuf> was populated with C<count> bytes of data. C<error> is the -errno value of any earlier detected error, or zero. +C<subbuf> was populated with C<count> bytes of data. On input, C<error> +contains the errno value of any earlier detected error, or zero. =item C<LIBNBD_READ_HOLE> = 2 -C<subbuf> represents a hole, and contains C<count> NUL bytes. C<error> -is the errno value of any earlier detected error, or zero. +C<subbuf> represents a hole, and contains C<count> NUL bytes. On input, +C<error> contains the errno value of any earlier detected error, or zero. =item C<LIBNBD_READ_ERROR> = 3 -C<count> is 0, so C<subbuf> is unusable. C<error> is the errno value -reported by the server as occurring while reading that C<offset>. +C<count> is 0, so C<subbuf> is unusable. On input, C<error> contains the +errno value reported by the server as occurring while reading that +C<offset>, regardless if any earlier error has been detected. =back @@ -1685,7 +1688,8 @@ parameters behave as documented in C<nbd_pread>."; Opaque "data"; CallbackPersist ("chunk", [ Opaque "data"; BytesIn ("subbuf", "count"); - UInt64 "offset"; Int "error"; + UInt64 "offset"; + Mutable (Int "error"); Int "status" ]); Flags "flags" ]; ret = RInt64; diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c index 25eab9d..cab72d6 100644 --- a/generator/states-reply-simple.c +++ b/generator/states-reply-simple.c @@ -61,11 +61,12 @@ /* guaranteed by START */ assert (cmd); if (cmd->cb.fn.read) { + int error = 0; + assert (cmd->error == 0); - errno = 0; if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data, cmd->count, - cmd->offset, 0, LIBNBD_READ_DATA) == -1) - cmd->error = errno ? errno : EPROTO; + cmd->offset, &error, LIBNBD_READ_DATA) == -1) + cmd->error = error ? error : EPROTO; } SET_NEXT_STATE (%^FINISH_COMMAND); diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index 24bead6..fa11dd6 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -292,15 +292,16 @@ return -1; } if (cmd->type == NBD_CMD_READ && cmd->cb.fn.read) { + int scratch = error; + /* Different from successful reads: inform the callback about the * current error rather than any earlier one. If the callback fails * without setting errno, then use the server's error below. */ - errno = 0; if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data + (offset - cmd->offset), - 0, offset, error, LIBNBD_READ_ERROR) == -1) + 0, offset, &scratch, LIBNBD_READ_ERROR) == -1) if (cmd->error == 0) - cmd->error = errno; + cmd->error = scratch; } } @@ -381,12 +382,13 @@ assert (cmd); /* guaranteed by CHECK */ if (cmd->cb.fn.read) { - errno = 0; + int error = cmd->error; + if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data + (offset - cmd->offset), - length - sizeof offset, offset, cmd->error, + length - sizeof offset, offset, &error, LIBNBD_READ_DATA) == -1) if (cmd->error == 0) - cmd->error = errno ? errno : EPROTO; + cmd->error = error ? error : EPROTO; } SET_NEXT_STATE (%FINISH); @@ -441,12 +443,13 @@ */ memset (cmd->data + offset, 0, length); if (cmd->cb.fn.read) { - errno = 0; + int error = cmd->error; + if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data + offset, length, - cmd->offset + offset, cmd->error, + cmd->offset + offset, &error, LIBNBD_READ_HOLE) == -1) if (cmd->error == 0) - cmd->error = errno ? errno : EPROTO; + cmd->error = error ? error : EPROTO; } SET_NEXT_STATE(%FINISH); diff --git a/interop/structured-read.c b/interop/structured-read.c index cf8b893..46a7a80 100644 --- a/interop/structured-read.c +++ b/interop/structured-read.c @@ -47,7 +47,7 @@ struct data { static int read_cb (void *opaque, const void *bufv, size_t count, uint64_t offset, - int error, int status) + int *error, int status) { struct data *data = opaque; const char *buf = bufv; @@ -55,7 +55,7 @@ read_cb (void *opaque, const void *bufv, size_t count, uint64_t offset, /* The NBD spec allows chunks to be reordered; we are relying on the * fact that qemu-nbd does not do so. */ - assert (!error || (data->fail && data->count == 1)); + assert (!*error || (data->fail && data->count == 1 && *error == EPROTO)); assert (data->count-- > 0); switch (status) { @@ -93,7 +93,7 @@ read_cb (void *opaque, const void *bufv, size_t count, uint64_t offset, if (data->fail) { /* Something NBD servers can't send */ - errno = data->count == 1 ? EPROTO : ECONNREFUSED; + *error = data->count == 1 ? EPROTO : ECONNREFUSED; return -1; } return 0; diff --git a/lib/internal.h b/lib/internal.h index 7d87fa4..f827957 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -234,7 +234,7 @@ struct socket { typedef int (*extent_fn) (void *data, const char *metacontext, uint64_t offset, uint32_t *entries, size_t nr_entries); typedef int (*read_fn) (void *data, const void *buf, size_t count, - uint64_t offset, int error, int status); + uint64_t offset, int *error, int status); struct command_cb { void *opaque; diff --git a/python/t/405-pread-structured.py b/python/t/405-pread-structured.py index 1bfa162..9ed2090 100644 --- a/python/t/405-pread-structured.py +++ b/python/t/405-pread-structured.py @@ -16,6 +16,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import nbd +import errno h = nbd.NBD () h.connect_command (["nbdkit", "-s", "--exit-with-parent", "-v", @@ -24,10 +25,11 @@ h.connect_command (["nbdkit", "-s", "--exit-with-parent", "-v", expected = b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x08\x00\x00\x00\x00\x00\x00\x00\x10\x00\x00\x00\x00\x00\x00\x00\x18\x00\x00\x00\x00\x00\x00\x00 \x00\x00\x00\x00\x00\x00\x00(\x00\x00\x00\x00\x00\x00\x000\x00\x00\x00\x00\x00\x00\x008\x00\x00\x00\x00\x00\x00\x00@\x00\x00\x00\x00\x00\x00\x00H\x00\x00\x00\x00\x00\x00\x00P\x00\x00\x00\x00\x00\x00\x00X\x00\x00\x00\x00\x00\x00\x00`\x00\x00\x00\x00\x00\x00\x00h\x00\x00\x00\x00\x00\x00\x00p\x00\x00\x00\x00\x00\x00\x00x\x00\x00\x00\x00\x00\x00\x00\x80\x00\x00\x00\x00\x00\x00\x00\x88\x00\x00\x00\x00\x00\x00\x00\x90\x00\x00\x00\x00\x00\x00\x00\x98\x00\x00\x00\x00\x00\x00\x00\xa0\x00\x00\x00\x00\x00\x00\x00\xa8\x00\x00\x00\x00\x00\x00\x00\xb0\x00\x00\x00\x00\x00\x00\x00\xb8\x00\x00\x00\x00\x00\x00\x00\xc0\x00\x00\x00\x00\x00\x00\x00\xc8\x00\x00\x00\x00\x00\x00\x00\xd0\x00\x00\x00\x00\x00\x00\x00\xd8\x00\x00\x00\x00\x00\x00\x00\xe0\x00\x00\x00\x00\x00\x00\x00\xe8\x00\x00\x00\x00\x00\x00\x00\xf0\x00\x00\x00\x00\x00\x00\x00\xf8\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x01\x08\x00\x00\x00\x00\x00\x00\x01\x10\x00\x00\x00\x00\x00\x00\x01\x18\x00\x00\x00\x00\x00\x00\x01 \x00\x00\x00\x00\x00\x00\x01(\x00\x00\x00\x00\x00\x00\x010\x00\x00\x00\x00\x00\x00\x018\x00\x00\x00\x00\x00\x00\x01@\x00\x00\x00\x00\x00\x00\x01H\x00\x00\x00\x00\x00\x00\x01P\x00\x00\x00\x00\x00\x00\x01X\x00\x00\x00\x00\x00\x00\x01`\x00\x00\x00\x00\x00\x00\x01h\x00\x00\x00\x00\x00\x00\x01p\x00\x00\x00\x00\x00\x00\x01x\x00\x00\x00\x00\x00\x00\x01\x80\x00\x00\x00\x00\x00\x00\x01\x88\x00\x00\x00\x00\x00\x00\x01\x90\x00\x00\x00\x00\x00\x00\x01\x98\x00\x00\x00\x00\x00\x00\x01\xa0\x00\x00\x00\x00\x00\x00\x01\xa8\x00\x00\x00\x00\x00\x00\x01\xb0\x00\x00\x00\x00\x00\x00\x01\xb8\x00\x00\x00\x00\x00\x00\x01\xc0\x00\x00\x00\x00\x00\x00\x01\xc8\x00\x00\x00\x00\x00\x00\x01\xd0\x00\x00\x00\x00\x00\x00\x01\xd8\x00\x00\x00\x00\x00\x00\x01\xe0\x00\x00\x00\x00\x00\x00\x01\xe8\x00\x00\x00\x00\x00\x00\x01\xf0\x00\x00\x00\x00\x00\x00\x01\xf8' def f (data, buf2, offset, err, s): + assert err.value == 0 + err.value = errno.EPROTO assert data == 42 assert buf2 == expected assert offset == 0 - assert err == 0 assert s == nbd.READ_DATA buf = h.pread_structured (512, 0, 42, f) @@ -41,3 +43,11 @@ buf = h.pread_structured (512, 0, 42, f, nbd.CMD_FLAG_DF) print ("%r" % buf) assert buf == expected + +try: + buf = h.pread_structured (512, 0, 43, f, nbd.CMD_FLAG_DF) + assert False +except Exception: + # Need a way for python to access last NBD error... + # assert nbd.get_errno == errno.EPROTO + pass diff --git a/tests/oldstyle.c b/tests/oldstyle.c index 38f5130..0207bf8 100644 --- a/tests/oldstyle.c +++ b/tests/oldstyle.c @@ -36,7 +36,7 @@ static const char *progname; static int pread_cb (void *data, const void *buf, size_t count, uint64_t offset, - int error, int status) + int *error, int status) { int *calls = data; ++*calls; @@ -49,7 +49,7 @@ pread_cb (void *data, const void *buf, size_t count, uint64_t offset, fprintf (stderr, "%s: callback called with wrong offset\n", progname); exit (EXIT_FAILURE); } - if (error != 0) { + if (*error != 0) { fprintf (stderr, "%s: callback called with unexpected error\n", progname); exit (EXIT_FAILURE); } @@ -64,7 +64,7 @@ pread_cb (void *data, const void *buf, size_t count, uint64_t offset, } if (*calls > 1) { - errno = EPROTO; /* Something NBD servers can't send */ + *error = ECONNREFUSED; /* Something NBD servers can't send */ return -1; } @@ -143,7 +143,7 @@ main (int argc, char *argv[]) fprintf (stderr, "%s: expected failure from callback\n", argv[0]); exit (EXIT_FAILURE); } - if (nbd_get_errno () != EPROTO) { + if (nbd_get_errno () != ECONNREFUSED) { fprintf (stderr, "%s: wrong errno value after failed callback\n", argv[0]); exit (EXIT_FAILURE); } -- 2.20.1

2 1

Re: [Libguestfs] [libguestfs/libguestfs] when compiling v1.40.2: rake aborted! (#40)
by Richard W.M. Jones 27 Jun '19

27 Jun '19

On Wed, Jun 26, 2019 at 05:38:28AM -0700, Samuel Bernardo wrote: > When compilling libguestfs 1.40.2 I receive the following linking error: > > ``` > linking shared-object _guestfs.so > /usr/lib/gcc/x86_64-pc-linux-gnu/8.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: actions-1.o: in function `guestfs_int_ruby_f2fs_expand': > actions-1.c:(.text+0x1521): undefined reference to `guestfs_f2fs_expand' > /usr/lib/gcc/x86_64-pc-linux-gnu/8.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: actions-2.o: in function `guestfs_int_ruby_lvm_scan': > actions-2.c:(.text+0x2e80): undefined reference to `guestfs_lvm_scan' > /usr/lib/gcc/x86_64-pc-linux-gnu/8.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: actions-5.o: in function `guestfs_int_ruby_inspect_get_osinfo': > actions-5.c:(.text+0x1d42): undefined reference to `guestfs_inspect_get_osinfo' > collect2: error: ld returned 1 exit status > make[3]: *** [Makefile:259: _guestfs.so] Error 1 > make[3]: Leaving directory '/var/tmp/portage/app-emulation/libguestfs-1.40.2/work/libguestfs-1.40.2/ruby/ext/guestfs' > rake aborted! > ``` Unclear - would need to see the complete build log to even begin to guess at what's going on here. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org

2 2

[PATCH 1/9] Rust bindings: Add Rust bindings
by Hiroyuki Katsura 27 Jun '19

27 Jun '19

From: Hiroyuki_Katsura <hiroyuki.katsura.0513(a)gmail.com> --- Makefile.am | 4 ++++ configure.ac | 3 +++ generator/Makefile.am | 3 +++ generator/bindtests.ml | 3 +++ generator/bindtests.mli | 1 + generator/main.ml | 5 +++++ generator/rust.ml | 34 ++++++++++++++++++++++++++++++++++ generator/rust.mli | 19 +++++++++++++++++++ m4/guestfs-rust.m4 | 30 ++++++++++++++++++++++++++++++ rust/Cargo.toml | 6 ++++++ rust/Makefile.am | 29 +++++++++++++++++++++++++++++ rust/src/.gitkeep | 0 rust/tests/.gitkeep | 0 13 files changed, 137 insertions(+) create mode 100644 generator/rust.ml create mode 100644 generator/rust.mli create mode 100644 m4/guestfs-rust.m4 create mode 100644 rust/Cargo.toml create mode 100644 rust/Makefile.am create mode 100644 rust/src/.gitkeep create mode 100644 rust/tests/.gitkeep diff --git a/Makefile.am b/Makefile.am index b5f33f62b..69142cf41 100644 --- a/Makefile.am +++ b/Makefile.am @@ -151,6 +151,10 @@ endif if HAVE_GOLANG SUBDIRS += golang golang/examples endif +if HAVE_RUST +SUBDIRS += rust +endif + # Unconditional because nothing is built yet. SUBDIRS += csharp diff --git a/configure.ac b/configure.ac index 6b701bef2..f9bdbe54b 100644 --- a/configure.ac +++ b/configure.ac @@ -161,6 +161,8 @@ HEADING([Checking for Go]) m4_include([m4/guestfs-golang.m4]) HEADING([Checking for GObject Introspection]) m4_include([m4/guestfs-gobject.m4]) +HEADING([Checking for Rust]) +m4_include([m4/guestfs-rust.m4]) HEADING([Checking for Vala]) VAPIGEN_CHECK @@ -315,6 +317,7 @@ AC_CONFIG_FILES([Makefile ruby/Rakefile ruby/examples/Makefile ruby/ext/guestfs/extconf.rb + rust/Makefile sparsify/Makefile sysprep/Makefile test-data/Makefile diff --git a/generator/Makefile.am b/generator/Makefile.am index 07872d49c..676c08ce5 100644 --- a/generator/Makefile.am +++ b/generator/Makefile.am @@ -105,6 +105,8 @@ sources = \ p2v_config.mli \ ruby.ml \ ruby.mli \ + rust.ml \ + rust.mli \ structs.ml \ structs.mli \ tests_c_api.ml \ @@ -163,6 +165,7 @@ objects = \ lua.cmo \ GObject.cmo \ golang.cmo \ + rust.cmo \ bindtests.cmo \ errnostring.cmo \ customize.cmo \ diff --git a/generator/bindtests.ml b/generator/bindtests.ml index 58d7897b3..41aef47ea 100644 --- a/generator/bindtests.ml +++ b/generator/bindtests.ml @@ -983,6 +983,9 @@ and generate_php_bindtests () = dump "bindtests" +and generate_rust_bindtests () = + generate_header CStyle GPLv2plus; + (* Language-independent bindings tests - we do it this way to * ensure there is parity in testing bindings across all languages. *) diff --git a/generator/bindtests.mli b/generator/bindtests.mli index 6f469b3a1..0e18a4c44 100644 --- a/generator/bindtests.mli +++ b/generator/bindtests.mli @@ -28,3 +28,4 @@ val generate_perl_bindtests : unit -> unit val generate_php_bindtests : unit -> unit val generate_python_bindtests : unit -> unit val generate_ruby_bindtests : unit -> unit +val generate_rust_bindtests : unit -> unit diff --git a/generator/main.ml b/generator/main.ml index 7974550c5..5de89138c 100644 --- a/generator/main.ml +++ b/generator/main.ml @@ -372,6 +372,11 @@ Run it from the top source directory using the command output_to "p2v/virt-p2v-kernel-config.pod" P2v_config.generate_p2v_kernel_config_pod; + output_to "rust/src/lib.rs" + Rust.generate_rust; + output_to "rust/tests/bind_test.rs" + Bindtests.generate_rust_bindtests; + (* Generate the list of files generated -- last. *) printf "generated %d lines of code\n" (get_lines_generated ()); let files = List.sort compare (get_files_generated ()) in diff --git a/generator/rust.ml b/generator/rust.ml new file mode 100644 index 000000000..83afdfe73 --- /dev/null +++ b/generator/rust.ml @@ -0,0 +1,34 @@ +(* libguestfs + * Copyright (C) 2019 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +*) + +(* Please read generator/README first. *) + +open Std_utils +open Types +open Utils +open Pr +open Docstrings +open Optgroups +open Actions +open Structs +open C +open Events + + +let generate_rust () = + generate_header CStyle LGPLv2plus; \ No newline at end of file diff --git a/generator/rust.mli b/generator/rust.mli new file mode 100644 index 000000000..4fef55d4e --- /dev/null +++ b/generator/rust.mli @@ -0,0 +1,19 @@ +(* libguestfs + * Copyright (C) 2009-2019 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +*) + +val generate_rust: unit -> unit \ No newline at end of file diff --git a/m4/guestfs-rust.m4 b/m4/guestfs-rust.m4 new file mode 100644 index 000000000..f46ce960b --- /dev/null +++ b/m4/guestfs-rust.m4 @@ -0,0 +1,30 @@ +# libguestfs +# Copyright (C) 2009-2019 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +dnl Rust +AC_ARG_ENABLE([rust], + AS_HELP_STRING([--disable-rust], [disable Rust language bindings]), + [], + [enable_rust=yes]) +AS_IF([test "x$enable_rust" != "xno"],[ + AC_CHECK_PROG([RUSTC],[rustc],[rustc],[no]) + AC_CHECK_PROG([CARGO],[cargo],[cargo],[no]) +],[ + RUSTC=no + CARGO=no + ]) +AM_CONDITIONAL([HAVE_RUST],[test "x$RUSTC" != "xno" && test "x$CARGO" != "xno"]) \ No newline at end of file diff --git a/rust/Cargo.toml b/rust/Cargo.toml new file mode 100644 index 000000000..e730ee830 --- /dev/null +++ b/rust/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "rust" +version = "0.1.0" +edition = "2018" + +[dependencies] diff --git a/rust/Makefile.am b/rust/Makefile.am new file mode 100644 index 000000000..e8bf27894 --- /dev/null +++ b/rust/Makefile.am @@ -0,0 +1,29 @@ +# libguestfs golang bindings +# Copyright (C) 2019 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +include $(top_srcdir)/subdir-rules.mk + +generator_built = \ + src/lib.rs + +EXTRA_DIST = \ + $(generator_built) + +if HAVE_RUST + +endif + diff --git a/rust/src/.gitkeep b/rust/src/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/rust/tests/.gitkeep b/rust/tests/.gitkeep new file mode 100644 index 000000000..e69de29bb -- 2.20.1 (Apple Git-117)

2 23

Few libnbd questions/concerns
by Martin Kletzander 27 Jun '19

27 Jun '19

Here are few things I found out when using libnbd that might be perfectly fine or maybe just an oversight, but I wanted to point them out. It's nothing major. When running a program with `nbdkit -U - --run ...`, the $nbd parameter gets expanded to nbd:unix:/path/to/socket. When this string is passed to nbd_connect_uri(), it does not return an error (even though it is not a valid URL), but what's more it treats it as "nbd://localhost", which might be a completely different server (that actually happened to me and it was kind of confusing). When nbd_block_status() callback returns -1, the nbd_block_status() function reports: command failed: Protocol error (errno = 71) which is a bit confusing to me. I could be nicer to have that report that it really was the callback that caused this. One last thing is that I could not find any definition for the flags of "base:allocation" metadata (NBD_STATE_HOLE and NBD_STATE_ZERO). It might just be things that are still missing due to an early stage of libnbd, or it might not belong there or it might be me misusing something. Have a nice day, Martin

3 6

[nbdkit PATCH] iso: Shell-quote an alternative isoprog
by Eric Blake 26 Jun '19

26 Jun '19

Otherwise, a user can do things like "nbdkit iso . prog='date;prog'" to run unintended commands in addition to their alternative isoprog. This is not a CVE (since nbdkit isn't running with any more privileges than the user running those commands themselves), but shows the frailty of relying on the shell to parse subsidiary commands rather than exec()ing them directly. This patch also doesn't resolve the fact that we are also passing params= through shell parsing (if we don't like that, we should consider changing the interface to make the user write param='-V' param='My Disk Image' and use shell_quote() over each param, rather than the current params='-V "My Disk Image"'), but does try to enhance the docs to point it out with more clarity. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- I'm pushing this now, but we may want to reconsider the iso plugin exposing params= that is intentionally designed for another round of shell parsing, as a followup patch. Ideally, we want to avoid ever passing user-supplied data through another shell invocation without first re-quoting it. plugins/iso/nbdkit-iso-plugin.pod | 4 +++- plugins/iso/iso.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/iso/nbdkit-iso-plugin.pod b/plugins/iso/nbdkit-iso-plugin.pod index 90e26f01..597c3c50 100644 --- a/plugins/iso/nbdkit-iso-plugin.pod +++ b/plugins/iso/nbdkit-iso-plugin.pod @@ -62,7 +62,9 @@ For example: would specify Joliet (I<-J>), Rock Ridge (I<-r>) and TRANS.TBL (I<-T>) extensions, and specify the volume ID (I<-V>) as C<My Disk Image>. -Take care when quoting this parameter. +Take care when quoting this parameter; nbdkit passes the resulting +string through another layer of shell interpretation without any +sanity checks for unquoted shell metacharacters. =item B<prog=>mkisofs diff --git a/plugins/iso/iso.c b/plugins/iso/iso.c index 4728ff32..5634bac9 100644 --- a/plugins/iso/iso.c +++ b/plugins/iso/iso.c @@ -94,7 +94,8 @@ make_iso (void) return -1; } - fprintf (fp, "%s -quiet", isoprog); + shell_quote (isoprog, fp); + fprintf (fp, " -quiet"); if (params) fprintf (fp, " %s", params); for (i = 0; i < nr_dirs; ++i) { -- 2.20.1

2 3

[nbdkit PATCH v2 0/2] adding nbdkit --run '$uri'
by Eric Blake 26 Jun '19

26 Jun '19

Since v1: - new patch to add uri_quote() - rebase on top of other recent patches needed while auditing shell_quote() - use uri_quote() instead of shell_quote() for producing $uri Eric Blake (2): common/utils: Add uri_quote and tests captive: Support $uri in --run docs/nbdkit-captive.pod | 8 ++- common/utils/utils.h | 1 + common/utils/test-quotes.c | 108 +++++++++++++++++++++++++++++++++++++ common/utils/utils.c | 27 ++++++++++ server/captive.c | 14 ++++- .gitignore | 1 + common/utils/Makefile.am | 11 ++++ 7 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 common/utils/test-quotes.c -- 2.20.1

2 3

[nbdkit PATCH] captive: Support $uri in --run
by Eric Blake 26 Jun '19

26 Jun '19

The existing --run '$nbd' outputs an older form that differs between libguestfs and qemu, and which is not always a valid URI. For historical compatibility, we probably can't change that; but we can instead add a new '$uri' that outputs a valid URI. Note that the libguestfs '$nbd' TCP form is already a valid URI, but that libguestfs still needs to be taught about the nbd+unix:// scheme, so for there, you are still better off using '$nbd'; but for qemu, using '$uri' already works since qemu v1.3.0 in 2012. Note that the NBD project has not actually yet posted a link for the preferred URI syntax; once Rich is happy with his work on that project, we can touch this up to link to that page. Reported-by: Martin Kletzander <mkletzan(a)redhat.com> Signed-off-by: Eric Blake <eblake(a)redhat.com> --- docs/nbdkit-captive.pod | 8 +++++++- server/captive.c | 14 +++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod index 6f69cca5..59df6690 100644 --- a/docs/nbdkit-captive.pod +++ b/docs/nbdkit-captive.pod @@ -42,9 +42,15 @@ The following shell variables are available in the I<--run> argument: =over 4 +=item C<$uri> + +A URI that refers to the nbdkit port or socket in the preferred form +documented by the NBD project. + =item C<$nbd> -A URL that refers to the nbdkit port or socket. +An older URL that refers to the nbdkit port or socket in a manner +specific to certain tools. Note there is some magic here, since qemu and guestfish URLs have a different format, so nbdkit tries to guess which you are running. If diff --git a/server/captive.c b/server/captive.c index 6971af2e..c5274f65 100644 --- a/server/captive.c +++ b/server/captive.c @@ -66,7 +66,19 @@ run_command (void) exit (EXIT_FAILURE); } - /* Construct $nbd "URL". Unfortunately guestfish and qemu take + /* Construct $uri. */ + fprintf (fp, "uri="); + if (port) { + fprintf (fp, "nbd://localhost:"); + shell_quote (port, fp); + } + else if (unixsocket) { + fprintf (fp, "nbd+unix://\\?socket="); + shell_quote (unixsocket, fp); + } + fprintf (fp, "\n"); + + /* Construct older $nbd "URL". Unfortunately guestfish and qemu take * different syntax, so try to guess which one we need. */ fprintf (fp, "nbd="); -- 2.20.1

3 3

[PATCH libnbd] generator: Add Mutable type to the generator.
by Richard W.M. Jones 25 Jun '19

25 Jun '19

Mutable (Int n) => int *n This can currently only be used for callback arguments of type int (not for other types, nor for any ordinary function arguments), but it could be implemented more generally in future. --- generator/generator | 75 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/generator/generator b/generator/generator index e1a97a5..72d37c8 100755 --- a/generator/generator +++ b/generator/generator @@ -843,6 +843,7 @@ and arg = | Flags of string (* NBD_CMD_FLAG_* flags *) | Int of string (* small int *) | Int64 of string (* 64 bit signed int *) +| Mutable of arg (* mutable argument, eg. int* *) | Opaque of string (* opaque object, void* in C *) | Path of string (* filename or path *) | SockAddrAndLen of string * string (* struct sockaddr * + socklen_t *) @@ -2708,6 +2709,7 @@ let rec name_of_arg = function | Int n -> [n] | Int64 n -> [n] | Opaque n -> [n] +| Mutable arg -> name_of_arg arg | Path n -> [n] | SockAddrAndLen (n, len) -> [n; len] | String n -> [n] @@ -2741,6 +2743,8 @@ let rec print_c_arg_list ?(handle = false) args = | Flags n -> pr "uint32_t %s" n | Int n -> pr "int %s" n | Int64 n -> pr "int64_t %s" n + | Mutable (Int n) -> pr "int *%s" n + | Mutable arg -> assert false | Opaque n -> pr "void *%s" n | Path n | String n -> pr "const char *%s" n @@ -3261,14 +3265,20 @@ let print_python_binding name { args; ret } = pr " for (size_t i = 0; i < %s; ++i)\n" len; pr " PyList_SET_ITEM (py_%s, i, PyLong_FromUnsignedLong (%s[i]));\n" n n | BytesIn _ -> () + | Mutable (Int n) -> + pr " PyObject *py_%s_dict = PyImport_GetModuleDict ();\n" n; + pr " PyObject *py_%s_mod = PyMapping_GetItemString (py_%s_dict, \"ctypes\");\n" n n; + pr " PyObject *py_%s = PyObject_CallMethod (py_%s_mod, \"c_int\", \"i\", *%s);\n" n n n | Opaque n -> pr " struct %s_%s_data *_data = %s;\n" name cb_name n | String n | UInt64 n -> () (* The following not yet implemented for callbacks XXX *) | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ | Callback _ | CallbackPersist _ - | Flags _ | Int _ | Int64 _ | Path _ | SockAddrAndLen _ | StringList _ + | BytesPersistIn _ | BytesPersistOut _ + | Callback _ | CallbackPersist _ + | Flags _ | Int _ | Int64 _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ | UInt _ | UInt32 _ -> assert false ) args; pr "\n"; @@ -3278,13 +3288,16 @@ let print_python_binding name { args; ret } = function | ArrayAndLen (UInt32 n, len) -> pr " \"O\"" | BytesIn (n, len) -> pr " \"y#\"" + | Mutable (Int n) -> pr " \"O\"" | Opaque n -> pr " \"O\"" | String n -> pr " \"s\"" | UInt64 n -> pr " \"K\"" (* The following not yet implemented for callbacks XXX *) | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ | Callback _ | CallbackPersist _ - | Flags _ | Int _ | Int64 _ | Path _ | SockAddrAndLen _ | StringList _ + | BytesPersistIn _ | BytesPersistOut _ + | Callback _ | CallbackPersist _ + | Flags _ | Int _ | Int64 _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ | UInt _ | UInt32 _ -> assert false ) args; pr " \")\""; @@ -3292,13 +3305,16 @@ let print_python_binding name { args; ret } = function | ArrayAndLen (UInt32 n, _) -> pr ", py_%s" n | BytesIn (n, len) -> pr ", %s, (int) %s" n len + | Mutable (Int n) -> pr ", py_%s" n | Opaque _ -> pr ", _data->data" | String n | UInt64 n -> pr ", %s" n (* The following not yet implemented for callbacks XXX *) | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ | Callback _ | CallbackPersist _ - | Flags _ | Int _ | Int64 _ | Path _ | SockAddrAndLen _ | StringList _ + | BytesPersistIn _ | BytesPersistOut _ + | Callback _ | CallbackPersist _ + | Flags _ | Int _ | Int64 _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ | UInt _ | UInt32 _ -> assert false ) args; pr ");\n"; @@ -3327,14 +3343,21 @@ let print_python_binding name { args; ret } = function | ArrayAndLen (UInt32 n, _) -> pr " Py_DECREF (py_%s);\n" n + | Mutable (Int n) -> + pr " PyObject *py_%s_ret = PyObject_CallMethod (py_%s, \"value\", \"\");\n" n n; + pr " *%s = PyLong_AsLong (py_%s_ret);\n" n n; + pr " Py_DECREF (py_%s_ret);\n" n; + pr " Py_DECREF (py_%s);\n" n | BytesIn _ | String _ | UInt64 _ | Opaque _ -> () (* The following not yet implemented for callbacks XXX *) | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ | Callback _ | CallbackPersist _ - | Flags _ | Int _ | Int64 _ | Path _ | SockAddrAndLen _ | StringList _ + | BytesPersistIn _ | BytesPersistOut _ + | Callback _ | CallbackPersist _ + | Flags _ | Int _ | Int64 _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ | UInt _ | UInt32 _ -> assert false ) args; pr " return ret;\n"; @@ -3388,6 +3411,8 @@ let print_python_binding name { args; ret } = | Int64 n -> pr " int64_t %s_i64;\n" n; pr " long long %s; /* really int64_t */\n" n + | Mutable arg -> + pr " PyObject *%s;\n" (List.hd (name_of_arg arg)) | Opaque _ -> () | Path n -> pr " char *%s = NULL;\n" n | SockAddrAndLen (n, _) -> @@ -3429,6 +3454,7 @@ let print_python_binding name { args; ret } = | Int _ | Int64 _ | Opaque _ + | Mutable _ | Path _ | SockAddrAndLen _ | String _ @@ -3452,6 +3478,7 @@ let print_python_binding name { args; ret } = | Flags n -> pr " \"I\"" | Int n -> pr " \"i\"" | Int64 n -> pr " \"L\"" + | Mutable _ -> pr " \"O\"" | Opaque _ -> pr " \"O\"" | Path n -> pr " \"O&\"" | SockAddrAndLen (n, _) -> pr " \"O\"" @@ -3476,6 +3503,7 @@ let print_python_binding name { args; ret } = | Flags n -> pr ", &%s" n | Int n -> pr ", &%s" n | 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 | SockAddrAndLen (n, _) -> pr ", &%s" n @@ -3525,6 +3553,8 @@ let print_python_binding name { args; ret } = | Flags n -> pr " %s_u32 = %s;\n" n n | Int _ -> () | Int64 n -> pr " %s_i64 = %s;\n" n n + | Mutable _ -> + pr " abort (); /* Mutable for normal Python parameters not impl */\n" | Opaque n -> () | Path _ -> () | SockAddrAndLen _ -> @@ -3553,6 +3583,7 @@ let print_python_binding name { args; ret } = | Flags n -> pr ", %s_u32" n | Int n -> pr ", %s" n | Int64 n -> pr ", %s_i64" n + | Mutable arg -> pr ", /* XXX */ (void *) %s" (List.hd (name_of_arg arg)) | Opaque n -> pr ", %s_data" (find_callback n) | Path n -> pr ", %s" n | SockAddrAndLen (n, _) -> pr ", /* XXX */ (void *) %s, 0" n @@ -3587,6 +3618,7 @@ let print_python_binding name { args; ret } = | Flags _ | Int _ | Int64 _ + | Mutable _ | Opaque _ | Path _ | SockAddrAndLen _ @@ -3632,6 +3664,7 @@ let print_python_binding name { args; ret } = | Flags _ -> () | Int _ -> () | Int64 _ -> () + | Mutable _ -> () | Opaque _ -> () | Path n -> pr " free (%s);\n" n | SockAddrAndLen _ -> () @@ -3730,6 +3763,7 @@ class NBD (object): | Flags n -> n, Some "0" | Int n -> n, None | Int64 n -> n, None + | Mutable arg -> List.hd (name_of_arg arg), None | Opaque n -> n, None | Path n -> n, None | SockAddrAndLen (n, _) -> n, None @@ -3820,6 +3854,7 @@ and ocaml_arg_to_string = function | OCamlArg (Flags _) -> assert false (* see above *) | OCamlArg (Int _) -> "int" | OCamlArg (Int64 _) -> "int64" + | OCamlArg (Mutable arg) -> ocaml_arg_to_string (OCamlArg arg) ^ " ref" | OCamlArg (Opaque n) -> sprintf "'%s" n | OCamlArg (Path _) -> "string" | OCamlArg (SockAddrAndLen _) -> "string" (* XXX not impl *) @@ -3985,13 +4020,13 @@ let print_ocaml_binding (name, { args; ret }) = List.map ( function | ArrayAndLen (UInt32 n, _) | BytesIn (n, _) - | String n | UInt64 n | Opaque n -> + | String n | UInt64 n | Opaque n | Mutable (Int n) -> n ^ "v" (* The following not yet implemented for callbacks XXX *) | ArrayAndLen _ | Bool _ | BytesOut _ | BytesPersistIn _ | BytesPersistOut _ | Callback _ | CallbackPersist _ - | Flags _ | Int _ | Int64 _ | Path _ + | Flags _ | Int _ | Int64 _ | Path _ | Mutable _ | SockAddrAndLen _ | StringList _ | UInt _ | UInt32 _ -> assert false ) args in @@ -4025,10 +4060,15 @@ let print_ocaml_binding (name, { args; ret }) = pr " const struct callback_data *_%s = %s;\n" n n; pr " fnv = *_%s->cb;\n" n; pr " %sv = *_%s->data;\n" n n + | Mutable (Int n) -> + pr " %sv = caml_alloc_tuple (1);\n" n; + pr " Store_field (%sv, 0, Val_int (*%s));\n" n n (* The following not yet implemented for callbacks XXX *) | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ | Callback _ | CallbackPersist _ - | Flags _ | Int _ | Int64 _ | Path _ | SockAddrAndLen _ | StringList _ + | BytesPersistIn _ | BytesPersistOut _ + | Callback _ | CallbackPersist _ + | Flags _ | Int _ | Int64 _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ | UInt _ | UInt32 _ -> assert false ) args; @@ -4047,6 +4087,15 @@ let print_ocaml_binding (name, { args; ret }) = pr " caml_format_exception (Extract_exception (rv)));\n"; pr " CAMLreturnT (int, -1);\n"; pr " }\n"; + + List.iter ( + function + (* Mutable must be copied back to the C pointer. *) + | Mutable (Int n) -> + pr " *%s = Int_val (Field (%sv, 0));\n" n n + | _ -> () + ) args; + pr "\n"; pr " CAMLreturnT (int, 0);\n"; pr "}\n"; @@ -4153,6 +4202,7 @@ let print_ocaml_binding (name, { args; ret }) = pr " int %s = Int_val (%sv);\n" n n | OCamlArg (Int64 n) -> pr " int64_t %s = Int64_val (%sv);\n" n n + | OCamlArg (Mutable _) -> assert false | OCamlArg (Opaque n) -> (match find_callback n with | Callback (cb_name, _) -> @@ -4252,6 +4302,7 @@ let print_ocaml_binding (name, { args; ret }) = | OCamlArg (Flags _) | OCamlArg (Int _) | OCamlArg (Int64 _) + | OCamlArg (Mutable _) | OCamlArg (Opaque _) | OCamlArg (Path _) | OCamlArg (String _) -- 2.22.0

2 3

[libnbd PATCH] states: Never block state machine inside REPLY
by Eric Blake 25 Jun '19

25 Jun '19

When processing a server reply within the REPLY subgroup, we will often hit a situation where recv() requires us to block until the next NotifyRead. But since NotifyRead is the only permitted external action while in this group, we are effectively blocking CmdIssue and NotifyWrite events from happening until the server finishes the in-progress reply, even though those events have no strict dependence on the server's progress. The solution is similar to commit dd101bde - any time we need to pause the reply cycle, we need to save enough information to recall where we left off then return to the READY state, then teach REPLY.START to check if we are starting a fresh reply or resuming an earlier reply. The state machine will still be blocked until the next POLLIN, but now is in a position to also accept CmdIssue and NotifyWrite without delay. With this patch in place, the only time is_state_processing is true is during the ISSUE_COMMAND group when it is blocked on NotifyWrite. Thus, once handshaking is complete, we can reliably equate nbd_aio_get_direction() == DIRECTION_READ with is_ready(), nbd_aio_get_direction() == DIRECTION_BOTH with is_processing() in the ISSUE_COMMAND substate. Oddly enough, I am not getting any measurable performance difference with this patch applied and using examples/threaded-reads-and-writes coupled with nbdkit. My explanation is that in the common case, once a server has something to send, it is going to send the entire reply as fast as it can, rather than sending a partial reply and waiting; and our attempts to keep up to 64 commands in flight mean that our real bottleneck is not sending our next command (even if we have to wait for the server's reply to finish), but at the server (if we are saturating the server's amount of in-flight requests, the server won't read our next request until it has finished its current reply). Still, I'm sure that it is possible to construct corner cases where this shows more of an effect. --- Applies on top of my series to add nbd_aio_pread_callback. generator/generator | 22 +++++++++--------- generator/states-reply-simple.c | 4 ++++ generator/states-reply-structured.c | 32 +++++++++++++++++++++++++ generator/states-reply.c | 36 ++++++++++++++++++++++++++++- lib/internal.h | 1 + 5 files changed, 83 insertions(+), 12 deletions(-) diff --git a/generator/generator b/generator/generator index 630260b..68a4fdf 100755 --- a/generator/generator +++ b/generator/generator @@ -682,14 +682,14 @@ and reply_state_machine = [ default_state with name = "START"; comment = "Prepare to receive a reply from the remote server"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { default_state with name = "RECV_REPLY"; comment = "Receive a reply from the remote server"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { @@ -723,7 +723,7 @@ and simple_reply_state_machine = [ default_state with name = "RECV_READ_PAYLOAD"; comment = "Receiving the read payload for a simple reply"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; ] @@ -740,7 +740,7 @@ and structured_reply_state_machine = [ default_state with name = "RECV_REMAINING"; comment = "Receiving the remaining part of a structured reply"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { @@ -754,49 +754,49 @@ and structured_reply_state_machine = [ default_state with name = "RECV_ERROR"; comment = "Receive a structured reply error header"; - external_events = [ NotifyRead, "" ]; + external_events = [] }; State { default_state with name = "RECV_ERROR_MESSAGE"; comment = "Receive a structured reply error message"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { default_state with name = "RECV_ERROR_TAIL"; comment = "Receive a structured reply error tail"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { default_state with name = "RECV_OFFSET_DATA"; comment = "Receive a structured reply offset-data header"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { default_state with name = "RECV_OFFSET_DATA_DATA"; comment = "Receive a structured reply offset-data block of data"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { default_state with name = "RECV_OFFSET_HOLE"; comment = "Receive a structured reply offset-hole header"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { default_state with name = "RECV_BS_ENTRIES"; comment = "Receive a structured reply block-status payload"; - external_events = [ NotifyRead, "" ]; + external_events = []; }; State { diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c index ddc91ce..3b63d07 100644 --- a/generator/states-reply-simple.c +++ b/generator/states-reply-simple.c @@ -53,6 +53,10 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: + save_reply_state (h); + SET_NEXT_STATE (%.READY); + return 0; case 0: /* guaranteed by START */ assert (cmd); diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index 00659de..594525e 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -38,6 +38,10 @@ REPLY.STRUCTURED_REPLY.RECV_REMAINING: switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: + save_reply_state (h); + SET_NEXT_STATE (%.READY); + return 0; case 0: SET_NEXT_STATE (%CHECK); } return 0; @@ -154,6 +158,10 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: + save_reply_state (h); + SET_NEXT_STATE (%.READY); + return 0; case 0: length = be32toh (h->sbuf.sr.structured_reply.length); msglen = be16toh (h->sbuf.sr.payload.error.error.len); @@ -176,6 +184,10 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: + save_reply_state (h); + SET_NEXT_STATE (%.READY); + return 0; case 0: length = be32toh (h->sbuf.sr.structured_reply.length); msglen = be16toh (h->sbuf.sr.payload.error.error.len); @@ -219,6 +231,10 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: + save_reply_state (h); + SET_NEXT_STATE (%.READY); + return 0; case 0: error = be32toh (h->sbuf.sr.payload.error.error.error); type = be16toh (h->sbuf.sr.structured_reply.type); @@ -268,6 +284,10 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: + save_reply_state (h); + SET_NEXT_STATE (%.READY); + return 0; case 0: length = be32toh (h->sbuf.sr.structured_reply.length); offset = be64toh (h->sbuf.sr.payload.offset_data.offset); @@ -324,6 +344,10 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: + save_reply_state (h); + SET_NEXT_STATE (%.READY); + return 0; case 0: length = be32toh (h->sbuf.sr.structured_reply.length); offset = be64toh (h->sbuf.sr.payload.offset_data.offset); @@ -349,6 +373,10 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: + save_reply_state (h); + SET_NEXT_STATE (%.READY); + return 0; case 0: offset = be64toh (h->sbuf.sr.payload.offset_hole.offset); length = be32toh (h->sbuf.sr.payload.offset_hole.length); @@ -410,6 +438,10 @@ switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: + save_reply_state (h); + SET_NEXT_STATE (%.READY); + return 0; case 0: length = be32toh (h->sbuf.sr.structured_reply.length); diff --git a/generator/states-reply.c b/generator/states-reply.c index 54f98c5..99e54f6 100644 --- a/generator/states-reply.c +++ b/generator/states-reply.c @@ -16,10 +16,43 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ -/* State machine for receiving reply messages from the server. */ +#include <assert.h> + +/* State machine for receiving reply messages from the server. + * + * Note that we never block while in this sub-group. If there is + * insufficient data to finish parsing a reply, requiring us to block + * until POLLIN, we instead track where in the state machine we left + * off, then return to READY to actually block. Then, on entry to + * REPLY.START, we can tell if this is the start of a new reply (rlen + * is 0, stay put), a continuation of the preamble (reply_cmd is NULL, + * resume with RECV_REPLY), or a continuation from any other location + * (reply_cmd contains the state to jump to). + */ + +static void +save_reply_state (struct nbd_handle *h) +{ + assert (h->reply_cmd); + assert (h->rlen); + h->reply_cmd->state = get_next_state (h); +} + +/*----- End of prologue. -----*/ /* STATE MACHINE */ { REPLY.START: + /* If rlen is non-zero, we are resuming an earlier reply cycle. */ + if (h->rlen > 0) { + if (h->reply_cmd) { + assert (nbd_internal_is_state_processing (h->reply_cmd->state)); + SET_NEXT_STATE (h->reply_cmd->state); + } + else + SET_NEXT_STATE (%RECV_REPLY); + return 0; + } + /* This state is entered when a read notification is received in the * READY state. Therefore we know the socket is readable here. * Reading a zero length now would indicate that the socket has been @@ -66,6 +99,7 @@ REPLY.RECV_REPLY: switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; + case 1: SET_NEXT_STATE (%.READY); return 0; case 0: SET_NEXT_STATE (%CHECK_SIMPLE_OR_STRUCTURED_REPLY); } return 0; diff --git a/lib/internal.h b/lib/internal.h index a1e27df..662ff7a 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -253,6 +253,7 @@ struct command_in_flight { uint32_t count; void *data; /* Buffer for read/write */ struct command_cb cb; + enum state state; /* State to resume with on next POLLIN */ bool data_seen; /* For read, true if at least one data chunk seen */ uint32_t error; /* Local errno value */ }; -- 2.20.1

2 6