similar to: [PATCH NOT WORKING nbdkit] vddk: Restructure plugin to allow greater parallelism.

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

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

Probably needs a bit of cleanup, but seems like it is generally the right direction. One thing I've noticed is that the expect test randomly (but rarely) hangs :-( I guess something is racey but I don't know what at the moment. Rich.

I suppose we may as well implement the only other flag too ... It's not clear what this does, something like O_DIRECT I imagine. Rich.

Couple of fixes to return correct data and one nice-to-have clean-up which is not needed. I just find it nicer to read. Martin Kletzander (3): vddk: Use a separate handle for single-link=true vddk: Do not report hole extents to be zero with single-link=true vddk: Eliminate one needless goto plugins/vddk/vddk.c | 48 +++++++++++++++++++++++++++++++++------------ 1 file changed, 36

From: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- plugins/vddk/nbdkit-vddk-plugin.pod | 7 ++++++- plugins/vddk/vddk-structs.h | 1 + plugins/vddk/vddk.c | 10 ++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod

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

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.

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

Make it behave more or less like a real VDDK. Of course it ignores all parameters passed and emulates a blank disk, but that is sufficient to do some more realistic testing. --- plugins/vddk/vddk-stubs.h | 8 --- tests/dummy-vddk.c | 115 ++++++++++++++++++++++++++++++++++---- 2 files changed, 103 insertions(+), 20 deletions(-) diff --git a/plugins/vddk/vddk-stubs.h

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

Not sure what version we're up to, but this reimplements extents using the new simpler structure described in this thread: https://www.redhat.com/archives/libguestfs/2019-March/msg00077.html I also fixed most of the things that Eric pointed out in the previous review, although I need to go back over his replies and check I've got everything. This needs a bit more testing. However the

I'm not sure exactly which version we're up to, but let's say it's version 4. I'm a lot happier with this version: - all filters have been reviewed and changed where I think that's necessary - can_extents is properly defined and implemented now - NBD protocol is followed - I believe it addresses all previous review points where possible The "only" thing

The pool is only used for readonly connections, since writable connections usually take a lock on the server side and therefore you cannot open more than one. --- plugins/vddk/nbdkit-vddk-plugin.pod | 7 + plugins/vddk/vddk.c | 201 ++++++++++++++++++++++------ 2 files changed, 164 insertions(+), 44 deletions(-) diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod

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

On Thu, Aug 6, 2020, 16:16 Richard W.M. Jones <rjones@redhat.com> wrote: > The pool is only used for readonly connections, since writable > connections usually take a lock on the server side and therefore you > cannot open more than one. > --- > plugins/vddk/nbdkit-vddk-plugin.pod | 7 + > plugins/vddk/vddk.c | 201 ++++++++++++++++++++++------ > 2

I want to post this but mainly for discussion and early review. It's not safe for these patches to all go upstream yet (because not all filters have been checked/adjusted), but if any patches were to go upstream then probably 1 & 2 only are safe. File, VDDK, memory and data plugins all work, although I have only done minimal testing on them. The current tests, such as they are, all

If you have a plugin which either creates background threads itself or uses a library that creates background threads, it turns out you cannot create these in .get_ready (or earlier). The reason is that nbdkit forks when either daemonizing itself or using the --run option, and fork cancels all the background threads in the child process (the daemonized or captive nbdkit). The only good solution

As I've spent really too long today investigating this, I want to document this in a public email, even though there's nothing really that interesting here. One thing you find from search for VDD 6.7 / VixDiskLib_QueryAllocatedBlocks issues with Google is that we must be one of the very few users out there. And the other thing is that it's quite broken. All testing was done using

On 3/20/19 5:11 PM, Richard W.M. Jones wrote: > This uses a new API VixDiskLib_QueryAllocatedBlocks provided in > VDDK >= 6.7. > > Thanks: Martin Kletzander. > --- > plugins/vddk/vddk-structs.h | 15 +++- > plugins/vddk/vddk.c | 138 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 152 insertions(+), 1 deletion(-) > > +static int >