search for: vddk_handl

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

...*/ -#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS +#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL /* Lock protecting open/close calls - see above. */ static pthread_mutex_t open_close_lock = PTHREAD_MUTEX_INITIALIZER; -/* The per-connection handle. */ +struct handle; struct vddk_handle { + VixDiskLibHandle vddk_handle; + bool in_use; + struct handle *h; +}; +DEFINE_VECTOR_TYPE(vddk_handles, struct vddk_handle) + +/* The per-connection handle. */ +struct handle { VixDiskLibConnectParams *params; /* connection parameters */ VixDiskLibConnection connection; /* connection *...

...EAD_MODEL_SERIALIZE_REQUESTS > +#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL > > /* Lock protecting open/close calls - see above. */ > static pthread_mutex_t open_close_lock = PTHREAD_MUTEX_INITIALIZER; > > -/* The per-connection handle. */ > +struct handle; > struct vddk_handle { > + VixDiskLibHandle vddk_handle; > + bool in_use; > + struct handle *h; > +}; > +DEFINE_VECTOR_TYPE(vddk_handles, struct vddk_handle) > + > +/* The per-connection handle. */ > +struct handle { > VixDiskLibConnectParams *params; /* connection parameters */ >...

...T_THREAD_MODEL_SERIALIZE_ALL_REQUESTS > +#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS > + > +/* Lock protecting open/close calls - see above. */ > +static pthread_mutex_t open_close_lock = PTHREAD_MUTEX_INITIALIZER; > > /* The per-connection handle. */ > struct vddk_handle { > @@ -524,6 +531,7 @@ free_connect_params (VixDiskLibConnectParams *params) > static void * > vddk_open (int readonly) > { > + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); > struct vddk_handle *h; > VixError err; > uint32_t flags; > @@ -616,6 +62...

...*/ -#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS +#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS + +/* Lock protecting open/close calls - see above. */ +static pthread_mutex_t open_close_lock = PTHREAD_MUTEX_INITIALIZER; /* The per-connection handle. */ struct vddk_handle { VixDiskLibConnectParams *params; /* connection parameters */ VixDiskLibConnection connection; /* connection */ VixDiskLibHandle handle; /* disk handle */ + int readonly; }; static inline VixDiskLibConnectParams * @@ -557,6 +569,7 @@ free_connect_params (VixDiskLibConnectPar...

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.

...*/ -#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS +#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS + +/* Lock protecting open/close calls - see above. */ +static pthread_mutex_t open_close_lock = PTHREAD_MUTEX_INITIALIZER; /* The per-connection handle. */ struct vddk_handle { @@ -524,6 +531,7 @@ free_connect_params (VixDiskLibConnectParams *params) static void * vddk_open (int readonly) { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); struct vddk_handle *h; VixError err; uint32_t flags; @@ -616,6 +624,7 @@ vddk_open (int readonly) static void...

On Wed, Aug 05, 2020 at 02:39:44PM +0300, Nir Soffer wrote: > Can we use something like the file plugin? thread pool of workers, > each keeping open vddk handle, and serving requests in parallel from > the same nbd socket? Yes, but this isn't implemented in the plugins, it's implemented in the server. The server always uses a thread pool, but plugins can opt for more or less

..._ALL_REQUESTS serializes requests across all handles. SERIALIZE_REQUESTS serializes requests within each handle, but multiple parallel requests can happen to different handles. > but it sounds to me like we need PARALLEL. > > With parallel we will have multiple threads using the same > vddk_handle, which can > be thread safe since we control this struct. > > The struct can be: > > struct vddk_item { > VixDiskLibConnectParams *params; /* connection parameters */ > VixDiskLibConnection connection; /* connection */ > VixDiskLibHandle handle; /* disk hand...

...LIZE_REQUESTS, with an extra mutex added > by the plugin around VixDiskLib_Open and _Close calls. I'm not sure what is the difference between SERIALIZE_REQUESTS and SERIALIZE_ALL_REQUESTS, but it sounds to me like we need PARALLEL. With parallel we will have multiple threads using the same vddk_handle, which can be thread safe since we control this struct. The struct can be: struct vddk_item { VixDiskLibConnectParams *params; /* connection parameters */ VixDiskLibConnection connection; /* connection */ VixDiskLibHandle handle; /* disk handle */ struct vddk_item *next; /* nex...

...e performance. In this mode every coroutine will do the I/O when it is ready, instead of waiting for other coroutines and submit the I/O in the right order. > > but it sounds to me like we need PARALLEL. > > > > With parallel we will have multiple threads using the same > > vddk_handle, which can > > be thread safe since we control this struct. > > > > The struct can be: > > > > struct vddk_item { > > VixDiskLibConnectParams *params; /* connection parameters */ > > VixDiskLibConnection connection; /* connection */ > > VixDisk...

...eryAllocatedBlocks"); + VixDiskLib_FreeBlockList = dlsym (dl, "VixDiskLib_FreeBlockList"); } static void @@ -570,6 +579,133 @@ vddk_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset) return 0; } +static int +vddk_can_extents (void *handle) +{ + struct vddk_handle *h = handle; + VixError err; + VixDiskLibBlockList *block_list; + + /* This call was added in VDDK 6.7. In earlier versions the + * function pointer will be NULL and we cannot query extents. + */ + if (VixDiskLib_QueryAllocatedBlocks == NULL) { + nbdkit_debug ("can_extents: VixDis...

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

...@redhat.com> --- plugins/vddk/vddk.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c index ca6ae6d6ff63..8cf54f6e2b96 100644 --- a/plugins/vddk/vddk.c +++ b/plugins/vddk/vddk.c @@ -428,6 +428,7 @@ struct vddk_handle { VixDiskLibConnectParams *params; /* connection parameters */ VixDiskLibConnection connection; /* connection */ VixDiskLibHandle handle; /* disk handle */ + VixDiskLibHandle handle_single_link; /* disk handle for single_link=true */ }; static inline VixDiskLibConnectParams *...

...ddk/vddk.c | 138 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 152 insertions(+), 1 deletion(-) > > +static int > +vddk_extents (void *handle, uint32_t count, uint64_t offset, uint32_t flags, > + struct nbdkit_extents *extents) > +{ > + struct vddk_handle *h = handle; > + bool req_one = flags & NBDKIT_FLAG_REQ_ONE; > + uint64_t position, end, start_sector; > + > + position = offset; > + ... > + > + for (i = 0; i < block_list->numBlocks; ++i) { > + uint64_t offset, length; > + > + offset = bloc...

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

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

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

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