[PATCH nbdkit] tests: Remove QEMU_IO / HAVE_QEMU_IO.
by Richard W.M. Jones
Currently we test for qemu-img, socat, ss, certtool, etc at run time,
but we test for qemu-io at compile time (in ./configure). This commit
removes this inconsistency.
I would consider the opposite patch (which makes qemu-img etc tested
at configure time).
The main advantage of testing for these binaries at run time is that
tests are not "silently" omitted. Instead tests with insufficient
dependencies are now reported as ‘SKIP’, which I think gives packagers
a better overview of their coverage of the test suite.
I recently changed the Fedora build so we run the test suite on every
architecture, and it's interesting that x86_64 is now running and
passing 40 tests, but some architectures (s390x for instance) only
manage 17 passes with the rest being skipped for multiple reasons.
Disadvantages:
- Can't override the location of these binaries eg by setting
QEMU_IO=/opt/qemu/bin/qemu-io ./configure
- ./configure doesn't report missing soft dependencies.
There are also other test dependencies that we test at compile time
but we cannot change that (guestfish, libguestfs).
Rich.
6 years, 11 months
virt-sparsify and iSCSI
by Shmuel Melamud
Hi!
This question emerged during testing of oVirt sparsify integration.
oVirt now is able to run virt-sparsify on VM disks. But virt-sparsify
seems to have no effect on iSCSI disks.
Does virt-sparsify work on iSCSI disks? Maybe in sum situation, with
some configuration of iSCSI server, with some specific virt-sparsify
options?
Shmuel
6 years, 11 months
nbdkit filter error handling
by Eric Blake
I've given some more thoughts to error handling in nbdkit filters, and
am writing this while it is still fresh on my mind (even though I won't
have time to code any of it until next week).
Right now, plugins are allowed to call either nbdkit_set_error OR to
leave a value in errno; the latter works only if errno_is_preserved was
set. If a plugin returns -1 but does not set an error through either
method, then nbdkit assumes EIO as the error value.
But filters may want/need to perform error recovery (perhaps recognizing
an ENOSPC on write, by resizing and then retrying the operation).
Filters have access to errno after a client call fails, but cannot rely
on errno having the right value - they have to be able to get at the
thread-local value that was stored during nbdkit_set_error.
Furthermore, if a filter does ANYTHING non-trivial after the client
returns an error, it is likely to clobber the errno value that the
plugin set (if the client relied on errno instead of nbdkit_set_error) -
which may be okay if the filter itself wants to return a new value, but
not so good if it was just logging things but wants to preserve the
plugin's error. At the same time, if the plugin set the error via
nbdkit_set_error, then changing errno won't make a difference; the way
nbdkit handles errors is that the thread-local value takes priority and
errno is only a fallback if the thread-local value was not set.
So, we can either mandate that filters that want to do non-trivial work
after calling into next_ops must check what the plugin set (hoist the
connections.c:get_error into nbdkit_get_error), and then MUST call
nbdkit_set_error to change the error (meaning that filters should not
rely on errno for setting the value) - or we can change the semantics of
the backend and filters to say that instead of returning -1, we return
the actual error code. Plugins still return -1 for backward
compatibility, but the conversion from plugin to backend semantics in
plugins.c will convert the errno-or-nbdkit_set_error value into its
return; and then the filter stack will see the return value directly,
and can then return that same value without having to mess with either
errno or nbdkit_set_error.
At this point, I'm leaning towards this semantic change as being less
boilerplate in filters.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
6 years, 11 months
[PATCH nbdkit] tests: Rename and rework test-ipv4.sh so it tests IPv6
by Richard W.M. Jones
I wanted to change this test so it tries connections on both IPv4 &
IPv6. Having it connect on both is the easy bit. Harder was making
it not fail on machines that don't have IPv6 stack (or IPv4 in some
rare cases). TBH I wasn't able to test this, but it seems like this
should work.
In the end we want to modify nbdkit so it can listen on only certain
interfaces, but that's a bit more complicated.
Rich.
6 years, 11 months
[PATCH nbdkit] filters: cache, cow: Handle bitmap overflow on 32 bit architectures.
by Richard W.M. Jones
When compiling on armv7 it becomes clear from the compiler warnings
that the current code is wrong.
The bitmap has to be allocated in virtual memory, so use size_t to
describe the length of the bitmap. When changing the length of the
bitmap, compute the new size as an unsigned 64 bit int, and then check
whether or not it is too large to fit into size_t before casting it.
---
filters/cache/cache.c | 13 ++++++++++---
filters/cow/cow.c | 13 ++++++++++---
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/filters/cache/cache.c b/filters/cache/cache.c
index 7410f0d..9473f2c 100644
--- a/filters/cache/cache.c
+++ b/filters/cache/cache.c
@@ -72,7 +72,7 @@ static int fd = -1;
static uint8_t *bitmap;
/* Size of the bitmap in bytes. */
-static uint64_t bm_size;
+static size_t bm_size;
enum bm_entry {
BLOCK_NOT_CACHED = 0,
@@ -167,7 +167,14 @@ blk_set_size (uint64_t new_size)
{
uint8_t *new_bm;
const size_t old_bm_size = bm_size;
- size_t new_bm_size = DIV_ROUND_UP (new_size, BLKSIZE*8/2);
+ uint64_t new_bm_size_u64 = DIV_ROUND_UP (new_size, BLKSIZE*8/2);
+ size_t new_bm_size;
+
+ if (new_bm_size_u64 > SIZE_MAX) {
+ nbdkit_error ("bitmap too large for this architecture");
+ return -1;
+ }
+ new_bm_size = (size_t) new_bm_size_u64;
new_bm = realloc (bitmap, new_bm_size);
if (new_bm == NULL) {
@@ -179,7 +186,7 @@ blk_set_size (uint64_t new_size)
if (old_bm_size < new_bm_size)
memset (&bitmap[old_bm_size], 0, new_bm_size-old_bm_size);
- nbdkit_debug ("cache: bitmap resized to %" PRIu64 " bytes", new_bm_size);
+ nbdkit_debug ("cache: bitmap resized to %zu bytes", new_bm_size);
if (ftruncate (fd, new_size) == -1) {
nbdkit_error ("ftruncate: %m");
diff --git a/filters/cow/cow.c b/filters/cow/cow.c
index 2b023af..5c2fcd0 100644
--- a/filters/cow/cow.c
+++ b/filters/cow/cow.c
@@ -97,7 +97,7 @@ static int fd = -1;
static uint8_t *bitmap;
/* Size of the bitmap in bytes. */
-static uint64_t bm_size;
+static size_t bm_size;
static void
cow_load (void)
@@ -153,7 +153,14 @@ blk_set_size (uint64_t new_size)
{
uint8_t *new_bm;
const size_t old_bm_size = bm_size;
- size_t new_bm_size = DIV_ROUND_UP (new_size, BLKSIZE*8);
+ uint64_t new_bm_size_u64 = DIV_ROUND_UP (new_size, BLKSIZE*8);
+ size_t new_bm_size;
+
+ if (new_bm_size_u64 > SIZE_MAX) {
+ nbdkit_error ("bitmap too large for this architecture");
+ return -1;
+ }
+ new_bm_size = (size_t) new_bm_size_u64;
new_bm = realloc (bitmap, new_bm_size);
if (new_bm == NULL) {
@@ -165,7 +172,7 @@ blk_set_size (uint64_t new_size)
if (old_bm_size < new_bm_size)
memset (&bitmap[old_bm_size], 0, new_bm_size-old_bm_size);
- nbdkit_debug ("cow: bitmap resized to %" PRIu64 " bytes", new_bm_size);
+ nbdkit_debug ("cow: bitmap resized to %zu bytes", new_bm_size);
if (ftruncate (fd, new_size) == -1) {
nbdkit_error ("ftruncate: %m");
--
2.13.2
6 years, 11 months
RFC: async plugin support
by Shaun McDowell
I've created a workable async framework with the latest nbdkit code and a
sample file plugin using aio to service requests. This allows for a single
worker thread to issue multiple requests in parallel via a callback
mechanism. Existing locking mechanisms are preserved and if an async thread
model sets THREAD_MODEL < PARALLEL then nbdkit continues to properly
serialize.
The changes are split into 3 commits:
1) backend plumbing changes required to keep the memory for requests around
until completed
2) the async plugin backend
3) a sample plugin that uses aio and the async plugin backend
The patches would be large so rather than post them all here I've included
a link to the github diff
https://github.com/libguestfs/nbdkit/compare/master...dev-cloudbd:async
6 years, 11 months
[PATCH v2 1/3] daemon: Reimplement 'part_get_mbr_part_type' API in OCaml.
by Mykola Ivanets
Instead of parsing 'parted' output OCaml implementation relies on the following facts:
1. The function is applicable for MBR partitions only (as noted in documentation and as function name suggests).
2. An attempt to call the function for non-MBR partition fails with "part_get_mbr_part_type can only be used on MBR Partitions" error and NULL is returned.
3. MBR partition table can hold up to 4 "primary" partitions.
4. Partition with number >= 5 is logical partition
5. Extnded partition number is <= 4 and has MBR id 0x05 or 0x0f (http://thestarman.pcministry.com/asm/mbr/PartTypes.htm; https://en.wikipedia.org/wiki/Partition_type).
---
daemon/parted.c | 106 ----------------------------------------------
daemon/parted.ml | 14 ++++++
daemon/parted.mli | 2 +
generator/actions_core.ml | 1 +
4 files changed, 17 insertions(+), 106 deletions(-)
diff --git a/daemon/parted.c b/daemon/parted.c
index e5435b5..d67c6c5 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -602,112 +602,6 @@ do_part_get_name (const char *device, int partnum)
}
}
-char *
-do_part_get_mbr_part_type (const char *device, int partnum)
-{
- CLEANUP_FREE char *parttype;
- char *part_type;
-
- parttype = do_part_get_parttype (device);
- if (parttype == NULL)
- return NULL;
-
- /* machine parseable output by 'parted -m' did not provide
- * partition type info.
- * Use traditional style.
- */
- CLEANUP_FREE char *out = print_partition_table (device, false);
- if (!out)
- return NULL;
-
- CLEANUP_FREE_STRING_LIST char **lines = split_lines (out);
-
- if (!lines)
- return NULL;
-
- size_t start = 0, end = 0, row;
-
- for (row = 0; lines[row] != NULL; ++row)
- if (STRPREFIX (lines[row], "Number")) {
- start = row + 1;
- break;
- }
-
- if (start == 0) {
- reply_with_error ("parted output has no \"Number\" line");
- return NULL;
- }
-
- for (row = start; lines[row] != NULL; ++row)
- if (STREQ (lines[row], "")) {
- end = row;
- break;
- }
-
- if (end == 0) {
- reply_with_error ("parted output has no blank after end of table");
- return NULL;
- }
-
- /* Now parse the lines. */
- size_t i;
- int64_t temp_int64;
- int part_num;
- char temp_type[16] = {'\0'};
- for (i = 0, row = start; row < end; ++i, ++row) {
- if (STREQ (parttype, "gpt")) {
- memcpy (temp_type, "primary", strlen ("primary"));
- if (sscanf (lines[row], "%d%" SCNi64 "B%" SCNi64 "B%" SCNi64 "B",
- &part_num,
- &temp_int64,
- &temp_int64,
- &temp_int64) != 4) {
- reply_with_error ("could not parse row from output of parted print command: %s", lines[row]);
- return NULL;
- }
- } else {
- if (sscanf (lines[row], "%d%" SCNi64 "B%" SCNi64 "B%" SCNi64 "B" "%15s",
- &part_num,
- &temp_int64,
- &temp_int64,
- &temp_int64,
- temp_type) != 5) {
- reply_with_error ("could not parse row from output of parted print command: %s", lines[row]);
- return NULL;
- }
- }
-
- if (part_num != partnum)
- continue;
-
- if (STRPREFIX (temp_type, "primary")) {
- part_type = strdup ("primary");
- if (part_type == NULL)
- goto error;
- } else if (STRPREFIX (temp_type, "logical")) {
- part_type = strdup ("logical");
- if (part_type == NULL)
- goto error;
- } else if (STRPREFIX (temp_type, "extended")) {
- part_type = strdup ("extended");
- if (part_type == NULL)
- goto error;
- } else
- goto error;
-
- return part_type;
- }
-
- if (row == end) {
- reply_with_error ("could not find partnum: %d", partnum);
- return NULL;
- }
-
- error:
- reply_with_error ("strdup failed");
- return NULL;
-}
-
static char *
extract_uuid (const char *value)
{
diff --git a/daemon/parted.ml b/daemon/parted.ml
index ce8da8a..75d9d37 100644
--- a/daemon/parted.ml
+++ b/daemon/parted.ml
@@ -125,6 +125,20 @@ let part_get_parttype device =
| _ ->
failwithf "%s: cannot parse the output of parted" device
+let part_get_mbr_part_type device partnum =
+ let parttype = part_get_parttype device in
+ let mbr_id = part_get_mbr_id device partnum in
+
+ (* 0x05 - extended partition id within the first 1024 cylinders.
+ * 0x0f - extended partition id beyond the first 1024 cylinders.
+ *)
+ match parttype, partnum, mbr_id with
+ | "msdos", (1|2|3|4), (0x05|0x0f) -> "extended"
+ | "msdos", (1|2|3|4), _ -> "primary"
+ | "msdos", _, _ -> "logical"
+ | _, _, _ ->
+ failwithf "part_get_mbr_part_type can only be used on MBR Partitions"
+
let part_set_gpt_attributes device partnum attributes =
if partnum <= 0 then failwith "partition number must be >= 1";
diff --git a/daemon/parted.mli b/daemon/parted.mli
index d547f2f..0f59d29 100644
--- a/daemon/parted.mli
+++ b/daemon/parted.mli
@@ -28,6 +28,8 @@ val part_list : string -> partition list
val part_get_parttype : string -> string
+val part_get_mbr_part_type : string -> int -> string
+
val part_get_gpt_type : string -> int -> string
val part_get_gpt_guid : string -> int -> string
val part_get_gpt_attributes : string -> int -> int64
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index 544cb6e..307e414 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -9213,6 +9213,7 @@ All data will be zeroed, but metadata and the like is preserved." };
{ defaults with
name = "part_get_mbr_part_type"; added = (1, 29, 32);
style = RString (RPlainString, "partitiontype"), [String (Device, "device"); Int "partnum"], [];
+ impl = OCaml "Parted.part_get_mbr_part_type";
tests = [
InitEmpty, Always, TestResultString (
[["part_init"; "/dev/sda"; "mbr"];
--
2.9.5
6 years, 11 months
[nbdkit PATCH] errors: Use lighter-weight locking
by Eric Blake
Commit d02d9c9d used pthread_mutex to avoid interleaving output.
However, the standard provides flockfile() for grouping related
FILE* I/O that must not be interleaved; and that may be
lighter-weight than rolling our own locking.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
Pushing this one; as a related conversation on another mailing list
reminded me about flockfile().
src/errors.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/src/errors.c b/src/errors.c
index c1efae6..471e3ea 100644
--- a/src/errors.c
+++ b/src/errors.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013-2017 Red Hat Inc.
+ * Copyright (C) 2013-2018 Red Hat Inc.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -44,21 +44,16 @@
#include "nbdkit-plugin.h"
#include "internal.h"
-/* Used to group piecemeal message construction into atomic output. */
-static pthread_mutex_t errors_lock = PTHREAD_MUTEX_INITIALIZER;
-
static void
lock (void)
{
- int r = pthread_mutex_lock (&errors_lock);
- assert (!r);
+ flockfile (stderr);
}
static void
unlock (void)
{
- int r = pthread_mutex_unlock (&errors_lock);
- assert (!r);
+ funlockfile (stderr);
}
/* Called with lock taken. */
--
2.14.3
6 years, 11 months
