January 2018 - Libguestfs - libguestfs List Archives

[PATCH nbdkit] tests: Remove QEMU_IO / HAVE_QEMU_IO.
by Richard W.M. Jones 29 Jan '18

29 Jan '18

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.

2 3

virt-sparsify and iSCSI
by Shmuel Melamud 29 Jan '18

29 Jan '18

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

2 1

nbdkit filter error handling
by Eric Blake 27 Jan '18

27 Jan '18

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

1 0

[PATCH nbdkit v2] tests: Rename and rework test-ipv4.sh so it tests
by Richard W.M. Jones 27 Jan '18

27 Jan '18

This is my second go at reworking the ‘test-ipv4.sh’ test. Instead of merely connecting to the server using socat, use ‘qemu-img info’ so that the test is slightly more realistic. Also clean up old PID and log files before starting the test. Previous version was posted here: https://www.redhat.com/archives/libguestfs/2018-January/msg00253.html Rich.

1 1

[PATCH nbdkit] tests: Rename and rework test-ipv4.sh so it tests IPv6
by Richard W.M. Jones 26 Jan '18

26 Jan '18

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.

1 1

[PATCH nbdkit] filters: cache, cow: Handle bitmap overflow on 32 bit architectures.
by Richard W.M. Jones 26 Jan '18

26 Jan '18

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

2 2

IRC question: OCaml 4.06 and hivex
by Richard W.M. Jones 26 Jan '18

26 Jan '18

> 22:35 < cspack> has anyone tried to build hivex with ocaml 4.06? There seems to be some incompatibility there. I had to add two patches when building for Fedora: https://src.fedoraproject.org/rpms/hivex/tree/master They are both upstream, but not in a released version of hivex yet. https://github.com/libguestfs/hivex/commits/master Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/

1 0

RFC: async plugin support
by Shaun McDowell 26 Jan '18

26 Jan '18

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

1 0

[PATCH v2 1/3] daemon: Reimplement 'part_get_mbr_part_type' API in OCaml.
by Mykola Ivanets 25 Jan '18

25 Jan '18

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

1 2

[nbdkit PATCH] errors: Use lighter-weight locking
by Eric Blake 25 Jan '18

25 Jan '18

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

2 1