[Libguestfs] [PATCH nbdkit 1/2] vddk: Relax threading model: SERIALIZE_ALL_REQUESTS -> SERIALIZE_REQUESTS.

[Nir Soffer]

On Thu, Aug 6, 2020, 16:16 Richard W.M. Jones <rjones at 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
> --- a/plugins/vddk/vddk.c
> +++ b/plugins/vddk/vddk.c
> @@ -478,11 +478,18 @@ vddk_dump_plugin (void)
>  #endif
>  }
>
> -/* XXX To really do threading correctly in accordance with the VDDK
> - * documentation, we must do all open/close calls from a single
> - * thread.  This is a huge pain.
> +/* The rules on threads and VDDK are 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.
>

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_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
>  vddk_close (void *handle)
>  {
> +  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock);
>    struct vddk_handle *h = handle;
>
>    DEBUG_CALL ("VixDiskLib_Close", "handle");
> --
> 2.27.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20200806/60a1b93f/attachment.htm>