[Libguestfs] [PATCH nbdkit 2/2] vddk: Drive library loading from libdir parameter.

Richard W.M. Jones rjones at redhat.com
Thu Feb 13 14:01:23 UTC 2020


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.

Instead rely on a relatively undocumented feature of dlopen which is
that when we pass in a full path it will try to load dependent
libraries from the same directory.

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 Berrange, Ming Xie, Eric Blake.
---
 plugins/vddk/nbdkit-vddk-plugin.pod | 29 ++++++++++++++++++++---------
 configure.ac                        |  1 +
 plugins/vddk/vddk.c                 | 15 +++++++++++++--
 tests/test-vddk-real.sh             | 12 ++----------
 tests/test-vddk.sh                  | 19 +++++++++++++------
 5 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod
index f34b9fba..5f32988b 100644
--- a/plugins/vddk/nbdkit-vddk-plugin.pod
+++ b/plugins/vddk/nbdkit-vddk-plugin.pod
@@ -230,17 +230,22 @@ 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 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 can two choices: 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
+
+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
@@ -250,6 +255,12 @@ 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/configure.ac b/configure.ac
index fa902945..d71f06e4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -130,6 +130,7 @@ AC_PROG_INSTALL
 AC_PROG_CPP
 AC_CANONICAL_HOST
 AC_SYS_LARGEFILE
+AC_CHECK_SIZEOF([long])
 
 AC_C_PROTOTYPES
 test "x$U" != "x" && AC_MSG_ERROR([Compiler not ANSI compliant])
diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c
index db61c1d8..c49eebcd 100644
--- a/plugins/vddk/vddk.c
+++ b/plugins/vddk/vddk.c
@@ -256,7 +256,18 @@ load_library (void)
 
   /* Load the library. */
   for (i = 0; i < sizeof sonames / sizeof sonames[0]; ++i) {
-    dl = dlopen (sonames[i], RTLD_NOW);
+    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/lib%d/%s",
+                  libdir ? : VDDK_LIBDIR, 8*SIZEOF_LONG, sonames[i]) == -1) {
+      nbdkit_error ("asprintf: %m");
+      exit (EXIT_FAILURE);
+    }
+
+    dl = dlopen (path, RTLD_NOW);
     if (dl != NULL)
       break;
     if (i == 0) {
@@ -268,7 +279,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);
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..c2df4d69 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,19 @@ source ./functions.sh
 set -e
 set -x
 
-rm -f test-vddk.out
-cleanup_fn rm -f test-vddk.out
+# Real VDDK only works on x86-64.  While this test doesn't test real
+# VDDK, we do need to know that we're on a 64 bit architecture
+# (because of the need for the lib64 path), so we might as well only
+# test x86-64.
+requires test $(uname -m) = x86_64
 
-LD_LIBRARY_PATH=.libs:$LD_LIBRARY_PATH \
-LIBRARY_PATH=.libs:$LIBRARY_PATH \
-nbdkit vddk --dump-plugin > test-vddk.out
+rm -rf vmware-vix-disklib-distrib test-vddk.out
+cleanup_fn rm -rf vmware-vix-disklib-distrib test-vddk.out
+
+mkdir -p vmware-vix-disklib-distrib/lib64
+cp .libs/libvixDiskLib.so* vmware-vix-disklib-distrib/lib64/
+
+nbdkit vddk libdir=vmware-vix-disklib-distrib --dump-plugin > test-vddk.out
 cat test-vddk.out
 
 grep ^vddk_default_libdir= test-vddk.out
-- 
2.25.0




More information about the Libguestfs mailing list