[libvirt] [PATCH] Call curl_global_init from virInitialize to avoid thread-safety issues

curl_global_init is not thread-safe. curl_easy_init might call curl_global_init when it was no called before. But curl_easy_init can be called from different threads by the ESX driver. Therefore, call curl_global_init from virInitialize to stop curl_easy_init from calling it. Reported by Benjamin Wang. --- configure.ac | 10 +++++++++- src/libvirt.c | 8 ++++++++ src/xenapi/xenapi_driver.c | 1 - 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 6d50985..0013d9d 100644 --- a/configure.ac +++ b/configure.ac @@ -2299,9 +2299,12 @@ dnl LIBCURL_CFLAGS="" LIBCURL_LIBS="" +have_curl=no if test "$with_esx" = "yes" || test "$with_esx" = "check" || test "$with_xenapi" = "yes" || test "$with_xenapi" = "check"; then PKG_CHECK_MODULES(LIBCURL, libcurl >= $LIBCURL_REQUIRED, [ + have_curl=yes + if test "$with_esx" = "check"; then with_esx=yes fi @@ -2326,6 +2329,11 @@ if test "$with_esx" = "yes" || test "$with_esx" = "check" || test "$with_xenapi" ]) fi +if test "$have_curl" = "yes" ; then + AC_DEFINE_UNQUOTED([HAVE_LIBCURL], 1, [whether libcurl is available]) +fi +AM_CONDITIONAL([HAVE_LIBCURL], [test "$have_curl" = "yes"]) + if test "$with_esx" = "yes" ; then AC_DEFINE_UNQUOTED([WITH_ESX], 1, [whether ESX driver is enabled]) @@ -3081,7 +3089,7 @@ AC_MSG_NOTICE([Libraries]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ libxml: $LIBXML_CFLAGS $LIBXML_LIBS]) AC_MSG_NOTICE([ dlopen: $DLOPEN_LIBS]) -if test "$with_esx" = "yes" ; then +if test "$have_curl" = "yes" ; then AC_MSG_NOTICE([ libcurl: $LIBCURL_CFLAGS $LIBCURL_LIBS]) else AC_MSG_NOTICE([ libcurl: no]) diff --git a/src/libvirt.c b/src/libvirt.c index ada9a86..76e4401 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -41,6 +41,10 @@ # include <winsock2.h> #endif +#ifdef HAVE_LIBCURL +# include <curl/curl.h> +#endif + #include "virterror_internal.h" #include "logging.h" #include "datatypes.h" @@ -418,6 +422,10 @@ virInitialize(void) virNetTLSInit(); +#if HAVE_LIBCURL + curl_global_init(CURL_GLOBAL_DEFAULT); +#endif + VIR_DEBUG("register drivers"); #if HAVE_WINSOCK2_H diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 2fac561..d60ad23 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -171,7 +171,6 @@ xenapiOpen (virConnectPtr conn, virConnectAuthPtr auth, xmlInitParser(); xmlKeepBlanksDefault(0); xen_init(); - curl_global_init(CURL_GLOBAL_ALL); privP->session = xen_session_login_with_password(call_func, privP, username, password, xen_api_latest_version); -- 1.7.4.1

On Sat, Oct 06, 2012 at 08:10:07PM +0200, Matthias Bolte wrote:
curl_global_init is not thread-safe. curl_easy_init might call curl_global_init when it was no called before. But curl_easy_init can be called from different threads by the ESX driver. Therefore, call curl_global_init from virInitialize to stop curl_easy_init from calling it.
Reported by Benjamin Wang. --- configure.ac | 10 +++++++++- src/libvirt.c | 8 ++++++++ src/xenapi/xenapi_driver.c | 1 - 3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac index 6d50985..0013d9d 100644 --- a/configure.ac +++ b/configure.ac @@ -2299,9 +2299,12 @@ dnl
LIBCURL_CFLAGS="" LIBCURL_LIBS="" +have_curl=no
if test "$with_esx" = "yes" || test "$with_esx" = "check" || test "$with_xenapi" = "yes" || test "$with_xenapi" = "check"; then PKG_CHECK_MODULES(LIBCURL, libcurl >= $LIBCURL_REQUIRED, [ + have_curl=yes + if test "$with_esx" = "check"; then with_esx=yes fi @@ -2326,6 +2329,11 @@ if test "$with_esx" = "yes" || test "$with_esx" = "check" || test "$with_xenapi" ]) fi
+if test "$have_curl" = "yes" ; then + AC_DEFINE_UNQUOTED([HAVE_LIBCURL], 1, [whether libcurl is available]) +fi +AM_CONDITIONAL([HAVE_LIBCURL], [test "$have_curl" = "yes"]) +
if test "$with_esx" = "yes" ; then AC_DEFINE_UNQUOTED([WITH_ESX], 1, [whether ESX driver is enabled]) @@ -3081,7 +3089,7 @@ AC_MSG_NOTICE([Libraries]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ libxml: $LIBXML_CFLAGS $LIBXML_LIBS]) AC_MSG_NOTICE([ dlopen: $DLOPEN_LIBS]) -if test "$with_esx" = "yes" ; then +if test "$have_curl" = "yes" ; then AC_MSG_NOTICE([ libcurl: $LIBCURL_CFLAGS $LIBCURL_LIBS]) else AC_MSG_NOTICE([ libcurl: no]) diff --git a/src/libvirt.c b/src/libvirt.c index ada9a86..76e4401 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -41,6 +41,10 @@ # include <winsock2.h> #endif
+#ifdef HAVE_LIBCURL +# include <curl/curl.h> +#endif + #include "virterror_internal.h" #include "logging.h" #include "datatypes.h" @@ -418,6 +422,10 @@ virInitialize(void)
virNetTLSInit();
+#if HAVE_LIBCURL + curl_global_init(CURL_GLOBAL_DEFAULT); +#endif + VIR_DEBUG("register drivers");
#if HAVE_WINSOCK2_H diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 2fac561..d60ad23 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -171,7 +171,6 @@ xenapiOpen (virConnectPtr conn, virConnectAuthPtr auth, xmlInitParser(); xmlKeepBlanksDefault(0); xen_init(); - curl_global_init(CURL_GLOBAL_ALL);
privP->session = xen_session_login_with_password(call_func, privP, username, password, xen_api_latest_version);
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

