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