vbox: Stop hardcoding a single path for VBoxXPCOMC.so

This partly reverts df90ca76.

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.

configure.ac

@@ -234,8 +234,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],
@@ -341,51 +341,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"],

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

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,113 +65,104 @@
 *******************************************************************************/
 /** 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. */
 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));
 }

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