Re: [Libguestfs] [nbdkit PATCH v3] vddk: Drive library loading from libdir parameter.
On 2/13/20 4:46 PM, Eric Blake wrote: 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. One other depressing note: https://sourceware.org/bugzilla/show_bug.cgi?id=15971 https://sourceware.org/bugzilla/show_bug.cgi?id=11839 Both gdb and glibc need patches applied before you can debug anything loaded via dlmopen(); ideas for patches were last discussed 7 years ago, but still nothing has landed. We're unlikely to need gdb debugging of vddk code itself (after all, it's already a proprietary library, so you can't follow along with source code), but the lack of debuggability of the shim code makes maintenance that much harder (it already cost me some time today when I had to frequently recompile and resort to printf debugging) - if we decide to go with a patch along these lines, we'd better be sure it is robust enough to not need to be inspecting the code in gdb any time soon. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [nbdkit PATCH] vddk: Make 'file=' a magic key
Since it is required, it might as well be magic ;)
Signed-off-by: Eric Blake
---
As written, applies on top of my v3 patch for dlopen; but it would
be easy enough to rebase and take this one now even if we aren't
sure about the dlopen stuff
plugins/vddk/nbdkit-vddk-plugin.pod | 5 -
plugins/vddk/vddk.c | 3 ++-
tests/test-vddk.sh | 3 +--
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod
b/plugins/vddk/nbdkit-vddk-plugin.pod
index f0748def..11d12c3f 100644
--- a/plugins/vddk/nbdkit-vddk-plugin.pod
+++ b/plugins/vddk/nbdkit-vddk-plugin.pod
@@ -4,7 +4,7 @@ nbdkit-vddk-plugin - nbdkit VMware VDDK plugin
=head1 SYNOPSIS
- nbdkit vddk file=FILENAME
+ nbdkit vddk [file=]FILENAME
[config=FILENAME] [cookie=COOKIE] [libdir=LIBRARY]
[nfchostport=PORT] [single-link=true]
[password=PASSWORD | password=- | password=+FILENAME
@@ -135,6 +135,9 @@ If a VM has multiple disks, nbdkit can only serve one at a
time. To
serve more than one you must run multiple copies of nbdkit. (See
L below).
+C is a magic config key and may be omitted in most cases.
+See L.
+
=item BPATHNAME
This sets the path of the VMware VDDK distribution.
diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c
index 6deb0a0b..344b4e6b 100644
--- a/plugins/vddk/vddk.c
+++ b/plugins/vddk/vddk.c
@@ -416,7 +416,7 @@ vddk_config_complete (void)
}
#define vddk_config_help \
- "file= (required) The filename (eg. VMDK file) to serve.\n" \
+ "[file=] (required) The filename (eg. VMDK file) to serve.\n" \
"Many optional parameters are supported, see nbdkit-vddk-plugin(3)."
static void
@@ -901,6 +901,7 @@ static struct nbdkit_plugin plugin = {
.config= vddk_config,
.config_complete = vddk_config_complete,
.config_help = vddk_config_help,
+ .magic_config_key = "file",
.dump_plugin = vddk_dump_plugin,
.open = vddk_open,
.close = vddk_close,
diff --git a/tests/test-vddk.sh b/tests/test-vddk.sh
index d99ebf88..6933f716 100755
--- a/tests/test-vddk.sh
+++ b/tests/test-vddk.sh
@@ -48,8 +48,7 @@ grep ^vddk_default_libdir= test-vddk.out
# 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 || :
+nbdkit vddk libdir=.libs /dev/null --run ':' 2> test-vddk.err || :
cat test-vddk.err
grep '/.libs/nosuch' test-vddk.err
--
2.24.1
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [nbdkit PATCH v3] vddk: Drive library loading from libdir parameter.
From: "Richard W.M. Jones" 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>) is located on a -non-standard path, you may need to set C or modify -F before this plugin will work. In addition you may -want to set the C 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. The reason for this is that the VDDK library is shipped +with recompiled libraries like F and F +that can conflict with system libraries. -Usually the
[Libguestfs] [PATCH v3 0/1] tools: add '--blocksize' option for C-based tools
From: Nikolay Ivanets This patch depends on changes in 'common' sub-module posted here: https://www.redhat.com/archives/libguestfs/2020-February/msg00150.html v3 fixes issue found during code review: - options now appear in alphabetical order v2: Almost the same as v1 except '--blocksize' option description is moved into a common submodule (similar to key-option.pod). v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00097.html Nikolay Ivanets (1): tools: add '--blocksize' option for C-based tools align/Makefile.am | 1 + align/scan.c | 8 align/virt-alignment-scan.pod | 2 ++ cat/Makefile.am | 1 + cat/cat.c | 8 cat/filesystems.c | 8 cat/log.c | 8 cat/ls.c | 8 cat/tail.c| 8 cat/virt-cat.pod | 2 ++ cat/virt-filesystems.pod | 2 ++ cat/virt-log.pod | 2 ++ cat/virt-ls.pod | 2 ++ cat/virt-tail.pod | 2 ++ df/Makefile.am| 1 + df/main.c | 8 df/virt-df.pod| 2 ++ diff/diff.c | 8 diff/virt-diff.pod| 2 ++ edit/edit.c | 8 edit/virt-edit.pod| 2 ++ fish/fish.c | 8 fish/guestfish.pod| 2 ++ format/Makefile.am| 1 + format/format.c | 8 format/virt-format.pod| 2 ++ fuse/guestmount.c | 8 fuse/guestmount.pod | 2 ++ inspector/inspector.c | 8 inspector/virt-inspector.pod | 2 ++ rescue/Makefile.am| 1 + rescue/rescue.c | 8 rescue/virt-rescue.pod| 2 ++ 33 files changed, 145 insertions(+) -- 2.17.2 ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH v3 1/1] tools: add '--blocksize' option for C-based tools
From: Nikolay Ivanets
This patch adds '--blocksize' command line option for guestfish and
other C-based tools. This option allows specifying disk sector size.
---
align/Makefile.am | 1 +
align/scan.c | 8
align/virt-alignment-scan.pod | 2 ++
cat/Makefile.am | 1 +
cat/cat.c | 8
cat/filesystems.c | 8
cat/log.c | 8
cat/ls.c | 8
cat/tail.c| 8
cat/virt-cat.pod | 2 ++
cat/virt-filesystems.pod | 2 ++
cat/virt-log.pod | 2 ++
cat/virt-ls.pod | 2 ++
cat/virt-tail.pod | 2 ++
df/Makefile.am| 1 +
df/main.c | 8
df/virt-df.pod| 2 ++
diff/diff.c | 8
diff/virt-diff.pod| 2 ++
edit/edit.c | 8
edit/virt-edit.pod| 2 ++
fish/fish.c | 8
fish/guestfish.pod| 2 ++
format/Makefile.am| 1 +
format/format.c | 8
format/virt-format.pod| 2 ++
fuse/guestmount.c | 8
fuse/guestmount.pod | 2 ++
inspector/inspector.c | 8
inspector/virt-inspector.pod | 2 ++
rescue/Makefile.am| 1 +
rescue/rescue.c | 8
rescue/virt-rescue.pod| 2 ++
33 files changed, 145 insertions(+)
diff --git a/align/Makefile.am b/align/Makefile.am
index 1c698561b..6ff4aa3bf 100644
--- a/align/Makefile.am
+++ b/align/Makefile.am
@@ -66,6 +66,7 @@ stamp-virt-alignment-scan.pod: virt-alignment-scan.pod
$(PODWRAPPER) \
--man virt-alignment-scan.1 \
--html $(top_builddir)/website/virt-alignment-scan.1.html \
+ --path $(top_srcdir)/common/options \
--license GPLv2+ \
--warning safe \
$<
diff --git a/align/scan.c b/align/scan.c
index b9f29868c..7209bc02a 100644
--- a/align/scan.c
+++ b/align/scan.c
@@ -87,6 +87,8 @@ usage (int status)
" %s [--options] -a disk.img [-a disk.img ...]\n"
"Options:\n"
" -a|--add image Add image\n"
+ " --blocksize[=512|4096]\n"
+ " Set sector size of the disk for -a
option\n"
" -c|--connect uri Specify libvirt URI for -d option\n"
" -d|--domain guestAdd disks from libvirt guest\n"
" --format[=raw|..]Force disk format for -a option\n"
@@ -116,6 +118,7 @@ main (int argc, char *argv[])
static const char options[] = "a:c:d:P:qvVx";
static const struct option long_options[] = {
{ "add", 1, 0, 'a' },
+{ "blocksize", 2, 0, 0 },
{ "connect", 1, 0, 'c' },
{ "domain", 1, 0, 'd' },
{ "format", 2, 0, 0 },
@@ -131,6 +134,8 @@ main (int argc, char *argv[])
struct drv *drvs = NULL;
const char *format = NULL;
bool format_consumed = true;
+ int blocksize = 0;
+ bool blocksize_consumed = true;
int c;
int option_index;
int exit_code;
@@ -153,6 +158,8 @@ main (int argc, char *argv[])
display_short_options (options);
else if (STREQ (long_options[option_index].name, "format")) {
OPTION_format;
+ } else if (STREQ (long_options[option_index].name, "blocksize")) {
+OPTION_blocksize;
} else if (STREQ (long_options[option_index].name, "uuid")) {
uuid = 1;
} else
@@ -215,6 +222,7 @@ main (int argc, char *argv[])
usage (EXIT_FAILURE);
CHECK_OPTION_format_consumed;
+ CHECK_OPTION_blocksize_consumed;
/* virt-alignment-scan has two modes. If the user didn't specify
* any drives, then we do the scan on every libvirt guest. That's
diff --git a/align/virt-alignment-scan.pod b/align/virt-alignment-scan.pod
index 19953546e..83645f4d0 100644
--- a/align/virt-alignment-scan.pod
+++ b/align/virt-alignment-scan.pod
@@ -121,6 +121,8 @@ force a particular format use the I<--format=..> option.
Add a remote disk. See L.
+__INCLUDE:blocksize-option.pod__
+
=item B<-c> URI
=item B<--connect> URI
diff --git a/cat/Makefile.am b/cat/Makefile.am
index 1d013e8dd..01e13abda 100644
--- a/cat/Makefile.am
+++ b/cat/Makefile.am
@@ -200,6 +200,7 @@ stamp-virt-filesystems.pod: virt-filesystems.pod
$(PODWRAPPER) \
--man virt-filesystems.1 \
--html $(top_builddir)/website/virt-filesystems.1.html \
+ --path $(top_srcdir)/common/options \
--license GPLv2+ \
--warning safe \
$<
diff --git a/cat/cat.c b/cat/cat.c
index 4f0d7035e..a968640ce 100644
--- a/cat/cat.c
+++ b/cat/cat.c
@@ -66,6 +66,8 @@ usage (int status)
" %s [--options] -a disk.img [-a disk.img ...] file [file
...]\n"
"Options:\n"
" -a|--add image Add image\n"
+ "
[Libguestfs] [common PATCH v4 1/1] options: add '--blocksize' option for C-based tools
From: Nikolay Ivanets
This patch adds '--blocksize' command line option parsing and handling
for guestfish and other C-based tools which share the same code from
this sub-module.
'--blocksize' will be a common for almost all libguestfs-based tools and
thus parameter description will be repeated all the time. Let's move
it into blocksize-option.pod and include everywhere we need.
---
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
diff --git a/options/Makefile.am b/options/Makefile.am
index 28940f1..394f668 100644
--- a/options/Makefile.am
+++ b/options/Makefile.am
@@ -18,7 +18,8 @@
include $(top_srcdir)/subdir-rules.mk
EXTRA_DIST = \
- key-option.pod
+ key-option.pod \
+ blocksize-option.pod
# liboptions.la contains guestfish code which is used in other
# C tools for options parsing and a few other things
diff --git a/options/blocksize-option.pod b/options/blocksize-option.pod
new file mode 100644
index 000..7b96ebf
--- /dev/null
+++ b/options/blocksize-option.pod
@@ -0,0 +1,11 @@
+=item B<--blocksize=512>
+
+=item B<--blocksize=4096>
+
+=item B<--blocksize>
+
+This parameter sets the sector size of the disk image. It affects all
+explicitly added subsequent disks after this parameter. Using
+I<--blocksize> with no argument switches the disk sector size to the
+default value which is usually 512 bytes. See also
+L.
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, _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, _optargs);
if (r == -1)
diff --git a/options/options.h b/options/options.h
index 9b78302..4716e6f 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,16 +166,18 @@ extern void mount_mps (struct mp *mp);
extern void free_drives (struct drv *drv);
[Libguestfs] [common PATCH v4 0/1] options: add '--blocksize' option for C-based tools
From: Nikolay Ivanets 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 ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit v2 2/3] NOT WORKING: vddk: Drive library loading from libdir parameter.
On 2/13/20 10:04 AM, Richard W.M. Jones wrote:
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-then-making-the-dlopen-call).
So currently we must have nbdkit start up with $LD_LIBRARY_PATH set,
in other words how it works at the moment.
Here's my rough [untested] idea, after reading
https://hackerboss.com/overriding-system-functions-for-fun-and-profit/.
Create a shim shared library that implements only one function - a
replacement dlopen(), something like:
#define _GNU_SOURCE
#include
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.
*/
char *override_dir;
void *dlopen(const char *filename, int flags)
{
if (override_dir && !strchr(filename, '/'))
... code to rewrite filename into override_dir/filename
return orig_dlopen (filename, flags);
}
void
_init(void)
{
orig_dlopen = dlsym(RTLD_NEXT, "dlopen");
}
then in our vddk plugin, we dlmopen(LM_ID_NEWLM, "shim", flags) to
create a new namespace, dlinfo() to learn the id of that namespace, then
override_dir = libdir; dlmopen(id, "vddk", flags) so that vddk gets
loaded into that same namespace but where all of its relative loads get
rewritten into absolute loads.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH nbdkit v2 2/3] NOT WORKING: vddk: Drive library loading from libdir parameter.
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>) is located on a
-non-standard path, you may need to set C or modify
-F before this plugin will work. In addition you may
-want to set the C 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. The reason for this is that the VDDK library is shipped
+with recompiled libraries like F and F
+that can conflict with system libraries.
-Usually the VDDK distribution directory should be passed as the
-C parameter and set C to the F
-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 parameter to point to
+F (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 and
F<$HOME/.vmware/config>, but you can override this using the C
parameter.
+=head2 No need to set C
+
+In nbdkit E 1.16 you had to set the environment variable
+C when using this plugin. In nbdkit E 1.18 this
+is I recommended.
+
=head1 FILE PARAMETER
The C 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 (, "%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;
+
[Libguestfs] [PATCH nbdkit v2 3/3] NOT WORKING vddk: Use dlmopen to isolate VDDK.
---
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 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 487ebba6..30dd6375 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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH nbdkit v2 2/3] NOT WORKING: vddk: Drive library loading from libdir parameter.
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-then-making-the-dlopen-call). So currently we must have nbdkit start up with $LD_LIBRARY_PATH set, in other words how it works at the moment. Rich. ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH nbdkit v2 1/3] vddk: Delay loading VDDK until config_complete.
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, _fn, _fn, _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
___
Libguestfs mailing
Re: [Libguestfs] [PATCH nbdkit 2/2] vddk: Drive library loading from libdir parameter.
On Thu, Feb 13, 2020 at 08:23:32AM -0600, Eric Blake wrote:
> On 2/13/20 8:01 AM, Richard W.M. Jones wrote:
> >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.
>
> Do you want to use Dan's preferred UTF-8 spelling?
I'll fix this. Hope mxie doesn't mind being "romanized" :-)
> >+if (asprintf (, "%s/lib%d/%s",
> >+ libdir ? : VDDK_LIBDIR, 8*SIZEOF_LONG, sonames[i]) == -1)
> >{
>
> could you just spell this sizeof(long)*CHAR_BITS?
>
> I'm guessing that the vddk files always ship with a dir/lib32/xxx.so
> and a dir/lib64/xxx.so convention?
>
> Or, can we just hard-code the name '/lib64/', since...
It uses lib32/ or lib64/. i386 support was dropped in VDDK > 5.1.1,
but in theory we support it.
> But the idea looks sane to me. I don't have VDDK libraries installed
> so I can't readily test it, but assume this helps.
There's a larger problem with this patch as we discussed on IRC, so
I'll try to spin a second version.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit 2/2] vddk: Drive library loading from libdir parameter.
On 2/13/20 8:01 AM, Richard W.M. Jones wrote:
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.
Do you want to use Dan's preferred UTF-8 spelling?
---
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>) is located on a
-non-standard path, you may need to set C or modify
-F before this plugin will work. In addition you may
-want to set the C 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. The reason for this is that the VDDK library is shipped
+with recompiled libraries like F and F
+that conflict with system libraries.
Maybe "that can conflict", since we saw the issues on RHEL8 but not
RHEL7 (so whether there is a conflict depends on what version VDDK was
compiled against, rather than an unconditional event)
-Usually the VDDK distribution directory should be passed as the
-C parameter and set C to the F
-subdirectory:
+You can two choices: Place VDDK in the default libdir which is
s/can/have/
+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 parameter to point to
+F (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 and
F<$HOME/.vmware/config>, but you can override this using the C
parameter.
+=head2 No need to set C
+
+In nbdkit E 1.16 you had to set the environment variable
+C when using this plugin. In nbdkit E 1.18 this
+is I recommended.
+
=head1 FILE PARAMETER
The C 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])
Is this strictly necessary, or...
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 (, "%s/lib%d/%s",
+ libdir ? : VDDK_LIBDIR, 8*SIZEOF_LONG, sonames[i]) == -1) {
could you just spell this sizeof(long)*CHAR_BITS?
I'm guessing that the vddk files always ship with a dir/lib32/xxx.so and
a dir/lib64/xxx.so convention?
Or, can we just hard-code the name '/lib64/', since...
+++ 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
Re: [Libguestfs] [PATCH nbdkit] NOT WORKING vddk: Use dlmopen to isolate VDDK.
On Thu, Feb 13, 2020 at 02:06:47PM +, Richard W.M. Jones wrote:
> ---
> 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 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) {
The error when this patch is applied is:
nbdkit: debug: VDDK call: VixDiskLib_InitEx (5, 1, _fn, _fn,
_fn, /home/rjones/tmp/vddk-5.1.1/vmware-vix-disklib-distrib, NULL)
nbdkit: debug: VixDiskLib: linuxVerifySSLCertificates is 0
nbdkit: debug: VixDiskLib: config options: libdir
'/home/rjones/tmp/vddk-5.1.1/vmware-vix-disklib-distrib', tmpDir
'/tmp/vmware-rjones'.
nbdkit: debug: OBJLIB-LIB : Objlib initialized.
nbdkit: debug: VixDiskLib: Attempting to locate advanced transport module in
"/home/rjones/tmp/vddk-5.1.1/vmware-vix-disklib-distrib".
/home/rjones/d/nbdkit/server/nbdkit: error while loading shared libraries:
libcrypto.so.0.9.8: cannot open shared object file: No such file or directory
What appears to happen here is that libvixDiskLib.so.6 is loaded and
runs (see the VixDiskLib debug messages which come from VDDK itself).
However VDDK tries to open "Advanced Transport" (which is a VDDK
extension) and I think it's trying itself to use dlopen() and that is
failing.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH nbdkit] NOT WORKING vddk: Use dlmopen to isolate VDDK.
---
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 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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH nbdkit 2/2] vddk: Drive library loading from libdir parameter.
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>) is located on a
-non-standard path, you may need to set C or modify
-F before this plugin will work. In addition you may
-want to set the C 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. The reason for this is that the VDDK library is shipped
+with recompiled libraries like F and F
+that conflict with system libraries.
-Usually the VDDK distribution directory should be passed as the
-C parameter and set C to the F
-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 parameter to point to
+F (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 and
F<$HOME/.vmware/config>, but you can override this using the C
parameter.
+=head2 No need to set C
+
+In nbdkit E 1.16 you had to set the environment variable
+C when using this plugin. In nbdkit E 1.18 this
+is I recommended.
+
=head1 FILE PARAMETER
The C 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 (, "%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)
[Libguestfs] [PATCH nbdkit 1/2] vddk: Delay loading VDDK until config_complete.
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, _fn, _fn, _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
___
Libguestfs mailing
Re: [Libguestfs] [PATCH 2/2] firstboot: schedule firstboot as delayed task
> > I have really no clue about this. > > > > What is "~dpnx0"? > > I wanted to documented that in a comment at first. But then I decided > against that as it seemed that as it seemed such patterns are used in > multiple places in the code (ergo everyone knows). > > Looking now I see two occurrence in convert_windows.ml and one in > firstboot.ml. I guess picking one of those for the comment should be > enough. > For completeness, "~dpnx0" expands into the name of the running script (0) with full path (p), including drive letter (d) and filename (n) with extension (x). Tomas ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 2/2] firstboot: schedule firstboot as delayed task
On Tue, 11 Feb 2020 14:36:24 +
"Richard W.M. Jones" wrote:
> On Thu, Nov 21, 2019 at 12:04:18PM +0100, Tomáš Golembiovský wrote:
> > Instead of running firstboot scripts during early boot schedule a task
> > delayed for 1-2 minute.
> >
> > During the first boot, after virt-v2v conversion, Windows installs the
> > drivers injected by virit-v2v. When this installation is finished
> > Windows enforces some kind of internal reboot. This unfortunately
> > terminates any running firstboot scritps thus killing for example the
> > installation of qemu-ga MSI.
> >
> > Hopefully delaying the installtion to some later time can also fix
> > problem we sometimes saw on Windows 2012R2 when installing RHEV-APT,
> > where the installer terminated immediately with the error:
> >
> > Failed to connect to server. Error: 0x8007045B
> >
> > Signed-off-by: Tomáš Golembiovský
> > ---
> > mlcustomize/firstboot.ml | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/mlcustomize/firstboot.ml b/mlcustomize/firstboot.ml
> > index c3ebfd9..b4ca181 100644
> > --- a/mlcustomize/firstboot.ml
> > +++ b/mlcustomize/firstboot.ml
> > @@ -286,10 +286,18 @@ set log=%%firstboot%%\\log.txt
> > set scripts=%%firstboot%%\\scripts
> > set scripts_done=%%firstboot%%\\scripts-done
> >
> > -call :main >> \"%%log%%\" 2>&1
> > +call :main %%1 >> \"%%log%%\" 2>&1
> > exit /b
> >
> > :main
> > +
> > +if not '%%1' == 'real' (
> > +REM schedule delayed task
> > +schtasks.exe /Delete /TN Firstboot /F
> > +powershell.exe -command \"$d = (get-date).AddSeconds(119);
> > schtasks.exe /Create /SC ONCE /ST $d.ToString('HH:mm') /SD
> > $d.ToString('MM/dd/') /RU SYSTEM /TN Firstboot /TR \\\"%%~dpnx0
> > real\\\"\"
> > +exit /b
> > +)
> > +
> > echo starting firstboot service
>
> I have really no clue about this.
>
> What is "~dpnx0"?
I wanted to documented that in a comment at first. But then I decided
against that as it seemed that as it seemed such patterns are used in
multiple places in the code (ergo everyone knows).
Looking now I see two occurrence in convert_windows.ml and one in
firstboot.ml. I guess picking one of those for the comment should be
enough.
>
> I wwill say that in general we do need a way that we can order
> firstboot scripts so that some run before others. At the moment the
> Windows static IP script sometimes runs before the network device is
> created (see https://bugzilla.redhat.com/1788823). I guess we could
> hack that by making the sleep time above settable when registering a
> firstboot script, but what we really need is some kind of dependency
> system. I have no idea if Windows has a mechanism to do this already.
Such ordering or dependency mechanism would be nice, but I guess that's
beyond what command line tools provide. Maybe we could script that to
some degree if we turn the main firstboot script completely into
PowerShell. But that would mean opening a can of worms.
Tomas
>
> Rich.
>
> --
> Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> virt-df lists disk usage of guests without needing to install any
> software inside the virtual machine. Supports Linux and Windows.
> http://people.redhat.com/~rjones/virt-df/
>
--
Tomáš Golembiovský
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH v2 0/1] tools: add '--blocksize' option for C-based tools
On Wed, Feb 12, 2020 at 03:02:40AM +0200, Mykola Ivanets wrote: > From: Nikolay Ivanets > > This patch depends on changes in 'common' sub-module posted here: > https://www.redhat.com/archives/libguestfs/2020-February/msg00099.html > > v2: > Almost the same as v1 except '--blocksize' option description is moved > into a common submodule (similar to key-option.pod). Use of the POD file fragment is a good improvement in this version, but these options still need to appear in alphabetical order. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [common PATCH v2 1/1] options: add '--blocksize' option for C-based tools
This version fixes the trailing whitespace problem, and having the "blocksize-option.pod" file makes sense as well. (One day we should do the same for --format and other options). Still a certain amount of whitespace-change-only hunks going on though. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 1/1] tools: add '--blocksize' option for C-based tools
On Tue, Feb 11, 2020 at 11:51:17PM +0200, Mykola Ivanets wrote:
> diff --git a/align/scan.c b/align/scan.c
> index b9f29868c..5a456d3d6 100644
> --- a/align/scan.c
> +++ b/align/scan.c
> @@ -90,6 +90,8 @@ usage (int status)
>" -c|--connect uri Specify libvirt URI for -d option\n"
>" -d|--domain guestAdd disks from libvirt guest\n"
>" --format[=raw|..]Force disk format for -a option\n"
> + " --blocksize[=512|4096]\n"
> + " Set sector size of the disk for -a
> option\n"
>" --help Display brief help\n"
>" -P nr_threadsUse at most nr_threads\n"
>" -q|--quiet No output, just exit code\n"
These options need to be kept in alphabetical order.
> @@ -119,6 +121,7 @@ main (int argc, char *argv[])
> { "connect", 1, 0, 'c' },
> { "domain", 1, 0, 'd' },
> { "format", 2, 0, 0 },
> +{ "blocksize", 2, 0, 0 },
> { "help", 0, 0, HELP_OPTION },
> { "long-options", 0, 0, 0 },
> { "quiet", 0, 0, 'q' },
And here.
> diff --git a/align/virt-alignment-scan.pod b/align/virt-alignment-scan.pod
> index 19953546e..21b5339e0 100644
> --- a/align/virt-alignment-scan.pod
> +++ b/align/virt-alignment-scan.pod
> @@ -162,6 +162,18 @@ If you have untrusted raw-format guest disk images, you
> should use
> this option to specify the disk format. This avoids a possible
> security problem with malicious guests (CVE-2010-3851).
>
> +=item B<--blocksize=512>
> +
> +=item B<--blocksize=4096>
> +
> +=item B<--blocksize>
> +
> +This parameter sets the sector size of the disk image. Similar to
> +I<--format> option it affects all subsequent I<-a> options. Using
> +I<--blocksize> with no argument switches disk sector size to the
> +default value which usually 512 bytes. See also
> +L.
> +
> =item B<-P> nr_threads
And here.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [common PATCH] options: add '--blocksize' option for C-based tools
On Tue, Feb 11, 2020 at 11:41:20PM +0200, Mykola Ivanets wrote:
> 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) */
Trailing whitespace on this line.
> -#define OPTION_c\
> +#define OPTION_c \
>libvirt_uri = optarg
>
> -#define OPTION_d\
> +#define OPTION_d \
>option_d (optarg, )
>
> -#define OPTION_D\
> +#define OPTION_D \
>option_d (optarg, )
>
> -#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)
There are loads of whitespace-change-only hunks in this patch. Are
they necessary?
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
