On Thu, Apr 09, 2009 at 11:25:08AM +0200, Pritesh Kothari wrote:
Hi All,
Resending the second patch [PATCH 2/2] after fixing the Bug mentioned by
Florian on the list.
Regards,
Pritesh
On Wednesday 08 April 2009 14:20:29 Pritesh Kothari wrote:
> Hi All,
>
> I have attached a patch which when applied on the HEAD as of today would
> allow virtualbox support in libvirt. It takes cares of all the stuff
> mentioned on the list earlier. Still if I have missed anything, please do
> tell me.
>
> The patch works very well with the VirtualBox OSE version and the 2.2
> release.
>
> [PATCH 1/2] contains diff of files already in libvirt.
> [PATCH 2/2] contains new files needed for VirtualBox support.
diff --git a/src/vbox/vbox_XPCOMCGlue.c b/src/vbox/vbox_XPCOMCGlue.c
new file mode 100644
index 0000000..8e0a771
--- /dev/null
+++ b/src/vbox/vbox_XPCOMCGlue.c
@@ -0,0 +1,215 @@
+/** @file vbox_XPCOMCGlue.c
+ * Glue code for dynamically linking to VBoxXPCOMC.
+ */
+
+/*******************************************************************************
+* Global Variables *
+*******************************************************************************/
+/** 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;
+/** boolean for checking if the VBOX_APP_HOME is already set by the users */
+int g_bVAHSet = 0;
I don't much like the static fixed size error message buffer
here. It only appears to be used in one function:
+ * 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.
+ */
+static int tryLoadOne(const char *pszHome)
+{
+ size_t cchHome = pszHome ? strlen(pszHome) : 0;
+ size_t cbReq;
+ char szBuf[4096];
+ int rc = -1;
+
+ /*
+ * Construct the full name.
+ */
+ cbReq = cchHome + sizeof("/" DYNLIB_NAME);
+ if (cbReq > sizeof(szBuf))
+ {
+ sprintf(g_szVBoxErrMsg, "path buffer too small: %u bytes required",
(unsigned)cbReq);
+ return -1;
+ }
+ memcpy(szBuf, pszHome, cchHome);
+ szBuf[cchHome] = '/';
+ cchHome++;
+ memcpy(&szBuf[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 (!g_bVAHSet)
+ {
+ /* Override it as we know that user didn't set it
+ * and that we only did it in previous iteration
+ */
+ setenv("VBOX_APP_HOME", pszHome, 1);
+ }
+ g_hVBoxXPCOMC = dlopen(szBuf, 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;
+ rc = 0;
+ }
+ else
+ sprintf(g_szVBoxErrMsg, "%.80s: pfnGetFunctions(%#x) failed",
+ szBuf, VBOX_XPCOMC_VERSION);
+ }
+ else
+ sprintf(g_szVBoxErrMsg, "dlsym(%.80s/%.32s): %128s",
+ szBuf, VBOX_GET_XPCOMC_FUNCTIONS_SYMBOL_NAME, dlerror());
+ if (rc != 0)
+ {
+ dlclose(g_hVBoxXPCOMC);
+ g_hVBoxXPCOMC = NULL;
+ }
+ }
+ else
+ sprintf(g_szVBoxErrMsg, "dlopen(%.80s): %128s", szBuf, dlerror());
+ return rc;
+}
And the caller of this function just calls the formal libvirt error
reporting function via vboxError():
+static int vboxInitialize(virConnectPtr conn, vboxGlobalData *data)
{
+
+ if (VBoxCGlueInit() != 0) {
+ vboxError(conn, VIR_ERR_INTERNAL_ERROR,"%s\n", g_szVBoxErrMsg);
+ goto cleanup;
+ }
+
+ if (g_pVBoxFuncs == NULL) {
+ vboxError(conn, VIR_ERR_INTERNAL_ERROR,"%s\n", g_szVBoxErrMsg);
+ goto cleanup;
+ }
+
+ g_pVBoxFuncs->pfnComInitialize(&data->vboxObj,
&data->vboxSession);
+ if (data->vboxObj == NULL)
+ goto cleanup;
+ if (data->vboxSession == NULL)
+ goto cleanup;
+
+ return 0;
+
+cleanup:
+ return -1;
+}
So, I think it'd be better to get rid of g_szVBoxErrMsg, and just have
tryLoadOne() call vboxError() (or virRaiseErrorHelper) directly.
+
+static virDrvOpenStatus vboxOpen(virConnectPtr conn,
+ virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+ int flags ATTRIBUTE_UNUSED) {
+ vboxGlobalData *data;
+ uid_t uid = getuid();
+
+ if (errorval == -1)
+ return VIR_DRV_OPEN_DECLINED;
+
+ if (conn->uri == NULL) {
+ conn->uri = xmlParseURI(uid ? "vbox:///session" :
"vbox:///system");
+ if (conn->uri == NULL) {
+ vboxError(conn, VIR_ERR_NO_DOMAIN, NULL);
+ return VIR_DRV_OPEN_ERROR;
Minor bug here, the VIR_ERR_NO_DOMAIN error isn't the correct code for an
URI parsing error :-)
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|