[nbdkit PATCH] blocksize: Fix .extents when plugin changes type within minblock
by Eric Blake
It is easy to demonstrate that our blocksize filter has a bug: if the
minblock= setting is a higher granularity than the underlying plugin
actually supports, and the client is trying to collect block status
for the entire disk, a mid-block transition in the plugin can result
in the filter rounding a request so small that it no longer makes
progress, causing the client to see:
nbd.Error: nbd_block_status: block-status: command failed: Invalid argument (EINVAL)
Better is to round mid-block transitions up to the block size; even
though the client can make read/write requests smaller than the block
alignment, they should not see transitions in status at that
granularity.
Fixes: 2515532316
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I'm pushing this already since the tests prove it makes a difference,
but it was tricky enough that I thought it worth documenting on-list.
filters/blocksize/Makefile.am | 6 +-
tests/Makefile.am | 4 +-
filters/blocksize/blocksize.c | 40 ++++++++++---
tests/test-blocksize-extents.sh | 102 ++++++++++++++++++++++++++++++++
4 files changed, 140 insertions(+), 12 deletions(-)
create mode 100755 tests/test-blocksize-extents.sh
diff --git a/filters/blocksize/Makefile.am b/filters/blocksize/Makefile.am
index 54a09bdb..0cb6b1f8 100644
--- a/filters/blocksize/Makefile.am
+++ b/filters/blocksize/Makefile.am
@@ -1,5 +1,5 @@
# nbdkit
-# Copyright (C) 2018 Red Hat Inc.
+# Copyright (C) 2018-2020 Red Hat Inc.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -43,12 +43,16 @@ nbdkit_blocksize_filter_la_SOURCES = \
nbdkit_blocksize_filter_la_CPPFLAGS = \
-I$(top_srcdir)/include \
-I$(top_srcdir)/common/include \
+ -I$(top_srcdir)/common/utils \
$(NULL)
nbdkit_blocksize_filter_la_CFLAGS = $(WARNINGS_CFLAGS)
nbdkit_blocksize_filter_la_LDFLAGS = \
-module -avoid-version -shared $(SHARED_LDFLAGS) \
-Wl,--version-script=$(top_srcdir)/filters/filters.syms \
$(NULL)
+nbdkit_blocksize_filter_la_LIBADD = \
+ $(top_builddir)/common/utils/libutils.la \
+ $(NULL)
if HAVE_POD
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 481291e6..c0c7e981 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1199,8 +1199,8 @@ test_layers_filter3_la_LDFLAGS = \
$(NULL)
# blocksize filter test.
-TESTS += test-blocksize.sh
-EXTRA_DIST += test-blocksize.sh
+TESTS += test-blocksize.sh test-blocksize-extents.sh
+EXTRA_DIST += test-blocksize.sh test-blocksize-extents.sh
# cache filter test.
TESTS += \
diff --git a/filters/blocksize/blocksize.c b/filters/blocksize/blocksize.c
index 8504f985..c3a2d60d 100644
--- a/filters/blocksize/blocksize.c
+++ b/filters/blocksize/blocksize.c
@@ -43,6 +43,7 @@
#include <nbdkit-filter.h>
+#include "cleanup.h"
#include "minmax.h"
#include "rounding.h"
@@ -372,16 +373,37 @@ blocksize_extents (struct nbdkit_next_ops *next_ops, void *nxdata,
void *handle, uint32_t count, uint64_t offset,
uint32_t flags, struct nbdkit_extents *extents, int *err)
{
- /* Ask the plugin for blocksize-aligned data. Since the extents
- * list start is set to the real offset, everything before the
- * offset is ignored automatically. Also we only need to ask for
- * maxlen of data, because it's fine to return less than the full
- * count as long as we're making progress.
+ /* Ask the plugin for blocksize-aligned data. Copying that into the
+ * callers' extents will then take care of truncating unaligned
+ * ends. Also we only need to ask for maxlen of data, because it's
+ * fine to return less than the full count as long as we're making
+ * progress.
*/
- return next_ops->extents (nxdata,
- MIN (count, maxlen),
- ROUND_DOWN (offset, minblock),
- flags, extents, err);
+ CLEANUP_EXTENTS_FREE struct nbdkit_extents *extents2 = NULL;
+ size_t i;
+ struct nbdkit_extent e;
+
+ extents2 = nbdkit_extents_new (ROUND_DOWN (offset, minblock),
+ ROUND_UP (offset + count, minblock));
+ if (extents2 == NULL) {
+ *err = errno;
+ return -1;
+ }
+
+ if (nbdkit_extents_aligned (next_ops, nxdata,
+ MIN (ROUND_UP (count, minblock), maxlen),
+ ROUND_DOWN (offset, minblock),
+ flags, minblock, extents2, err) == -1)
+ return -1;
+
+ for (i = 0; i < nbdkit_extents_count (extents2); ++i) {
+ e = nbdkit_get_extent (extents2, i);
+ if (nbdkit_add_extent (extents, e.offset, e.length, e.type) == -1) {
+ *err = errno;
+ return -1;
+ }
+ }
+ return 0;
}
static int
diff --git a/tests/test-blocksize-extents.sh b/tests/test-blocksize-extents.sh
new file mode 100755
index 00000000..156777ad
--- /dev/null
+++ b/tests/test-blocksize-extents.sh
@@ -0,0 +1,102 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2018-2020 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# Demonstrate a fix for a bug where blocksize used to cause extents failures
+
+source ./functions.sh
+set -e
+set -x
+
+requires nbdsh --base-allocation -c 'exit(not h.supports_uri())'
+
+# Script a server that requires 512-byte aligned requests, reports only one
+# extent at a time, and with a hole placed unaligned to 4k bounds
+exts='
+if test $3 -gt $(( 32 * 1024 )); then
+ echo "EINVAL request too large" 2>&1
+ exit 1
+fi
+if test $(( ($3|$4) & 511 )) != 0; then
+ echo "EINVAL request unaligned" 2>&1
+ exit 1
+fi
+type=
+if test $(( $4 >= 512 && $4 < 8 * 1024 )) = 1; then
+ type=hole,zero
+fi
+echo $4 512 $type
+'
+
+# We also need an nbdsh script to parse all extents, coalescing adjacent
+# types for simplicity, as well as testing some unaligned probes.
+export script='
+size = h.get_size()
+offs = 0
+entries = []
+def f (metacontext, offset, e, err):
+ global entries
+ global offs
+ assert offs == offset
+ for length, flags in zip(*[iter(e)] * 2):
+ if entries and flags == entries[-1][1]:
+ entries[-1] = (entries[-1][0] + length, flags)
+ else:
+ entries.append((length, flags))
+ offs = offs + length
+
+# Test a loop over the entire device
+while offs < size:
+ h.block_status (size - offs, offs, f)
+assert entries == [(4096, 0), (4096, 3), (57344, 0)]
+
+# Unaligned status queries must also work
+offs = 1
+entries = []
+h.block_status (1, offs, f, nbd.CMD_FLAG_REQ_ONE)
+assert entries == [(1, 0)]
+
+offs = 512
+entries = []
+h.block_status (512, offs, f)
+assert entries == [(3584, 0)]
+
+offs = 4095
+entries=[]
+while offs < 4097:
+ h.block_status (4097 - offs, offs, f, nbd.CMD_FLAG_REQ_ONE)
+assert entries == [(1, 0), (1, 3)]
+'
+
+# Now run everything
+nbdkit -U - --filter=blocksize eval minblock=4k maxlen=32k \
+ get_size='echo 64k' pread='exit 1' extents="$exts" \
+ --run 'nbdsh --base-allocation -u $uri -c "$script"'
--
2.27.0
4 years, 4 months
[RFC nbdkit PATCH 0/3] aligned .extents
by Eric Blake
Ultimately, both the blocksize and swab filters want to return aligned
extents to the client. I'm posting this as a snapshot of my work in
progress on how I plan to get there (it's not quite working yet, but
I'm done for today and wanted to at least document my ideas).
I might also add a convenience function for nbdkit_extents_offset,
since we have a number of filters that repeat the same code for
translating all extents from the plugin by an offset.
Eric Blake (3):
vector: Add VECT_remove
extents: Add nbdkit_extents_aligned()
RFC swab: Re-enable .extents
docs/nbdkit-filter.pod | 18 +++++++++
include/nbdkit-filter.h | 5 +++
common/utils/vector.h | 10 +++++
server/extents.c | 80 +++++++++++++++++++++++++++++++++++++-
server/nbdkit.syms | 1 +
common/utils/test-vector.c | 26 ++++++++++++-
filters/swab/swab.c | 16 ++++----
7 files changed, 146 insertions(+), 10 deletions(-)
--
2.27.0
4 years, 4 months
[nbdkit PATCH] nbd: Add vsock-cid= transport option
by Eric Blake
With new enough libnbd, we already support vsock by virtue of uri=;
however, it's also nice to allow direct exposure of the
nbd_connect_vsock() api.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
plugins/nbd/nbdkit-nbd-plugin.pod | 29 +++++++++++---
plugins/nbd/nbd.c | 63 ++++++++++++++++++++++++-------
2 files changed, 73 insertions(+), 19 deletions(-)
diff --git a/plugins/nbd/nbdkit-nbd-plugin.pod b/plugins/nbd/nbdkit-nbd-plugin.pod
index e4ac40d1..bd980209 100644
--- a/plugins/nbd/nbdkit-nbd-plugin.pod
+++ b/plugins/nbd/nbdkit-nbd-plugin.pod
@@ -6,6 +6,7 @@ nbdkit-nbd-plugin - proxy / forward to another NBD server
nbdkit nbd { command=COMMAND [arg=ARG [...]] |
hostname=HOST [port=PORT] |
+ vhost-cid=CID [port=PORT] |
socket=SOCKNAME |
socket-fd=FD |
[uri=]URI }
@@ -40,9 +41,9 @@ With L<qemu-nbd(8)>, read and write qcow2 files with nbdkit.
=head1 PARAMETERS
-One of B<socket>, B<hostname> (optionally with B<port>), B<uri>,
-B<socket-fd> or B<command> must be given to specify which NBD server
-to forward to:
+One of B<socket>, B<hostname> (optionally with B<port>), B<vsock-cid>
+(optionally with B<port>), B<uri>, B<socket-fd> or B<command> must be
+given to specify which NBD server to forward to:
=over 4
@@ -71,7 +72,22 @@ This option implies C<shared=true>.
=item B<port=>PORT
Connect to the NBD server at the remote C<HOST> using a TCP socket.
-The optional port parameter overrides the default port (10809).
+The optional port parameter overrides the default port (10809), and
+may be a 16-bit number or a non-numeric string to look up the
+well-known port for a service name.
+
+=item B<vhost-cid=>CID
+
+=item B<port=>PORT
+
+Connect to the NBD server at the given vsock C<CID> (for example, in a
+guest VM, using the cid 2 will connect to a server in the host). The
+optional port parameter overrides the default port (10809), and must
+be a 32-bit number. This only works for platforms with the
+C<AF_VSOCK> family of sockets and libnbd new enough to use it;
+C<nbdkit --dump-plugin nbd> will contain C<libnbd_vsock=1> if this is
+the case. For more details on AF_VSOCK, see
+L<nbdkit-service(1)/AF_VSOCK>.
=item B<socket=>SOCKNAME
@@ -87,8 +103,9 @@ option implies C<shared=true>.
When C<uri> is supplied, decode C<URI> to determine the address to
connect to. A URI can specify a TCP connection (such as
-C<nbd://localhost:10809/export>) or a Unix socket (such as
-C<nbd+unix:///export?socket=/path/to/sock>). Remember you may need to
+C<nbd://localhost:10809/export>), a Unix socket (such as
+C<nbd+unix:///export?socket=/path/to/sock>), or a vsock connection
+(such as C<nbd+vsock:///2:10809/export>). Remember you may need to
quote the parameter to protect it from the shell.
The C<uri> parameter is only available when the plugin was compiled
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index e446d52b..3f84375b 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -46,6 +46,7 @@
#include <semaphore.h>
#include <poll.h>
#include <fcntl.h>
+#include <sys/socket.h>
#include <libnbd.h>
@@ -61,6 +62,12 @@
DEFINE_VECTOR_TYPE(string_vector, const char *);
+#if !defined AF_VSOCK || !LIBNBD_HAVE_NBD_CONNECT_VSOCK
+#define USE_VSOCK 0
+#else
+#define USE_VSOCK 1
+#endif
+
/* The per-transaction details */
struct transaction {
int64_t cookie;
@@ -87,8 +94,15 @@ static char *sockname;
/* Connect to server via TCP socket */
static const char *hostname;
+
+/* Valid with TCP or VSOCK */
static const char *port;
+/* Connect to server via AF_VSOCK socket */
+static const char *raw_cid;
+static uint32_t cid;
+static uint32_t vport;
+
/* Connect to a command. */
static string_vector command = empty_vector;
@@ -126,11 +140,8 @@ nbdplug_unload (void)
free (command.ptr); /* the strings are statically allocated */
}
-/* Called for each key=value passed on the command line. This plugin
- * accepts socket=<sockname>, hostname=<hostname>/port=<port>, or
- * [uri=]<uri> (exactly one connection required), and optional
- * parameters export=<name>, retry=<n>, shared=<bool> and various
- * tls settings.
+/* Called for each key=value passed on the command line. See
+ * nbdplug_config_help for the various keys recognized.
*/
static int
nbdplug_config (const char *key, const char *value)
@@ -148,6 +159,10 @@ nbdplug_config (const char *key, const char *value)
hostname = value;
else if (strcmp (key, "port") == 0)
port = value;
+ else if (strcmp (key, "vsock_cid") == 0 ||
+ strcmp (key, "vsock-cid") == 0 ||
+ strcmp (key, "cid") == 0)
+ raw_cid = value;
else if (strcmp (key, "uri") == 0)
uri = value;
else if (strcmp (key, "command") == 0 || strcmp (key, "arg") == 0) {
@@ -220,23 +235,25 @@ static int
nbdplug_config_complete (void)
{
int c = !!sockname + !!hostname + !!uri +
- (command.size > 0) + (socket_fd >= 0);
+ (command.size > 0) + (socket_fd >= 0) + !!raw_cid;
/* Check the user passed exactly one connection parameter. */
if (c > 1) {
nbdkit_error ("cannot mix Unix ‘socket’, TCP ‘hostname’/‘port’, "
- "‘command’, ‘socket-fd’ and ‘uri’ parameters");
+ "‘vhost-cid’, ‘command’, ‘socket-fd’ and ‘uri’ parameters");
return -1;
}
if (c == 0) {
- nbdkit_error ("exactly one of ‘socket’, ‘hostname’, ‘command’, ‘socket-fd’ "
- "and ‘uri’ parameters must be specified");
+ nbdkit_error ("exactly one of ‘socket’, ‘hostname’, ‘vhost-cid’, "
+ "‘command’, ‘socket-fd’ and ‘uri’ parameters must be "
+ "specified");
return -1;
}
- /* Port, if present, should only be used with hostname. */
- if (port && !hostname) {
- nbdkit_error ("‘port’ parameter should only be used with ‘hostname’");
+ /* Port, if present, should only be used with hostname or vsock-cid. */
+ if (port && !(hostname || raw_cid)) {
+ nbdkit_error ("‘port’ parameter should only be used with ‘hostname’ or "
+ "‘vsock-cid’");
return -1;
}
@@ -266,6 +283,18 @@ nbdplug_config_complete (void)
if (!port)
port = "10809";
}
+ else if (raw_cid) {
+#if !USE_VSOCK
+ nbdkit_error ("libnbd was compiled without vsock support");
+ return -1;
+#else
+ if (!port)
+ port = "10809";
+ if (nbdkit_parse_uint32_t ("vsock_cid", raw_cid, &cid) == -1 ||
+ nbdkit_parse_uint32_t ("port", port, &vport) == -1)
+ return -1;
+#endif
+ }
else if (command.size > 0) {
/* Add NULL sentinel to the command. */
if (string_vector_append (&command, NULL) == -1) {
@@ -320,7 +349,8 @@ nbdplug_after_fork (void)
"[uri=]<URI> URI of an NBD socket to connect to (if supported).\n" \
"socket=<SOCKNAME> The Unix socket to connect to.\n" \
"hostname=<HOST> The hostname for the TCP socket to connect to.\n" \
- "port=<PORT> TCP port or service name to use (default 10809).\n" \
+ "port=<PORT> TCP/VHOST port or service name to use (default 10809).\n" \
+ "vhost_cid=<CID> The cid for the VSOCK socket to connect to.\n" \
"command=<COMMAND> Command to run.\n" \
"arg=<ARG> Parameters for command.\n" \
"socket-fd=<FD> Socket file descriptor to connect to.\n" \
@@ -346,6 +376,7 @@ nbdplug_dump_plugin (void)
printf ("libnbd_version=%s\n", nbd_get_version (nbd));
printf ("libnbd_tls=%d\n", nbd_supports_tls (nbd));
printf ("libnbd_uri=%d\n", nbd_supports_uri (nbd));
+ printf ("libnbd_vsock=%d\n", USE_VSOCK);
nbd_close (nbd);
}
@@ -545,6 +576,12 @@ nbdplug_open_handle (int readonly)
r = nbd_connect_unix (h->nbd, sockname);
else if (hostname)
r = nbd_connect_tcp (h->nbd, hostname, port);
+ else if (raw_cid)
+#if !USE_VSOCK
+ abort ();
+#else
+ r = nbd_connect_vsock (h->nbd, cid, vport);
+#endif
else if (command.size > 0)
r = nbd_connect_systemd_socket_activation (h->nbd, (char **) command.ptr);
else if (socket_fd >= 0)
--
2.27.0
4 years, 4 months
[nbdkit PATCH] RFC tests: Avoid odd test behavior under NDEBUG
by Eric Blake
While we support compilation with CFLAGS=-DNDEBUG for the brave user
desiring to avoid assertion overhead, this causes the tests to be less
powerful. Fortunately, a quick test of './configure CFLAGS=-DNDEBUG'
didn't hit any test failures, but it seems better to just ensure that
assertions always work in our tests, even if they are turned off for
the main binary.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I don't know if I like this one. Since 'make check' passed in a build
with CFLAGS=-DNDEBUG, I'm not sure we need it.
common/include/test-ascii-ctype.c | 1 +
common/include/test-ascii-string.c | 1 +
common/include/test-byte-swapping.c | 1 +
common/include/test-current-dir-name.c | 1 +
common/include/test-isaligned.c | 1 +
common/include/test-ispowerof2.c | 1 +
common/include/test-iszero.c | 1 +
common/include/test-minmax.c | 1 +
common/include/test-nextnonzero.c | 1 +
common/bitmap/test-bitmap.c | 1 +
common/utils/test-quotes.c | 1 +
common/utils/test-vector.c | 1 +
tests/test-layers-filter.c | 1 +
tests/test-pause.c | 1 +
tests/test-stdio-plugin.c | 1 +
15 files changed, 15 insertions(+)
diff --git a/common/include/test-ascii-ctype.c b/common/include/test-ascii-ctype.c
index 85a78fa5..6e0b6626 100644
--- a/common/include/test-ascii-ctype.c
+++ b/common/include/test-ascii-ctype.c
@@ -34,6 +34,7 @@
#include <stdio.h>
#include <stdlib.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include "ascii-ctype.h"
diff --git a/common/include/test-ascii-string.c b/common/include/test-ascii-string.c
index 0fa4a483..6bbf9997 100644
--- a/common/include/test-ascii-string.c
+++ b/common/include/test-ascii-string.c
@@ -34,6 +34,7 @@
#include <stdio.h>
#include <stdlib.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include "ascii-string.h"
diff --git a/common/include/test-byte-swapping.c b/common/include/test-byte-swapping.c
index e95a17db..a5e61a1f 100644
--- a/common/include/test-byte-swapping.c
+++ b/common/include/test-byte-swapping.c
@@ -36,6 +36,7 @@
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include "byte-swapping.h"
diff --git a/common/include/test-current-dir-name.c b/common/include/test-current-dir-name.c
index bb2bfd12..40872f2b 100644
--- a/common/include/test-current-dir-name.c
+++ b/common/include/test-current-dir-name.c
@@ -37,6 +37,7 @@
#include <stdint.h>
#include <string.h>
#include <unistd.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include "get-current-dir-name.h"
diff --git a/common/include/test-isaligned.c b/common/include/test-isaligned.c
index adad3266..45415e37 100644
--- a/common/include/test-isaligned.c
+++ b/common/include/test-isaligned.c
@@ -36,6 +36,7 @@
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include "isaligned.h"
diff --git a/common/include/test-ispowerof2.c b/common/include/test-ispowerof2.c
index af3e3b9b..4ea7fdde 100644
--- a/common/include/test-ispowerof2.c
+++ b/common/include/test-ispowerof2.c
@@ -36,6 +36,7 @@
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include "ispowerof2.h"
diff --git a/common/include/test-iszero.c b/common/include/test-iszero.c
index 1a4f09ff..d53efb4f 100644
--- a/common/include/test-iszero.c
+++ b/common/include/test-iszero.c
@@ -36,6 +36,7 @@
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include "iszero.h"
diff --git a/common/include/test-minmax.c b/common/include/test-minmax.c
index 7d4173ca..ce559b91 100644
--- a/common/include/test-minmax.c
+++ b/common/include/test-minmax.c
@@ -36,6 +36,7 @@
#include <stdlib.h>
#include <stdint.h>
#include <limits.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include <float.h>
diff --git a/common/include/test-nextnonzero.c b/common/include/test-nextnonzero.c
index 86aee5ec..0c3ad5e3 100644
--- a/common/include/test-nextnonzero.c
+++ b/common/include/test-nextnonzero.c
@@ -36,6 +36,7 @@
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include "nextnonzero.h"
diff --git a/common/bitmap/test-bitmap.c b/common/bitmap/test-bitmap.c
index cbbbbef8..2323bb7f 100644
--- a/common/bitmap/test-bitmap.c
+++ b/common/bitmap/test-bitmap.c
@@ -40,6 +40,7 @@
#include <stdarg.h>
#include <string.h>
#include <errno.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include <nbdkit-plugin.h>
diff --git a/common/utils/test-quotes.c b/common/utils/test-quotes.c
index 72e54fc7..0783be27 100644
--- a/common/utils/test-quotes.c
+++ b/common/utils/test-quotes.c
@@ -37,6 +37,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include <stdbool.h>
diff --git a/common/utils/test-vector.c b/common/utils/test-vector.c
index a2c0db10..be3696b7 100644
--- a/common/utils/test-vector.c
+++ b/common/utils/test-vector.c
@@ -35,6 +35,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include "vector.h"
diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c
index 66d80835..b77df258 100644
--- a/tests/test-layers-filter.c
+++ b/tests/test-layers-filter.c
@@ -35,6 +35,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include <nbdkit-filter.h>
diff --git a/tests/test-pause.c b/tests/test-pause.c
index ba772d4f..09f76c76 100644
--- a/tests/test-pause.c
+++ b/tests/test-pause.c
@@ -40,6 +40,7 @@
#include <string.h>
#include <unistd.h>
#include <time.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include <sys/types.h>
#include <sys/socket.h>
diff --git a/tests/test-stdio-plugin.c b/tests/test-stdio-plugin.c
index 6af1ba90..f29bf3f7 100644
--- a/tests/test-stdio-plugin.c
+++ b/tests/test-stdio-plugin.c
@@ -32,6 +32,7 @@
#include <config.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
#include <assert.h>
#include <fcntl.h>
#include <stdbool.h>
--
2.27.0
4 years, 4 months
Re: [Libguestfs] Makefile probably incorrect
by Richard W.M. Jones
On Mon, Jul 06, 2020 at 07:52:42PM -0300, Ezequiel Santos wrote:
> Actually, I was trying to compile it from the AUR repository (
> https://aur.archlinux.org/packages/libguestfs-git/) when I came across this
> problem.
> Now, answering your question:
> *1. Are there any files under common/ ?*
> Yes. There are several files there in, including Makefile.am,
> guestfs_protocol.x, but not guestfs_protocol.c or guestfs_protocol.h
>
> However, I was talking to the package maintainer and he's stated that
>
> > rpcgen is used to create the files that are missed in your builds:
> >
> > $ grep -n rpcgen makepkg.out
> > 3426:checking for rpcgen... rpcgen
> > 4712:checking for rpcgen... rpcgen
> > 5409:rpcgen -c -o guestfs_protocol.c-t guestfs_protocol.x
> > 5410:rpcgen -h -o guestfs_protocol.h-t guestfs_protocol.x
> > 5749:rpcgen -h -o rc_protocol.h-t rc_protocol.x
> > 5750:rpcgen -c -o rc_protocol.c-t rc_protocol.x
> >
> > So I think the solution would be to add *rpcsvc-proto* (provider of
> > rpcgen) to makedepends. On feedback that it works I'll make this update.
> >
>
> I installed *rpcsvc-proto *and indeed it created the missing files
> (including the .c and .h file mentioned above) and it eliminated that
> compiler error (just now I got a different one, which I'll try solving
> later),
>
> However, it is of my concern that, as the maintainer of that package
> mentioned:
>
> > This is a bit messy:
> >
> > 1. error should have been triggered at configure stage when rpcgen was
> > not found, rather than at compile stage.
> > 2. http://libguestfs.org/guestfs-building.1.html does mention in
> > section *Full list of requirements* that *The rpcgen tool is optional,
> > except if you want to compile from git*. However, there is section *BUILDING
> > FROM GIT* which also lists specific dependencies, but not rpcgen; so
> > if you are directly looking for git build instructions, *you'll miss
> > it. *
> >
> > I personally tried agree with him, because I tried to find info on how to
> properly compile this and although I didn't try manually fetching the code
> from git, I could find enough information to solve this problem and thus I
> think *there is *something to be fixed upstream, even if it's just
> documentation.
Why is the package being built from git? The tarballs contain all the
files needed, and rpcgen isn't needed when building from the tarball.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
4 years, 4 months
[PATCH] python: Add type hints
by Sam Eiderman
Since support for python2 is dropped we can use the new python3 syntax
for type hints.
Signed-off-by: Sam Eiderman <sameid(a)google.com>
---
generator/python.ml | 39 +++++++++++++++++++++++++++++++++++----
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/generator/python.ml b/generator/python.ml
index f0d6b5d96..3640ee39a 100644
--- a/generator/python.ml
+++ b/generator/python.ml
@@ -685,6 +685,7 @@ logvols = g.lvs()
import os
import sys
import libguestfsmod
+from typing import Union, List, Tuple, Optional
";
@@ -802,14 +803,44 @@ class GuestFS(object):
fun f ->
let ret, args, optargs = f.style in
let len_name = String.length f.name in
+ let ret_type_hint =
+ match ret with
+ | RErr -> "None"
+ | RInt _ | RInt64 _ -> "int"
+ | RBool _ -> "bool"
+ | RConstOptString _ -> "Optional[str]"
+ | RConstString _ | RString _ -> "str"
+ | RBufferOut _ -> "bytes"
+ | RStringList _ -> "List[str]"
+ | RStruct _ -> "dict"
+ | RStructList _ -> "List[dict]"
+ | RHashtable _ -> "Union[List[Tuple[str, str]], dict]" in
+ let type_hint_of_argt arg =
+ match arg with
+ | String _ -> ": str"
+ | OptString _ -> ": Optional[str]"
+ | Bool _ -> ": bool"
+ | Int _ | Int64 _ -> ": int"
+ | BufferIn _ -> ": bytes"
+ | StringList _ -> ": List[str]"
+ | Pointer _ -> ""
+ in
+ let type_hint_of_optargt optarg =
+ match optarg with
+ | OBool _ -> "bool"
+ | OInt _ | OInt64 _ -> "int"
+ | OString _ -> "str"
+ | OStringList _ -> "List[str]"
+ in
let decl_string =
"self" ^
- map_join (fun arg ->sprintf ", %s" (name_of_argt arg))
+ map_join (fun arg ->sprintf ", %s%s" (name_of_argt arg) (type_hint_of_argt arg))
args ^
- map_join (fun optarg -> sprintf ", %s=None" (name_of_optargt optarg))
- optargs in
+ map_join (fun optarg -> sprintf ", %s: Optional[%s] = None" (name_of_optargt optarg) (type_hint_of_optargt optarg))
+ optargs ^
+ ") -> " ^ ret_type_hint ^ ":" in
pr "\n";
- pr " def %s(%s):\n"
+ pr " def %s(%s\n"
f.name (indent_python decl_string (9 + len_name) 78);
if is_documented f then (
--
2.27.0.212.ge8ba1cc988-goog
4 years, 4 months
[PATCH nbdkit 0/9] nbd: Implement command= and socket-fd= parameters.
by Richard W.M. Jones
I fixed the deadlock - turned out to be an actual bug in the nbd
plugin (see patch 8).
I changed the command syntax so it's now:
nbdkit nbd command=qemu arg=-f arg=qcow2 arg=/path/to/disk.qcow2
Nir wrote:
18:08 < nsoffer> rwmjones: regarding the nbd proxy patches, did you have specific flow that help us?
18:08 < nsoffer> rwmjones: or this is just a way to support qcow2 in the nbdkit pipeline?
A bit of both - it's nice if nbdkit can "open" qcow2 files.
Rich.
4 years, 4 months
Makefile probably incorrect
by Ezequiel Santos
I am a Manjaro user and was compiling libguestfs today from the AUR (it
doesn't reside on Arch/Manjaro repos) and got the following error:
"make[2]: Leaving directory
'/home/matador/AUR/libguestfs-git/src/libguestfs/common/errnostring'
Making all in common/protocol
make[2]: Entering directory
'/home/matador/AUR/libguestfs-git/src/libguestfs/common/protocol'
make[2]: *** No rule to make target 'guestfs_protocol.c', needed by 'all'.
Stop.
make[2]: Leaving directory
'/home/matador/AUR/libguestfs-git/src/libguestfs/common/protocol'
make[1]: *** [Makefile:2380: all-recursive] Error 1
make[1]: Leaving directory '/home/matador/AUR/libguestfs-git/src/libguestfs'
make: *** [Makefile:2280: all] Erro 2
==> ERRO: Uma falha ocorreu em build()."
Apparently a makefile error, as there's no
'guestfs_protocol.c' file in that directory (or in the whole source code,
in fact), although it is referenced by the Makefile.
4 years, 4 months
[PATCH v3] python: Fix UnicodeError in inspect_list_applications2() (RHBZ#1684004)
by Sam Eiderman
The python3 bindings create PyUnicode objects from application strings
on the guest (i.e. installed rpm, deb packages).
It is documented that rpm package fields such as description should be
utf8 encoded - however in some cases they are not a valid unicode
string, on SLES11 SP4 the encoding of the description of the following
packages is latin1 and they fail to be converted to unicode using
guestfs_int_py_fromstring() (which invokes PyUnicode_FromString()):
PackageKit
aaa_base
coreutils
dejavu
desktop-data-SLED
gnome-utils
hunspell
hunspell-32bit
hunspell-tools
libblocxx6
libexif
libgphoto2
libgtksourceview-2_0-0
libmpfr1
libopensc2
libopensc2-32bit
liborc-0_4-0
libpackagekit-glib10
libpixman-1-0
libpixman-1-0-32bit
libpoppler-glib4
libpoppler5
libsensors3
libtelepathy-glib0
m4
opensc
opensc-32bit
permissions
pinentry
poppler-tools
python-gtksourceview
splashy
syslog-ng
tar
tightvnc
xorg-x11
xorg-x11-xauth
yast2-mouse
Fix this by globally changing guestfs_int_py_fromstring()
and guestfs_int_py_fromstringsize() to fallback to latin1 decoding if
utf-8 decoding fails.
Using the "strict" error handler doesn't matter in the case of latin1
and has the same effect of "replace":
https://docs.python.org/3/library/codecs.html#error-handlers
Signed-off-by: Sam Eiderman <sameid(a)google.com>
---
python/handle.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/python/handle.c b/python/handle.c
index 2fb8c18f0..fe89dc58a 100644
--- a/python/handle.c
+++ b/python/handle.c
@@ -387,7 +387,7 @@ guestfs_int_py_fromstring (const char *str)
#if PY_MAJOR_VERSION < 3
return PyString_FromString (str);
#else
- return PyUnicode_FromString (str);
+ return guestfs_int_py_fromstringsize (str, strlen (str));
#endif
}
@@ -397,7 +397,12 @@ guestfs_int_py_fromstringsize (const char *str, size_t size)
#if PY_MAJOR_VERSION < 3
return PyString_FromStringAndSize (str, size);
#else
- return PyUnicode_FromStringAndSize (str, size);
+ PyObject *s = PyUnicode_FromString (str);
+ if (s == NULL) {
+ PyErr_Clear ();
+ s = PyUnicode_Decode (str, strlen(str), "latin1", "strict");
+ }
+ return s;
#endif
}
--
2.26.2.303.gf8c07b1a785-goog
4 years, 4 months
