[libvirt] [PATCH v2 0/2] vbox: address thread-safety issues

Dawid Zamirski dzamirski at datto.com
Mon Nov 7 22:00:43 UTC 2016


This patch series solves vbox driver thread-safety issues where
libvird process would segfault when there was more than one concurrent
vbox connection. The reason for the segfault was because the original
implementation called pfnComInitialize/Unintialize on each
virConnectOpen/Close calls which were setting pointers to ISession and
IVirtualBox instances that were held by the global g_pVBoxGlobalData.
This caused new connection to overwrite the pointer created by the
prior connection.

Changes since v1 [1]:

* use just vboxDriver object instead of vboxGlobalData and vboxPrivate
  structs. The vboxDriver takes advantage of libvirt object reference
  counting and more closely resembles the pattern of "global" drivers
  used by other drivers such as vz or xen. This also takes care of the
  original concern mentioned in v1 where allocation of the struct was
  not guarded by a mutex.

* use pfnClientInialize/Unintialize vbox API for newer vbox versions
  instead of the old pfnComInitialize/Unintialze which are marked as
  deprecated by upstream vbox.

While this series adresses the issues I've been targetting, i.e. avoid
libvird sefault with concurrent vbox connections,  the libvirt vbox
driver still incorrectly calls vbox's init/unint APIs as those must be
called from the same thread according to vbox source code and docs.
Furthermore, if one is running vbox 'debug' build, it will refuse to
intialize as it's not called from the main thread due to asserts [2].
Those asserts are compiled away on 'release' builds so it appears to
work, but may cause some unwanted side effects. Unfortunately, I don't
think it's possible to have libvirt driver call those APIs from the
same thread as libvirt is a client-server architecture and each libvir
API call from the connection starts a new thread, so I guess this is 
not doeable right now :-/

[1] https://www.redhat.com/archives/libvir-list/2016-September/msg01306.html
[2] https://github.com/mdaniel/virtualbox-org-svn-vbox-trunk/blob/master/src/VBox/Main/glue/initterm.cpp#L376

Dawid Zamirski (2):
  vbox: change how vbox API is initialized.
  vbox: get rid of g_pVBoxGlobalData

 src/vbox/vbox_common.c        | 561 +++++++++++++++++++++++-------------------
 src/vbox/vbox_common.h        |   2 +-
 src/vbox/vbox_driver.c        |   1 +
 src/vbox/vbox_network.c       |  24 +-
 src/vbox/vbox_storage.c       |  20 +-
 src/vbox/vbox_tmpl.c          | 434 ++++++++++++++++----------------
 src/vbox/vbox_uniformed_api.h | 110 ++++-----
 7 files changed, 612 insertions(+), 540 deletions(-)

-- 
2.9.3




More information about the libvir-list mailing list