search for: vixdisklib_open

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

...n (void) * https://code.vmware.com/docs/11750/virtual-disk-development-kit-programming-guide/GUID-6BE903E8-DC70-46D9-98E4-E34A2002C2AD.html * * Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS. Since nbdkit - * 1.22 we changed this to SERIALIZE_REQUESTS and added a mutex around - * calls to VixDiskLib_Open and VixDiskLib_Close. This is not quite - * within the letter of the rules, but is within the spirit. + * 1.22 we changed this to PARALLEL, added a mutex around calls to + * VixDiskLib_Open and VixDiskLib_Close, and use a pool of disk + * handles. This is not quite within the letter of the rules,...

.../code.vmware.com/docs/11750/virtual-disk-development-kit-programming-guide/GUID-6BE903E8-DC70-46D9-98E4-E34A2002C2AD.html > * > * Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS. Since nbdkit > - * 1.22 we changed this to SERIALIZE_REQUESTS and added a mutex around > - * calls to VixDiskLib_Open and VixDiskLib_Close. This is not quite > - * within the letter of the rules, but is within the spirit. > + * 1.22 we changed this to PARALLEL, added a mutex around calls to > + * VixDiskLib_Open and VixDiskLib_Close, and use a pool of disk > + * handles. This is not quite within the...

...ags |= VIXDISKLIB_FLAG_OPEN_UNBUFFERED; @@ -538,6 +537,21 @@ vddk_open (int readonly) nbdkit_debug ("transport mode: %s", VixDiskLib_GetTransportMode (h->handle)); + if (single_link) { + flags |= VIXDISKLIB_FLAG_OPEN_SINGLE_LINK; + + DEBUG_CALL ("VixDiskLib_Open", + "connection, %s, %d, &handle_single_link", filename, flags); + err = VixDiskLib_Open (h->connection, filename, flags, &h->handle_single_link); + if (err != VIX_OK) { + VDDK_ERROR (err, "VixDiskLib_Open: %s", filename); +...

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

...lel thread_model=parallel The VDDK plugin currently uses SERIALIZE_ALL_REQUESTS: $ nbdkit vddk --dump-plugin | grep thread max_thread_model=serialize_all_requests thread_model=serialize_all_requests The proposal is to use SERIALIZE_REQUESTS, with an extra mutex added by the plugin around VixDiskLib_Open and _Close calls. PARALLEL is not possible. > This is kind of ugly but simple, and it works great for the file > plugin - we get better > performance than qemu-nbd. > > But since we get low throughput even when we have 10 concurrent > handles for 10 different disks, I'm sure...

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

.../code.vmware.com/docs/11750/virtual-disk-development-kit-programming-guide/GUID-6BE903E8-DC70-46D9-98E4-E34A2002C2AD.html > + * > + * Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS. Since nbdkit > + * 1.22 we changed this to SERIALIZE_REQUESTS and added a mutex around > + * calls to VixDiskLib_Open and VixDiskLib_Close. This is not quite > + * within the letter of the rules, but is within the spirit. > The document is very clear about using the same thread for open an close. Using a lock is not the same. I think Eric already wrote about this. */ > -#define THREAD_MODEL NBDKIT_T...

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

...{ nbdkit_error ("unknown parameter '%s'", key); return -1; @@ -464,6 +472,8 @@ vddk_open (int readonly) flags = 0; if (readonly) flags |= VIXDISKLIB_FLAG_OPEN_READ_ONLY; + if (single_link) + flags |= VIXDISKLIB_FLAG_OPEN_SINGLE_LINK; DEBUG_CALL ("VixDiskLib_Open", "connection, %s, %d, &handle", filename, flags); -- 2.20.1

...E_ALL_REQUESTS: > > > > $ nbdkit vddk --dump-plugin | grep thread > > max_thread_model=serialize_all_requests > > thread_model=serialize_all_requests > > > > The proposal is to use SERIALIZE_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, SERIALIZE_ALL_REQUESTS serializes requests across all handles. SERIALIZE_REQUESTS serializes requests within each handle, but multiple parallel requests can happen to di...

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.

...re here: + * https://code.vmware.com/docs/11750/virtual-disk-development-kit-programming-guide/GUID-6BE903E8-DC70-46D9-98E4-E34A2002C2AD.html + * + * Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS. Since nbdkit + * 1.22 we changed this to SERIALIZE_REQUESTS and added a mutex around + * calls to VixDiskLib_Open and VixDiskLib_Close. This is not quite + * within the letter of the rules, but is within the spirit. + * + * We also enabled multi-conn for readonly connections since it + * appears possible for clients to open multiple handles even if they + * point to the same server/disk. */ -#define THREAD_...

...re here: + * https://code.vmware.com/docs/11750/virtual-disk-development-kit-programming-guide/GUID-6BE903E8-DC70-46D9-98E4-E34A2002C2AD.html + * + * Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS. Since nbdkit + * 1.22 we changed this to SERIALIZE_REQUESTS and added a mutex around + * calls to VixDiskLib_Open and VixDiskLib_Close. This is not quite + * within the letter of the rules, but is within the spirit. */ -#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 pt...

...parameter '%s'", key); > return -1; >@@ -464,6 +472,8 @@ vddk_open (int readonly) > flags = 0; > if (readonly) > flags |= VIXDISKLIB_FLAG_OPEN_READ_ONLY; >+ if (single_link) >+ flags |= VIXDISKLIB_FLAG_OPEN_SINGLE_LINK; > > DEBUG_CALL ("VixDiskLib_Open", > "connection, %s, %d, &handle", filename, flags); >-- >2.20.1 >

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.

...ror +VixDiskLib_ConnectEx (const VixDiskLibConnectParams *params, + char read_only, + const char *snapshot_ref, + const char *transport_modes, + VixDiskLibConnection *connection) +{ + return VIX_OK; +} + +VixError +VixDiskLib_Open (const VixDiskLibConnection connection, + const char *path, + uint32_t flags, + VixDiskLibHandle *handle) +{ + return VIX_OK; +} + +const char * +VixDiskLib_GetTransportMode (VixDiskLibHandle handle) +{ + return "file"; +} + +VixError +Vix...

...VDDK plugin currently uses SERIALIZE_ALL_REQUESTS: > > $ nbdkit vddk --dump-plugin | grep thread > max_thread_model=serialize_all_requests > thread_model=serialize_all_requests > > The proposal is to use SERIALIZE_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 ca...

...> > $ nbdkit vddk --dump-plugin | grep thread > > > max_thread_model=serialize_all_requests > > > thread_model=serialize_all_requests > > > > > > The proposal is to use SERIALIZE_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, > > SERIALIZE_ALL_REQUESTS serializes requests across all handles. > SERIALIZE_REQUESTS serializes requests within each handle, > but multiple p...

[NB: Adding PUBLIC mailing list because this is upstream discussion] On Mon, Aug 03, 2020 at 06:27:04PM +0100, Richard W.M. Jones wrote: > On Mon, Aug 03, 2020 at 06:03:23PM +0300, Nir Soffer wrote: > > On Mon, Aug 3, 2020 at 5:47 PM Richard W.M. Jones <rjones@redhat.com> wrote: > > All this make sense, but when we upload 10 disks we have 10 connections > > but still we