February 2020 - Libguestfs - libguestfs List Archives

[nbdkit PATCH v3] vddk: Drive library loading from libdir parameter.
by Eric Blake 14 Feb '20

14 Feb '20

From: "Richard W.M. Jones" <rjones(a)redhat.com> Do not use LD_LIBRARY_PATH to locate the VDDK library. Setting this always causes problems because VDDK comes bundled with broken replacements for system libraries, such as libcrypto.so and libstdc++.so. Two problems this causes which we have seen in the real world: (1) User does ‘export LD_LIBRARY_PATH=vmware-vix-disklib-distrib’ and that breaks lots of ordinary utilities on their system. (2) nbdkit vddk --run subcommand inherits the LD_LIBRARY_PATH environment variable from nbdkit, and common commands such as 'qemu-img' break, relying on complex workarounds like saving and restoring the original LD_LIBRARY_PATH in the subcommand. While dlopen() _does_ allow us to pass in an absolute path name to a library (which picks up all immediate dependencies of that library from the same directory, regardless of LD_LIBRARY_PATH), that only catches immediate dependencies; but VDDK itself calls a subsequent dlopen() with a relative name, and that subsequent load no longer searches the directory we supplied explicitly. However, we can't call setenv() to change LD_LIBRARY_PATH dynamically, since (for security reasons) ld.so uses only a value of the environment variable cached prior to main(). Instead, we can fix the problem by relying on the fact that Linux provides dlmopen(), which opens a shared library in a new namespace. We first load a shim library into this namespace which overrides the dlopen() present in the main executable, then load the VDDK library. All further dlopen() calls made during VDDK initialization now go through our shim, at which point we can rewrite them to be absolute calls before the real dlopen() kicks in. Note this may break some callers who are not using libdir and expecting LD_LIBRARY_PATH to work, so it's a change in behaviour which we will have to highlight prominently in the 1.18 release notes. Thanks: Dan Berrangé, Ming Xie, Eric Blake. [eblake: Heavy modifications to Rich's initial patch, including the idea of a shim library] --- Replaces patches 2-3/3 of v2, but keeps 1/3 of that series applied as-is. Passes 'make check' for me, although we might still need a tweak for how to resolve "nbdkit-shim-dlopen.so" from its installed location. I don't have VDDK installed locally, so I can't guarantee how this works with the REAL vddk library, but the fact that 'make check' passes proves that I got the dlmopen() stuff working correctly. plugins/vddk/Makefile.am | 14 ++++- plugins/vddk/nbdkit-vddk-plugin.pod | 39 +++++++++--- plugins/vddk/shim.c | 92 +++++++++++++++++++++++++++++ plugins/vddk/shim.h | 49 +++++++++++++++ plugins/vddk/vddk.c | 73 +++++++++++++++++++++-- tests/Makefile.am | 1 + tests/dummy-vddk.c | 20 ++++++- tests/test-vddk-real.sh | 12 +--- tests/test-vddk.sh | 22 +++++-- wrapper.c | 10 ++-- 10 files changed, 294 insertions(+), 38 deletions(-) create mode 100644 plugins/vddk/shim.c create mode 100644 plugins/vddk/shim.h diff --git a/plugins/vddk/Makefile.am b/plugins/vddk/Makefile.am index b806a7d9..dbb1cc9c 100644 --- a/plugins/vddk/Makefile.am +++ b/plugins/vddk/Makefile.am @@ -1,5 +1,5 @@ # nbdkit -# Copyright (C) 2013-2018 Red Hat Inc. +# Copyright (C) 2013-2020 Red Hat Inc. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions are @@ -38,7 +38,7 @@ EXTRA_DIST = \ if HAVE_VDDK -plugin_LTLIBRARIES = nbdkit-vddk-plugin.la +plugin_LTLIBRARIES = nbdkit-vddk-plugin.la libnbdkit-shim-dlopen.la nbdkit_vddk_plugin_la_SOURCES = \ vddk.c \ @@ -63,6 +63,16 @@ nbdkit_vddk_plugin_la_LDFLAGS = \ -Wl,--version-script=$(top_srcdir)/plugins/plugins.syms \ $(NULL) +libnbdkit_shim_dlopen_la_SOURCES = \ + shim.c \ + shim.h \ + $(NULL) +libnbdkit_shim_dlopen_la_CFLAGS = $(WARNINGS_CFLAGS) +libnbdkit_shim_dlopen_la_LDFLAGS = \ + -module -no-undefined -shared -avoid-version -rpath /nowhere \ + -ldl \ + $(NULL) + if HAVE_POD man_MANS = nbdkit-vddk-plugin.1 diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod index f34b9fba..f0748def 100644 --- a/plugins/vddk/nbdkit-vddk-plugin.pod +++ b/plugins/vddk/nbdkit-vddk-plugin.pod @@ -230,26 +230,47 @@ This parameter is ignored for backwards compatibility. =head1 LIBRARY AND CONFIG FILE LOCATIONS -If the VDDK library (F<libvixDiskLib.so.I<N>>) is located on a -non-standard path, you may need to set C<LD_LIBRARY_PATH> or modify -F</etc/ld.so.conf> before this plugin will work. In addition you may -want to set the C<libdir> parameter so that the VDDK library can load -plugins like Advanced Transport. +The VDDK library should not be placed on a system library path such as +F</usr/lib>. The reason for this is that the VDDK library is shipped +with recompiled libraries like F<libcrypto.so> and F<libstdc++.so> +that can conflict with system libraries. -Usually the VDDK distribution directory should be passed as the -C<libdir> parameter and set C<LD_LIBRARY_PATH> to the F<lib64> -subdirectory: +You have two choices: + +=over 4 + +=item * + +Place VDDK in the default libdir which is compiled into this plugin, +for example: + + $ nbdkit vddk --dump-plugin | grep ^vddk_default_libdir + vddk_default_libdir=/usr/lib64/vmware-vix-disklib + +=item * + +But the most common way is to set the C<libdir> parameter to point to +F<vmware-vix-disklib-distrib/> (which you can unpack anywhere you +like), and this plugin will find the VDDK library from there. For +example: - LD_LIBRARY_PATH=/path/to/vmware-vix-disklib-distrib/lib64 \ nbdkit vddk \ libdir=/path/to/vmware-vix-disklib-distrib \ file=file.vmdk +=back + VDDK itself looks in a few default locations for the optional configuration file, usually including F</etc/vmware/config> and F<$HOME/.vmware/config>, but you can override this using the C<config> parameter. +=head2 No need to set C<LD_LIBRARY_PATH> + +In nbdkit E<le> 1.16 you had to set the environment variable +C<LD_LIBRARY_PATH> when using this plugin. In nbdkit E<ge> 1.18 this +is I<not> recommended. + =head1 FILE PARAMETER The C<file> parameter can either be a local file, in which case it diff --git a/plugins/vddk/shim.c b/plugins/vddk/shim.c new file mode 100644 index 00000000..0819c81a --- /dev/null +++ b/plugins/vddk/shim.c @@ -0,0 +1,92 @@ +/* nbdkit + * Copyright (C) 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. + */ + +/* This file provides a shim around dlopen, to convert relative + * library requests from VDDK into absolute requests. This library is + * designed to be the first thing loaded into an empty dl namespace, + * in order to override dlopen for all other shared libraries later + * loaded in the same namespace; but because it is in a new namespace, + * it cannot use exported nbdkit functions from the main namespace. + */ + +#include <config.h> + +#include <dlfcn.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +static void *(*orig_dlopen) (const char *filename, int flags); + +/* NULL for normal behavior, or set when we want to override + * relative dlopen()s to instead be an absolute open with prefix. + */ +static char *dir; + +int nbdkit_set_dlopen_prefix (const char *newdir) +{ + free (dir); + if (newdir) { + dir = strdup (newdir); + if (!dir) + return -1; + } + else + dir = NULL; + return 0; +} + +void *dlopen(const char *filename, int flags) +{ + char *tmp = NULL; + void *ret; + + if (dir && ! strchr (filename, '/')) { + /* Best effort: we can't report an error through nbdkit, and can't + * influence dlerror; if allocation fails, just rely on the + * original relative request. + */ + if (asprintf (&tmp, "%s/%s", dir, filename) >= 0) + filename = tmp; + } + + ret = orig_dlopen (filename, flags); + free (tmp); + return ret; +} + +static void constructor (void) __attribute__ ((constructor)); +static void +constructor (void) +{ + orig_dlopen = dlsym (RTLD_NEXT, "dlopen"); +} diff --git a/plugins/vddk/shim.h b/plugins/vddk/shim.h new file mode 100644 index 00000000..a9c202f0 --- /dev/null +++ b/plugins/vddk/shim.h @@ -0,0 +1,49 @@ +/* nbdkit + * Copyright (C) 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. + */ + +/* This file provides a shim around dlopen, to convert relative library + * requests from VDDK into absolute requests. + */ + +#ifndef NBDKIT_SHIM_H +#define NBDKIT_SHIM_H + +#include <config.h> + +#include <dlfcn.h> +#include <stdio.h> +#include <stdlib.h> + +typedef int (*nbdkit_set_dlopen_prefix_function)(const char *); +extern int nbdkit_set_dlopen_prefix (const char *dir); + +#endif /* NBDKIT_SHIM_H */ diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c index db61c1d8..6deb0a0b 100644 --- a/plugins/vddk/vddk.c +++ b/plugins/vddk/vddk.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -40,6 +40,7 @@ #include <string.h> #include <unistd.h> #include <dlfcn.h> +#include <libgen.h> #define NBDKIT_API_VERSION 2 @@ -50,6 +51,7 @@ #include "minmax.h" #include "rounding.h" +#include "shim.h" #include "vddk-structs.h" /* Enable extra disk info debugging with: -D vddk.diskinfo=1 */ @@ -71,7 +73,8 @@ int vddk_debug_extents; #define VDDK_MAJOR 5 #define VDDK_MINOR 1 -static void *dl = NULL; /* dlopen handle */ +static void *shimdl = NULL; /* shim dlopen handle */ +static void *dl = NULL; /* vddk dlopen handle */ static int init_called = 0; /* was InitEx called */ static char *config = NULL; /* config */ @@ -153,6 +156,8 @@ vddk_unload (void) } if (dl) dlclose (dl); + if (shimdl) + dlclose (shimdl); free (config); free (libdir); free (password); @@ -248,17 +253,67 @@ load_library (void) { static const char *sonames[] = { /* Prefer the newest library in case multiple exist. */ + "lib64/libvixDiskLib.so.6", "libvixDiskLib.so.6", + "lib64/libvixDiskLib.so.5", "libvixDiskLib.so.5", }; size_t i; CLEANUP_FREE char *orig_error = NULL; + Lmid_t id; + nbdkit_set_dlopen_prefix_function set_prefix; + + /* The main namespace already has a resolved dlopen() symbol (or + * this plugin wouldn't be running), and it's too late to change + * LD_LIBRARY_PATH. However, we can use dlmopen() to create a new + * namespace, where we first load our shim library. Then all + * subsequent libraries loaded in that namespace will call our shim + * dlopen(), which lets us correct the relative dlopen()s performed + * later by VDDK initialization. + */ + shimdl = dlmopen (LM_ID_NEWLM, "libnbdkit-shim-dlopen.so", + RTLD_NOW); + if (!shimdl) { + nbdkit_error("failed to load dlopen shim: %s", dlerror()); + exit (EXIT_FAILURE); + } + if (dlinfo(shimdl, RTLD_DI_LMID, &id)) { + nbdkit_error("failed to learn dlopen shim id: %s", dlerror()); + exit (EXIT_FAILURE); + } + *(void **) (&set_prefix) = dlsym (shimdl, "nbdkit_set_dlopen_prefix"); + if (!set_prefix) { + nbdkit_error ("required dlopen shim symbol \"nbdkit_set_dlopen_prefix\" " + "is missing: %s", dlerror ()); + exit (EXIT_FAILURE); + } + nbdkit_debug("dlopen shim loaded with namespace id %d", (int) id); /* Load the library. */ for (i = 0; i < sizeof sonames / sizeof sonames[0]; ++i) { - dl = dlopen (sonames[i], RTLD_NOW); - if (dl != NULL) + CLEANUP_FREE char *path; + + /* Set the full path so that dlopen will preferentially load the + * system libraries from the same directory. + */ + if (asprintf (&path, "%s/%s", libdir, sonames[i]) == -1) { + nbdkit_error ("asprintf: %m"); + exit (EXIT_FAILURE); + } + + dl = dlmopen (id, path, RTLD_NOW); + if (dl != NULL) { + /* Inform the shim about our desired prefix of libdir. This may + * modify path in-place, but that's okay. + */ + char *dir = dirname (path); + if (set_prefix (dir) == -1) { + nbdkit_error ("unable to set prefix for dlopen shim: %m"); + exit (EXIT_FAILURE); + } + nbdkit_debug("dlopen shim prefix set to %s", dir); break; + } if (i == 0) { orig_error = dlerror (); if (orig_error) @@ -268,7 +323,7 @@ load_library (void) if (dl == NULL) { nbdkit_error ("%s\n\n" "If '%s' is located on a non-standard path you may need to\n" - "set $LD_LIBRARY_PATH or edit /etc/ld.so.conf.\n\n" + "set libdir=/path/to/vmware-vix-disklib-distrib.\n\n" "See the nbdkit-vddk-plugin(1) man page for details.", orig_error ? : "(unknown error)", sonames[0]); exit (EXIT_FAILURE); @@ -331,6 +386,14 @@ vddk_config_complete (void) #undef missing } + if (!libdir) { + libdir = strdup (VDDK_LIBDIR); + if (!libdir) { + nbdkit_error ("strdup: %m"); + return -1; + } + } + load_library (); /* Initialize VDDK library. */ diff --git a/tests/Makefile.am b/tests/Makefile.am index e1284231..8ad4b928 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -758,6 +758,7 @@ libvixDiskLib_la_CXXFLAGS = $(WARNINGS_CFLAGS) # https://lists.gnu.org/archive/html/libtool/2007-07/msg00067.html libvixDiskLib_la_LDFLAGS = \ -shared -version-number 6:0:0 -rpath /nowhere \ + -ldl \ $(NULL) endif HAVE_VDDK diff --git a/tests/dummy-vddk.c b/tests/dummy-vddk.c index ed5d3712..dee3773f 100644 --- a/tests/dummy-vddk.c +++ b/tests/dummy-vddk.c @@ -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 @@ -39,6 +39,8 @@ #include <stdio.h> #include <stdlib.h> #include <stdint.h> +#include <dlfcn.h> +#include <assert.h> #include "vddk-structs.h" @@ -49,6 +51,12 @@ VixDiskLib_InitEx (uint32_t major, uint32_t minor, VixDiskLibGenericLogFunc *panic_function, const char *lib_dir, const char *config_file) { - /* Do nothing, only exit with no error. */ + /* Simulate the fact that real vddk calls dlopen("relative"), to see + * that our shim kicks in and rewrites it. We pass invalid flags, so + * the dlopen fails, but only after going through our shim. + */ + dlopen("nosuch", RTLD_LAZY); + + /* dlopen() should have failed, but if id didn't exit, neither to we. */ return VIX_OK; } diff --git a/tests/test-vddk-real.sh b/tests/test-vddk-real.sh index 52c91232..d9cf3319 100755 --- a/tests/test-vddk-real.sh +++ b/tests/test-vddk-real.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # nbdkit -# Copyright (C) 2018-2019 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 @@ -51,15 +51,7 @@ cleanup_fn rm -f $files qemu-img create -f vmdk test-vddk-real.vmdk 100M -export old_ld_library_path="$LD_LIBRARY_PATH" -export LD_LIBRARY_PATH="$vddkdir/lib64:$LD_LIBRARY_PATH" - nbdkit -f -v -U - \ --filter=readahead \ vddk libdir="$vddkdir" file=test-vddk-real.vmdk \ - --run ' - # VDDK library path breaks qemu-img, we must restore the - # original path here. - export LD_LIBRARY_PATH="$old_ld_library_path" - qemu-img convert $nbd -O raw test-vddk-real.out -' + --run 'qemu-img convert $nbd -O raw test-vddk-real.out' diff --git a/tests/test-vddk.sh b/tests/test-vddk.sh index 19b946b6..d99ebf88 100755 --- a/tests/test-vddk.sh +++ b/tests/test-vddk.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # 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 @@ -34,12 +34,22 @@ source ./functions.sh set -e set -x -rm -f test-vddk.out -cleanup_fn rm -f test-vddk.out +rm -rf test-vddk.out test-vddk.err +cleanup_fn rm -rf test-vddk.out test-vddk.err -LD_LIBRARY_PATH=.libs:$LD_LIBRARY_PATH \ -LIBRARY_PATH=.libs:$LIBRARY_PATH \ -nbdkit vddk --dump-plugin > test-vddk.out +# --dump-plugin does not initialize VDDK,... +nbdkit vddk libdir=.libs --dump-plugin > test-vddk.out cat test-vddk.out +test ! -f test-vddk.err grep ^vddk_default_libdir= test-vddk.out + +# ...but passing a filename does. Initializing our dummy library causes +# a load that we know will fail, but the important part is that dlopen's +# error message lists an absolute file even though we passed a relative +# name, showing that our shim did adjust it. +nbdkit vddk libdir=.libs \ + file=/dev/null --run ':' 2> test-vddk.err || : +cat test-vddk.err + +grep '/.libs/nosuch' test-vddk.err diff --git a/wrapper.c b/wrapper.c index 6aef81a1..37705608 100644 --- a/wrapper.c +++ b/wrapper.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2017-2019 Red Hat Inc. + * Copyright (C) 2017-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -168,12 +168,14 @@ main (int argc, char *argv[]) } } - /* Needed for plugins written in OCaml. */ + /* Needed for plugins written in OCaml, and for in-tree testing of VDDK. */ s = getenv ("LD_LIBRARY_PATH"); if (s) - r = asprintf (&s, "%s/plugins/ocaml/.libs:%s", builddir, s); + r = asprintf (&s, "%s/plugins/ocaml/.libs:%s/plugins/vddk/.libs:%s", + builddir, builddir, s); else - r = asprintf (&s, "%s/plugins/ocaml/.libs", builddir); + r = asprintf (&s, "%s/plugins/ocaml/.libs:%s/plugins/vddk/.libs", + builddir, builddir); if (r < 0) { perror ("asprintf"); exit (EXIT_FAILURE); -- 2.24.1

1 1

[common PATCH v4 0/1] options: add '--blocksize' option for C-based tools
by Mykola Ivanets 13 Feb '20

13 Feb '20

From: Nikolay Ivanets <stenavin(a)gmail.com> v4 fixes issues found during code review: - whitespace-change-only hunks are removed - options are alphabetically orderred now v3 is just a spelling correction spotted by Eric Blake https://www.redhat.com/archives/libguestfs/2020-February/msg00111.html In v2 I've moved '--blocksize' parameter description into the separate file called blocksize-option.pod so we can include it everywhere we need similar to key-option.pod. https://www.redhat.com/archives/libguestfs/2020-February/msg00099.html v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00096.html Nikolay Ivanets (1): options: add '--blocksize' option for C-based tools options/Makefile.am | 3 ++- options/blocksize-option.pod | 11 ++++++++++ options/options.c | 13 +++++++++++- options/options.h | 41 ++++++++++++++++++++++++++++-------- 4 files changed, 57 insertions(+), 11 deletions(-) create mode 100644 options/blocksize-option.pod -- 2.17.2

1 1

[PATCH nbdkit v2 2/3] NOT WORKING: vddk: Drive library loading from libdir parameter.
by Richard W.M. Jones 13 Feb '20

13 Feb '20

I couldn't get this to work in the end. This is the latest non-working version. This email documents what doesn't work for the permanent record. The central problem is that VDDK InitEx() appears to dlopen() various of its own plugins. Although I wasn't able to capture exactly what dlopen() command it is running, the plugins cannot be loaded because they rely on the recompiled system libraries (libcrypto.so.X etc) and it cannot find those because $LD_LIBRARY_PATH is not set. Setting $LD_LIBRARY_PATH using setenv around the call to InitEx() does not work because glibc only consults $LD_LIBRARY_PATH when the program starts up. Various workarounds have been suggested for this but none of them are pleasant (https://stackoverflow.com/questions/6713692/problems-with-using-setenv-and-…) So currently we must have nbdkit start up with $LD_LIBRARY_PATH set, in other words how it works at the moment. Rich.

2 4

[PATCH nbdkit 1/2] vddk: Delay loading VDDK until config_complete.
by Richard W.M. Jones 13 Feb '20

13 Feb '20

We were previously dlopen-ing it in the load() method. This is very early and in particular means that the only possible way to configure where we find the library is through environment variables and not through config parameters. Also it's not necessary as we don't call any functions from the library (such as VixDiskLib_InitEx) until config_complete. This change is neutral refactoring as currently we _do_ configure the location through an environment variable (LD_LIBRARY_PATH). --- plugins/vddk/vddk.c | 100 ++++++++++++++++++++++---------------------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c index 5d3764d6..db61c1d8 100644 --- a/plugins/vddk/vddk.c +++ b/plugins/vddk/vddk.c @@ -143,54 +143,7 @@ error_function (const char *fs, va_list args) nbdkit_error ("%s", str); } -/* Load and unload the plugin. */ -static void -vddk_load (void) -{ - static const char *sonames[] = { - /* Prefer the newest library in case multiple exist. */ - "libvixDiskLib.so.6", - "libvixDiskLib.so.5", - }; - size_t i; - CLEANUP_FREE char *orig_error = NULL; - - /* Load the library. */ - for (i = 0; i < sizeof sonames / sizeof sonames[0]; ++i) { - dl = dlopen (sonames[i], RTLD_NOW); - if (dl != NULL) - break; - if (i == 0) { - orig_error = dlerror (); - if (orig_error) - orig_error = strdup (orig_error); - } - } - if (dl == NULL) { - nbdkit_error ("%s\n\n" - "If '%s' is located on a non-standard path you may need to\n" - "set $LD_LIBRARY_PATH or edit /etc/ld.so.conf.\n\n" - "See the nbdkit-vddk-plugin(1) man page for details.", - orig_error ? : "(unknown error)", sonames[0]); - exit (EXIT_FAILURE); - } - - /* Load symbols. */ -#define STUB(fn,ret,args) \ - do { \ - fn = dlsym (dl, #fn); \ - if (fn == NULL) { \ - nbdkit_error ("required VDDK symbol \"%s\" is missing: %s", \ - #fn, dlerror ()); \ - exit (EXIT_FAILURE); \ - } \ - } while (0) -#define OPTIONAL_STUB(fn,ret,args) fn = dlsym (dl, #fn) -#include "vddk-stubs.h" -#undef STUB -#undef OPTIONAL_STUB -} - +/* Unload the plugin. */ static void vddk_unload (void) { @@ -289,6 +242,54 @@ vddk_config (const char *key, const char *value) return 0; } +/* Load the VDDK library. */ +static void +load_library (void) +{ + static const char *sonames[] = { + /* Prefer the newest library in case multiple exist. */ + "libvixDiskLib.so.6", + "libvixDiskLib.so.5", + }; + size_t i; + CLEANUP_FREE char *orig_error = NULL; + + /* Load the library. */ + for (i = 0; i < sizeof sonames / sizeof sonames[0]; ++i) { + dl = dlopen (sonames[i], RTLD_NOW); + if (dl != NULL) + break; + if (i == 0) { + orig_error = dlerror (); + if (orig_error) + orig_error = strdup (orig_error); + } + } + if (dl == NULL) { + nbdkit_error ("%s\n\n" + "If '%s' is located on a non-standard path you may need to\n" + "set $LD_LIBRARY_PATH or edit /etc/ld.so.conf.\n\n" + "See the nbdkit-vddk-plugin(1) man page for details.", + orig_error ? : "(unknown error)", sonames[0]); + exit (EXIT_FAILURE); + } + + /* Load symbols. */ +#define STUB(fn,ret,args) \ + do { \ + fn = dlsym (dl, #fn); \ + if (fn == NULL) { \ + nbdkit_error ("required VDDK symbol \"%s\" is missing: %s", \ + #fn, dlerror ()); \ + exit (EXIT_FAILURE); \ + } \ + } while (0) +#define OPTIONAL_STUB(fn,ret,args) fn = dlsym (dl, #fn) +#include "vddk-stubs.h" +#undef STUB +#undef OPTIONAL_STUB +} + static int vddk_config_complete (void) { @@ -330,6 +331,8 @@ vddk_config_complete (void) #undef missing } + load_library (); + /* Initialize VDDK library. */ DEBUG_CALL ("VixDiskLib_InitEx", "%d, %d, &debug_fn, &error_fn, &error_fn, %s, %s", @@ -831,7 +834,6 @@ static struct nbdkit_plugin plugin = { .name = "vddk", .longname = "VMware VDDK plugin", .version = PACKAGE_VERSION, - .load = vddk_load, .unload = vddk_unload, .config = vddk_config, .config_complete = vddk_config_complete, -- 2.25.0

2 3

[PATCH nbdkit] NOT WORKING vddk: Use dlmopen to isolate VDDK.
by Richard W.M. Jones 13 Feb '20

13 Feb '20

--- configure.ac | 5 +++++ plugins/vddk/vddk.c | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/configure.ac b/configure.ac index d71f06e4..57626a76 100644 --- a/configure.ac +++ b/configure.ac @@ -321,6 +321,11 @@ AC_SEARCH_LIBS([dlsym], [dl dld], [ ]) LIBS="$old_LIBS" +old_LIBS="$LIBS" +LIBS="$LIBS -ldl" +AC_CHECK_FUNCS([dlmopen]) +LIBS="$old_LIBS" + dnl Test if <iconv.h> header can build working binaries. dnl dnl On FreeBSD: iconv and libiconv both exist, both can be installed diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c index c49eebcd..b988946b 100644 --- a/plugins/vddk/vddk.c +++ b/plugins/vddk/vddk.c @@ -267,7 +267,11 @@ load_library (void) exit (EXIT_FAILURE); } +#ifdef HAVE_DLMOPEN + dl = dlmopen (LM_ID_NEWLM, path, RTLD_NOW); +#else dl = dlopen (path, RTLD_NOW); +#endif if (dl != NULL) break; if (i == 0) { -- 2.25.0

1 1

[PATCH v2 0/1] tools: add '--blocksize' option for C-based tools
by Mykola Ivanets 13 Feb '20

13 Feb '20

3 3

[common PATCH v2 0/1] options: add '--blocksize' option for C-based tools
by Mykola Ivanets 13 Feb '20

13 Feb '20

From: Nikolay Ivanets <stenavin(a)gmail.com> In v2 I've moved '--blocksize' parameter description into the separate file called blocksize-option.pod so we can include it everywhere we need similar to key-option.pod. v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00096.html Nikolay Ivanets (1): options: add '--blocksize' option for C-based tools options/Makefile.am | 3 +- options/blocksize-option.pod | 11 +++ options/options.c | 13 +++- options/options.h | 125 +++++++++++++++++++++-------------- 4 files changed, 99 insertions(+), 53 deletions(-) create mode 100644 options/blocksize-option.pod -- 2.17.2

3 3

[PATCH 0/1] tools: add '--blocksize' option for C-based tools
by Mykola Ivanets 13 Feb '20

13 Feb '20

2 2

[common PATCH] options: add '--blocksize' option for C-based tools
by Mykola Ivanets 13 Feb '20

13 Feb '20

From: Nikolay Ivanets <stenavin(a)gmail.com> This patch adds '--blocksize' command line option parsing and handling for guestfish and other C-based tools which share the same code. --- options/options.c | 13 ++++- options/options.h | 125 +++++++++++++++++++++++++++------------------- 2 files changed, 86 insertions(+), 52 deletions(-) diff --git a/options/options.c b/options/options.c index fe63da9..63221ea 100644 --- a/options/options.c +++ b/options/options.c @@ -49,7 +49,8 @@ * Handle the guestfish I<-a> option on the command line. */ void -option_a (const char *arg, const char *format, struct drv **drvsp) +option_a (const char *arg, const char *format, int blocksize, + struct drv **drvsp) { struct uri uri; struct drv *drv; @@ -69,6 +70,7 @@ option_a (const char *arg, const char *format, struct drv **drvsp) drv->type = drv_a; drv->a.filename = uri.path; drv->a.format = format; + drv->a.blocksize = blocksize; free (uri.protocol); } @@ -82,6 +84,7 @@ option_a (const char *arg, const char *format, struct drv **drvsp) drv->uri.password = uri.password; drv->uri.format = format; drv->uri.orig_uri = arg; + drv->uri.blocksize = blocksize; } drv->next = *drvsp; @@ -137,6 +140,10 @@ add_drives_handle (guestfs_h *g, struct drv *drv, size_t drive_index) ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK; ad_optargs.discard = drv->a.discard; } + if (drv->a.blocksize) { + ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK; + ad_optargs.blocksize = drv->a.blocksize; + } r = guestfs_add_drive_opts_argv (g, drv->a.filename, &ad_optargs); if (r == -1) @@ -170,6 +177,10 @@ add_drives_handle (guestfs_h *g, struct drv *drv, size_t drive_index) ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK; ad_optargs.secret = drv->uri.password; } + if (drv->uri.blocksize) { + ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK; + ad_optargs.blocksize = drv->uri.blocksize; + } r = guestfs_add_drive_opts_argv (g, drv->uri.path, &ad_optargs); if (r == -1) diff --git a/options/options.h b/options/options.h index 9b78302..9f3a1e7 100644 --- a/options/options.h +++ b/options/options.h @@ -65,6 +65,7 @@ struct drv { const char *format; /* format (NULL == autodetect) */ const char *cachemode;/* cachemode (NULL == default) */ const char *discard; /* discard (NULL == disable) */ + int blocksize; /* blocksize (0 == default) */ } a; struct { char *path; /* disk path */ @@ -74,6 +75,7 @@ struct drv { char *password; /* password - can be NULL */ const char *format; /* format (NULL == autodetect) */ const char *orig_uri; /* original URI (for error messages etc.) */ + int blocksize; /* blocksize (0 == default) */ } uri; struct { char *guest; /* guest name */ @@ -156,7 +158,7 @@ extern struct key_store *key_store_import_key (struct key_store *ks, const struc extern void free_key_store (struct key_store *ks); /* in options.c */ -extern void option_a (const char *arg, const char *format, struct drv **drvsp); +extern void option_a (const char *arg, const char *format, int blocksize, struct drv **drvsp); extern void option_d (const char *arg, struct drv **drvsp); extern char add_drives_handle (guestfs_h *g, struct drv *drv, size_t drive_index); #define add_drives(drv) add_drives_handle (g, drv, 0) @@ -164,76 +166,87 @@ extern void mount_mps (struct mp *mp); extern void free_drives (struct drv *drv); extern void free_mps (struct mp *mp); -#define OPTION_a \ - do { \ - option_a (optarg, format, &drvs); \ - format_consumed = true; \ +#define OPTION_a \ + do { \ + option_a (optarg, format, blocksize, &drvs); \ + format_consumed = true; \ + blocksize_consumed = true; \ } while (0) -#define OPTION_A \ - do { \ - option_a (optarg, format, &drvs2); \ - format_consumed = true; \ +#define OPTION_A \ + do { \ + option_a (optarg, format, blocksize, &drvs2); \ + format_consumed = true; \ + blocksize_consumed = true; \ } while (0) -#define OPTION_c \ +#define OPTION_c \ libvirt_uri = optarg -#define OPTION_d \ +#define OPTION_d \ option_d (optarg, &drvs) -#define OPTION_D \ +#define OPTION_D \ option_d (optarg, &drvs2) -#define OPTION_format \ - do { \ - if (!optarg || STREQ (optarg, "")) \ - format = NULL; \ - else \ - format = optarg; \ - format_consumed = false; \ +#define OPTION_format \ + do { \ + if (!optarg || STREQ (optarg, "")) \ + format = NULL; \ + else \ + format = optarg; \ + format_consumed = false; \ } while (0) -#define OPTION_i \ +#define OPTION_blocksize \ + do { \ + if (!optarg || STREQ (optarg, "")) \ + blocksize = 0; \ + else if (sscanf (optarg, "%d", &blocksize) != 1) \ + error (EXIT_FAILURE, 0, _("--blocksize option is not numeric")); \ + blocksize_consumed = false; \ + } while (0) + +#define OPTION_i \ inspector = 1 -#define OPTION_m \ - mp = malloc (sizeof (struct mp)); \ - if (!mp) \ - error (EXIT_FAILURE, errno, "malloc"); \ - mp->fstype = NULL; \ - mp->options = NULL; \ - mp->mountpoint = (char *) "/"; \ - p = strchr (optarg, ':'); \ - if (p) { \ - *p = '\0'; \ - p++; \ - mp->mountpoint = p; \ - p = strchr (p, ':'); \ - if (p) { \ - *p = '\0'; \ - p++; \ - mp->options = p; \ - p = strchr (p, ':'); \ - if (p) { \ - *p = '\0'; \ - p++; \ - mp->fstype = p; \ - } \ - } \ - } \ - mp->device = optarg; \ - mp->next = mps; \ +#define OPTION_m \ + mp = malloc (sizeof (struct mp)); \ + if (!mp) \ + error (EXIT_FAILURE, errno, "malloc"); \ + mp->fstype = NULL; \ + mp->options = NULL; \ + mp->mountpoint = (char *) "/"; \ + p = strchr (optarg, ':'); \ + if (p) { \ + *p = '\0'; \ + p++; \ + mp->mountpoint = p; \ + p = strchr (p, ':'); \ + if (p) { \ + *p = '\0'; \ + p++; \ + mp->options = p; \ + p = strchr (p, ':'); \ + if (p) { \ + *p = '\0'; \ + p++; \ + mp->fstype = p; \ + } \ + } \ + } \ + mp->device = optarg; \ + mp->next = mps; \ mps = mp -#define OPTION_n \ +#define OPTION_n \ guestfs_set_autosync (g, 0) -#define OPTION_r \ +#define OPTION_r \ read_only = 1 -#define OPTION_v \ - verbose++; \ +#define OPTION_v \ + verbose++; \ guestfs_set_verbose (g, verbose) #define OPTION_V \ @@ -267,4 +280,14 @@ extern void free_mps (struct mp *mp); } \ } while (0) +#define CHECK_OPTION_blocksize_consumed \ + do { \ + if (!blocksize_consumed) { \ + fprintf (stderr, \ + _("%s: --blocksize parameter must appear before -a parameter\n"), \ + getprogname ()); \ + exit (EXIT_FAILURE); \ + } \ + } while (0) + #endif /* OPTIONS_H */ -- 2.17.2

2 1

[nbdkit PATCH] filters: Remove most next_* wrappers
by Eric Blake 12 Feb '20

12 Feb '20

With our recent cleanups to nxdata, the only remaining difference between functions like backend_open() and next_open() was the signature (one used void*, the other struct backend *); the API is compatible. All of our filters are in-tree, and we don't promise API/ABI stability, but it is still a lot of files to touch, so the simplest solution to avoid the redundant hop through wrapper functions is to change our header to compile with a typedef, which is either 'struct backend *' for internal code or 'void *' for filters. With this in place, we can now delete most of the next_* wrappers, by pointing to corresponding backend_* functions instead; the few exceptions are .config and .config_complete (the public API returns an integer, but the backend code exits on failure and thus returns void), and .preconnect (we don't have a backend_preconnect function). Signed-off-by: Eric Blake <eblake(a)redhat.com> --- The alternative to the #ifdef'd typedef is to touch all of the filters/*.c files to change 'void *nxdata' to 'struct backend *nxdata'. include/nbdkit-filter.h | 119 ++++++++++++----------- server/filters.c | 208 +++++----------------------------------- server/internal.h | 3 +- 3 files changed, 89 insertions(+), 241 deletions(-) diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h index f7705d67..50b3d55a 100644 --- a/include/nbdkit-filter.h +++ b/include/nbdkit-filter.h @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -49,47 +49,56 @@ extern "C" { #define NBDKIT_ZERO_EMULATE 1 #define NBDKIT_ZERO_NATIVE 2 +#ifdef NBDKIT_INTERNAL +/* Opaque type encapsulating all information needed for calling into + * the next filter or plugin. + */ +typedef struct backend backend; +#else +typedef void backend; +#endif + /* Next ops. */ -typedef int nbdkit_next_config (void *nxdata, +typedef int nbdkit_next_config (backend *nxdata, const char *key, const char *value); -typedef int nbdkit_next_config_complete (void *nxdata); -typedef int nbdkit_next_preconnect (void *nxdata, int readonly); -typedef int nbdkit_next_open (void *nxdata, int readonly); +typedef int nbdkit_next_config_complete (backend *nxdata); +typedef int nbdkit_next_preconnect (backend *nxdata, int readonly); +typedef int nbdkit_next_open (backend *nxdata, int readonly); struct nbdkit_next_ops { /* Performs close + open on the underlying chain. * Used by the retry filter. */ - int (*reopen) (void *nxdata, int readonly); + int (*reopen) (backend *nxdata, int readonly); /* The rest of the next ops are the same as normal plugin operations. */ - int64_t (*get_size) (void *nxdata); - - int (*can_write) (void *nxdata); - int (*can_flush) (void *nxdata); - int (*is_rotational) (void *nxdata); - int (*can_trim) (void *nxdata); - int (*can_zero) (void *nxdata); - int (*can_fast_zero) (void *nxdata); - int (*can_extents) (void *nxdata); - int (*can_fua) (void *nxdata); - int (*can_multi_conn) (void *nxdata); - int (*can_cache) (void *nxdata); - - int (*pread) (void *nxdata, void *buf, uint32_t count, uint64_t offset, + int64_t (*get_size) (backend *nxdata); + + int (*can_write) (backend *nxdata); + int (*can_flush) (backend *nxdata); + int (*is_rotational) (backend *nxdata); + int (*can_trim) (backend *nxdata); + int (*can_zero) (backend *nxdata); + int (*can_fast_zero) (backend *nxdata); + int (*can_extents) (backend *nxdata); + int (*can_fua) (backend *nxdata); + int (*can_multi_conn) (backend *nxdata); + int (*can_cache) (backend *nxdata); + + int (*pread) (backend *nxdata, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*pwrite) (void *nxdata, + int (*pwrite) (backend *nxdata, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*flush) (void *nxdata, uint32_t flags, int *err); - int (*trim) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, - int *err); - int (*zero) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, - int *err); - int (*extents) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, - struct nbdkit_extents *extents, int *err); - int (*cache) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, - int *err); + int (*flush) (backend *nxdata, uint32_t flags, int *err); + int (*trim) (backend *nxdata, uint32_t count, uint64_t offset, + uint32_t flags, int *err); + int (*zero) (backend *nxdata, uint32_t count, uint64_t offset, + uint32_t flags, int *err); + int (*extents) (backend *nxdata, uint32_t count, uint64_t offset, + uint32_t flags, struct nbdkit_extents *extents, int *err); + int (*cache) (backend *nxdata, uint32_t count, uint64_t offset, + uint32_t flags, int *err); }; /* Extent functions. */ @@ -127,66 +136,66 @@ struct nbdkit_filter { void (*load) (void); void (*unload) (void); - int (*config) (nbdkit_next_config *next, void *nxdata, + int (*config) (nbdkit_next_config *next, backend *nxdata, const char *key, const char *value); - int (*config_complete) (nbdkit_next_config_complete *next, void *nxdata); + int (*config_complete) (nbdkit_next_config_complete *next, backend *nxdata); const char *config_help; int (*thread_model) (void); - int (*preconnect) (nbdkit_next_preconnect *next, void *nxdata, int readonly); + int (*preconnect) (nbdkit_next_preconnect *next, backend *nxdata, + int readonly); - void * (*open) (nbdkit_next_open *next, void *nxdata, + void * (*open) (nbdkit_next_open *next, backend *nxdata, int readonly); void (*close) (void *handle); - int (*prepare) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*prepare) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle, int readonly); - int (*finalize) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*finalize) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); - int64_t (*get_size) (struct nbdkit_next_ops *next_ops, void *nxdata, + int64_t (*get_size) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); - int (*can_write) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*can_write) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); - int (*can_flush) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*can_flush) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); int (*is_rotational) (struct nbdkit_next_ops *next_ops, - void *nxdata, - void *handle); - int (*can_trim) (struct nbdkit_next_ops *next_ops, void *nxdata, + backend *nxdata, void *handle); + int (*can_trim) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); - int (*can_zero) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*can_zero) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); - int (*can_fast_zero) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*can_fast_zero) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); - int (*can_extents) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*can_extents) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); - int (*can_fua) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*can_fua) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); - int (*can_multi_conn) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*can_multi_conn) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); - int (*can_cache) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*can_cache) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle); - int (*pread) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*pread) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*pwrite) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*pwrite) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*flush) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*flush) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle, uint32_t flags, int *err); - int (*trim) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*trim) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*zero) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*zero) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*extents) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*extents) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err); - int (*cache) (struct nbdkit_next_ops *next_ops, void *nxdata, + int (*cache) (struct nbdkit_next_ops *next_ops, backend *nxdata, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err); }; diff --git a/server/filters.c b/server/filters.c index 92b0ceb3..8985ebeb 100644 --- a/server/filters.c +++ b/server/filters.c @@ -127,9 +127,8 @@ filter_dump_fields (struct backend *b) } static int -next_config (void *nxdata, const char *key, const char *value) +next_config (struct backend *b, const char *key, const char *value) { - struct backend *b = nxdata; b->config (b, key, value); return 0; } @@ -151,9 +150,8 @@ filter_config (struct backend *b, const char *key, const char *value) } static int -next_config_complete (void *nxdata) +next_config_complete (struct backend *b) { - struct backend *b = nxdata; b->config_complete (b); return 0; } @@ -173,13 +171,6 @@ filter_config_complete (struct backend *b) b->next->config_complete (b->next); } -static int -next_preconnect (void *nxdata, int readonly) -{ - struct backend *b_next = nxdata; - return b_next->preconnect (b_next, readonly); -} - static int filter_preconnect (struct backend *b, int readonly) { @@ -188,7 +179,7 @@ filter_preconnect (struct backend *b, int readonly) debug ("%s: preconnect", b->name); if (f->filter.preconnect) - return f->filter.preconnect (next_preconnect, b->next, readonly); + return f->filter.preconnect (b->next->preconnect, b->next, readonly); else return b->next->preconnect (b->next, readonly); } @@ -202,14 +193,6 @@ plugin_magic_config_key (struct backend *b) return b->next->magic_config_key (b->next); } -static int -next_open (void *nxdata, int readonly) -{ - struct backend *b_next = nxdata; - - return backend_open (b_next, readonly); -} - static void * filter_open (struct backend *b, int readonly) { @@ -220,7 +203,7 @@ filter_open (struct backend *b, int readonly) * inner-to-outer ordering. */ if (f->filter.open) - handle = f->filter.open (next_open, b->next, readonly); + handle = f->filter.open (backend_open, b->next, readonly); else if (backend_open (b->next, readonly) == -1) handle = NULL; else @@ -237,171 +220,26 @@ filter_close (struct backend *b, void *handle) f->filter.close (handle); } -/* The next_functions structure contains pointers to backend - * functions. These are only needed for type safety (nxdata is void - * pointer, backend_* functions expect a struct backend * parameter). - * nxdata is a pointer to the next backend in the linked list. - */ - -static int -next_reopen (void *nxdata, int readonly) -{ - struct backend *b_next = nxdata; - return backend_reopen (b_next, readonly); -} - -static int64_t -next_get_size (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_get_size (b_next); -} - -static int -next_can_write (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_can_write (b_next); -} - -static int -next_can_flush (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_can_flush (b_next); -} - -static int -next_is_rotational (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_is_rotational (b_next); -} - -static int -next_can_trim (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_can_trim (b_next); -} - -static int -next_can_zero (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_can_zero (b_next); -} - -static int -next_can_fast_zero (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_can_fast_zero (b_next); -} - -static int -next_can_extents (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_can_extents (b_next); -} - -static int -next_can_fua (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_can_fua (b_next); -} - -static int -next_can_multi_conn (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_can_multi_conn (b_next); -} - -static int -next_can_cache (void *nxdata) -{ - struct backend *b_next = nxdata; - return backend_can_cache (b_next); -} - -static int -next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset, - uint32_t flags, int *err) -{ - struct backend *b_next = nxdata; - return backend_pread (b_next, buf, count, offset, flags, err); -} - -static int -next_pwrite (void *nxdata, const void *buf, uint32_t count, uint64_t offset, - uint32_t flags, int *err) -{ - struct backend *b_next = nxdata; - return backend_pwrite (b_next, buf, count, offset, flags, err); -} - -static int -next_flush (void *nxdata, uint32_t flags, int *err) -{ - struct backend *b_next = nxdata; - return backend_flush (b_next, flags, err); -} - -static int -next_trim (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, - int *err) -{ - struct backend *b_next = nxdata; - return backend_trim (b_next, count, offset, flags, err); -} - -static int -next_zero (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, - int *err) -{ - struct backend *b_next = nxdata; - return backend_zero (b_next, count, offset, flags, err); -} - -static int -next_extents (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, - struct nbdkit_extents *extents, int *err) -{ - struct backend *b_next = nxdata; - return backend_extents (b_next, count, offset, flags, extents, err); -} - -static int -next_cache (void *nxdata, uint32_t count, uint64_t offset, - uint32_t flags, int *err) -{ - struct backend *b_next = nxdata; - return backend_cache (b_next, count, offset, flags, err); -} - static struct nbdkit_next_ops next_ops = { - .reopen = next_reopen, - .get_size = next_get_size, - .can_write = next_can_write, - .can_flush = next_can_flush, - .is_rotational = next_is_rotational, - .can_trim = next_can_trim, - .can_zero = next_can_zero, - .can_fast_zero = next_can_fast_zero, - .can_extents = next_can_extents, - .can_fua = next_can_fua, - .can_multi_conn = next_can_multi_conn, - .can_cache = next_can_cache, - .pread = next_pread, - .pwrite = next_pwrite, - .flush = next_flush, - .trim = next_trim, - .zero = next_zero, - .extents = next_extents, - .cache = next_cache, + .reopen = backend_reopen, + .get_size = backend_get_size, + .can_write = backend_can_write, + .can_flush = backend_can_flush, + .is_rotational = backend_is_rotational, + .can_trim = backend_can_trim, + .can_zero = backend_can_zero, + .can_fast_zero = backend_can_fast_zero, + .can_extents = backend_can_extents, + .can_fua = backend_can_fua, + .can_multi_conn = backend_can_multi_conn, + .can_cache = backend_can_cache, + .pread = backend_pread, + .pwrite = backend_pwrite, + .flush = backend_flush, + .trim = backend_trim, + .zero = backend_zero, + .extents = backend_extents, + .cache = backend_cache, }; static int diff --git a/server/internal.h b/server/internal.h index 9d314bf8..eaec31ba 100644 --- a/server/internal.h +++ b/server/internal.h @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -40,6 +40,7 @@ #include <pthread.h> #define NBDKIT_API_VERSION 2 +#define NBDKIT_INTERNAL #include "nbdkit-plugin.h" #include "nbdkit-filter.h" #include "cleanup.h" -- 2.24.1

2 2