search for: vixdisklib_initex

...615af1..c5a68430 100644 --- a/plugins/vddk/vddk-stubs.h +++ b/plugins/vddk/vddk-stubs.h @@ -44,10 +44,6 @@ * which is the earliest version of VDDK that we support. */ -/* We treat these two stubs slightly specially for the benefit of - * tests/dummy-vddk.c. - */ -#ifndef NO_INITEX_STUB STUB (VixDiskLib_InitEx, VixError, (uint32_t major, uint32_t minor, @@ -56,13 +52,9 @@ STUB (VixDiskLib_InitEx, VixDiskLibGenericLogFunc *panic_function, const char *lib_dir, const char *config_file)); -#endif -#ifndef NO_EXIT_STUB STUB (VixDiskLib_Exit, void, (void));...

We had a query yesterday about the VDDK plugin and making it actually obey the weird "Multithreading Considerations" rules in the VDDK documentation (https://vdc-download.vmware.com/vmwb-repository/dcr-public/8f96698a-0e7b-4d67-bb6c-d18a1d101540/ef536a47-27cd-481a-90ef-76b38e75353c/vsphere-vddk-671-programming-guide.pdf) This patch is my attempt to implement this. The idea is that the

....\n" \ + "Many optional parameters are supported, see nbdkit-vddk-plugin(3)." + static int vddk_get_ready (void) { - VixError err; - load_library (true); + return 0; +} + +/* Defer VDDK initialization until after fork because it is known to + * create background threads from VixDiskLib_InitEx. Unfortunately + * error reporting from this callback is difficult, but we have + * already checked in .get_ready that the library is dlopenable. + * + * For various hangs and failures which were caused by background + * threads and fork see: + * https://bugzilla.redhat.com/show_bug.cgi?id=1846309...

This is my second attempt at this. The first version (also not working) was here: https://www.redhat.com/archives/libguestfs/2019-October/msg00062.html In part 1/2 I introduce a new .ready_to_serve plugin method which is called after forking and just before accepting any client connection. The idea would be that plugins could start background threads here. However this doesn't work well in

...@@ -453,18 +493,26 @@ 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. */ DEBUG_CALL ("VixDiskLib_InitEx", "%d, %d, &debug_fn, &error_fn, &error_fn, %s, %s", VDDK_MAJOR, VDDK_MINOR, - libdir ? : VDDK_LIBDIR, config ? : "NULL"); + libdir, config ? : "NULL"); err = VixDiskLib_InitEx (VDDK_MAJOR, VDD...

In v7: everything should work now! The re-exec code is slightly simplified, with Rich's suggestion to pass the original LD_LIBRARY_PATH rather than just the prefix being added, and I've now finished wiring up the initial dlopen() check into code that correctly computes the right prefix dir to add to LD_LIBRARY_PATH. Eric Blake (1): vddk: Add re-exec with altered environment Richard

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

--- 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])

On Tue, Feb 18, 2020 at 11:05:11AM -0600, Eric Blake wrote: > static const char *sonames[] = { > - /* Prefer the newest library in case multiple exist. */ > + /* Prefer the newest library in case multiple exist. Check two > + * possible directories: the usual VDDK installation puts .so > + * files in an arch-specific subdirectory of $libdir (although > + *

...} + } + } + /* For remote connections, check all the parameters have been * passed. Note that VDDK will segfault if parameters that it * expects are NULL (and there's no real way to tell what parameters @@ -347,7 +479,7 @@ vddk_config_complete (void) VDDK_ERROR (err, "VixDiskLib_InitEx"); exit (EXIT_FAILURE); } - init_called = 1; + init_called = true; return 0; } -- 2.24.1

...en-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/plu...

Probably needs a bit of cleanup, but seems like it is generally the right direction. One thing I've noticed is that the expect test randomly (but rarely) hangs :-( I guess something is racey but I don't know what at the moment. Rich.

Differences from v4: Patch 1 is simplified: I realized that since we already use -rdynamic for nbdkit (after all, we WANT our dlopen()d plugins to be able to call our nbdkit_* exports), it is only a matter of adding dlopen to the set of symbols that we export. With that done, there is no separate shared library needed; our dlopen shim is now part of nbdkit proper, and we don't have to tweak

I'm a lot happier with this version: no mucking with dlmopen(). It does add a bit more coupling between nbdkit proper and the vddk plugin (namely, nbdkit now exports a new function that the vddk plugin relies on), but patch 2 adds testsuite coverage of the new function so we shouldn't regress. Patch 1 and 2 are new, patch 3 is unchanged from when Rich posted it in v2, and patch 4 is

If you have a plugin which either creates background threads itself or uses a library that creates background threads, it turns out you cannot create these in .get_ready (or earlier). The reason is that nbdkit forks when either daemonizing itself or using the --run option, and fork cancels all the background threads in the child process (the daemonized or captive nbdkit). The only good solution

...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 + * t...