[libnbd PATCH 0/2] Tighten URI parser
by Eric Blake
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
4 years, 10 months
[libnbd PATCH] pread_structured: Change callback type to use Mutable error
by Eric Blake
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
4 years, 10 months
Re: [Libguestfs] [libguestfs/libguestfs] when compiling v1.40.2: rake aborted! (#40)
by Richard W.M. Jones
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
4 years, 10 months
[PATCH 1/9] Rust bindings: Add Rust bindings
by Hiroyuki Katsura
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)
4 years, 10 months
Few libnbd questions/concerns
by Martin Kletzander
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
4 years, 10 months
[nbdkit PATCH] iso: Shell-quote an alternative isoprog
by Eric Blake
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
4 years, 10 months
[nbdkit PATCH v2 0/2] adding nbdkit --run '$uri'
by Eric Blake
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
4 years, 10 months
[nbdkit PATCH] captive: Support $uri in --run
by Eric Blake
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
4 years, 11 months
[PATCH libnbd] generator: Add Mutable type to the generator.
by Richard W.M. Jones
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
4 years, 11 months
[libnbd PATCH] states: Never block state machine inside REPLY
by Eric Blake
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
4 years, 11 months