September 2018 - Libguestfs - libguestfs List Archives

[PATCH nbdkit v2] common: isaligned: Use a macro instead of relying on implicit truncation.
by Richard W.M. Jones 17 Sep '18

17 Sep '18

--- common/include/isaligned.h | 11 +++++------ plugins/file/file.c | 4 ++-- plugins/vddk/vddk.c | 8 ++++---- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/common/include/isaligned.h b/common/include/isaligned.h index e693820..81ce8a7 100644 --- a/common/include/isaligned.h +++ b/common/include/isaligned.h @@ -36,16 +36,15 @@ #include <assert.h> #include <stdbool.h> +#include <stdint.h> #include "ispowerof2.h" /* Return true if size is a multiple of align. align must be power of 2. */ -static inline bool -is_aligned (unsigned int size, unsigned int align) -{ - assert (is_power_of_2 (align)); - return !(size & (align - 1)); -} +#define IS_ALIGNED(size, align) ({ \ + assert (is_power_of_2 ((align))); \ + !((size) & ((align) - 1)); \ +}) #endif /* NBDKIT_ISALIGNED_H */ diff --git a/plugins/file/file.c b/plugins/file/file.c index 9d03f18..1391f62 100644 --- a/plugins/file/file.c +++ b/plugins/file/file.c @@ -397,13 +397,13 @@ file_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) #ifdef BLKZEROOUT /* For aligned range and block device, we can use BLKZEROOUT. */ - if (h->can_zeroout && is_aligned (offset | count, h->sector_size)) { + if (h->can_zeroout && IS_ALIGNED (offset | count, h->sector_size)) { uint64_t range[2] = {offset, count}; r = ioctl (h->fd, BLKZEROOUT, &range); if (r == 0) { if (file_debug_zero) - nbdkit_debug ("h->can_zeroout && is_aligned: " + nbdkit_debug ("h->can_zeroout && IS_ALIGNED: " "zero succeeded using BLKZEROOUT"); return 0; } diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c index f3b4539..9bfd4d2 100644 --- a/plugins/vddk/vddk.c +++ b/plugins/vddk/vddk.c @@ -511,11 +511,11 @@ vddk_pread (void *handle, void *buf, uint32_t count, uint64_t offset) VixError err; /* Align to sectors. */ - if (!is_aligned (offset, VIXDISKLIB_SECTOR_SIZE)) { + if (!IS_ALIGNED (offset, VIXDISKLIB_SECTOR_SIZE)) { nbdkit_error ("read is not aligned to sectors"); return -1; } - if (!is_aligned (count, VIXDISKLIB_SECTOR_SIZE)) { + if (!IS_ALIGNED (count, VIXDISKLIB_SECTOR_SIZE)) { nbdkit_error ("read is not aligned to sectors"); return -1; } @@ -544,11 +544,11 @@ vddk_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset) VixError err; /* Align to sectors. */ - if (!is_aligned (offset, VIXDISKLIB_SECTOR_SIZE)) { + if (!IS_ALIGNED (offset, VIXDISKLIB_SECTOR_SIZE)) { nbdkit_error ("read is not aligned to sectors"); return -1; } - if (!is_aligned (count, VIXDISKLIB_SECTOR_SIZE)) { + if (!IS_ALIGNED (count, VIXDISKLIB_SECTOR_SIZE)) { nbdkit_error ("read is not aligned to sectors"); return -1; } -- 2.19.0.rc0

3 7

[PATCH nbdkit v2] common: Introduce round up, down; and divide round
by Richard W.M. Jones 17 Sep '18

17 Sep '18

Since we're using ({ .. }) gcc/clang extension, let's rewrite the rounding.h change too. Rich.

2 3

[PATCH nbdkit 0/3] Add partitioning plugin.
by Richard W.M. Jones 17 Sep '18

17 Sep '18

nbdkit partitioning boot.img swap.img root.img ... creates a virtual disk by adding a partition table. In ancient times Xen used to do this. Rich.

3 8

nbdkit -c /dev/nbdX option
by Richard W.M. Jones 17 Sep '18

17 Sep '18

