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 Berrangé, Ming Xie, Eric Blake. --- plugins/vddk/nbdkit-vddk-plugin.pod | 39 ++++++++++++++++++++------- configure.ac | 1 + plugins/vddk/vddk.c | 42 ++++++++++++++++++++++++++--- tests/test-vddk-real.sh | 12 ++------- tests/test-vddk.sh | 19 ++++++++----- 5 files changed, 85 insertions(+), 28 deletions(-) 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/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..487ebba6 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); @@ -294,6 +305,8 @@ static int vddk_config_complete (void) { VixError err; + const char *saved_ld_library_path; + CLEANUP_FREE char *ld_library_path; if (filename == NULL) { nbdkit_error ("you must supply the file=<FILENAME> parameter " @@ -333,7 +346,24 @@ vddk_config_complete (void) load_library (); - /* Initialize VDDK library. */ + /* Initialize VDDK library. + * + * We have to set LD_LIBRARY_PATH around this call. This is because + * InitEx() loads plugins like Advanced Transport, but cannot find + * them unless the environment variable is set. + * + * This is OK as we're single-threaded here, but if VDDK decided in + * future to lazily load plugins later on then this would break. + */ + saved_ld_library_path = getenv ("LD_LIBRARY_PATH"); + if (asprintf (&ld_library_path, "%s/lib%d", + libdir ? : VDDK_LIBDIR, 8*SIZEOF_LONG) == -1) { + nbdkit_error ("asprintf: %m"); + exit (EXIT_FAILURE); + } + nbdkit_debug ("setting LD_LIBRARY_PATH=%s", ld_library_path); + setenv ("LD_LIBRARY_PATH", ld_library_path, 1); + DEBUG_CALL ("VixDiskLib_InitEx", "%d, %d, &debug_fn, &error_fn, &error_fn, %s, %s", VDDK_MAJOR, VDDK_MINOR, @@ -349,6 +379,12 @@ vddk_config_complete (void) } init_called = 1; + nbdkit_debug ("restoring LD_LIBRARY_PATH"); + if (saved_ld_library_path) + setenv ("LD_LIBRARY_PATH", saved_ld_library_path, 1); + else + unsetenv ("LD_LIBRARY_PATH"); + return 0; } 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 _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs