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

[Richard W.M. Jones]

On Thu, Aug 06, 2020 at 06:31:39PM +0300, Nir Soffer wrote:
> 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 agree it's not within the letter of the rules, as it says in
the comment.  However it does appear to work.

Rich.

> 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
> >
> >

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/