[libvirt] Making the Thread-safety issues with vbox driver ?

[Jean-Baptiste Rouault]

On Wednesday 04 April 2012 17:56:12 Jean-Baptiste Rouault wrote:
> On Monday 16 January 2012 11:34:53 Matthias Bolte wrote:
> > Okay, without looking deeper into this here are some ideas:
> > 
> > The XPCOM API libvirt uses might not be threadsafe, or needs to be
> > initialized for every thread that wants to use it. Currently its only
> > initialized for the thread that opens the driver. I know that this is
> > the case on Windows were VirtualBox uses MSCOM for its API and you
> > need to call CoInitialize on every thread. This is currently not done
> > for the MSCOM glue in libvirt, so I know that on Windows the
> > VirtualBox driver is not threadsafe currently. Also I didn't look into
> > a solution for this yet. Maybe we need a thread local variable that
> > holds whether (MS/XP)COM was already initialized for this thread and
> > add a check to every driver function to initialize it when needed.
> > 
> > Did you try to open a connection for each thread instead of trying to
> > share one? If that works reliable it might indicate that there is an
> > VirtualBox API initialization problem.
> 
> I tried today with one connection for each thread and it works.
> 
> I changed the vbox driver so that the pfnComInitialize function is called
> only when the first connection is opened : it breaks the test, even with
> one connection per thread.
> 
> I guess we'll have to use a thread local variable as you suggested, unless
> someone has a better idea to handle this problem.

Hi,

I looked deeper into these thread-safety issues, once a new connection is 
opened for each thread, everything works well.
However, opening and closing connections isn't thread-safe at all for two 
reasons :

- VirtualBox C bindings initialization and uninitialization functions aren't 
thread-safe. I talked about it with upstream on IRC and they are probably not 
going to fix it, but would accept a patch fixing the issue. I'm going to contact 
upstream again to get some advices so I can write a patch.

- In the libvirt vbox driver, for each new connection, modification of the 
global variable g_pVBoxGlobalData isn't protected (see line 1040 of 
vbox_tmpl.c). 

First of all, is it really necessary to replace it on each new connection, or 
would it be ok to initialize it only when the first connection is opened ?

A global mutex is needed in the vbox driver to protect access to 
g_pVBoxGlobalData, the vboxRegister() function seems to be the best place to 
initialize such a mutex unless there is another entry point to do this ?

-- 
Jean-Baptiste ROUAULT
Ingénieur R&D - diateam : Architectes de l'information
Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051