[libvirt] [PATCH] vbox: Stop hardcoding a single path for VBoxXPCOMC.so
Matthias Bolte
matthias.bolte at googlemail.com
Fri Oct 22 19:44:22 UTC 2010
This partly reverts df90ca7661b0a789bd790ccf8258a4527c13eb8d.
Don't disable the VirtualBox driver when configure can't find
VBoxXPCOMC.so, rely on detection at runtime again instead.
Keep --with-vbox=/path/to/virtualbox intact, added to for:
https://bugzilla.redhat.com/show_bug.cgi?id=609185
Detection order for VBoxXPCOMC.so:
1. VBOX_APP_HOME environment variable
2. configure provided location
3. hardcoded list of known locations
4. dynamic linker search path
Also cleanup the glue code and improve error reporting.
---
Patch is based on Daniel's suggestion in:
https://www.redhat.com/archives/libvir-list/2010-October/msg00852.html
configure.ac | 53 +---------
po/POTFILES.in | 1 +
src/vbox/vbox_XPCOMCGlue.c | 232 +++++++++++++++++++++++---------------------
src/vbox/vbox_XPCOMCGlue.h | 14 ---
4 files changed, 130 insertions(+), 170 deletions(-)
diff --git a/configure.ac b/configure.ac
index e41f2b5..40d6b69 100644
--- a/configure.ac
+++ b/configure.ac
@@ -225,8 +225,8 @@ AC_ARG_WITH([xenapi],
AC_HELP_STRING([--with-xenapi], [add XenAPI support @<:@default=check@:>@]),[],[with_xenapi=check])
AC_ARG_WITH([vbox],
AC_HELP_STRING([--with-vbox=@<:@PFX@:>@],
- [VirtualBox XPCOMC location @<:@default=check@:>@]),[],
- [with_vbox=check])
+ [VirtualBox XPCOMC location @<:@default=yes@:>@]),[],
+ [with_vbox=yes])
AC_ARG_WITH([lxc],
AC_HELP_STRING([--with-lxc], [add Linux Container support @<:@default=check@:>@]),[],[with_lxc=check])
AC_ARG_WITH([one],
@@ -332,51 +332,10 @@ dnl
vbox_xpcomc_dir=
-if test "x$with_vbox" = "xyes" || test "x$with_vbox" = "xcheck"; then
- AC_MSG_CHECKING([for VirtualBox XPCOMC location])
-
- for vbox in \
- /usr/lib/virtualbox/VBoxXPCOMC.so \
- /usr/lib64/virtualbox/VBoxXPCOMC.so \
- /usr/lib/VirtualBox/VBoxXPCOMC.so \
- /opt/virtualbox/VBoxXPCOMC.so \
- /opt/VirtualBox/VBoxXPCOMC.so \
- /opt/virtualbox/i386/VBoxXPCOMC.so \
- /opt/VirtualBox/i386/VBoxXPCOMC.so \
- /opt/virtualbox/amd64/VBoxXPCOMC.so \
- /opt/VirtualBox/amd64/VBoxXPCOMC.so \
- /usr/local/lib/virtualbox/VBoxXPCOMC.so \
- /usr/local/lib/VirtualBox/VBoxXPCOMC.so \
- /Applications/VirtualBox.app/Contents/MacOS/VBoxXPCOMC.dylib \
- ; do
- if test -f "$vbox"; then
- vbox_xpcomc_dir=`AS_DIRNAME(["$vbox"])`
- break
- fi
- done
-
- if test -n "$vbox_xpcomc_dir"; then
- AC_MSG_RESULT([$vbox_xpcomc_dir])
- with_vbox=yes
- else
- if test "x$with_vbox" = "xcheck"; then
- AC_MSG_RESULT([not found, disabling VirtualBox driver])
- with_vbox=no
- else
- AC_MSG_RESULT([not found])
- AC_MSG_ERROR([VirtualBox XPCOMC is required for the VirtualBox driver])
- fi
- fi
-else
- if test "x$with_vbox" != "xno"; then
- if test -f ${with_vbox}/VBoxXPCOMC.so || \
- test -f ${with_vbox}/VBoxXPCOMC.dylib; then
- vbox_xpcomc_dir=$with_vbox
- with_vbox=yes
- else
- AC_MSG_ERROR([$with_vbox does not contain VirtualBox XPCOMC])
- fi
- fi
+if test "x$with_vbox" != "xyes" && test "x$with_vbox" != "xno"; then
+ # intentionally don't do any further checks here on the provided path
+ vbox_xpcomc_dir=$with_vbox
+ with_vbox=yes
fi
AC_DEFINE_UNQUOTED([VBOX_XPCOMC_DIR], ["$vbox_xpcomc_dir"],
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 541335e..dafef47 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -93,6 +93,7 @@ src/util/util.c
src/util/virtaudit.c
src/util/virterror.c
src/util/xml.c
+src/vbox/vbox_XPCOMCGlue.c
src/vbox/vbox_driver.c
src/vbox/vbox_tmpl.c
src/xen/proxy_internal.c
diff --git a/src/vbox/vbox_XPCOMCGlue.c b/src/vbox/vbox_XPCOMCGlue.c
index e8b0bd1..7648e54 100644
--- a/src/vbox/vbox_XPCOMCGlue.c
+++ b/src/vbox/vbox_XPCOMCGlue.c
@@ -32,13 +32,18 @@
#include <config.h>
-#include <stdio.h>
-#include <string.h>
#include <stdlib.h>
-#include <stdarg.h>
#include <dlfcn.h>
+#include <stdbool.h>
#include "vbox_XPCOMCGlue.h"
+#include "internal.h"
+#include "memory.h"
+#include "util.h"
+#include "logging.h"
+#include "virterror_internal.h"
+
+#define VIR_FROM_THIS VIR_FROM_VBOX
/*******************************************************************************
@@ -60,8 +65,6 @@
*******************************************************************************/
/** The dlopen handle for VBoxXPCOMC. */
void *g_hVBoxXPCOMC = NULL;
-/** The last load error. */
-char g_szVBoxErrMsg[256];
/** Pointer to the VBoxXPCOMC function table. */
PCVBOXXPCOM g_pVBoxFuncs = NULL;
/** Pointer to VBoxGetXPCOMCFunctions for the loaded VBoxXPCOMC so/dylib/dll. */
@@ -69,104 +72,97 @@ PFNVBOXGETXPCOMCFUNCTIONS g_pfnGetFunctions = NULL;
/**
- * Wrapper for setting g_szVBoxErrMsg. Can be an empty stub.
- *
- * @param fAlways When 0 the g_szVBoxErrMsg is only set if empty.
- * @param pszFormat The format string.
- * @param ... The arguments.
- */
-static void setErrMsg(int fAlways, const char *pszFormat, ...)
-{
-#ifndef LIBVIRT_VERSION
- if ( fAlways
- || !g_szVBoxErrMsg[0])
- {
- va_list va;
- va_start(va, pszFormat);
- vsnprintf(g_szVBoxErrMsg, sizeof(g_szVBoxErrMsg), pszFormat, va);
- va_end(va);
- }
-#else /* libvirt */
- (void)fAlways;
- (void)pszFormat;
-#endif /* libvirt */
-}
-
-
-/**
* Try load VBoxXPCOMC.so/dylib/dll from the specified location and resolve all
* the symbols we need.
*
- * @returns 0 on success, -1 on failure.
- * @param pszHome The director where to try load VBoxXPCOMC from. Can
- * be NULL.
- * @param fSetAppHome Whether to set the VBOX_APP_HOME env.var. or not
- * (boolean).
+ * @returns 0 on success, -1 on failure and 1 if VBoxXPCOMC was not found.
+ * @param dir The directory where to try load VBoxXPCOMC from. Can
+ * be NULL.
+ * @param setAppHome Whether to set the VBOX_APP_HOME env.var. or not.
+ * @param ignoreMissing Whether to ignore missing library or not.
*/
-static int tryLoadOne(const char *pszHome, int fSetAppHome)
+static int tryLoadOne(const char *dir, bool setAppHome, bool ignoreMissing)
{
- size_t cchHome = pszHome ? strlen(pszHome) : 0;
- size_t cbBufNeeded;
- char szName[4096];
- int rc = -1;
+ int result = -1;
+ char *name = NULL;
+ PFNVBOXGETXPCOMCFUNCTIONS pfnGetFunctions;
+
+ if (dir != NULL) {
+ if (virAsprintf(&name, "%s/%s", dir, DYNLIB_NAME) < 0) {
+ virReportOOMError();
+ return -1;
+ }
- /*
- * Construct the full name.
- */
- cbBufNeeded = cchHome + sizeof("/" DYNLIB_NAME);
- if (cbBufNeeded > sizeof(szName))
- {
- setErrMsg(1, "path buffer too small: %u bytes needed",
- (unsigned)cbBufNeeded);
- return -1;
- }
- if (cchHome)
- {
- memcpy(szName, pszHome, cchHome);
- szName[cchHome] = '/';
- cchHome++;
+ if (!virFileExists(name)) {
+ if (!ignoreMissing) {
+ VIR_ERROR(_("Libaray '%s' doesn't exist"), name);
+ }
+
+ return -1;
+ }
+ } else {
+ name = strdup(DYNLIB_NAME);
+
+ if (name == NULL) {
+ virReportOOMError();
+ return -1;
+ }
}
- memcpy(&szName[cchHome], DYNLIB_NAME, sizeof(DYNLIB_NAME));
/*
* Try load it by that name, setting the VBOX_APP_HOME first (for now).
* Then resolve and call the function table getter.
*/
- if (fSetAppHome)
- {
- if (pszHome)
- setenv("VBOX_APP_HOME", pszHome, 1 /* always override */);
- else
+ if (setAppHome) {
+ if (dir != NULL) {
+ setenv("VBOX_APP_HOME", dir, 1 /* always override */);
+ } else {
unsetenv("VBOX_APP_HOME");
+ }
}
- g_hVBoxXPCOMC = dlopen(szName, RTLD_NOW | RTLD_LOCAL);
- if (g_hVBoxXPCOMC)
- {
- PFNVBOXGETXPCOMCFUNCTIONS pfnGetFunctions;
- pfnGetFunctions = (PFNVBOXGETXPCOMCFUNCTIONS)
- dlsym(g_hVBoxXPCOMC, VBOX_GET_XPCOMC_FUNCTIONS_SYMBOL_NAME);
- if (pfnGetFunctions)
- {
- g_pVBoxFuncs = pfnGetFunctions(VBOX_XPCOMC_VERSION);
- if (g_pVBoxFuncs)
- {
- g_pfnGetFunctions = pfnGetFunctions;
- return 0;
- }
- /* bail out */
- setErrMsg(1, "%.80s: pfnGetFunctions(%#x) failed",
- szName, VBOX_XPCOMC_VERSION);
- }
- else
- setErrMsg(1, "dlsym(%.80s/%.32s): %.128s",
- szName, VBOX_GET_XPCOMC_FUNCTIONS_SYMBOL_NAME, dlerror());
+ g_hVBoxXPCOMC = dlopen(name, RTLD_NOW | RTLD_LOCAL);
+
+ if (g_hVBoxXPCOMC == NULL) {
+ VIR_WARN("Could not dlopen '%s': %s", name, dlerror());
+ goto cleanup;
+ }
+
+ pfnGetFunctions = (PFNVBOXGETXPCOMCFUNCTIONS)
+ dlsym(g_hVBoxXPCOMC, VBOX_GET_XPCOMC_FUNCTIONS_SYMBOL_NAME);
+
+ if (pfnGetFunctions == NULL) {
+ VIR_ERROR(_("Could not dlsym %s from '%s': %s"),
+ VBOX_GET_XPCOMC_FUNCTIONS_SYMBOL_NAME, name, dlerror());
+ goto cleanup;
+ }
+
+ g_pVBoxFuncs = pfnGetFunctions(VBOX_XPCOMC_VERSION);
+
+ if (g_pVBoxFuncs == NULL) {
+ VIR_ERROR(_("Calling %s from '%s' failed"),
+ VBOX_GET_XPCOMC_FUNCTIONS_SYMBOL_NAME, name);
+ goto cleanup;
+ }
+
+ g_pfnGetFunctions = pfnGetFunctions;
+ result = 0;
+
+ if (dir != NULL) {
+ VIR_DEBUG("Found %s in '%s'", DYNLIB_NAME, dir);
+ } else {
+ VIR_DEBUG("Found %s in dynamic linker search path", DYNLIB_NAME);
+ }
+
+cleanup:
+ if (g_hVBoxXPCOMC != NULL && result < 0) {
dlclose(g_hVBoxXPCOMC);
g_hVBoxXPCOMC = NULL;
}
- else
- setErrMsg(0, "dlopen(%.80s): %.160s", szName, dlerror());
- return rc;
+
+ VIR_FREE(name);
+
+ return result;
}
@@ -175,34 +171,53 @@ static int tryLoadOne(const char *pszHome, int fSetAppHome)
* function pointers.
*
* @returns 0 on success, -1 on failure.
- *
- * @remark This should be considered moved into a separate glue library since
- * its its going to be pretty much the same for any user of VBoxXPCOMC
- * and it will just cause trouble to have duplicate versions of this
- * source code all around the place.
*/
int VBoxCGlueInit(void)
{
- /*
- * If the user specifies the location, try only that.
- */
- const char *pszHome = getenv("VBOX_APP_HOME");
- if (pszHome)
- return tryLoadOne(pszHome, 0);
+ int i;
+ static const char *knownDirs[] = {
+ "/usr/lib/virtualbox",
+ "/usr/lib/virtualbox-ose",
+ "/usr/lib64/virtualbox",
+ "/usr/lib64/virtualbox-ose",
+ "/usr/lib/VirtualBox",
+ "/opt/virtualbox",
+ "/opt/VirtualBox",
+ "/opt/virtualbox/i386",
+ "/opt/VirtualBox/i386",
+ "/opt/virtualbox/amd64",
+ "/opt/VirtualBox/amd64",
+ "/usr/local/lib/virtualbox",
+ "/usr/local/lib/VirtualBox",
+ "/Applications/VirtualBox.app/Contents/MacOS"
+ };
+ const char *home = getenv("VBOX_APP_HOME");
+
+ /* If the user specifies the location, try only that. */
+ if (home != NULL) {
+ if (tryLoadOne(home, false, false) < 0) {
+ return -1;
+ }
+ }
- /*
- * Try the configured location.
- */
- g_szVBoxErrMsg[0] = '\0';
+ /* Try the additionally configured location. */
+ if (VBOX_XPCOMC_DIR[0] != '\0') {
+ if (tryLoadOne(VBOX_XPCOMC_DIR, true, true) >= 0) {
+ return 0;
+ }
+ }
- if (tryLoadOne(VBOX_XPCOMC_DIR, 1) == 0)
- return 0;
+ /* Try the known locations. */
+ for (i = 0; i < ARRAY_CARDINALITY(knownDirs); ++i) {
+ if (tryLoadOne(knownDirs[i], true, true) >= 0) {
+ return 0;
+ }
+ }
- /*
- * Finally try the dynamic linker search path.
- */
- if (tryLoadOne(NULL, 1) == 0)
+ /* Finally try the dynamic linker search path. */
+ if (tryLoadOne(NULL, false, true) >= 0) {
return 0;
+ }
/* No luck, return failure. */
return -1;
@@ -214,14 +229,13 @@ int VBoxCGlueInit(void)
*/
void VBoxCGlueTerm(void)
{
- if (g_hVBoxXPCOMC)
- {
+ if (g_hVBoxXPCOMC != NULL) {
#if 0 /* VBoxRT.so doesn't like being reloaded. See @bugref{3725}. */
dlclose(g_hVBoxXPCOMC);
#endif
g_hVBoxXPCOMC = NULL;
}
+
g_pVBoxFuncs = NULL;
g_pfnGetFunctions = NULL;
- memset(g_szVBoxErrMsg, 0, sizeof(g_szVBoxErrMsg));
}
diff --git a/src/vbox/vbox_XPCOMCGlue.h b/src/vbox/vbox_XPCOMCGlue.h
index c04eefa..6c44030 100644
--- a/src/vbox/vbox_XPCOMCGlue.h
+++ b/src/vbox/vbox_XPCOMCGlue.h
@@ -32,26 +32,12 @@
/* This has to be the oldest version we support. */
# include "vbox_CAPI_v2_2.h"
-# ifdef __cplusplus
-extern "C" {
-# endif
-
-/** The dlopen handle for VBoxXPCOMC. */
-extern void *g_hVBoxXPCOMC;
-/** The last load error. */
-extern char g_szVBoxErrMsg[256];
/** Pointer to the VBoxXPCOMC function table. */
extern PCVBOXXPCOM g_pVBoxFuncs;
/** Pointer to VBoxGetXPCOMCFunctions for the loaded VBoxXPCOMC so/dylib/dll. */
extern PFNVBOXGETXPCOMCFUNCTIONS g_pfnGetFunctions;
-
int VBoxCGlueInit(void);
void VBoxCGlueTerm(void);
-
-# ifdef __cplusplus
-}
-# endif
-
#endif
--
1.7.0.4
More information about the libvir-list
mailing list