2012/10/8 Daniel P. Berrange <berrange@redhat.com>:
On Sat, Oct 06, 2012 at 08:10:07PM +0200, Matthias Bolte wrote:
curl_global_init is not thread-safe. curl_easy_init might call curl_global_init when it was no called before. But curl_easy_init can be called from different threads by the ESX driver. Therefore, call curl_global_init from virInitialize to stop curl_easy_init from calling it.
Reported by Benjamin Wang. --- configure.ac | 10 +++++++++- src/libvirt.c | 8 ++++++++ src/xenapi/xenapi_driver.c | 1 - 3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac index 6d50985..0013d9d 100644 --- a/configure.ac +++ b/configure.ac @@ -2299,9 +2299,12 @@ dnl
LIBCURL_CFLAGS="" LIBCURL_LIBS="" +have_curl=no
if test "$with_esx" = "yes" || test "$with_esx" = "check" || test "$with_xenapi" = "yes" || test "$with_xenapi" = "check"; then PKG_CHECK_MODULES(LIBCURL, libcurl >= $LIBCURL_REQUIRED, [ + have_curl=yes + if test "$with_esx" = "check"; then with_esx=yes fi @@ -2326,6 +2329,11 @@ if test "$with_esx" = "yes" || test "$with_esx" = "check" || test "$with_xenapi" ]) fi
+if test "$have_curl" = "yes" ; then + AC_DEFINE_UNQUOTED([HAVE_LIBCURL], 1, [whether libcurl is available]) +fi +AM_CONDITIONAL([HAVE_LIBCURL], [test "$have_curl" = "yes"]) +
if test "$with_esx" = "yes" ; then AC_DEFINE_UNQUOTED([WITH_ESX], 1, [whether ESX driver is enabled]) @@ -3081,7 +3089,7 @@ AC_MSG_NOTICE([Libraries]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ libxml: $LIBXML_CFLAGS $LIBXML_LIBS]) AC_MSG_NOTICE([ dlopen: $DLOPEN_LIBS]) -if test "$with_esx" = "yes" ; then +if test "$have_curl" = "yes" ; then AC_MSG_NOTICE([ libcurl: $LIBCURL_CFLAGS $LIBCURL_LIBS]) else AC_MSG_NOTICE([ libcurl: no]) diff --git a/src/libvirt.c b/src/libvirt.c index ada9a86..76e4401 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -41,6 +41,10 @@ # include <winsock2.h> #endif
+#ifdef HAVE_LIBCURL +# include <curl/curl.h> +#endif + #include "virterror_internal.h" #include "logging.h" #include "datatypes.h" @@ -418,6 +422,10 @@ virInitialize(void)
virNetTLSInit();
+#if HAVE_LIBCURL + curl_global_init(CURL_GLOBAL_DEFAULT); +#endif + VIR_DEBUG("register drivers");
#if HAVE_WINSOCK2_H diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 2fac561..d60ad23 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -171,7 +171,6 @@ xenapiOpen (virConnectPtr conn, virConnectAuthPtr auth, xmlInitParser(); xmlKeepBlanksDefault(0); xen_init(); - curl_global_init(CURL_GLOBAL_ALL);
privP->session = xen_session_login_with_password(call_func, privP, username, password, xen_api_latest_version);
ACK
Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com
participants (2)
-
Daniel P. Berrange
-
Matthias Bolte