On 2/18/20 11:28 AM, Richard W.M. Jones wrote:
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
> + * only VDDK 5 supported 32-bit); but in our testsuite is easier
> + * to write if we point libdir directly to a stub .so.
> + */
> + "lib64/libvixDiskLib.so.6",
> "libvixDiskLib.so.6",
> + "lib64/libvixDiskLib.so.5",
> + "lib32/libvixDiskLib.so.5",
> "libvixDiskLib.so.5",
Should we put #ifdef around the lib64/ and lib32/ versions?
Not necessary: dlopen() fails to open a .so from the wrong arch (on a
32-bit platform, even though lib64/libvixDiskLib.so.5 exists, it will
fail to load, and we'll then try dlopen()ing lib32/libvixDiskLib.so.5).
We don't re-exec until after a successful dlopen(), but it was not the
initial dlopen() that was giving us grief, but the fact that
VixDiskLib_InitEx() triggers a further dlopen() which in turn has an
unanchored dependency, and this still re-exec's before that point.
Another alternative might be to get rid of the lib32/ version
entirely, only compile the plugin for x86-64, and set the minimum
version of VDDK in the docs to 5.5.5 (see nbdkit-vddk-plugin.pod
section "SUPPORTED VERSIONS OF VDDK").
Would be reasonable as a followup patch.
But the series looks good now, so
ACK
Thanks for the tremendous amount of effort that went into the
apparently simple change :-)
You bet. Took both of us longer than expected, but I like the results.
And I got to learn a lot about ld.so (there's something satisfying about
understanding how the black magic of shared libraries actually works)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization:
qemu.org |
libvirt.org