11:14 a.m.
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 nbdkit
just added a way to request a prefix be added to any relative dlopen()
requests, feeding it the libdir= parameter learned during .config
after we confirm that directory worked for loading VDDK.
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: Several modifications to Rich's initial patch, mainly to take
advantage of the new nbdkit_set_dlopen_prefix]
---
plugins/vddk/nbdkit-vddk-plugin.pod | 39 +++++++++++++++++++------
plugins/vddk/vddk.c | 44 +++++++++++++++++++++++++----
tests/test-vddk-real.sh | 10 +------
tests/test-vddk.sh | 6 +---
4 files changed, 71 insertions(+), 28 deletions(-)
diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod
index a7300e94..11d12c3f 100644
--- a/plugins/vddk/nbdkit-vddk-plugin.pod
+++ b/plugins/vddk/nbdkit-vddk-plugin.pod
@@ -233,26 +233,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/vddk.c b/plugins/vddk/vddk.c
index 0abec68e..d3c4483e 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
@@ -247,8 +248,14 @@ static void
load_library (void)
{
static const char *sonames[] = {
- /* Prefer the newest library in case multiple exist. */
+ /* Prefer the newest library in case multiple exist. This list
+ * intentionally allows a user to point to either the top-level
+ * installation directory, or the lib64/ subdirectory thereof, in
+ * part because it makes our test setup easier.
+ */
+ "lib64/libvixDiskLib.so.6",
"libvixDiskLib.so.6",
+ "lib64/libvixDiskLib.so.5",
"libvixDiskLib.so.5",
};
size_t i;
@@ -256,9 +263,28 @@ load_library (void)
/* 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 = dlopen (path, RTLD_NOW);
+ if (dl != NULL) {
+ /* Now that we found the library, inform nbdkit's dlopen shim
+ * about our desired prefix based on libdir for all future
+ * loads. This may modify path in-place, but that's okay.
+ */
+ char *dir = dirname (path);
+ if (nbdkit_set_dlopen_prefix (dir) == -1)
+ exit (EXIT_FAILURE);
+ nbdkit_debug("dlopen shim prefix set to %s", dir);
break;
+ }
if (i == 0) {
orig_error = dlerror ();
if (orig_error)
@@ -268,7 +294,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 +357,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/test-vddk-real.sh b/tests/test-vddk-real.sh
index 3a888eac..f155025d 100755
--- a/tests/test-vddk-real.sh
+++ b/tests/test-vddk-real.sh
@@ -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" 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 9ee46aa8..927e29fd 100755
--- a/tests/test-vddk.sh
+++ b/tests/test-vddk.sh
@@ -37,15 +37,11 @@ set -x
rm -f test-vddk.out
cleanup_fn rm -f test-vddk.out
-LD_LIBRARY_PATH=.libs:$LD_LIBRARY_PATH \
-LIBRARY_PATH=.libs:$LIBRARY_PATH \
-nbdkit vddk --dump-plugin > test-vddk.out
+nbdkit vddk libdir=.libs --dump-plugin > test-vddk.out
cat test-vddk.out
grep ^vddk_default_libdir= test-vddk.out
# Also test our magic file= handling, even though the dummy driver doesn't
# really open a file.
-LD_LIBRARY_PATH=.libs:$LD_LIBRARY_PATH \
-LIBRARY_PATH=.libs:$LIBRARY_PATH \
nbdkit -U - vddk libdir=.libs /dev/null --run true
--
2.24.1