While I was thinking about what I'd write for my proposed nbdkit talk at FOSDEM next year, this strikes me as clunky: rm -f /tmp/sock nbdkit -U /tmp/sock file foo nbd-client -u /tmp/sock /dev/nbd0 ... nbd-client -d /dev/nbd0 killall nbdkit In qemu-nbd, all that is just this: qemu-nbd -c /dev/nbd0 foo It seems like it would be nice to implement an ‘nbdkit -c /dev/nbdX’ option. My first idea was: nbdkit -c /dev/nbdX ==> nbdkit -U [sock] & running: nbd-client -nofork -u [sock] /dev/nbdX However that still leaves the need for end users to manually delete the nbdX device & kill nbdkit. If I'm understanding the qemu code correctly, qemu-nbd uses its own internal NBD client running in a pthread, so it doesn't need nbd-client. So it can keep the client running next to the server and neatly handle cleanup of the device & server on exit. (In case it's not clear, the nbd-client userspace process does not persist with modern kernel NBD.) I can't think of a good way to implement this in nbdkit so if anyone's got any ideas ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html

1 0

[PATCH v2 nbdkit 0/5] tests: Move common functions into tests/functions.sh
by Richard W.M. Jones 13 Sep '18

13 Sep '18

v1 was here: https://www.redhat.com/archives/libguestfs/2018-September/msg00057.html v2: - Fix tab vs spaces in configure.ac. - To generate list of plugins, use printf instead of xargs. - Use 'source ./functions.sh' instead of 'source functions'. - functions.sh: Consistent quoting in foreach_plugin function. - functions.sh: Change the contract of start_nbdkit so it requires (and checks that) the first two parameters are '-P pidfile'. This avoids needing to iterate over the arguments. - functions.sh: Use {1..10} instead of seq. - functions.sh: Add a common mechanism for handling traps / cleanups (this is extra patch 4/5). - functions.sh: start_nbdkit manages its own cleanup function for killing nbdkit on exit

2 12

[PATCH] daemon: consider /etc/mdadm/mdadm.conf while inspecting mountpoints.
by Mykola Ivanets 13 Sep '18

13 Sep '18

