[libvirt] [PATCH] vbox: Stop hardcoding a single path for VBoxXPCOMC.so

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