similar to: [nbdkit PATCH] vddk: Make 'file=' a magic key

On Mon, Feb 17, 2020 at 12:45:58PM +0000, Richard W.M. Jones wrote: > On Thu, Feb 13, 2020 at 04:49:59PM -0600, Eric Blake wrote: > > Since it is required, it might as well be magic ;) > > > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > > > > As written, applies on top of my v3 patch for dlopen; but it would > > be easy enough to

On Thu, Feb 13, 2020 at 04:49:59PM -0600, Eric Blake wrote: > Since it is required, it might as well be magic ;) > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > > 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 > >

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

This is my second attempt at this. The first version (also not working) was here: https://www.redhat.com/archives/libguestfs/2019-October/msg00062.html In part 1/2 I introduce a new .ready_to_serve plugin method which is called after forking and just before accepting any client connection. The idea would be that plugins could start background threads here. However this doesn't work well in

In v7: everything should work now! The re-exec code is slightly simplified, with Rich's suggestion to pass the original LD_LIBRARY_PATH rather than just the prefix being added, and I've now finished wiring up the initial dlopen() check into code that correctly computes the right prefix dir to add to LD_LIBRARY_PATH. Eric Blake (1): vddk: Add re-exec with altered environment Richard

VDDK creates background threads. fork kills these, resulting in the library hanging or failing completely in certain configurations. --- plugins/vddk/vddk.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c index e6639b4d..a481e8bf 100644 --- a/plugins/vddk/vddk.c +++ b/plugins/vddk/vddk.c @@ -431,12

I'm a lot happier with this version: no mucking with dlmopen(). It does add a bit more coupling between nbdkit proper and the vddk plugin (namely, nbdkit now exports a new function that the vddk plugin relies on), but patch 2 adds testsuite coverage of the new function so we shouldn't regress. Patch 1 and 2 are new, patch 3 is unchanged from when Rich posted it in v2, and patch 4 is

Differences from v4: Patch 1 is simplified: I realized that since we already use -rdynamic for nbdkit (after all, we WANT our dlopen()d plugins to be able to call our nbdkit_* exports), it is only a matter of adding dlopen to the set of symbols that we export. With that done, there is no separate shared library needed; our dlopen shim is now part of nbdkit proper, and we don't have to tweak

From: "Richard W.M. Jones" <rjones@redhat.com> 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’

On Thu, Aug 6, 2020, 16:16 Richard W.M. Jones <rjones@redhat.com> wrote: > See comment in code and > https://www.redhat.com/archives/libguestfs/2020-August/msg00023.html > --- > plugins/vddk/vddk.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c > index c24da96f..5926e181 100644

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

We had a query yesterday about the VDDK plugin and making it actually obey the weird "Multithreading Considerations" rules in the VDDK documentation (https://vdc-download.vmware.com/vmwb-repository/dcr-public/8f96698a-0e7b-4d67-bb6c-d18a1d101540/ef536a47-27cd-481a-90ef-76b38e75353c/vsphere-vddk-671-programming-guide.pdf) This patch is my attempt to implement this. The idea is that the

I believe this roughly implements Nir's proposal here: https://www.redhat.com/archives/libguestfs/2020-August/msg00028.html Unfortunately it doesn't work for me. It actually slows things down quite a lot, for reasons I don't understand. Note the adjustment of the pool-max parameter and how it affects the total time. The results are quite reproducible. $ ./nbdkit -r -U - vddk

From: "Richard W.M. Jones" <rjones@redhat.com> 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’

From: "Richard W.M. Jones" <rjones@redhat.com> 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’

In theory this patch depends on this series: https://www.redhat.com/archives/libguestfs/2020-August/msg00021.html In practice I believe they're independent of each other, but the above series makes it easier to test. Rich.

See comment in code and https://www.redhat.com/archives/libguestfs/2020-August/msg00023.html --- plugins/vddk/vddk.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c index 31e58f66..95e030d4 100644 --- a/plugins/vddk/vddk.c +++ b/plugins/vddk/vddk.c @@ -511,17 +511,29 @@ vddk_dump_plugin (void) #endif

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

I like this change. I think we were overloading the config_complete method before to do two different things (complete configuration; do any allocation/housekeeping necessary before we can start serving). The only questions in my mind are whether we want this before 1.18, and whether the name ("get_ready") is a good one. Rich.

In can_extents we test whether the VDDK function VixDiskLib_QueryAllocatedBlocks works before declaring that extents are supported. This VDDK function can fail in several ways but most notable if the server doesn't support querying extents. In this case VDDK raises an error message through the error_function callback, which we turn into a call to nbdkit_error. In most cases this will cause