From: Nikolay Ivanets <stenavin(a)gmail.com> Inspection code checks /etc/mdadm.conf to map MD device paths listed in mdadm.conf to MD device paths in the guestfs appliance. However on some operating systems (e.g. Ubuntu) mdadm.conf has alternative location: /etc/mdadm/mdadm.conf. This patch consider an alternative location of mdadm.conf as well. --- daemon/inspect_fs_unix_fstab.ml | 13 ++++++++----- daemon/inspect_fs_unix_fstab.mli | 3 ++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/daemon/inspect_fs_unix_fstab.ml b/daemon/inspect_fs_unix_fstab.ml index 170440d2c..9d54dadda 100644 --- a/daemon/inspect_fs_unix_fstab.ml +++ b/daemon/inspect_fs_unix_fstab.ml @@ -38,14 +38,15 @@ let re_xdev = PCRE.compile "^/dev/(h|s|v|xv)d([a-z]+)(\\d*)$" let rec check_fstab ?(mdadm_conf = false) (root_mountable : Mountable.t) os_type = - let configfiles = - "/etc/fstab" :: if mdadm_conf then ["/etc/mdadm.conf"] else [] in + let mdadmfiles = + if mdadm_conf then ["/etc/mdadm.conf"; "/etc/mdadm/mdadm.conf"] else [] in + let configfiles = "/etc/fstab" :: mdadmfiles in with_augeas ~name:"check_fstab_aug" configfiles (check_fstab_aug mdadm_conf root_mountable os_type) and check_fstab_aug mdadm_conf root_mountable os_type aug = - (* Generate a map of MD device paths listed in /etc/mdadm.conf + (* Generate a map of MD device paths listed in mdadm.conf * to MD device paths in the guestfs appliance. *) let md_map = if mdadm_conf then map_md_devices aug else StringMap.empty in @@ -224,11 +225,13 @@ and map_md_devices aug = if StringMap.is_empty uuid_map then StringMap.empty else ( (* Get all arrays listed in mdadm.conf. *) - let entries = aug_matches_noerrors aug "/files/etc/mdadm.conf/array" in + let entries1 = aug_matches_noerrors aug "/files/etc/mdadm.conf/array" in + let entries2 = aug_matches_noerrors aug "/files/etc/mdadm/mdadm.conf/array" in + let entries = List.append entries1 entries2 in (* Log a debug entry if we've got md devices but nothing in mdadm.conf. *) if verbose () && entries = [] then - eprintf "warning: appliance has MD devices, but augeas returned no array matches in /etc/mdadm.conf\n%!"; + eprintf "warning: appliance has MD devices, but augeas returned no array matches in mdadm.conf\n%!"; List.fold_left ( fun md_map entry -> diff --git a/daemon/inspect_fs_unix_fstab.mli b/daemon/inspect_fs_unix_fstab.mli index 4778fd962..2ca06debc 100644 --- a/daemon/inspect_fs_unix_fstab.mli +++ b/daemon/inspect_fs_unix_fstab.mli @@ -24,7 +24,8 @@ val check_fstab : ?mdadm_conf:bool -> Mountable.t -> Inspect_types.os_type -> this function also knows how to map (eg) BSD device names into Linux/libguestfs device names. - [mdadm_conf] is true if you want to check [/etc/mdadm.conf] as well. + [mdadm_conf] is true if you want to check [/etc/mdadm.conf] or + [/etc/mdadm/mdadm.conf] as well. [root_mountable] is the [Mountable.t] of the root filesystem. (Note that the root filesystem must be mounted on sysroot before this -- 2.17.1

2 1

[PATCH nbdkit 0/4] tests: Move common functions into tests/functions.sh
by Richard W.M. Jones 11 Sep '18

11 Sep '18

Combine much common code into tests/functions.sh. Patch 1: Preparation for patch 3. Patch 2: Fix a long-standing bug in how man pages links are generated. Patch 3: Common code for iterating a test function over every plugin. Patch 4: Common code for starting nbdkit in a test and waiting for the PID file to appear. This is the largest and most complex of the patches but is basically repetitive. Rich.

2 12

[PATCH nbdkit v3 0/6] plugins: Implement magic config key.
by Richard W.M. Jones 10 Sep '18

10 Sep '18

v1: https://www.redhat.com/archives/libguestfs/2018-September/msg00024.html v2: https://www.redhat.com/archives/libguestfs/2018-September/msg00034.html v3: - Fixed is_config_key zero length test. - Fixed is_config_key so it uses strspn and is not O(n^2). - Changed >= 1.7 to >= 1.8 in the documentation. Rich.

2 8

question on nbdkit sh plugin
by Eric Blake 10 Sep '18

10 Sep '18

In the just-added nbdkit-sh-plugin.pod, you documented that unlike other plugins (where introspection or struct member population) can be used to determine which functionalities are supported, the shell plugin has to implement can_write and friends to return a special status 2 to document not supported, and 3 to indicate false, in part because you can't detect if a pwrite function is present without running it. But since pwrite normally takes additional parameters, could we instead document that nbdkit calling '/path/to/script pwrite <handle>' while intentionally omitting <count> and <offset> is sufficient for that to return specific status (2 if pwrite in general is not handled, 3 if it is handled but not appropriate for the given handle, and 0 if it works), without needing a 'can_write' entry point? I guess the only difference is that all the logic would go through the single pwrite interface and you wouln't need a can_write. Also, I spotted a potential bug in sh.c: +/* This is the generic function that calls the script. It can + * optionally write to the script's stdin and read from the script's + * stdout and stderr. It returns the raw error code and does no error + * processing. + */ +static int +call3 (const char *wbuf, size_t wbuflen, /* sent to stdin */ + char **rbuf, size_t *rbuflen, /* read from stdout */ + char **ebuf, size_t *ebuflen, /* read from stderr */ + const char **argv) /* script + parameters */ +{ ... + + execvp (argv[0], (char **) argv); + perror (argv[0]); + _exit (EXIT_FAILURE); + } perror() is not async-safe, but since nbdkit may be multithreaded, calling a non-async-safe function in between fork() and successful exec/exit() can deadlock or cause other problems. I don't know if it is worth trying to make that code safer, though. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

2 1

[PATCH nbdkit v2 0/6] plugins: Implement magic config key.
by Richard W.M. Jones 10 Sep '18

10 Sep '18

v1 was here: https://www.redhat.com/archives/libguestfs/2018-September/msg00024.html v2: - As discussed in the patch review, tighten up the characters permitted in keys. - Update documentation to note that relative paths can be made safe by prefixing with ./ and absolute paths do not need any extra steps. - I pushed patch 1/6 from the v1 series since it was just a trivial comment update. Rich.

2 10