[PATCH nbdkit v2] common: isaligned: Use a macro instead of relying on implicit truncation.
by Richard W.M. Jones
---
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
5 years, 10 months
nbdkit -c /dev/nbdX option
by Richard W.M. Jones
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
5 years, 10 months
[PATCH v2 nbdkit 0/5] tests: Move common functions into tests/functions.sh
by Richard W.M. Jones
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
5 years, 10 months
[PATCH] daemon: consider /etc/mdadm/mdadm.conf while inspecting mountpoints.
by Mykola Ivanets
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
5 years, 10 months
[PATCH nbdkit 0/4] tests: Move common functions into tests/functions.sh
by Richard W.M. Jones
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.
5 years, 10 months
question on nbdkit sh plugin
by Eric Blake
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
5 years, 10 months