[libvirt] [PATCH 0/4] vbox: address thread-safety issues.
This patch series solves (at least in my testing) vbox driver thread-safety issues that were also outlined on libvirt-users ML [1] and I was affected with. Those patches try to follow the suggestions made Matthias' [2] in that thread as close as possible. Here's where my patch differs from that suggesions: * vboxGlobalData - still needs to keep reference to ISession and IVirutalBox session because it's apparently not possible to have multiple instances created/destroyed safely with pfnComInitialize and pfnComUninitalize calls on per-connection basis. * as such vboxPrivate (the new struct introduced here) also has references to ISession and IVirutalBox (which are just referenes to the ones from the global) mainly to immitate ISession instance per-connection. Apparently newer VBOX SDKs introduced pfnClinetInitialize that can allegedly create multiple ISessions and we might want to take advantage of that in the future hopefully without making additional changes allover the driver code that this patch did. The gist of the change is in patch 3 and it also contains more in-depth explanation on how the issue is being resolved. Also, please note that patch 4 should be squashed into 3 as it was kept separate only for code-review purposes and 3rd patch won't compile without 4 applied on top. [1] https://www.redhat.com/archives/libvirt-users/2012-April/msg00122.html [2] https://www.redhat.com/archives/libvirt-users/2012-April/msg00125.htmlgq Dawid Zamirski (4): vbox: add vboxPrivate struct. vbox: replace vboxGlobalData with vboxPrivate. vbox: change API (un)initialization logic. vbox: update rest of the code to for prior changes. src/vbox/vbox_common.c | 275 ++++++++++++--------------- src/vbox/vbox_common.h | 32 ++-- src/vbox/vbox_network.c | 51 +++-- src/vbox/vbox_storage.c | 20 +- src/vbox/vbox_tmpl.c | 433 +++++++++++++++++++++--------------------- src/vbox/vbox_uniformed_api.h | 128 +++++++------ 6 files changed, 460 insertions(+), 479 deletions(-) -- 2.7.4
To be passed as per-connection context info instead of using vboxGlobalData that it will eventually replace in most cases. --- src/vbox/vbox_uniformed_api.h | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h index 74e9ac0..6ec5245 100644 --- a/src/vbox/vbox_uniformed_api.h +++ b/src/vbox/vbox_uniformed_api.h @@ -110,6 +110,36 @@ typedef struct { PCVBOXXPCOM pFuncs; /* The next is used for domainEvent */ + /* Async event handling */ + virObjectEventStatePtr domainEvents; + int fdWatch; + int volatile vboxCallBackRefCount; +# if defined(VBOX_API_VERSION) && VBOX_API_VERSION > 2002000 && VBOX_API_VERSION < 4000000 + IVirtualBoxCallback *vboxCallback; + nsIEventQueue *vboxQueue; +# else /* VBOX_API_VERSION <= 2002000 || VBOX_API_VERSION >= 4000000 || VBOX_API_VERSION undefined */ + void *vboxCallback; + void *vboxQueue; +# endif /* VBOX_API_VERSION <= 2002000 || VBOX_API_VERSION >= 4000000 || VBOX_API_VERSION undefined */ + + /* pointer back to the connection */ + virConnectPtr conn; +} vboxPrivate; + +typedef struct { + virMutex lock; + unsigned long version; + + virCapsPtr caps; + virDomainXMLOptionPtr xmlopt; + + IVirtualBox *vboxObj; + ISession *vboxSession; + + /** Our version specific API table pointer. */ + PCVBOXXPCOM pFuncs; + + /* The next is used for domainEvent */ # if defined(VBOX_API_VERSION) && VBOX_API_VERSION > 2002000 && VBOX_API_VERSION < 4000000 /* Async event handling */ -- 2.7.4
On Wed, Sep 28, 2016 at 01:41:33PM -0400, Dawid Zamirski wrote:
To be passed as per-connection context info instead of using vboxGlobalData that it will eventually replace in most cases. --- src/vbox/vbox_uniformed_api.h | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h index 74e9ac0..6ec5245 100644 --- a/src/vbox/vbox_uniformed_api.h +++ b/src/vbox/vbox_uniformed_api.h @@ -110,6 +110,36 @@ typedef struct { PCVBOXXPCOM pFuncs;
/* The next is used for domainEvent */ + /* Async event handling */ + virObjectEventStatePtr domainEvents; + int fdWatch; + int volatile vboxCallBackRefCount; +# if defined(VBOX_API_VERSION) && VBOX_API_VERSION > 2002000 && VBOX_API_VERSION < 4000000 + IVirtualBoxCallback *vboxCallback; + nsIEventQueue *vboxQueue; +# else /* VBOX_API_VERSION <= 2002000 || VBOX_API_VERSION >= 4000000 || VBOX_API_VERSION undefined */ + void *vboxCallback; + void *vboxQueue; +# endif /* VBOX_API_VERSION <= 2002000 || VBOX_API_VERSION >= 4000000 || VBOX_API_VERSION undefined */ + + /* pointer back to the connection */ + virConnectPtr conn; +} vboxPrivate; + +typedef struct { + virMutex lock; + unsigned long version; + + virCapsPtr caps; + virDomainXMLOptionPtr xmlopt; + + IVirtualBox *vboxObj; + ISession *vboxSession; + + /** Our version specific API table pointer. */ + PCVBOXXPCOM pFuncs; + + /* The next is used for domainEvent */ # if defined(VBOX_API_VERSION) && VBOX_API_VERSION > 2002000 && VBOX_API_VERSION < 4000000
/* Async event handling */
So after this patch the structure vboxGlobalData is changed, but none of the code is adjusted to that. I think that would cause some errors, but I don't have vbox installed to try that out. I'm quite sure, though. We are trying to separate comics in a way that you can compile and use the code after any commit so that bisecting works and we can clearly find out which particular commit causes problems.
-- 2.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Since those stucts are identical at the moment, this patch basically does s/vboxGlobalData \*data/vboxPrivate *data/ with type casts in vboxDriverLock/Unlock calls to keep the code working and take care of unavoidable diff noise to set the stage for further commits that actually address the issue for the patch series. --- src/vbox/vbox_common.c | 164 +++++++++++++++++++++--------------------- src/vbox/vbox_network.c | 24 +++---- src/vbox/vbox_storage.c | 20 +++--- src/vbox/vbox_tmpl.c | 146 ++++++++++++++++++------------------- src/vbox/vbox_uniformed_api.h | 54 +++++++------- 5 files changed, 204 insertions(+), 204 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 1472639..be6ff2d 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -57,7 +57,7 @@ VIR_LOG_INIT("vbox.vbox_common"); /* global vbox API, used for all common codes. */ static vboxUniformedAPI gVBoxAPI; -static int openSessionForMachine(vboxGlobalData *data, const unsigned char *dom_uuid, vboxIIDUnion *iid, +static int openSessionForMachine(vboxPrivate *data, const unsigned char *dom_uuid, vboxIIDUnion *iid, IMachine **machine, bool checkflag) { VBOX_IID_INITIALIZE(iid); @@ -286,7 +286,7 @@ vboxXMLConfInit(void) NULL, NULL); } -static int vboxInitialize(vboxGlobalData *data) +static int vboxInitialize(vboxPrivate *data) { if (gVBoxAPI.UPFN.Initialize(data) != 0) goto cleanup; @@ -348,7 +348,7 @@ static virCapsPtr vboxCapsInit(void) return NULL; } -static int vboxExtractVersion(vboxGlobalData *data) +static int vboxExtractVersion(vboxPrivate *data) { int ret = -1; PRUnichar *versionUtf16 = NULL; @@ -377,7 +377,7 @@ static int vboxExtractVersion(vboxGlobalData *data) return ret; } -static void vboxUninitialize(vboxGlobalData *data) +static void vboxUninitialize(vboxPrivate *data) { if (!data) return; @@ -397,7 +397,7 @@ vboxConnectOpen(virConnectPtr conn, virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { - vboxGlobalData *data = NULL; + vboxPrivate *data = NULL; uid_t uid = geteuid(); virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -466,7 +466,7 @@ vboxConnectOpen(virConnectPtr conn, static int vboxConnectClose(virConnectPtr conn) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; VIR_DEBUG("%s: in vboxClose", conn->driver->name); vboxUninitialize(data); @@ -478,7 +478,7 @@ static int vboxConnectClose(virConnectPtr conn) static int vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IConsole *console = NULL; vboxIIDUnion iid; IMachine *machine = NULL; @@ -530,19 +530,19 @@ vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED) return ret; } -static void vboxDriverLock(vboxGlobalData *data) +static void vboxDriverLock(vboxPrivate *data) { virMutexLock(&data->lock); } -static void vboxDriverUnlock(vboxGlobalData *data) +static void vboxDriverUnlock(vboxPrivate *data) { virMutexUnlock(&data->lock); } static int vboxConnectGetVersion(virConnectPtr conn, unsigned long *version) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; VIR_DEBUG("%s: in vboxGetVersion", conn->driver->name); vboxDriverLock(data); @@ -577,7 +577,7 @@ static int vboxConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) static int vboxConnectGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; PRUint32 maxCPUCount = 0; int ret = -1; @@ -604,7 +604,7 @@ vboxConnectGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED) static char *vboxConnectGetCapabilities(virConnectPtr conn) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; char *ret = NULL; if (!data->vboxObj) @@ -619,7 +619,7 @@ static char *vboxConnectGetCapabilities(virConnectPtr conn) static int vboxConnectListDomains(virConnectPtr conn, int *ids, int nids) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; PRUint32 state; nsresult rc; @@ -661,7 +661,7 @@ static int vboxConnectListDomains(virConnectPtr conn, int *ids, int nids) static int vboxConnectNumOfDomains(virConnectPtr conn) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; PRUint32 state; nsresult rc; @@ -700,7 +700,7 @@ static int vboxConnectNumOfDomains(virConnectPtr conn) static virDomainPtr vboxDomainLookupByID(virConnectPtr conn, int id) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; IMachine *machine; PRBool isAccessible = PR_FALSE; @@ -778,7 +778,7 @@ static virDomainPtr vboxDomainLookupByID(virConnectPtr conn, int id) virDomainPtr vboxDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; vboxIIDUnion iid; char *machineNameUtf8 = NULL; @@ -855,7 +855,7 @@ virDomainPtr vboxDomainLookupByUUID(virConnectPtr conn, static virDomainPtr vboxDomainLookupByName(virConnectPtr conn, const char *name) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; vboxIIDUnion iid; char *machineNameUtf8 = NULL; @@ -927,7 +927,7 @@ vboxDomainLookupByName(virConnectPtr conn, const char *name) } static void -vboxSetBootDeviceOrder(virDomainDefPtr def, vboxGlobalData *data, +vboxSetBootDeviceOrder(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { ISystemProperties *systemProperties = NULL; @@ -984,7 +984,7 @@ vboxSetBootDeviceOrder(virDomainDefPtr def, vboxGlobalData *data, } static void -vboxAttachDrivesNew(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxAttachDrivesNew(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { /* AttachDrives for 3.0 and later */ size_t i; @@ -1193,7 +1193,7 @@ vboxAttachDrivesNew(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine } static void -vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxAttachDrives(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { /* Here, About the vboxAttachDrives. In fact,there is * three different implementations. We name it as @@ -1264,7 +1264,7 @@ vboxAttachSound(virDomainDefPtr def, IMachine *machine) } static int -vboxAttachNetwork(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxAttachNetwork(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { ISystemProperties *systemProperties = NULL; PRUint32 chipsetType = ChipsetType_Null; @@ -1410,7 +1410,7 @@ vboxAttachNetwork(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } static void -vboxAttachSerial(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxAttachSerial(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { ISystemProperties *systemProperties = NULL; PRUint32 serialPortCount = 0; @@ -1486,7 +1486,7 @@ vboxAttachSerial(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } static void -vboxAttachParallel(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxAttachParallel(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { ISystemProperties *systemProperties = NULL; PRUint32 parallelPortCount = 0; @@ -1575,7 +1575,7 @@ vboxAttachVideo(virDomainDefPtr def, IMachine *machine) } static void -vboxAttachDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxAttachDisplay(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { int vrdpPresent = 0; int sdlPresent = 0; @@ -1722,7 +1722,7 @@ vboxAttachDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } static void -vboxAttachUSB(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxAttachUSB(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { IUSBCommon *USBCommon = NULL; size_t i = 0; @@ -1826,7 +1826,7 @@ vboxAttachUSB(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } static void -vboxAttachSharedFolder(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxAttachSharedFolder(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { size_t i; PRUnichar *nameUtf16; @@ -1855,7 +1855,7 @@ vboxAttachSharedFolder(virDomainDefPtr def, vboxGlobalData *data, IMachine *mach static virDomainPtr vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; IMachine *machine = NULL; IBIOSSettings *bios = NULL; vboxIIDUnion mchiid; @@ -2003,7 +2003,7 @@ vboxDomainDefineXML(virConnectPtr conn, const char *xml) } static void -detachDevices_common(vboxGlobalData *data, vboxIIDUnion *iidu) +detachDevices_common(vboxPrivate *data, vboxIIDUnion *iidu) { /* Block for checking if HDD's are attched to VM. * considering just IDE bus for now. Also skipped @@ -2040,7 +2040,7 @@ detachDevices_common(vboxGlobalData *data, vboxIIDUnion *iidu) static int vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IMachine *machine = NULL; vboxIIDUnion iid; nsresult rc; @@ -2083,7 +2083,7 @@ static int vboxDomainUndefine(virDomainPtr dom) static int vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine, vboxIIDUnion *iid) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; int vrdpPresent = 0; int sdlPresent = 0; int guiPresent = 0; @@ -2235,7 +2235,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine, vboxIIDUnion static int vboxDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; unsigned char uuid[VIR_UUID_BUFLEN] = {0}; nsresult rc; @@ -2340,7 +2340,7 @@ static virDomainPtr vboxDomainCreateXML(virConnectPtr conn, const char *xml, static int vboxDomainIsActive(virDomainPtr dom) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; vboxIIDUnion iid; char *machineNameUtf8 = NULL; @@ -2412,7 +2412,7 @@ static int vboxDomainIsPersistent(virDomainPtr dom) { /* All domains are persistent. However, we do want to check for * existence. */ - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion iid; IMachine *machine = NULL; int ret = -1; @@ -2435,7 +2435,7 @@ static int vboxDomainIsUpdated(virDomainPtr dom) { /* VBox domains never have a persistent state that differs from * current state. However, we do want to check for existence. */ - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion iid; IMachine *machine = NULL; int ret = -1; @@ -2456,7 +2456,7 @@ static int vboxDomainIsUpdated(virDomainPtr dom) static int vboxDomainSuspend(virDomainPtr dom) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IMachine *machine = NULL; vboxIIDUnion iid; IConsole *console = NULL; @@ -2507,7 +2507,7 @@ static int vboxDomainSuspend(virDomainPtr dom) static int vboxDomainResume(virDomainPtr dom) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IMachine *machine = NULL; vboxIIDUnion iid; IConsole *console = NULL; @@ -2558,7 +2558,7 @@ static int vboxDomainResume(virDomainPtr dom) static int vboxDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IMachine *machine = NULL; vboxIIDUnion iid; IConsole *console = NULL; @@ -2615,7 +2615,7 @@ static int vboxDomainShutdown(virDomainPtr dom) static int vboxDomainReboot(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IMachine *machine = NULL; vboxIIDUnion iid; IConsole *console = NULL; @@ -2663,7 +2663,7 @@ static int vboxDomainReboot(virDomainPtr dom, unsigned int flags) static int vboxDomainDestroyFlags(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IMachine *machine = NULL; vboxIIDUnion iid; IConsole *console = NULL; @@ -2729,7 +2729,7 @@ static char *vboxDomainGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED) { static int vboxDomainSetMemory(virDomainPtr dom, unsigned long memory) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IMachine *machine = NULL; vboxIIDUnion iid; PRUint32 state; @@ -2787,7 +2787,7 @@ static int vboxDomainSetMemory(virDomainPtr dom, unsigned long memory) static int vboxDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; char *machineName = NULL; PRUnichar *machineNameUtf16 = NULL; @@ -2871,7 +2871,7 @@ static int vboxDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) static int vboxDomainGetState(virDomainPtr dom, int *state, int *reason, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion domiid; IMachine *machine = NULL; PRUint32 mstate; @@ -2902,7 +2902,7 @@ static int vboxDomainGetState(virDomainPtr dom, int *state, static int vboxDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IMachine *machine = NULL; vboxIIDUnion iid; PRUint32 CPUCount = nvcpus; @@ -2956,7 +2956,7 @@ static int vboxDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) static int vboxDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; ISystemProperties *systemProperties = NULL; PRUint32 maxCPUCount = 0; int ret = -1; @@ -2993,7 +2993,7 @@ static int vboxDomainGetMaxVcpus(virDomainPtr dom) } static void -vboxHostDeviceGetXMLDesc(vboxGlobalData *data, virDomainDefPtr def, IMachine *machine) +vboxHostDeviceGetXMLDesc(vboxPrivate *data, virDomainDefPtr def, IMachine *machine) { IUSBCommon *USBCommon = NULL; PRBool enabled = PR_FALSE; @@ -3099,7 +3099,7 @@ vboxHostDeviceGetXMLDesc(vboxGlobalData *data, virDomainDefPtr def, IMachine *ma } static void -vboxDumpIDEHDDsNew(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxDumpIDEHDDsNew(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { /* dump IDE hdds if present */ vboxArray mediumAttachments = VBOX_ARRAY_INITIALIZER; @@ -3260,7 +3260,7 @@ vboxDumpIDEHDDsNew(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } static int -vboxDumpVideo(virDomainDefPtr def, vboxGlobalData *data ATTRIBUTE_UNUSED, +vboxDumpVideo(virDomainDefPtr def, vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine) { /* dump video options vram/2d/3d/directx/etc. */ @@ -3298,7 +3298,7 @@ vboxDumpVideo(virDomainDefPtr def, vboxGlobalData *data ATTRIBUTE_UNUSED, } static int -vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxDumpDisplay(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { /* dump display options vrdp/gui/sdl */ PRUnichar *keyUtf16 = NULL; @@ -3418,7 +3418,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } static void -vboxDumpSharedFolders(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +vboxDumpSharedFolders(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { /* shared folders */ vboxArray sharedFolders = VBOX_ARRAY_INITIALIZER; @@ -3479,7 +3479,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxGlobalData *data, IMachine *machi } static void -vboxDumpNetwork(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine, PRUint32 networkAdapterCount) +vboxDumpNetwork(virDomainDefPtr def, vboxPrivate *data, IMachine *machine, PRUint32 networkAdapterCount) { PRUint32 netAdpIncCnt = 0; size_t i = 0; @@ -3618,7 +3618,7 @@ vboxDumpNetwork(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine, PR } static void -vboxDumpAudio(virDomainDefPtr def, vboxGlobalData *data ATTRIBUTE_UNUSED, +vboxDumpAudio(virDomainDefPtr def, vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine) { /* dump sound card if active */ @@ -3658,7 +3658,7 @@ vboxDumpAudio(virDomainDefPtr def, vboxGlobalData *data ATTRIBUTE_UNUSED, } static void -vboxDumpSerial(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine, PRUint32 serialPortCount) +vboxDumpSerial(virDomainDefPtr def, vboxPrivate *data, IMachine *machine, PRUint32 serialPortCount) { PRUint32 serialPortIncCount = 0; size_t i = 0; @@ -3746,7 +3746,7 @@ vboxDumpSerial(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine, PRU } static void -vboxDumpParallel(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine, PRUint32 parallelPortCount) +vboxDumpParallel(virDomainDefPtr def, vboxPrivate *data, IMachine *machine, PRUint32 parallelPortCount) { PRUint32 parallelPortIncCount = 0; size_t i = 0; @@ -3821,7 +3821,7 @@ vboxDumpParallel(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine, P static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; virDomainDefPtr def = NULL; IMachine *machine = NULL; vboxIIDUnion iid; @@ -3989,7 +3989,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) static int vboxConnectListDefinedDomains(virConnectPtr conn, char ** const names, int maxnames) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; char *machineName = NULL; PRUnichar *machineNameUtf16 = NULL; @@ -4051,7 +4051,7 @@ static int vboxConnectListDefinedDomains(virConnectPtr conn, static int vboxConnectNumOfDefinedDomains(virConnectPtr conn) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; PRUint32 state; nsresult rc; @@ -4096,7 +4096,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, const char *xml, int mediaChangeOnly ATTRIBUTE_UNUSED) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IMachine *machine = NULL; vboxIIDUnion iid; PRUint32 state; @@ -4227,7 +4227,7 @@ static int vboxDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IMachine *machine = NULL; vboxIIDUnion iid; PRUint32 state; @@ -4340,7 +4340,7 @@ static int vboxDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, static int vboxCloseDisksRecursively(virDomainPtr dom, char *location) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; nsresult rc; size_t i = 0; PRUnichar *locationUtf = NULL; @@ -4441,7 +4441,7 @@ vboxSnapshotRedefine(virDomainPtr dom, * * Finally, we register the machine with the new virtualbox description file. */ - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion domiid; IMachine *machine = NULL; nsresult rc; @@ -5300,7 +5300,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, const char *xmlDesc, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; virDomainSnapshotDefPtr def = NULL; vboxIIDUnion domiid; IMachine *machine = NULL; @@ -5501,7 +5501,7 @@ vboxDomainSnapshotGetAll(virDomainPtr dom, } static ISnapshot * -vboxDomainSnapshotGet(vboxGlobalData *data, +vboxDomainSnapshotGet(vboxPrivate *data, virDomainPtr dom, IMachine *machine, const char *name) @@ -5554,7 +5554,7 @@ static int vboxSnapshotGetReadWriteDisks(virDomainSnapshotDefPtr def, virDomainSnapshotPtr snapshot) { virDomainPtr dom = snapshot->domain; - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion domiid; IMachine *machine = NULL; ISnapshot *snap = NULL; @@ -5775,7 +5775,7 @@ int vboxSnapshotGetReadOnlyDisks(virDomainSnapshotPtr snapshot, virDomainSnapshotDefPtr def) { virDomainPtr dom = snapshot->domain; - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion domiid; ISnapshot *snap = NULL; IMachine *machine = NULL; @@ -5994,7 +5994,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainPtr dom = snapshot->domain; - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion domiid; IMachine *machine = NULL; ISnapshot *snap = NULL; @@ -6139,7 +6139,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, static int vboxDomainSnapshotNum(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion iid; IMachine *machine = NULL; nsresult rc; @@ -6184,7 +6184,7 @@ static int vboxDomainSnapshotNum(virDomainPtr dom, unsigned int flags) static int vboxDomainSnapshotListNames(virDomainPtr dom, char **names, int nameslen, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion iid; IMachine *machine = NULL; nsresult rc; @@ -6265,7 +6265,7 @@ static virDomainSnapshotPtr vboxDomainSnapshotLookupByName(virDomainPtr dom, const char *name, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion iid; IMachine *machine = NULL; ISnapshot *snapshot = NULL; @@ -6294,7 +6294,7 @@ vboxDomainSnapshotLookupByName(virDomainPtr dom, const char *name, static int vboxDomainHasCurrentSnapshot(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion iid; IMachine *machine = NULL; ISnapshot *snapshot = NULL; @@ -6332,7 +6332,7 @@ vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainPtr dom = snapshot->domain; - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion iid; IMachine *machine = NULL; ISnapshot *snap = NULL; @@ -6395,7 +6395,7 @@ vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, static virDomainSnapshotPtr vboxDomainSnapshotCurrent(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion iid; IMachine *machine = NULL; ISnapshot *snapshot = NULL; @@ -6453,7 +6453,7 @@ static int vboxDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainPtr dom = snapshot->domain; - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion iid; IMachine *machine = NULL; ISnapshot *snap = NULL; @@ -6514,7 +6514,7 @@ static int vboxDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainPtr dom = snapshot->domain; - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion iid; IMachine *machine = NULL; ISnapshot *snap = NULL; @@ -6545,7 +6545,7 @@ static int vboxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainPtr dom = snapshot->domain; - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion domiid; IMachine *machine = NULL; ISnapshot *newSnapshot = NULL; @@ -6615,7 +6615,7 @@ static int vboxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } static int -vboxDomainSnapshotDeleteSingle(vboxGlobalData *data, +vboxDomainSnapshotDeleteSingle(vboxPrivate *data, IConsole *console, ISnapshot *snapshot) { @@ -6662,7 +6662,7 @@ vboxDomainSnapshotDeleteSingle(vboxGlobalData *data, } static int -vboxDomainSnapshotDeleteTree(vboxGlobalData *data, +vboxDomainSnapshotDeleteTree(vboxPrivate *data, IConsole *console, ISnapshot *snapshot) { @@ -6709,7 +6709,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) */ virDomainPtr dom = snapshot->domain; - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; virDomainSnapshotDefPtr def = NULL; char *defXml = NULL; vboxIIDUnion domiid; @@ -7146,7 +7146,7 @@ static int vboxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainPtr dom = snapshot->domain; - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIIDUnion domiid; IMachine *machine = NULL; ISnapshot *snap = NULL; @@ -7232,7 +7232,7 @@ vboxDomainScreenshot(virDomainPtr dom, unsigned int screen, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IConsole *console = NULL; vboxIIDUnion iid; IMachine *machine = NULL; @@ -7373,7 +7373,7 @@ vboxConnectListAllDomains(virConnectPtr conn, virDomainPtr **domains, unsigned int flags) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; char *machineNameUtf8 = NULL; PRUnichar *machineNameUtf16 = NULL; @@ -7585,7 +7585,7 @@ vboxNodeAllocPages(virConnectPtr conn ATTRIBUTE_UNUSED, static int vboxDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxArray machines = VBOX_ARRAY_INITIALIZER; vboxIIDUnion iid; char *machineNameUtf8 = NULL; @@ -7661,7 +7661,7 @@ vboxDomainSendKey(virDomainPtr dom, unsigned int flags) { int ret = -1; - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; IConsole *console = NULL; vboxIIDUnion iid; IMachine *machine = NULL; diff --git a/src/vbox/vbox_network.c b/src/vbox/vbox_network.c index 0b944b6..ec66fab 100644 --- a/src/vbox/vbox_network.c +++ b/src/vbox/vbox_network.c @@ -46,7 +46,7 @@ static vboxUniformedAPI gVBoxAPI; static int vboxConnectNumOfNetworks(virConnectPtr conn) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray networkInterfaces = VBOX_ARRAY_INITIALIZER; IHost *host = NULL; size_t i = 0; @@ -91,7 +91,7 @@ static int vboxConnectNumOfNetworks(virConnectPtr conn) static int vboxConnectListNetworks(virConnectPtr conn, char **const names, int nnames) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray networkInterfaces = VBOX_ARRAY_INITIALIZER; IHost *host = NULL; size_t i = 0; @@ -148,7 +148,7 @@ static int vboxConnectListNetworks(virConnectPtr conn, char **const names, int n static int vboxConnectNumOfDefinedNetworks(virConnectPtr conn) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray networkInterfaces = VBOX_ARRAY_INITIALIZER; IHost *host = NULL; size_t i = 0; @@ -193,7 +193,7 @@ static int vboxConnectNumOfDefinedNetworks(virConnectPtr conn) static int vboxConnectListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxArray networkInterfaces = VBOX_ARRAY_INITIALIZER; IHost *host = NULL; size_t i = 0; @@ -250,7 +250,7 @@ static int vboxConnectListDefinedNetworks(virConnectPtr conn, char **const names static virNetworkPtr vboxNetworkLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; PRUint32 interfaceType = 0; char *nameUtf8 = NULL; PRUnichar *nameUtf16 = NULL; @@ -302,7 +302,7 @@ static virNetworkPtr vboxNetworkLookupByUUID(virConnectPtr conn, const unsigned static virNetworkPtr vboxNetworkLookupByName(virConnectPtr conn, const char *name) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; PRUnichar *nameUtf16 = NULL; IHostNetworkInterface *networkInterface = NULL; PRUint32 interfaceType = 0; @@ -350,7 +350,7 @@ static virNetworkPtr vboxNetworkLookupByName(virConnectPtr conn, const char *nam } static PRUnichar * -vboxSocketFormatAddrUtf16(vboxGlobalData *data, virSocketAddrPtr addr) +vboxSocketFormatAddrUtf16(vboxPrivate *data, virSocketAddrPtr addr) { char *utf8 = NULL; PRUnichar *utf16 = NULL; @@ -369,7 +369,7 @@ vboxSocketFormatAddrUtf16(vboxGlobalData *data, virSocketAddrPtr addr) static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr conn, const char *xml, bool start) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; PRUnichar *networkInterfaceNameUtf16 = NULL; char *networkInterfaceNameUtf8 = NULL; PRUnichar *networkNameUtf16 = NULL; @@ -563,7 +563,7 @@ static virNetworkPtr vboxNetworkDefineXML(virConnectPtr conn, const char *xml) static int vboxNetworkUndefineDestroy(virNetworkPtr network, bool removeinterface) { - vboxGlobalData *data = network->conn->privateData; + vboxPrivate *data = network->conn->privateData; char *networkNameUtf8 = NULL; PRUnichar *networkInterfaceNameUtf16 = NULL; IHostNetworkInterface *networkInterface = NULL; @@ -668,7 +668,7 @@ static int vboxNetworkDestroy(virNetworkPtr network) static int vboxNetworkCreate(virNetworkPtr network) { - vboxGlobalData *data = network->conn->privateData; + vboxPrivate *data = network->conn->privateData; char *networkNameUtf8 = NULL; PRUnichar *networkInterfaceNameUtf16 = NULL; IHostNetworkInterface *networkInterface = NULL; @@ -739,7 +739,7 @@ static int vboxNetworkCreate(virNetworkPtr network) } static int -vboxSocketParseAddrUtf16(vboxGlobalData *data, const PRUnichar *utf16, +vboxSocketParseAddrUtf16(vboxPrivate *data, const PRUnichar *utf16, virSocketAddrPtr addr) { int result = -1; @@ -760,7 +760,7 @@ vboxSocketParseAddrUtf16(vboxGlobalData *data, const PRUnichar *utf16, static char *vboxNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags) { - vboxGlobalData *data = network->conn->privateData; + vboxPrivate *data = network->conn->privateData; virNetworkDefPtr def = NULL; virNetworkIPDefPtr ipdef = NULL; char *networkNameUtf8 = NULL; diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c index c849505..0b5d544 100644 --- a/src/vbox/vbox_storage.c +++ b/src/vbox/vbox_storage.c @@ -86,7 +86,7 @@ vboxStoragePoolLookupByName(virConnectPtr conn, const char *name) static int vboxStoragePoolNumOfVolumes(virStoragePoolPtr pool) { - vboxGlobalData *data = pool->conn->privateData; + vboxPrivate *data = pool->conn->privateData; vboxArray hardDisks = VBOX_ARRAY_INITIALIZER; PRUint32 hardDiskAccessible = 0; nsresult rc; @@ -127,7 +127,7 @@ static int vboxStoragePoolNumOfVolumes(virStoragePoolPtr pool) static int vboxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names, int nnames) { - vboxGlobalData *data = pool->conn->privateData; + vboxPrivate *data = pool->conn->privateData; vboxArray hardDisks = VBOX_ARRAY_INITIALIZER; PRUint32 numActive = 0; nsresult rc; @@ -183,7 +183,7 @@ vboxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names, int nname static virStorageVolPtr vboxStorageVolLookupByName(virStoragePoolPtr pool, const char *name) { - vboxGlobalData *data = pool->conn->privateData; + vboxPrivate *data = pool->conn->privateData; vboxArray hardDisks = VBOX_ARRAY_INITIALIZER; nsresult rc; size_t i; @@ -256,7 +256,7 @@ vboxStorageVolLookupByName(virStoragePoolPtr pool, const char *name) static virStorageVolPtr vboxStorageVolLookupByKey(virConnectPtr conn, const char *key) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; vboxIIDUnion hddIID; unsigned char uuid[VIR_UUID_BUFLEN]; IHardDisk *hardDisk = NULL; @@ -323,7 +323,7 @@ vboxStorageVolLookupByKey(virConnectPtr conn, const char *key) static virStorageVolPtr vboxStorageVolLookupByPath(virConnectPtr conn, const char *path) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; PRUnichar *hddPathUtf16 = NULL; IHardDisk *hardDisk = NULL; PRUnichar *hddNameUtf16 = NULL; @@ -401,7 +401,7 @@ static virStorageVolPtr vboxStorageVolCreateXML(virStoragePoolPtr pool, const char *xml, unsigned int flags) { - vboxGlobalData *data = pool->conn->privateData; + vboxPrivate *data = pool->conn->privateData; virStorageVolDefPtr def = NULL; PRUnichar *hddFormatUtf16 = NULL; PRUnichar *hddNameUtf16 = NULL; @@ -508,7 +508,7 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool, static int vboxStorageVolDelete(virStorageVolPtr vol, unsigned int flags) { - vboxGlobalData *data = vol->conn->privateData; + vboxPrivate *data = vol->conn->privateData; unsigned char uuid[VIR_UUID_BUFLEN]; IHardDisk *hardDisk = NULL; int deregister = 0; @@ -663,7 +663,7 @@ static int vboxStorageVolDelete(virStorageVolPtr vol, unsigned int flags) static int vboxStorageVolGetInfo(virStorageVolPtr vol, virStorageVolInfoPtr info) { - vboxGlobalData *data = vol->conn->privateData; + vboxPrivate *data = vol->conn->privateData; IHardDisk *hardDisk = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; PRUint32 hddstate; @@ -718,7 +718,7 @@ static int vboxStorageVolGetInfo(virStorageVolPtr vol, virStorageVolInfoPtr info static char *vboxStorageVolGetXMLDesc(virStorageVolPtr vol, unsigned int flags) { - vboxGlobalData *data = vol->conn->privateData; + vboxPrivate *data = vol->conn->privateData; IHardDisk *hardDisk = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; PRUnichar *hddFormatUtf16 = NULL; @@ -810,7 +810,7 @@ static char *vboxStorageVolGetXMLDesc(virStorageVolPtr vol, unsigned int flags) static char *vboxStorageVolGetPath(virStorageVolPtr vol) { - vboxGlobalData *data = vol->conn->privateData; + vboxPrivate *data = vol->conn->privateData; IHardDisk *hardDisk = NULL; PRUnichar *hddLocationUtf16 = NULL; char *hddLocationUtf8 = NULL; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 23f63f9..13869eb 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -206,12 +206,12 @@ static vboxGlobalData *g_pVBoxGlobalData; * vboxDriverLock and vboxDriverUnlock only be used in code for * 3.x release. */ -static void vboxDriverLock(vboxGlobalData *data) +static void vboxDriverLock(vboxPrivate *data) { virMutexLock(&data->lock); } -static void vboxDriverUnlock(vboxGlobalData *data) +static void vboxDriverUnlock(vboxPrivate *data) { virMutexUnlock(&data->lock); } @@ -309,14 +309,14 @@ typedef struct _vboxIID_v2_x_WIN32 vboxIID_v2_x_WIN32; # define IID_MEMBER(name) (iidu->vboxIID_v2_x_WIN32.name) static void -vboxIIDUnalloc_v2_x_WIN32(vboxGlobalData *data ATTRIBUTE_UNUSED, +vboxIIDUnalloc_v2_x_WIN32(vboxPrivate *data ATTRIBUTE_UNUSED, vboxIID_v2_x_WIN32 *iid ATTRIBUTE_UNUSED) { /* Nothing to free */ } static void -_vboxIIDUnalloc(vboxGlobalData *data ATTRIBUTE_UNUSED, +_vboxIIDUnalloc(vboxPrivate *data ATTRIBUTE_UNUSED, vboxIIDUnion *iid ATTRIBUTE_UNUSED) { /* Nothing to free */ @@ -329,13 +329,13 @@ vboxIIDToUUID_v2_x_WIN32(vboxIID_v2_x_WIN32 *iid, unsigned char *uuid) } static void -_vboxIIDToUUID(vboxGlobalData *data ATTRIBUTE_UNUSED, vboxIIDUnion *iidu, unsigned char *uuid) +_vboxIIDToUUID(vboxPrivate *data ATTRIBUTE_UNUSED, vboxIIDUnion *iidu, unsigned char *uuid) { vboxIIDToUUID_v2_x_WIN32(&iidu->vboxIID_v2_x_WIN32, uuid); } static void -vboxIIDFromUUID_v2_x_WIN32(vboxGlobalData *data, vboxIID_v2_x_WIN32 *iid, +vboxIIDFromUUID_v2_x_WIN32(vboxPrivate *data, vboxIID_v2_x_WIN32 *iid, const unsigned char *uuid) { vboxIIDUnalloc_v2_x_WIN32(data, iid); @@ -344,7 +344,7 @@ vboxIIDFromUUID_v2_x_WIN32(vboxGlobalData *data, vboxIID_v2_x_WIN32 *iid, } static void -_vboxIIDFromUUID(vboxGlobalData *data, vboxIIDUnion *iidu, +_vboxIIDFromUUID(vboxPrivate *data, vboxIIDUnion *iidu, const unsigned char *uuid) { vboxIIDFromUUID_v2_x_WIN32(data, &iidu->vboxIID_v2_x_WIN32, uuid); @@ -357,13 +357,13 @@ vboxIIDIsEqual_v2_x_WIN32(vboxIID_v2_x_WIN32 *iid1, vboxIID_v2_x_WIN32 *iid2) } static bool -_vboxIIDIsEqual(vboxGlobalData *data ATTRIBUTE_UNUSED, vboxIIDUnion *iidu1, vboxIIDUnion *iidu2) +_vboxIIDIsEqual(vboxPrivate *data ATTRIBUTE_UNUSED, vboxIIDUnion *iidu1, vboxIIDUnion *iidu2) { return vboxIIDIsEqual_v2_x_WIN32(&iidu1->vboxIID_v2_x_WIN32, &iidu2->vboxIID_v2_x_WIN32); } static void -vboxIIDFromArrayItem_v2_x_WIN32(vboxGlobalData *data, vboxIID_v2_x_WIN32 *iid, +vboxIIDFromArrayItem_v2_x_WIN32(vboxPrivate *data, vboxIID_v2_x_WIN32 *iid, vboxArray *array, int idx) { GUID *items = (GUID *)array->items; @@ -374,7 +374,7 @@ vboxIIDFromArrayItem_v2_x_WIN32(vboxGlobalData *data, vboxIID_v2_x_WIN32 *iid, } static void -_vboxIIDFromArrayItem(vboxGlobalData *data, vboxIIDUnion *iidu, +_vboxIIDFromArrayItem(vboxPrivate *data, vboxIIDUnion *iidu, vboxArray *array, int idx) { vboxIIDFromArrayItem_v2_x_WIN32(data, &iidu->vboxIID_v2_x_WIN32, array, idx); @@ -397,7 +397,7 @@ typedef struct _vboxIID_v2_x vboxIID_v2_x; # define IID_MEMBER(name) (iidu->vboxIID_v2_x.name) static void -vboxIIDUnalloc_v2_x(vboxGlobalData *data, vboxIID_v2_x *iid) +vboxIIDUnalloc_v2_x(vboxPrivate *data, vboxIID_v2_x *iid) { if (iid->value == NULL) return; @@ -409,7 +409,7 @@ vboxIIDUnalloc_v2_x(vboxGlobalData *data, vboxIID_v2_x *iid) } static void -_vboxIIDUnalloc(vboxGlobalData *data, vboxIIDUnion *iidu) +_vboxIIDUnalloc(vboxPrivate *data, vboxIIDUnion *iidu) { vboxIIDUnalloc_v2_x(data, &iidu->vboxIID_v2_x); } @@ -421,14 +421,14 @@ vboxIIDToUUID_v2_x(vboxIID_v2_x *iid, unsigned char *uuid) } static void -_vboxIIDToUUID(vboxGlobalData *data ATTRIBUTE_UNUSED, +_vboxIIDToUUID(vboxPrivate *data ATTRIBUTE_UNUSED, vboxIIDUnion *iidu, unsigned char *uuid) { vboxIIDToUUID_v2_x(&iidu->vboxIID_v2_x, uuid); } static void -vboxIIDFromUUID_v2_x(vboxGlobalData *data, vboxIID_v2_x *iid, +vboxIIDFromUUID_v2_x(vboxPrivate *data, vboxIID_v2_x *iid, const unsigned char *uuid) { vboxIIDUnalloc_v2_x(data, iid); @@ -440,7 +440,7 @@ vboxIIDFromUUID_v2_x(vboxGlobalData *data, vboxIID_v2_x *iid, } static void -_vboxIIDFromUUID(vboxGlobalData *data, vboxIIDUnion *iidu, +_vboxIIDFromUUID(vboxPrivate *data, vboxIIDUnion *iidu, const unsigned char *uuid) { vboxIIDFromUUID_v2_x(data, &iidu->vboxIID_v2_x, uuid); @@ -453,14 +453,14 @@ vboxIIDIsEqual_v2_x(vboxIID_v2_x *iid1, vboxIID_v2_x *iid2) } static bool -_vboxIIDIsEqual(vboxGlobalData *data ATTRIBUTE_UNUSED, +_vboxIIDIsEqual(vboxPrivate *data ATTRIBUTE_UNUSED, vboxIIDUnion *iidu1, vboxIIDUnion *iidu2) { return vboxIIDIsEqual_v2_x(&iidu1->vboxIID_v2_x, &iidu2->vboxIID_v2_x); } static void -vboxIIDFromArrayItem_v2_x(vboxGlobalData *data, vboxIID_v2_x *iid, +vboxIIDFromArrayItem_v2_x(vboxPrivate *data, vboxIID_v2_x *iid, vboxArray *array, int idx) { vboxIIDUnalloc_v2_x(data, iid); @@ -471,7 +471,7 @@ vboxIIDFromArrayItem_v2_x(vboxGlobalData *data, vboxIID_v2_x *iid, } static void -_vboxIIDFromArrayItem(vboxGlobalData *data, vboxIIDUnion *iidu, +_vboxIIDFromArrayItem(vboxPrivate *data, vboxIIDUnion *iidu, vboxArray *array, int idx) { vboxIIDFromArrayItem_v2_x(data, &iidu->vboxIID_v2_x, array, idx); @@ -496,7 +496,7 @@ typedef struct _vboxIID_v3_x vboxIID_v3_x; # define IID_MEMBER(name) (iidu->vboxIID_v3_x.name) static void -vboxIIDUnalloc_v3_x(vboxGlobalData *data, vboxIID_v3_x *iid) +vboxIIDUnalloc_v3_x(vboxPrivate *data, vboxIID_v3_x *iid) { if (iid->value != NULL && iid->owner) data->pFuncs->pfnUtf16Free(iid->value); @@ -506,13 +506,13 @@ vboxIIDUnalloc_v3_x(vboxGlobalData *data, vboxIID_v3_x *iid) } static void -_vboxIIDUnalloc(vboxGlobalData *data, vboxIIDUnion *iidu) +_vboxIIDUnalloc(vboxPrivate *data, vboxIIDUnion *iidu) { vboxIIDUnalloc_v3_x(data, &iidu->vboxIID_v3_x); } static void -vboxIIDToUUID_v3_x(vboxGlobalData *data, vboxIID_v3_x *iid, +vboxIIDToUUID_v3_x(vboxPrivate *data, vboxIID_v3_x *iid, unsigned char *uuid) { char *utf8 = NULL; @@ -525,14 +525,14 @@ vboxIIDToUUID_v3_x(vboxGlobalData *data, vboxIID_v3_x *iid, } static void -_vboxIIDToUUID(vboxGlobalData *data, vboxIIDUnion *iidu, +_vboxIIDToUUID(vboxPrivate *data, vboxIIDUnion *iidu, unsigned char *uuid) { vboxIIDToUUID_v3_x(data, &iidu->vboxIID_v3_x, uuid); } static void -vboxIIDFromUUID_v3_x(vboxGlobalData *data, vboxIID_v3_x *iid, +vboxIIDFromUUID_v3_x(vboxPrivate *data, vboxIID_v3_x *iid, const unsigned char *uuid) { char utf8[VIR_UUID_STRING_BUFLEN]; @@ -545,14 +545,14 @@ vboxIIDFromUUID_v3_x(vboxGlobalData *data, vboxIID_v3_x *iid, } static void -_vboxIIDFromUUID(vboxGlobalData *data, vboxIIDUnion *iidu, +_vboxIIDFromUUID(vboxPrivate *data, vboxIIDUnion *iidu, const unsigned char *uuid) { vboxIIDFromUUID_v3_x(data, &iidu->vboxIID_v3_x, uuid); } static bool -vboxIIDIsEqual_v3_x(vboxGlobalData *data, vboxIID_v3_x *iid1, +vboxIIDIsEqual_v3_x(vboxPrivate *data, vboxIID_v3_x *iid1, vboxIID_v3_x *iid2) { unsigned char uuid1[VIR_UUID_BUFLEN]; @@ -570,14 +570,14 @@ vboxIIDIsEqual_v3_x(vboxGlobalData *data, vboxIID_v3_x *iid1, } static bool -_vboxIIDIsEqual(vboxGlobalData *data, vboxIIDUnion *iidu1, +_vboxIIDIsEqual(vboxPrivate *data, vboxIIDUnion *iidu1, vboxIIDUnion *iidu2) { return vboxIIDIsEqual_v3_x(data, &iidu1->vboxIID_v3_x, &iidu2->vboxIID_v3_x); } static void -vboxIIDFromArrayItem_v3_x(vboxGlobalData *data, vboxIID_v3_x *iid, +vboxIIDFromArrayItem_v3_x(vboxPrivate *data, vboxIID_v3_x *iid, vboxArray *array, int idx) { vboxIIDUnalloc_v3_x(data, iid); @@ -587,7 +587,7 @@ vboxIIDFromArrayItem_v3_x(vboxGlobalData *data, vboxIID_v3_x *iid, } static void -_vboxIIDFromArrayItem(vboxGlobalData *data, vboxIIDUnion *iidu, +_vboxIIDFromArrayItem(vboxPrivate *data, vboxIIDUnion *iidu, vboxArray *array, int idx) { vboxIIDFromArrayItem_v3_x(data, &iidu->vboxIID_v3_x, array, idx); @@ -791,7 +791,7 @@ static virDomainState _vboxConvertState(PRUint32 state) #if VBOX_API_VERSION < 3001000 static void -_vboxAttachDrivesOld(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +_vboxAttachDrivesOld(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { size_t i; nsresult rc; @@ -1037,7 +1037,7 @@ _vboxAttachDrivesOld(virDomainDefPtr def, vboxGlobalData *data, IMachine *machin #elif VBOX_API_VERSION < 4000000 static void -_vboxAttachDrivesOld(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) +_vboxAttachDrivesOld(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) { size_t i; nsresult rc = 0; @@ -1264,7 +1264,7 @@ _vboxAttachDrivesOld(virDomainDefPtr def, vboxGlobalData *data, IMachine *machin static void _vboxAttachDrivesOld(virDomainDefPtr def ATTRIBUTE_UNUSED, - vboxGlobalData *data ATTRIBUTE_UNUSED, + vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine ATTRIBUTE_UNUSED) { vboxUnsupported(); @@ -1278,7 +1278,7 @@ _vboxDomainSnapshotRestore(virDomainPtr dom, IMachine *machine, ISnapshot *snapshot) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; vboxIID iid = VBOX_IID_INITIALIZER; nsresult rc; int ret = -1; @@ -1312,7 +1312,7 @@ _vboxDomainSnapshotRestore(virDomainPtr dom, IMachine *machine, ISnapshot *snapshot) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData; # if VBOX_API_VERSION < 5000000 IConsole *console = NULL; # endif /*VBOX_API_VERSION < 5000000*/ @@ -1422,7 +1422,7 @@ vboxCallbackOnMachineStateChange(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, int event = 0; int detail = 0; - vboxDriverLock(g_pVBoxGlobalData); + vboxDriverLock((vboxPrivate *) g_pVBoxGlobalData); VIR_DEBUG("IVirtualBoxCallback: %p, State: %d", pThis, state); DEBUGPRUnichar("machineId", machineId); @@ -1474,7 +1474,7 @@ vboxCallbackOnMachineStateChange(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, } } - vboxDriverUnlock(g_pVBoxGlobalData); + vboxDriverUnlock((vboxPrivate *) g_pVBoxGlobalData); return NS_OK; } @@ -1541,7 +1541,7 @@ vboxCallbackOnMachineRegistered(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, int event = 0; int detail = 0; - vboxDriverLock(g_pVBoxGlobalData); + vboxDriverLock((vboxPrivate *) g_pVBoxGlobalData); VIR_DEBUG("IVirtualBoxCallback: %p, registered: %s", pThis, registered ? "true" : "false"); DEBUGPRUnichar("machineId", machineId); @@ -1578,7 +1578,7 @@ vboxCallbackOnMachineRegistered(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, } } - vboxDriverUnlock(g_pVBoxGlobalData); + vboxDriverUnlock((vboxPrivate *) g_pVBoxGlobalData); return NS_OK; } @@ -1762,7 +1762,7 @@ vboxConnectDomainEventRegister(virConnectPtr conn, void *opaque, virFreeCallback freecb) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; int vboxRet = -1; nsresult rc; int ret = -1; @@ -1827,7 +1827,7 @@ static int vboxConnectDomainEventDeregister(virConnectPtr conn, virConnectDomainEventCallback callback) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; int cnt; int ret = -1; @@ -1866,7 +1866,7 @@ static int vboxConnectDomainEventRegisterAny(virConnectPtr conn, void *opaque, virFreeCallback freecb) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; int vboxRet = -1; nsresult rc; int ret = -1; @@ -1933,7 +1933,7 @@ static int vboxConnectDomainEventDeregisterAny(virConnectPtr conn, int callbackID) { - vboxGlobalData *data = conn->privateData; + vboxPrivate *data = conn->privateData; int cnt; int ret = -1; @@ -1976,7 +1976,7 @@ _registerDomainEvent(virHypervisorDriverPtr driver) #endif /* !(VBOX_API_VERSION == 2002000 || VBOX_API_VERSION >= 4000000) */ -static int _pfnInitialize(vboxGlobalData *data) +static int _pfnInitialize(vboxPrivate *data) { data->pFuncs = g_pfnGetFunctions(VBOX_XPCOMC_VERSION); if (data->pFuncs == NULL) @@ -1990,7 +1990,7 @@ static int _pfnInitialize(vboxGlobalData *data) } static int -_initializeDomainEvent(vboxGlobalData *data ATTRIBUTE_UNUSED) +_initializeDomainEvent(vboxPrivate *data ATTRIBUTE_UNUSED) { #if VBOX_API_VERSION <= 2002000 || VBOX_API_VERSION >= 4000000 /* No event queue functionality in 2.2.* and 4.* as of now */ @@ -2009,12 +2009,12 @@ _initializeDomainEvent(vboxGlobalData *data ATTRIBUTE_UNUSED) } static -void _registerGlobalData(vboxGlobalData *data ATTRIBUTE_UNUSED) +void _registerGlobalData(vboxPrivate *data ATTRIBUTE_UNUSED) { #if VBOX_API_VERSION == 2002000 vboxUnsupported(); #else /* VBOX_API_VERSION != 2002000 */ - g_pVBoxGlobalData = data; + g_pVBoxGlobalData = (vboxGlobalData *) data; #endif /* VBOX_API_VERSION != 2002000 */ } @@ -2022,7 +2022,7 @@ void _registerGlobalData(vboxGlobalData *data ATTRIBUTE_UNUSED) # if VBOX_API_VERSION < 3001000 static void -_detachDevices(vboxGlobalData *data ATTRIBUTE_UNUSED, +_detachDevices(vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine, PRUnichar *hddcnameUtf16) { /* Disconnect all the drives if present */ @@ -2032,7 +2032,7 @@ _detachDevices(vboxGlobalData *data ATTRIBUTE_UNUSED, } # else /* VBOX_API_VERSION >= 3001000 */ static void -_detachDevices(vboxGlobalData *data, IMachine *machine, +_detachDevices(vboxPrivate *data, IMachine *machine, PRUnichar *hddcnameUtf16 ATTRIBUTE_UNUSED) { /* get all the controller first, then the attachments and @@ -2084,7 +2084,7 @@ _detachDevices(vboxGlobalData *data, IMachine *machine, # endif /* VBOX_API_VERSION >= 3001000 */ static nsresult -_unregisterMachine(vboxGlobalData *data, vboxIIDUnion *iidu, IMachine **machine) +_unregisterMachine(vboxPrivate *data, vboxIIDUnion *iidu, IMachine **machine) { return data->vboxObj->vtbl->UnregisterMachine(data->vboxObj, IID_MEMBER(value), machine); } @@ -2098,7 +2098,7 @@ _deleteConfig(IMachine *machine) #else /* VBOX_API_VERSION >= 4000000 */ static void -_detachDevices(vboxGlobalData *data ATTRIBUTE_UNUSED, +_detachDevices(vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine ATTRIBUTE_UNUSED, PRUnichar *hddcnameUtf16 ATTRIBUTE_UNUSED) { @@ -2106,7 +2106,7 @@ _detachDevices(vboxGlobalData *data ATTRIBUTE_UNUSED, } static nsresult -_unregisterMachine(vboxGlobalData *data, vboxIIDUnion *iidu, IMachine **machine) +_unregisterMachine(vboxPrivate *data, vboxIIDUnion *iidu, IMachine **machine) { nsresult rc; vboxArray media = VBOX_ARRAY_INITIALIZER; @@ -2170,7 +2170,7 @@ _deleteConfig(IMachine *machine) static void _dumpIDEHDDsOld(virDomainDefPtr def, - vboxGlobalData *data, + vboxPrivate *data, IMachine *machine) { PRInt32 hddNum = 0; @@ -2279,7 +2279,7 @@ _dumpIDEHDDsOld(virDomainDefPtr def, static void _dumpDVD(virDomainDefPtr def, - vboxGlobalData *data, + vboxPrivate *data, IMachine *machine) { IDVDDrive *dvdDrive = NULL; @@ -2334,7 +2334,7 @@ _dumpDVD(virDomainDefPtr def, } static int -_attachDVD(vboxGlobalData *data, IMachine *machine, const char *src) +_attachDVD(vboxPrivate *data, IMachine *machine, const char *src) { IDVDDrive *dvdDrive = NULL; IDVDImage *dvdImage = NULL; @@ -2418,7 +2418,7 @@ _detachDVD(IMachine *machine) static void _dumpFloppy(virDomainDefPtr def, - vboxGlobalData *data, + vboxPrivate *data, IMachine *machine) { IFloppyDrive *floppyDrive = NULL; @@ -2476,7 +2476,7 @@ _dumpFloppy(virDomainDefPtr def, } static int -_attachFloppy(vboxGlobalData *data, IMachine *machine, const char *src) +_attachFloppy(vboxPrivate *data, IMachine *machine, const char *src) { IFloppyDrive *floppyDrive; IFloppyImage *floppyImage = NULL; @@ -2573,7 +2573,7 @@ _detachFloppy(IMachine *machine) static void _dumpIDEHDDsOld(virDomainDefPtr def ATTRIBUTE_UNUSED, - vboxGlobalData *data ATTRIBUTE_UNUSED, + vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine ATTRIBUTE_UNUSED) { vboxUnsupported(); @@ -2581,14 +2581,14 @@ _dumpIDEHDDsOld(virDomainDefPtr def ATTRIBUTE_UNUSED, static void _dumpDVD(virDomainDefPtr def ATTRIBUTE_UNUSED, - vboxGlobalData *data ATTRIBUTE_UNUSED, + vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine ATTRIBUTE_UNUSED) { vboxUnsupported(); } static int -_attachDVD(vboxGlobalData *data ATTRIBUTE_UNUSED, +_attachDVD(vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine ATTRIBUTE_UNUSED, const char *src ATTRIBUTE_UNUSED) { @@ -2605,14 +2605,14 @@ _detachDVD(IMachine *machine ATTRIBUTE_UNUSED) static void _dumpFloppy(virDomainDefPtr def ATTRIBUTE_UNUSED, - vboxGlobalData *data ATTRIBUTE_UNUSED, + vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine ATTRIBUTE_UNUSED) { vboxUnsupported(); } static int -_attachFloppy(vboxGlobalData *data ATTRIBUTE_UNUSED, +_attachFloppy(vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine ATTRIBUTE_UNUSED, const char *src ATTRIBUTE_UNUSED) { @@ -2629,7 +2629,7 @@ _detachFloppy(IMachine *machine ATTRIBUTE_UNUSED) #endif /* VBOX_API_VERSION >= 3001000 */ -static void _pfnUninitialize(vboxGlobalData *data) +static void _pfnUninitialize(vboxPrivate *data) { if (data->pFuncs) data->pFuncs->pfnComUninitialize(); @@ -2692,7 +2692,7 @@ static void _DEBUGIID(const char *msg, vboxIIDUnion *iidu) #endif /* VBOX_API_VERSION != 2002000 */ static void -_vboxIIDToUtf8(vboxGlobalData *data ATTRIBUTE_UNUSED, +_vboxIIDToUtf8(vboxPrivate *data ATTRIBUTE_UNUSED, vboxIIDUnion *iidu ATTRIBUTE_UNUSED, char **utf8 ATTRIBUTE_UNUSED) { @@ -2821,7 +2821,7 @@ _virtualboxGetHost(IVirtualBox *vboxObj, IHost **host) } static nsresult -_virtualboxCreateMachine(vboxGlobalData *data, virDomainDefPtr def, IMachine **machine, char *uuidstr ATTRIBUTE_UNUSED) +_virtualboxCreateMachine(vboxPrivate *data, virDomainDefPtr def, IMachine **machine, char *uuidstr ATTRIBUTE_UNUSED) { vboxIID iid = VBOX_IID_INITIALIZER; PRUnichar *machineNameUtf16 = NULL; @@ -3031,7 +3031,7 @@ _machineRemoveSharedFolder(IMachine *machine, PRUnichar *name) } static nsresult -_machineLaunchVMProcess(vboxGlobalData *data, +_machineLaunchVMProcess(vboxPrivate *data, IMachine *machine ATTRIBUTE_UNUSED, vboxIIDUnion *iidu ATTRIBUTE_UNUSED, PRUnichar *sessionType, PRUnichar *env, @@ -3329,13 +3329,13 @@ _machineSaveSettings(IMachine *machine) #if VBOX_API_VERSION < 4000000 static nsresult -_sessionOpen(vboxGlobalData *data, vboxIIDUnion *iidu, IMachine *machine ATTRIBUTE_UNUSED) +_sessionOpen(vboxPrivate *data, vboxIIDUnion *iidu, IMachine *machine ATTRIBUTE_UNUSED) { return data->vboxObj->vtbl->OpenSession(data->vboxObj, data->vboxSession, IID_MEMBER(value)); } static nsresult -_sessionOpenExisting(vboxGlobalData *data, vboxIIDUnion *iidu, IMachine *machine ATTRIBUTE_UNUSED) +_sessionOpenExisting(vboxPrivate *data, vboxIIDUnion *iidu, IMachine *machine ATTRIBUTE_UNUSED) { return data->vboxObj->vtbl->OpenExistingSession(data->vboxObj, data->vboxSession, IID_MEMBER(value)); } @@ -3349,13 +3349,13 @@ _sessionClose(ISession *session) #else /* VBOX_API_VERSION >= 4000000 */ static nsresult -_sessionOpen(vboxGlobalData *data, vboxIIDUnion *iidu ATTRIBUTE_UNUSED, IMachine *machine) +_sessionOpen(vboxPrivate *data, vboxIIDUnion *iidu ATTRIBUTE_UNUSED, IMachine *machine) { return machine->vtbl->LockMachine(machine, data->vboxSession, LockType_Write); } static nsresult -_sessionOpenExisting(vboxGlobalData *data, vboxIIDUnion *iidu ATTRIBUTE_UNUSED, IMachine *machine) +_sessionOpenExisting(vboxPrivate *data, vboxIIDUnion *iidu ATTRIBUTE_UNUSED, IMachine *machine) { return machine->vtbl->LockMachine(machine, data->vboxSession, LockType_Shared); } @@ -3930,7 +3930,7 @@ _vrdxServerSetEnabled(IVRDxServer *VRDxServer, PRBool enabled) } static nsresult -_vrdxServerGetPorts(vboxGlobalData *data ATTRIBUTE_UNUSED, +_vrdxServerGetPorts(vboxPrivate *data ATTRIBUTE_UNUSED, IVRDxServer *VRDxServer, virDomainGraphicsDefPtr graphics) { nsresult rc; @@ -3970,7 +3970,7 @@ _vrdxServerGetPorts(vboxGlobalData *data ATTRIBUTE_UNUSED, } static nsresult -_vrdxServerSetPorts(vboxGlobalData *data ATTRIBUTE_UNUSED, +_vrdxServerSetPorts(vboxPrivate *data ATTRIBUTE_UNUSED, IVRDxServer *VRDxServer, virDomainGraphicsDefPtr graphics) { nsresult rc = 0; @@ -4030,7 +4030,7 @@ _vrdxServerSetAllowMultiConnection(IVRDxServer *VRDxServer, PRBool enabled) } static nsresult -_vrdxServerGetNetAddress(vboxGlobalData *data ATTRIBUTE_UNUSED, +_vrdxServerGetNetAddress(vboxPrivate *data ATTRIBUTE_UNUSED, IVRDxServer *VRDxServer, PRUnichar **netAddress) { #if VBOX_API_VERSION >= 4000000 @@ -4046,7 +4046,7 @@ _vrdxServerGetNetAddress(vboxGlobalData *data ATTRIBUTE_UNUSED, } static nsresult -_vrdxServerSetNetAddress(vboxGlobalData *data ATTRIBUTE_UNUSED, +_vrdxServerSetNetAddress(vboxPrivate *data ATTRIBUTE_UNUSED, IVRDxServer *VRDxServer, PRUnichar *netAddress) { #if VBOX_API_VERSION < 4000000 @@ -4456,7 +4456,7 @@ _hostFindHostNetworkInterfaceByName(IHost *host, PRUnichar *name, } static nsresult -_hostCreateHostOnlyNetworkInterface(vboxGlobalData *data ATTRIBUTE_UNUSED, +_hostCreateHostOnlyNetworkInterface(vboxPrivate *data ATTRIBUTE_UNUSED, IHost *host, char *name ATTRIBUTE_UNUSED, IHostNetworkInterface **networkInterface) { diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h index 6ec5245..50041c0 100644 --- a/src/vbox/vbox_uniformed_api.h +++ b/src/vbox/vbox_uniformed_api.h @@ -172,8 +172,8 @@ typedef struct { /* Functions for pFuncs */ typedef struct { - int (*Initialize)(vboxGlobalData *data); - void (*Uninitialize)(vboxGlobalData *data); + int (*Initialize)(vboxPrivate *data); + void (*Uninitialize)(vboxPrivate *data); void (*ComUnallocMem)(PCVBOXXPCOM pFuncs, void *pv); void (*Utf16Free)(PCVBOXXPCOM pFuncs, PRUnichar *pwszString); void (*Utf8Free)(PCVBOXXPCOM pFuncs, char *pszString); @@ -184,12 +184,12 @@ typedef struct { /* Functions for vboxIID */ typedef struct { void (*vboxIIDInitialize)(vboxIIDUnion *iidu); - void (*vboxIIDUnalloc)(vboxGlobalData *data, vboxIIDUnion *iidu); - void (*vboxIIDToUUID)(vboxGlobalData *data, vboxIIDUnion *iidu, unsigned char *uuid); - void (*vboxIIDFromUUID)(vboxGlobalData *data, vboxIIDUnion *iidu, const unsigned char *uuid); - bool (*vboxIIDIsEqual)(vboxGlobalData *data, vboxIIDUnion *iidu1, vboxIIDUnion *iidu2); - void (*vboxIIDFromArrayItem)(vboxGlobalData *data, vboxIIDUnion *iidu, vboxArray *array, int idx); - void (*vboxIIDToUtf8)(vboxGlobalData *data, vboxIIDUnion *iidu, char **utf8); + void (*vboxIIDUnalloc)(vboxPrivate *data, vboxIIDUnion *iidu); + void (*vboxIIDToUUID)(vboxPrivate *data, vboxIIDUnion *iidu, unsigned char *uuid); + void (*vboxIIDFromUUID)(vboxPrivate *data, vboxIIDUnion *iidu, const unsigned char *uuid); + bool (*vboxIIDIsEqual)(vboxPrivate *data, vboxIIDUnion *iidu1, vboxIIDUnion *iidu2); + void (*vboxIIDFromArrayItem)(vboxPrivate *data, vboxIIDUnion *iidu, vboxArray *array, int idx); + void (*vboxIIDToUtf8)(vboxPrivate *data, vboxIIDUnion *iidu, char **utf8); void (*DEBUGIID)(const char *msg, vboxIIDUnion *iidu); } vboxUniformedIID; @@ -225,7 +225,7 @@ typedef struct { nsresult (*OpenMachine)(IVirtualBox *vboxObj, PRUnichar *settingsFile, IMachine **machine); nsresult (*GetSystemProperties)(IVirtualBox *vboxObj, ISystemProperties **systemProperties); nsresult (*GetHost)(IVirtualBox *vboxObj, IHost **host); - nsresult (*CreateMachine)(vboxGlobalData *data, virDomainDefPtr def, IMachine **machine, char *uuidstr); + nsresult (*CreateMachine)(vboxPrivate *data, virDomainDefPtr def, IMachine **machine, char *uuidstr); nsresult (*CreateHardDisk)(IVirtualBox *vboxObj, PRUnichar *format, PRUnichar *location, IHardDisk **hardDisk); nsresult (*RegisterMachine)(IVirtualBox *vboxObj, IMachine *machine); nsresult (*FindHardDisk)(IVirtualBox *vboxObj, PRUnichar *location, PRUint32 deviceType, @@ -250,7 +250,7 @@ typedef struct { PRUnichar *hostPath, PRBool writable, PRBool automount); nsresult (*RemoveSharedFolder)(IMachine *machine, PRUnichar *name); - nsresult (*LaunchVMProcess)(vboxGlobalData *data, IMachine *machine, + nsresult (*LaunchVMProcess)(vboxPrivate *data, IMachine *machine, vboxIIDUnion *iidu, PRUnichar *sessionType, PRUnichar *env, IProgress **progress); @@ -297,8 +297,8 @@ typedef struct { /* Functions for ISession */ typedef struct { - nsresult (*Open)(vboxGlobalData *data, vboxIIDUnion *iidu, IMachine *machine); - nsresult (*OpenExisting)(vboxGlobalData *data, vboxIIDUnion *iidu, IMachine *machine); + nsresult (*Open)(vboxPrivate *data, vboxIIDUnion *iidu, IMachine *machine); + nsresult (*OpenExisting)(vboxPrivate *data, vboxIIDUnion *iidu, IMachine *machine); nsresult (*GetConsole)(ISession *session, IConsole **console); nsresult (*GetMachine)(ISession *session, IMachine **machine); nsresult (*Close)(ISession *session); @@ -408,17 +408,17 @@ typedef struct { typedef struct { nsresult (*GetEnabled)(IVRDxServer *VRDxServer, PRBool *enabled); nsresult (*SetEnabled)(IVRDxServer *VRDxServer, PRBool enabled); - nsresult (*GetPorts)(vboxGlobalData *data, IVRDxServer *VRDxServer, + nsresult (*GetPorts)(vboxPrivate *data, IVRDxServer *VRDxServer, virDomainGraphicsDefPtr graphics); - nsresult (*SetPorts)(vboxGlobalData *data, IVRDxServer *VRDxServer, + nsresult (*SetPorts)(vboxPrivate *data, IVRDxServer *VRDxServer, virDomainGraphicsDefPtr graphics); nsresult (*GetReuseSingleConnection)(IVRDxServer *VRDxServer, PRBool *enabled); nsresult (*SetReuseSingleConnection)(IVRDxServer *VRDxServer, PRBool enabled); nsresult (*GetAllowMultiConnection)(IVRDxServer *VRDxServer, PRBool *enabled); nsresult (*SetAllowMultiConnection)(IVRDxServer *VRDxServer, PRBool enabled); - nsresult (*GetNetAddress)(vboxGlobalData *data, IVRDxServer *VRDxServer, + nsresult (*GetNetAddress)(vboxPrivate *data, IVRDxServer *VRDxServer, PRUnichar **netAddress); - nsresult (*SetNetAddress)(vboxGlobalData *data, IVRDxServer *VRDxServer, + nsresult (*SetNetAddress)(vboxPrivate *data, IVRDxServer *VRDxServer, PRUnichar *netAddress); } vboxUniformedIVRDxServer; @@ -516,7 +516,7 @@ typedef struct { IHostNetworkInterface **networkInterface); nsresult (*FindHostNetworkInterfaceByName)(IHost *host, PRUnichar *name, IHostNetworkInterface **networkInterface); - nsresult (*CreateHostOnlyNetworkInterface)(vboxGlobalData *data, + nsresult (*CreateHostOnlyNetworkInterface)(vboxPrivate *data, IHost *host, char *name, IHostNetworkInterface **networkInterface); nsresult (*RemoveHostOnlyNetworkInterface)(IHost *host, vboxIIDUnion *iidu, @@ -584,19 +584,19 @@ typedef struct { uint32_t APIVersion; uint32_t XPCOMCVersion; /* vbox APIs */ - int (*initializeDomainEvent)(vboxGlobalData *data); - void (*registerGlobalData)(vboxGlobalData *data); - void (*detachDevices)(vboxGlobalData *data, IMachine *machine, PRUnichar *hddcnameUtf16); - nsresult (*unregisterMachine)(vboxGlobalData *data, vboxIIDUnion *iidu, IMachine **machine); + int (*initializeDomainEvent)(vboxPrivate *data); + void (*registerGlobalData)(vboxPrivate *data); + void (*detachDevices)(vboxPrivate *data, IMachine *machine, PRUnichar *hddcnameUtf16); + nsresult (*unregisterMachine)(vboxPrivate *data, vboxIIDUnion *iidu, IMachine **machine); void (*deleteConfig)(IMachine *machine); - void (*vboxAttachDrivesOld)(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine); + void (*vboxAttachDrivesOld)(virDomainDefPtr def, vboxPrivate *data, IMachine *machine); virDomainState (*vboxConvertState)(PRUint32 state); - void (*dumpIDEHDDsOld)(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine); - void (*dumpDVD)(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine); - int (*attachDVD)(vboxGlobalData *data, IMachine *machine, const char *src); + void (*dumpIDEHDDsOld)(virDomainDefPtr def, vboxPrivate *data, IMachine *machine); + void (*dumpDVD)(virDomainDefPtr def, vboxPrivate *data, IMachine *machine); + int (*attachDVD)(vboxPrivate *data, IMachine *machine, const char *src); int (*detachDVD)(IMachine *machine); - void (*dumpFloppy)(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine); - int (*attachFloppy)(vboxGlobalData *data, IMachine *machine, const char *src); + void (*dumpFloppy)(virDomainDefPtr def, vboxPrivate *data, IMachine *machine); + int (*attachFloppy)(vboxPrivate *data, IMachine *machine, const char *src); int (*detachFloppy)(IMachine *machine); int (*snapshotRestore)(virDomainPtr dom, IMachine *machine, ISnapshot *snapshot); void (*registerDomainEvent)(virHypervisorDriverPtr driver); -- 2.7.4
On Wed, Sep 28, 2016 at 01:41:34PM -0400, Dawid Zamirski wrote:
Since those stucts are identical at the moment, this patch basically does s/vboxGlobalData \*data/vboxPrivate *data/ with type casts in vboxDriverLock/Unlock calls to keep the code working and take care of unavoidable diff noise to set the stage for further commits that actually address the issue for the patch series. --- src/vbox/vbox_common.c | 164 +++++++++++++++++++++--------------------- src/vbox/vbox_network.c | 24 +++---- src/vbox/vbox_storage.c | 20 +++--- src/vbox/vbox_tmpl.c | 146 ++++++++++++++++++------------------- src/vbox/vbox_uniformed_api.h | 54 +++++++------- 5 files changed, 204 insertions(+), 204 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 23f63f9..13869eb 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -206,12 +206,12 @@ static vboxGlobalData *g_pVBoxGlobalData;
So the g_pVBoxGlobalData is still pointer to vboxGlobalData, but ...
@@ -1312,7 +1312,7 @@ _vboxDomainSnapshotRestore(virDomainPtr dom, IMachine *machine, ISnapshot *snapshot) { - vboxGlobalData *data = dom->conn->privateData; + vboxPrivate *data = dom->conn->privateData;
in all cases you change here, like this...
# if VBOX_API_VERSION < 5000000 IConsole *console = NULL; # endif /*VBOX_API_VERSION < 5000000*/ @@ -1422,7 +1422,7 @@ vboxCallbackOnMachineStateChange(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, int event = 0; int detail = 0;
- vboxDriverLock(g_pVBoxGlobalData); + vboxDriverLock((vboxPrivate *) g_pVBoxGlobalData);
and this (and others) you *just* cast it to different struct. That's not good. I'm guessing this is still just a differently separated commit.
Since VBOX API initialization method (pfnComInitialize) is not thread-safe and must be called from the primary thread, calling it in every vboxConnectOpen (as we used to do) leads to segmentation faults when multiple connections are in use. Therefore the initalization and unitialization logic has been changed as the following: * _registerGlobalData allocates g_pVBoxGlobalData (only when not already allocated) and calls VBOX's pfnComInitialize to grab references to ISession and IVirtualBox objects. This ensures that pfnComInitialize is called when the first connection is established. * _pfnInitialize sets up the virConnectPtr->privateData (aka vboxPrivateData) for each connection by copying references to ISession and IVirtualBox from g_pVBoxGlobalData so that the rest of the driver API can use it without referencing the global. Each time this happens, a conntionCount is incremented on g_pVBoxGlobalData to keep track of when it's safe to call pfnComUnitialize. One of the reasons for existence of per-connection vboxPrivateData rather than completely relying on vboxGlobalData, is that more modern VBOX APIs provide pfnClientInitialize (since 4.2.20 and 4.3.4) and pfnClientThreadInitialize (5.0+) that allow to create multiple instances of ISession so if the code ever gets ported to support those newer APIs it should create much less diff noise as all API implementations are already updated to assume per-connection ISession/IVirutalBox instances. * _pfnUnitilalize decrements connectionCount in g_pVBoxGlobalData and once it is down to 0, it calls pfnComUnitialize and g_pVBoxGlobalData if free'd. This ensures that pfnComInitialize and pfnComUnitialize pair is called only once, even when multiple concurrent connections are in use. --- src/vbox/vbox_common.c | 32 ++++-------- src/vbox/vbox_tmpl.c | 113 ++++++++++++++++++++++++++++-------------- src/vbox/vbox_uniformed_api.h | 57 +++++++-------------- 3 files changed, 106 insertions(+), 96 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index be6ff2d..1728275 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -294,18 +294,6 @@ static int vboxInitialize(vboxPrivate *data) if (gVBoxAPI.domainEventCallbacks && gVBoxAPI.initializeDomainEvent(data) != 0) goto cleanup; - if (data->vboxObj == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("IVirtualBox object is null")); - goto cleanup; - } - - if (data->vboxSession == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("ISession object is null")); - goto cleanup; - } - return 0; cleanup: @@ -382,12 +370,12 @@ static void vboxUninitialize(vboxPrivate *data) if (!data) return; - gVBoxAPI.UPFN.Uninitialize(data); - - virObjectUnref(data->caps); virObjectUnref(data->xmlopt); if (gVBoxAPI.domainEventCallbacks) virObjectEventStateFree(data->domainEvents); + + gVBoxAPI.UPFN.Uninitialize(data); + VIR_FREE(data); } @@ -398,6 +386,8 @@ vboxConnectOpen(virConnectPtr conn, unsigned int flags) { vboxPrivate *data = NULL; + vboxGlobalData *globalData = NULL; + uid_t uid = geteuid(); virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -438,8 +428,12 @@ vboxConnectOpen(virConnectPtr conn, if (VIR_ALLOC(data) < 0) return VIR_DRV_OPEN_ERROR; - if (!(data->caps = vboxCapsInit()) || - vboxInitialize(data) < 0 || + globalData = gVBoxAPI.registerGlobalData(); + + if (!globalData || !(globalData->caps = vboxCapsInit())) + return VIR_DRV_OPEN_ERROR; + + if (vboxInitialize(data) < 0 || vboxExtractVersion(data) < 0 || !(data->xmlopt = vboxXMLConfInit())) { vboxUninitialize(data); @@ -451,12 +445,8 @@ vboxConnectOpen(virConnectPtr conn, vboxUninitialize(data); return VIR_DRV_OPEN_ERROR; } - - data->conn = conn; } - if (gVBoxAPI.hasStaticGlobalData) - gVBoxAPI.registerGlobalData(data); conn->privateData = data; VIR_DEBUG("in vboxOpen"); diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 13869eb..37dcd3e 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -102,6 +102,8 @@ typedef IMedium IHardDisk; VIR_LOG_INIT("vbox.vbox_tmpl"); +static vboxGlobalData *g_pVBoxGlobalData; + #define vboxUnsupported() \ VIR_WARN("No %s in current vbox version %d.", __FUNCTION__, VBOX_API_VERSION); @@ -167,22 +169,6 @@ if (strUtf16) {\ (unsigned)(iid)->m3[7]);\ }\ -#if VBOX_API_VERSION > 2002000 - -/* g_pVBoxGlobalData has to be global variable, - * there is no other way to make the callbacks - * work other then having g_pVBoxGlobalData as - * global, because the functions namely AddRef, - * Release, etc consider it as global and you - * can't change the function definition as it - * is XPCOM nsISupport::* function and it expects - * them that way - */ - -static vboxGlobalData *g_pVBoxGlobalData; - -#endif /* !(VBOX_API_VERSION == 2002000) */ - #if VBOX_API_VERSION < 4000000 # define VBOX_SESSION_OPEN(/* in */ iid_value, /* unused */ machine) \ @@ -1976,19 +1962,6 @@ _registerDomainEvent(virHypervisorDriverPtr driver) #endif /* !(VBOX_API_VERSION == 2002000 || VBOX_API_VERSION >= 4000000) */ -static int _pfnInitialize(vboxPrivate *data) -{ - data->pFuncs = g_pfnGetFunctions(VBOX_XPCOMC_VERSION); - if (data->pFuncs == NULL) - return -1; -#if VBOX_XPCOMC_VERSION == 0x00010000U - data->pFuncs->pfnComInitialize(&data->vboxObj, &data->vboxSession); -#else /* !(VBOX_XPCOMC_VERSION == 0x00010000U) */ - data->pFuncs->pfnComInitialize(IVIRTUALBOX_IID_STR, &data->vboxObj, ISESSION_IID_STR, &data->vboxSession); -#endif /* !(VBOX_XPCOMC_VERSION == 0x00010000U) */ - return 0; -} - static int _initializeDomainEvent(vboxPrivate *data ATTRIBUTE_UNUSED) { @@ -2009,13 +1982,38 @@ _initializeDomainEvent(vboxPrivate *data ATTRIBUTE_UNUSED) } static -void _registerGlobalData(vboxPrivate *data ATTRIBUTE_UNUSED) +vboxGlobalData* _registerGlobalData(void) { -#if VBOX_API_VERSION == 2002000 - vboxUnsupported(); -#else /* VBOX_API_VERSION != 2002000 */ - g_pVBoxGlobalData = (vboxGlobalData *) data; -#endif /* VBOX_API_VERSION != 2002000 */ + if (g_pVBoxGlobalData == NULL && VIR_ALLOC(g_pVBoxGlobalData) == 0) { + g_pVBoxGlobalData->pFuncs = g_pfnGetFunctions(VBOX_XPCOMC_VERSION); + if (g_pVBoxGlobalData->pFuncs == NULL) + return NULL; + +#if VBOX_XPCOMC_VERSION == 0x00010000U + g_pVBoxGlobalData->pFuncs->pfnComInitialize(&g_pVBoxGlobalData->vboxObj, + &g_pVBoxGlobalData->vboxSession); +#else /* !(VBOX_XPCOMC_VERSION == 0x00010000U) */ + g_pVBoxGlobalData->pFuncs->pfnComInitialize(IVIRTUALBOX_IID_STR, + &g_pVBoxGlobalData->vboxObj, + ISESSION_IID_STR, + &g_pVBoxGlobalData->vboxSession); +#endif /* !(VBOX_XPCOMC_VERSION == 0x00010000U) */ + } + + + if (g_pVBoxGlobalData->vboxObj == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("IVirtualBox object is null")); + return NULL; + } + + if (g_pVBoxGlobalData->vboxSession == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("ISession object is null")); + return NULL; + } + + return g_pVBoxGlobalData; } #if VBOX_API_VERSION < 4000000 @@ -2629,10 +2627,51 @@ _detachFloppy(IMachine *machine ATTRIBUTE_UNUSED) #endif /* VBOX_API_VERSION >= 3001000 */ +static int _pfnInitialize(vboxPrivate *data) +{ + int ret = -1; + + virMutexLock(&g_pVBoxGlobalData->lock); + + if (g_pVBoxGlobalData->vboxObj && g_pVBoxGlobalData->vboxSession) { + data->vboxObj = g_pVBoxGlobalData->vboxObj; + data->vboxSession = g_pVBoxGlobalData->vboxSession; + + g_pVBoxGlobalData->vboxObj->vtbl->nsisupports.AddRef((nsISupports *) g_pVBoxGlobalData->vboxObj); + g_pVBoxGlobalData->vboxSession->vtbl->nsisupports.AddRef((nsISupports *) g_pVBoxGlobalData->vboxSession); + + data->caps = virObjectRef(g_pVBoxGlobalData->caps); + + g_pVBoxGlobalData->connectionCount++; + + ret = 0; + } + + virMutexUnlock(&g_pVBoxGlobalData->lock); + + return ret; +} + static void _pfnUninitialize(vboxPrivate *data) { - if (data->pFuncs) - data->pFuncs->pfnComUninitialize(); + if (!data || !g_pVBoxGlobalData) + return; + + virMutexLock(&g_pVBoxGlobalData->lock); + + g_pVBoxGlobalData->connectionCount--; + + data->vboxObj->vtbl->nsisupports.Release((nsISupports *) data->vboxObj); + data->vboxSession->vtbl->nsisupports.Release((nsISupports *) data->vboxSession); + virObjectUnref(data->caps); + + if (g_pVBoxGlobalData->connectionCount == 0) { + g_pVBoxGlobalData->pFuncs->pfnComUninitialize(); + virMutexUnlock(&g_pVBoxGlobalData->lock); + VIR_FREE(g_pVBoxGlobalData); + } else { + virMutexUnlock(&g_pVBoxGlobalData->lock); + } } static void _pfnComUnallocMem(PCVBOXXPCOM pFuncs, void *pv) diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h index 50041c0..dac44e6 100644 --- a/src/vbox/vbox_uniformed_api.h +++ b/src/vbox/vbox_uniformed_api.h @@ -96,24 +96,28 @@ typedef union { PRInt32 resultCode; } resultCodeUnion; +/* "extends" IVirtualBoxCallback to provide members for context info */ +typedef struct { + struct IVirtualBoxCallback_vtbl *vtbl; + virConnectPtr conn; + int vboxCallBackRefCount; +} VirtualBoxCallback; + typedef struct { virMutex lock; unsigned long version; - virCapsPtr caps; virDomainXMLOptionPtr xmlopt; + /* references to members of g_pVBoxGlobalData */ + virCapsPtr caps; IVirtualBox *vboxObj; ISession *vboxSession; - /** Our version specific API table pointer. */ - PCVBOXXPCOM pFuncs; - /* The next is used for domainEvent */ /* Async event handling */ virObjectEventStatePtr domainEvents; int fdWatch; - int volatile vboxCallBackRefCount; # if defined(VBOX_API_VERSION) && VBOX_API_VERSION > 2002000 && VBOX_API_VERSION < 4000000 IVirtualBoxCallback *vboxCallback; nsIEventQueue *vboxQueue; @@ -121,49 +125,26 @@ typedef struct { void *vboxCallback; void *vboxQueue; # endif /* VBOX_API_VERSION <= 2002000 || VBOX_API_VERSION >= 4000000 || VBOX_API_VERSION undefined */ - - /* pointer back to the connection */ - virConnectPtr conn; } vboxPrivate; typedef struct { virMutex lock; - unsigned long version; - virCapsPtr caps; - virDomainXMLOptionPtr xmlopt; - - IVirtualBox *vboxObj; - ISession *vboxSession; /** Our version specific API table pointer. */ PCVBOXXPCOM pFuncs; - /* The next is used for domainEvent */ -# if defined(VBOX_API_VERSION) && VBOX_API_VERSION > 2002000 && VBOX_API_VERSION < 4000000 - - /* Async event handling */ - virObjectEventStatePtr domainEvents; - int fdWatch; - IVirtualBoxCallback *vboxCallback; - nsIEventQueue *vboxQueue; - - int volatile vboxCallBackRefCount; - - /* pointer back to the connection */ - virConnectPtr conn; - -# else /* VBOX_API_VERSION <= 2002000 || VBOX_API_VERSION >= 4000000 || VBOX_API_VERSION undefined */ - - virObjectEventStatePtr domainEvents; - int fdWatch; - void *vboxCallback; - void *vboxQueue; - int volatile vboxCallBackRefCount; - virConnectPtr conn; - -# endif /* VBOX_API_VERSION <= 2002000 || VBOX_API_VERSION >= 4000000 || VBOX_API_VERSION undefined */ + /* vbox objects shared for all connections. + * + * The pfnComInitialize does not support multiple sessions + * per process, therefore IVirutalBox and ISession must + * be global. + */ + IVirtualBox *vboxObj; + ISession *vboxSession; + /* keeps track of vbox connection count */ + int volatile connectionCount; } vboxGlobalData; /* vboxUniformedAPI gives vbox_common.c a uniformed layer to see -- 2.7.4
On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote:
Since VBOX API initialization method (pfnComInitialize) is not thread-safe and must be called from the primary thread, calling it in every vboxConnectOpen (as we used to do) leads to segmentation faults when multiple connections are in use. Therefore the initalization and unitialization logic has been changed as the following:
* _registerGlobalData allocates g_pVBoxGlobalData (only when not already allocated) and calls VBOX's pfnComInitialize to grab references to ISession and IVirtualBox objects. This ensures that pfnComInitialize is called when the first connection is established.
The allocation is not guarded by any lock, so there's still a race. I think there should be a global struct that has only some lock in it and whatever global data you need, the struct will be initialized on the first call to any function (check out VIR_ONCE_GLOBAL_INIT) and then the connection (or global data or how it's called) would be reference counted (just like you have). It's just that having the reference count in the object you will be reallocating over and over again for each connection is not really good. I don't understand how vbox works, but if initializing g_pVBoxGlobalData does not make any connection, and ISession does (which would make sense), we could keep the global data around and add ISession for each connection. I guess that's something you're talking about below.
* _pfnInitialize sets up the virConnectPtr->privateData (aka vboxPrivateData) for each connection by copying references to ISession and IVirtualBox from g_pVBoxGlobalData so that the rest of the driver API can use it without referencing the global. Each time this happens, a conntionCount is incremented on g_pVBoxGlobalData to keep track of when it's safe to call pfnComUnitialize. One of the reasons for existence of per-connection vboxPrivateData rather than completely relying on vboxGlobalData, is that more modern VBOX APIs provide pfnClientInitialize (since 4.2.20 and 4.3.4) and pfnClientThreadInitialize (5.0+) that allow to create multiple instances of ISession so if the code ever gets ported to support those newer APIs it should create much less diff noise as all API implementations are already updated to assume per-connection ISession/IVirutalBox instances.
* _pfnUnitilalize decrements connectionCount in g_pVBoxGlobalData and once it is down to 0, it calls pfnComUnitialize and g_pVBoxGlobalData if free'd. This ensures that pfnComInitialize and pfnComUnitialize pair is called only once, even when multiple concurrent connections are in use.
That's not true if there is a connection being made while it is being free()d or it's being allocated in two threads, etc. Unless I missed something, that is.
On Tue, 2016-10-11 at 16:22 +0200, Martin Kletzander wrote:
On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote:
The allocation is not guarded by any lock, so there's still a race. I think there should be a global struct that has only some lock in it and whatever global data you need, the struct will be initialized on the first call to any function (check out VIR_ONCE_GLOBAL_INIT) and then the connection (or global data or how it's called) would be reference counted (just like you have). It's just that having the reference count in the object you will be reallocating over and over again for each connection is not really good.
Thanks, I see, I'll address this in v2
I don't understand how vbox works, but if initializing g_pVBoxGlobalData does not make any connection, and ISession does (which would make sense), we could keep the global data around and add ISession for each connection. I guess that's something you're talking about below.
Neither I'm super familiar with vbox internals, but your suggestion sounds reasonable, so I'll dive into that code in vbox source to find out.
* _pfnInitialize sets up the virConnectPtr->privateData (aka vboxPrivateData) for each connection by copying references to ISession and IVirtualBox from g_pVBoxGlobalData so that the rest of the driver API can use it without referencing the global. Each time this happens, a conntionCount is incremented on g_pVBoxGlobalData to keep track of when it's safe to call pfnComUnitialize. One of the reasons for existence of per-connection vboxPrivateData rather than completely relying on vboxGlobalData, is that more modern VBOX APIs provide pfnClientInitialize (since 4.2.20 and 4.3.4) and pfnClientThreadInitialize (5.0+) that allow to create multiple instances of ISession so if the code ever gets ported to support those newer APIs it should create much less diff noise as all API implementations are already updated to assume per-connection ISession/IVirutalBox instances.
* _pfnUnitilalize decrements connectionCount in g_pVBoxGlobalData and once it is down to 0, it calls pfnComUnitialize and g_pVBoxGlobalData if free'd. This ensures that pfnComInitialize and pfnComUnitialize pair is called only once, even when multiple concurrent connections are in use.
That's not true if there is a connection being made while it is being free()d or it's being allocated in two threads, etc. Unless I missed something, that is.
Understood, though I figure that your initial suggestion to guard allocation of g_pVBoxGlobalData should take care of this?
On Tue, Oct 11, 2016 at 10:58:47AM -0400, Dawid Zamirski wrote:
On Tue, 2016-10-11 at 16:22 +0200, Martin Kletzander wrote:
On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote:
The allocation is not guarded by any lock, so there's still a race. I think there should be a global struct that has only some lock in it and whatever global data you need, the struct will be initialized on the first call to any function (check out VIR_ONCE_GLOBAL_INIT) and then the connection (or global data or how it's called) would be reference counted (just like you have). It's just that having the reference count in the object you will be reallocating over and over again for each connection is not really good.
Thanks, I see, I'll address this in v2
I don't understand how vbox works, but if initializing g_pVBoxGlobalData does not make any connection, and ISession does (which would make sense), we could keep the global data around and add ISession for each connection. I guess that's something you're talking about below.
Neither I'm super familiar with vbox internals, but your suggestion sounds reasonable, so I'll dive into that code in vbox source to find out.
* _pfnInitialize sets up the virConnectPtr->privateData (aka vboxPrivateData) for each connection by copying references to ISession and IVirtualBox from g_pVBoxGlobalData so that the rest of the driver API can use it without referencing the global. Each time this happens, a conntionCount is incremented on g_pVBoxGlobalData to keep track of when it's safe to call pfnComUnitialize. One of the reasons for existence of per-connection vboxPrivateData rather than completely relying on vboxGlobalData, is that more modern VBOX APIs provide pfnClientInitialize (since 4.2.20 and 4.3.4) and pfnClientThreadInitialize (5.0+) that allow to create multiple instances of ISession so if the code ever gets ported to support those newer APIs it should create much less diff noise as all API implementations are already updated to assume per-connection ISession/IVirutalBox instances.
* _pfnUnitilalize decrements connectionCount in g_pVBoxGlobalData and once it is down to 0, it calls pfnComUnitialize and g_pVBoxGlobalData if free'd. This ensures that pfnComInitialize and pfnComUnitialize pair is called only once, even when multiple concurrent connections are in use.
That's not true if there is a connection being made while it is being free()d or it's being allocated in two threads, etc. Unless I missed something, that is.
Understood, though I figure that your initial suggestion to guard allocation of g_pVBoxGlobalData should take care of this?
Yes, that would do.
On Tue, 2016-10-11 at 10:58 -0400, Dawid Zamirski wrote:
On Tue, 2016-10-11 at 16:22 +0200, Martin Kletzander wrote:
On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote:
I don't understand how vbox works, but if initializing g_pVBoxGlobalData does not make any connection, and ISession does (which would make sense), we could keep the global data around and add ISession for each connection. I guess that's something you're talking about below.
Neither I'm super familiar with vbox internals, but your suggestion sounds reasonable, so I'll dive into that code in vbox source to find out.
So after a quick glance at vbox initalization code, it seems that both IVirtualBox and ISession are reference counted global instances (g_VirtualBox and g_Session [1]) and both are allocated and released in pfnComInitialize [2] and pfnComUninitialize [3] implementation calls. As such I won't be able to separate out ISession instances to be per- connection while keeping global IVirtualBox reference. [1] https://github.com/mdaniel/virtualbox-org-svn-vbox-trunk/blob/maste r/src/VBox/Main/cbinding/VBoxCAPI.cpp#L46-L49 [2] https://github.com/mdaniel/virtualbox-org-svn-vbox-trunk/blob/maste r/src/VBox/Main/cbinding/VBoxCAPI.cpp#L480-L481 [3] https://github.com/mdaniel/virtualbox-org-svn-vbox-trunk/blob/maste r/src/VBox/Main/cbinding/VBoxCAPI.cpp#L493-L502
On Tue, Oct 11, 2016 at 03:44:45PM -0400, Dawid Zamirski wrote:
On Tue, 2016-10-11 at 10:58 -0400, Dawid Zamirski wrote:
On Tue, 2016-10-11 at 16:22 +0200, Martin Kletzander wrote:
On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote:
I don't understand how vbox works, but if initializing g_pVBoxGlobalData does not make any connection, and ISession does (which would make sense), we could keep the global data around and add ISession for each connection. I guess that's something you're talking about below.
Neither I'm super familiar with vbox internals, but your suggestion sounds reasonable, so I'll dive into that code in vbox source to find out.
So after a quick glance at vbox initalization code, it seems that both IVirtualBox and ISession are reference counted global instances (g_VirtualBox and g_Session [1]) and both are allocated and released in pfnComInitialize [2] and pfnComUninitialize [3] implementation calls. As such I won't be able to separate out ISession instances to be per- connection while keeping global IVirtualBox reference.
I don't see any reference counting from their side. Anyway, it looks like we cannot separate them, but we can get rid of them when we stop supporting the old version that has only the legacy initialization support. It's super-weird that they have global variable AND they still need to get that passed. It's like a combination of wrong and non-reentrant design. Anyway, they call it legacy, so I guess we should move to the new one if possible. But I think you already mentioned that, I'm just not sure.
[1] https://github.com/mdaniel/virtualbox-org-svn-vbox-trunk/blob/maste r/src/VBox/Main/cbinding/VBoxCAPI.cpp#L46-L49 [2] https://github.com/mdaniel/virtualbox-org-svn-vbox-trunk/blob/maste r/src/VBox/Main/cbinding/VBoxCAPI.cpp#L480-L481 [3] https://github.com/mdaniel/virtualbox-org-svn-vbox-trunk/blob/maste r/src/VBox/Main/cbinding/VBoxCAPI.cpp#L493-L502
On Tue, 2016-10-11 at 10:58 -0400, Dawid Zamirski wrote:
On Tue, 2016-10-11 at 16:22 +0200, Martin Kletzander wrote:
On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote:
The allocation is not guarded by any lock, so there's still a race. I think there should be a global struct that has only some lock in it and whatever global data you need, the struct will be initialized on the first call to any function (check out VIR_ONCE_GLOBAL_INIT) and then the connection (or global data or how it's called) would be reference counted (just like you have). It's just that having the reference count in the object you will be reallocating over and over again for each connection is not really good.
Thanks, I see, I'll address this in v2
So after my initial attempts at handling this as suggested (create vboxDriver struct and initialize in VIR_ONCE_GLOBAL_INIT), I've stumbled upon virStateDriver structs used in other drivers that apparently has .stateInitialize, .stateCleanup that to me look like perfect places to call vbox's pfnComInitialize pfnComUnintialize pairs. Would this be a good idea to utilize the virStateDriver for this or should I stick with VIR_ONCE_GLOBAL_INIT way? PS. Thanks a lot for reviewing my code and guiding me through it all.
On Wed, Oct 12, 2016 at 04:00:52PM -0400, Dawid Zamirski wrote:
On Tue, 2016-10-11 at 10:58 -0400, Dawid Zamirski wrote:
On Tue, 2016-10-11 at 16:22 +0200, Martin Kletzander wrote:
On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote:
The allocation is not guarded by any lock, so there's still a race. I think there should be a global struct that has only some lock in it and whatever global data you need, the struct will be initialized on the first call to any function (check out VIR_ONCE_GLOBAL_INIT) and then the connection (or global data or how it's called) would be reference counted (just like you have). It's just that having the reference count in the object you will be reallocating over and over again for each connection is not really good.
Thanks, I see, I'll address this in v2
So after my initial attempts at handling this as suggested (create vboxDriver struct and initialize in VIR_ONCE_GLOBAL_INIT), I've stumbled upon virStateDriver structs used in other drivers that apparently has .stateInitialize, .stateCleanup that to me look like perfect places to call vbox's pfnComInitialize pfnComUnintialize pairs. Would this be a good idea to utilize the virStateDriver for this or should I stick with VIR_ONCE_GLOBAL_INIT way?
I just now realized you said the issue we are talking about crashes the libvirt daemon, right? And it does not use virStateDriver? That's weird. You are connecting to vbox:///session, but the crash happens in the libvirt daemon. That means we switched the vbox driver to the daemo-side driver, so I'd expect it to be stateful driver. However it doesn't use virStateDriver. I'm Cc'ing Michal to let him look at it as he's more handy in the whole vbox area.
PS. Thanks a lot for reviewing my code and guiding me through it all.
On Thu, 2016-10-13 at 12:04 +0200, Martin Kletzander wrote:
On Wed, Oct 12, 2016 at 04:00:52PM -0400, Dawid Zamirski wrote:
So after my initial attempts at handling this as suggested (create vboxDriver struct and initialize in VIR_ONCE_GLOBAL_INIT), I've stumbled upon virStateDriver structs used in other drivers that apparently has .stateInitialize, .stateCleanup that to me look like perfect places to call vbox's pfnComInitialize pfnComUnintialize pairs. Would this be a good idea to utilize the virStateDriver for this or should I stick with VIR_ONCE_GLOBAL_INIT way?
I just now realized you said the issue we are talking about crashes the libvirt daemon, right? And it does not use virStateDriver? That's weird. You are connecting to vbox:///session, but the crash happens in the libvirt daemon. That means we switched the vbox driver to the daemo-side driver, so I'd expect it to be stateful driver. However it doesn't use virStateDriver. I'm Cc'ing Michal to let him look at it as he's more handy in the whole vbox area.
PS. Thanks a lot for reviewing my code and guiding me through it all.
Yes, it's crashing libvirtd daemon, however the local one that is started on behalf of vbox:///session owned by the user that started it (not the system-wide one running as root). One thing I realized is that putting pfnComInitialize into the .stateInitialize will likely make VBoxSVC start together with libvirtd which IMO is not desired. I might still utilize the state driver to at least initialize vboxDriver object and obtain pFuncs reference and then defer pfnComInitialize to the first connection that comes in, similarly as my original patch does. That way, I might be able to make vbox driver follow the initialization pattern used by other drivers and hopefully get rid of the g_pVBoxGlobalData.
On 13.10.2016 23:33, Dawid Zamirski wrote:
On Thu, 2016-10-13 at 12:04 +0200, Martin Kletzander wrote:
On Wed, Oct 12, 2016 at 04:00:52PM -0400, Dawid Zamirski wrote:
So after my initial attempts at handling this as suggested (create vboxDriver struct and initialize in VIR_ONCE_GLOBAL_INIT), I've stumbled upon virStateDriver structs used in other drivers that apparently has .stateInitialize, .stateCleanup that to me look like perfect places to call vbox's pfnComInitialize pfnComUnintialize pairs. Would this be a good idea to utilize the virStateDriver for this or should I stick with VIR_ONCE_GLOBAL_INIT way?
I just now realized you said the issue we are talking about crashes the libvirt daemon, right? And it does not use virStateDriver? That's weird. You are connecting to vbox:///session, but the crash happens in the libvirt daemon. That means we switched the vbox driver to the daemo-side driver, so I'd expect it to be stateful driver. However it doesn't use virStateDriver. I'm Cc'ing Michal to let him look at it as he's more handy in the whole vbox area.
PS. Thanks a lot for reviewing my code and guiding me through it all.
Yes, it's crashing libvirtd daemon, however the local one that is started on behalf of vbox:///session owned by the user that started it (not the system-wide one running as root). One thing I realized is that putting pfnComInitialize into the .stateInitialize will likely make VBoxSVC start together with libvirtd which IMO is not desired. I might still utilize the state driver to at least initialize vboxDriver object and obtain pFuncs reference and then defer pfnComInitialize to the first connection that comes in, similarly as my original patch does. That way, I might be able to make vbox driver follow the initialization pattern used by other drivers and hopefully get rid of the g_pVBoxGlobalData.
Yes, this sounds reasonable. We certainly do not want to have a permanent 'connection' to vboxsvc from the time daemon starts. It would be pure waste of resources. Michal
On 13.10.2016 18:04, Martin Kletzander wrote:
On Wed, Oct 12, 2016 at 04:00:52PM -0400, Dawid Zamirski wrote:
On Tue, 2016-10-11 at 10:58 -0400, Dawid Zamirski wrote:
On Tue, 2016-10-11 at 16:22 +0200, Martin Kletzander wrote:
On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote:
The allocation is not guarded by any lock, so there's still a race. I think there should be a global struct that has only some lock in it and whatever global data you need, the struct will be initialized on the first call to any function (check out VIR_ONCE_GLOBAL_INIT) and then the connection (or global data or how it's called) would be reference counted (just like you have). It's just that having the reference count in the object you will be reallocating over and over again for each connection is not really good.
Thanks, I see, I'll address this in v2
So after my initial attempts at handling this as suggested (create vboxDriver struct and initialize in VIR_ONCE_GLOBAL_INIT), I've stumbled upon virStateDriver structs used in other drivers that apparently has .stateInitialize, .stateCleanup that to me look like perfect places to call vbox's pfnComInitialize pfnComUnintialize pairs. Would this be a good idea to utilize the virStateDriver for this or should I stick with VIR_ONCE_GLOBAL_INIT way?
I just now realized you said the issue we are talking about crashes the libvirt daemon, right? And it does not use virStateDriver? That's weird. You are connecting to vbox:///session, but the crash happens in the libvirt daemon. That means we switched the vbox driver to the daemo-side driver, so I'd expect it to be stateful driver.
Yes, couple of drivers were switched to server side drivers because of some licensing nonsense.
However it doesn't use virStateDriver.
That's because vbox is stateless driver. Just like esx for instance. Michal
On Tue, 2016-10-11 at 16:22 +0200, Martin Kletzander wrote:
On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote:
* _pfnUnitilalize decrements connectionCount in g_pVBoxGlobalData and once it is down to 0, it calls pfnComUnitialize and g_pVBoxGlobalData if free'd. This ensures that pfnComInitialize and pfnComUnitialize pair is called only once, even when multiple concurrent connections are in use.
That's not true if there is a connection being made while it is being free()d or it's being allocated in two threads, etc. Unless I missed something, that is.
On a second thought, seeing how both _pfnInitialize and _pfnUnintialize (each called in virConnectOpen/Close respectively) obtain a mutex on g_pVBoxGlobalData, I think my original statement holds true, that is, ignoring the unsafe allocation of the global as you pointed out earlier.
On Tue, Oct 11, 2016 at 11:23:13AM -0400, Dawid Zamirski wrote:
On Tue, 2016-10-11 at 16:22 +0200, Martin Kletzander wrote:
On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote:
* _pfnUnitilalize decrements connectionCount in g_pVBoxGlobalData and once it is down to 0, it calls pfnComUnitialize and g_pVBoxGlobalData if free'd. This ensures that pfnComInitialize and pfnComUnitialize pair is called only once, even when multiple concurrent connections are in use.
That's not true if there is a connection being made while it is being free()d or it's being allocated in two threads, etc. Unless I missed something, that is.
On a second thought, seeing how both _pfnInitialize and _pfnUnintialize (each called in virConnectOpen/Close respectively) obtain a mutex on g_pVBoxGlobalData, I think my original statement holds true, that is, ignoring the unsafe allocation of the global as you pointed out earlier.
They can't take mutex on g_pVBoxGlobalData if it is not initialized (allocated) yet. Or is there another one?
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
This commit should be squashed to previous one. Keeping it separate for code-review purposes only as it's mostly noise due to changes of the struct definitions in previous patch: * vboxPrivate (aka old vboxGlobalData *data argument) no longer needs to be passed around as g_pVBoxGlobalData is the only one that has pFuncs reference. * for vbox 3.x, the even handling code was updated to use VirtuaBoxCallback struct (that is modeled after IVirtualBoxCallback) but includes additional members to carry contextual info so that g_pVBoxGlobalData does not have to be used in the callbacks. The additional members keep virConnectPtr reference and callback reference counter (taken out from old vboxGlobalData) P.S. VBox 3.x callback handling code updates were only compile-tested as VBOX 3 is no longer supported by upstream and does not work on any reasonably modern linux distro. --- src/vbox/vbox_common.c | 101 +++++++---------- src/vbox/vbox_common.h | 32 +++--- src/vbox/vbox_network.c | 31 +++-- src/vbox/vbox_tmpl.c | 258 ++++++++++++++++++------------------------ src/vbox/vbox_uniformed_api.h | 49 ++++---- 5 files changed, 206 insertions(+), 265 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 1728275..3c98a34 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -520,24 +520,12 @@ vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED) return ret; } -static void vboxDriverLock(vboxPrivate *data) -{ - virMutexLock(&data->lock); -} - -static void vboxDriverUnlock(vboxPrivate *data) -{ - virMutexUnlock(&data->lock); -} - static int vboxConnectGetVersion(virConnectPtr conn, unsigned long *version) { vboxPrivate *data = conn->privateData; VIR_DEBUG("%s: in vboxGetVersion", conn->driver->name); - vboxDriverLock(data); *version = data->version; - vboxDriverUnlock(data); return 0; } @@ -600,9 +588,7 @@ static char *vboxConnectGetCapabilities(virConnectPtr conn) if (!data->vboxObj) return ret; - vboxDriverLock(data); ret = virCapabilitiesFormatXML(data->caps); - vboxDriverUnlock(data); return ret; } @@ -1712,7 +1698,7 @@ vboxAttachDisplay(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) } static void -vboxAttachUSB(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) +vboxAttachUSB(virDomainDefPtr def, IMachine *machine) { IUSBCommon *USBCommon = NULL; size_t i = 0; @@ -1816,7 +1802,7 @@ vboxAttachUSB(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) } static void -vboxAttachSharedFolder(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) +vboxAttachSharedFolder(virDomainDefPtr def, IMachine *machine) { size_t i; PRUnichar *nameUtf16; @@ -1957,8 +1943,8 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags vboxAttachParallel(def, data, machine); vboxAttachVideo(def, machine); vboxAttachDisplay(def, data, machine); - vboxAttachUSB(def, data, machine); - vboxAttachSharedFolder(def, data, machine); + vboxAttachUSB(def, machine); + vboxAttachSharedFolder(def, machine); /* Save the machine settings made till now and close the * session. also free up the mchiid variable used. @@ -2020,7 +2006,7 @@ detachDevices_common(vboxPrivate *data, vboxIIDUnion *iidu) if (NS_SUCCEEDED(rc)) { rc = gVBoxAPI.UISession.GetMachine(data->vboxSession, &machine); if (NS_SUCCEEDED(rc) && machine) { - gVBoxAPI.detachDevices(data, machine, hddcnameUtf16); + gVBoxAPI.detachDevices(machine, hddcnameUtf16); gVBoxAPI.UIMachine.SaveSettings(machine); } gVBoxAPI.UISession.Close(data->vboxSession); @@ -2983,7 +2969,7 @@ static int vboxDomainGetMaxVcpus(virDomainPtr dom) } static void -vboxHostDeviceGetXMLDesc(vboxPrivate *data, virDomainDefPtr def, IMachine *machine) +vboxHostDeviceGetXMLDesc(virDomainDefPtr def, IMachine *machine) { IUSBCommon *USBCommon = NULL; PRBool enabled = PR_FALSE; @@ -3408,7 +3394,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) } static void -vboxDumpSharedFolders(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) +vboxDumpSharedFolders(virDomainDefPtr def, IMachine *machine) { /* shared folders */ vboxArray sharedFolders = VBOX_ARRAY_INITIALIZER; @@ -3469,7 +3455,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxPrivate *data, IMachine *machine) } static void -vboxDumpNetwork(virDomainDefPtr def, vboxPrivate *data, IMachine *machine, PRUint32 networkAdapterCount) +vboxDumpNetwork(virDomainDefPtr def, IMachine *machine, PRUint32 networkAdapterCount) { PRUint32 netAdpIncCnt = 0; size_t i = 0; @@ -3648,7 +3634,7 @@ vboxDumpAudio(virDomainDefPtr def, vboxPrivate *data ATTRIBUTE_UNUSED, } static void -vboxDumpSerial(virDomainDefPtr def, vboxPrivate *data, IMachine *machine, PRUint32 serialPortCount) +vboxDumpSerial(virDomainDefPtr def, IMachine *machine, PRUint32 serialPortCount) { PRUint32 serialPortIncCount = 0; size_t i = 0; @@ -3736,7 +3722,7 @@ vboxDumpSerial(virDomainDefPtr def, vboxPrivate *data, IMachine *machine, PRUint } static void -vboxDumpParallel(virDomainDefPtr def, vboxPrivate *data, IMachine *machine, PRUint32 parallelPortCount) +vboxDumpParallel(virDomainDefPtr def, IMachine *machine, PRUint32 parallelPortCount) { PRUint32 parallelPortIncCount = 0; size_t i = 0; @@ -3947,24 +3933,24 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) * into the common code. */ if (gVBoxAPI.oldMediumInterface) - gVBoxAPI.dumpIDEHDDsOld(def, data, machine); + gVBoxAPI.dumpIDEHDDsOld(def, machine); else vboxDumpIDEHDDsNew(def, data, machine); - vboxDumpSharedFolders(def, data, machine); - vboxDumpNetwork(def, data, machine, networkAdapterCount); + vboxDumpSharedFolders(def, machine); + vboxDumpNetwork(def, machine, networkAdapterCount); vboxDumpAudio(def, data, machine); if (gVBoxAPI.oldMediumInterface) { - gVBoxAPI.dumpDVD(def, data, machine); - gVBoxAPI.dumpFloppy(def, data, machine); + gVBoxAPI.dumpDVD(def, machine); + gVBoxAPI.dumpFloppy(def, machine); } - vboxDumpSerial(def, data, machine, serialPortCount); - vboxDumpParallel(def, data, machine, parallelPortCount); + vboxDumpSerial(def, machine, serialPortCount); + vboxDumpParallel(def, machine, parallelPortCount); /* dump USB devices/filters if active */ - vboxHostDeviceGetXMLDesc(data, def, machine); + vboxHostDeviceGetXMLDesc(def, machine); ret = virDomainDefFormat(def, data->caps, virDomainDefFormatConvertXMLFlags(flags)); @@ -4565,7 +4551,7 @@ vboxSnapshotRedefine(virDomainPtr dom, _("Unable to get the read write medium id")); goto cleanup; } - gVBoxAPI.UIID.vboxIIDToUtf8(data, &iid, &uuid); + gVBoxAPI.UIID.vboxIIDToUtf8(&iid, &uuid); vboxIIDUnalloc(&iid); rc = gVBoxAPI.UIMedium.GetFormat(readWriteMedium, &formatUtf); @@ -4673,7 +4659,7 @@ vboxSnapshotRedefine(virDomainPtr dom, _("Unable to get hard disk id")); goto cleanup; } - gVBoxAPI.UIID.vboxIIDToUtf8(data, &iid, &uuid); + gVBoxAPI.UIID.vboxIIDToUtf8(&iid, &uuid); vboxIIDUnalloc(&iid); rc = gVBoxAPI.UIMedium.GetFormat(readOnlyMedium, &formatUtf); @@ -4704,7 +4690,7 @@ vboxSnapshotRedefine(virDomainPtr dom, VIR_FREE(uuid); goto cleanup; } - gVBoxAPI.UIID.vboxIIDToUtf8(data, &parentiid, &parentUuid); + gVBoxAPI.UIID.vboxIIDToUtf8(&parentiid, &parentUuid); vboxIIDUnalloc(&parentiid); rc = gVBoxAPI.UIMedium.Close(readOnlyMedium); @@ -4988,7 +4974,7 @@ vboxSnapshotRedefine(virDomainPtr dom, VIR_FREE(disk); goto cleanup; } - gVBoxAPI.UIID.vboxIIDToUtf8(data, &iid, &uuid); + gVBoxAPI.UIID.vboxIIDToUtf8(&iid, &uuid); disk->uuid = uuid; vboxIIDUnalloc(&iid); @@ -5001,7 +4987,7 @@ vboxSnapshotRedefine(virDomainPtr dom, } gVBoxAPI.UIMedium.GetId(parentDisk, &parentiid); - gVBoxAPI.UIID.vboxIIDToUtf8(data, &parentiid, &parentUuid); + gVBoxAPI.UIID.vboxIIDToUtf8(&parentiid, &parentUuid); vboxIIDUnalloc(&parentiid); if (virVBoxSnapshotConfAddHardDiskToMediaRegistry(disk, snapshotMachineDesc->mediaRegistry, @@ -5086,7 +5072,7 @@ vboxSnapshotRedefine(virDomainPtr dom, (unsigned)rc); goto cleanup; } - gVBoxAPI.UIID.vboxIIDToUtf8(data, &parentiid, &parentUuid); + gVBoxAPI.UIID.vboxIIDToUtf8(&parentiid, &parentUuid); vboxIIDUnalloc(&parentiid); VBOX_UTF8_TO_UTF16("VDI", &formatUtf16); @@ -5135,7 +5121,7 @@ vboxSnapshotRedefine(virDomainPtr dom, (unsigned)rc); goto cleanup; } - gVBoxAPI.UIID.vboxIIDToUtf8(data, &iid, &uuid); + gVBoxAPI.UIID.vboxIIDToUtf8(&iid, &uuid); disk->uuid = uuid; vboxIIDUnalloc(&iid); @@ -5491,10 +5477,7 @@ vboxDomainSnapshotGetAll(virDomainPtr dom, } static ISnapshot * -vboxDomainSnapshotGet(vboxPrivate *data, - virDomainPtr dom, - IMachine *machine, - const char *name) +vboxDomainSnapshotGet(virDomainPtr dom, IMachine *machine, const char *name) { ISnapshot **snapshots = NULL; ISnapshot *snapshot = NULL; @@ -5569,7 +5552,7 @@ static int vboxSnapshotGetReadWriteDisks(virDomainSnapshotDefPtr def, if (openSessionForMachine(data, dom->uuid, &domiid, &machine, false) < 0) goto cleanup; - if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name))) + if (!(snap = vboxDomainSnapshotGet(dom, machine, snapshot->name))) goto cleanup; rc = gVBoxAPI.UISnapshot.GetId(snap, &snapIid); @@ -5579,7 +5562,7 @@ static int vboxSnapshotGetReadWriteDisks(virDomainSnapshotDefPtr def, goto cleanup; } - gVBoxAPI.UIID.vboxIIDToUtf8(data, &snapIid, &snapshotUuidStr); + gVBoxAPI.UIID.vboxIIDToUtf8(&snapIid, &snapshotUuidStr); vboxIIDUnalloc(&snapIid); rc = gVBoxAPI.UISnapshot.GetMachine(snap, &snapMachine); if (NS_FAILED(rc)) { @@ -5789,7 +5772,7 @@ int vboxSnapshotGetReadOnlyDisks(virDomainSnapshotPtr snapshot, if (openSessionForMachine(data, dom->uuid, &domiid, &machine, false) < 0) goto cleanup; - if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name))) + if (!(snap = vboxDomainSnapshotGet(dom, machine, snapshot->name))) goto cleanup; rc = gVBoxAPI.UISnapshot.GetMachine(snap, &snapMachine); @@ -6006,7 +5989,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, if (openSessionForMachine(data, dom->uuid, &domiid, &machine, false) < 0) goto cleanup; - if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name))) + if (!(snap = vboxDomainSnapshotGet(dom, machine, snapshot->name))) goto cleanup; if (VIR_ALLOC(def) < 0 || !(def->dom = virDomainDefNew())) @@ -6269,7 +6252,7 @@ vboxDomainSnapshotLookupByName(virDomainPtr dom, const char *name, if (openSessionForMachine(data, dom->uuid, &iid, &machine, false) < 0) goto cleanup; - if (!(snapshot = vboxDomainSnapshotGet(data, dom, machine, name))) + if (!(snapshot = vboxDomainSnapshotGet(dom, machine, name))) goto cleanup; ret = virGetDomainSnapshot(dom, name); @@ -6340,7 +6323,7 @@ vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, if (openSessionForMachine(data, dom->uuid, &iid, &machine, false) < 0) goto cleanup; - if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name))) + if (!(snap = vboxDomainSnapshotGet(dom, machine, snapshot->name))) goto cleanup; rc = gVBoxAPI.UISnapshot.GetParent(snap, &parent); @@ -6461,7 +6444,7 @@ static int vboxDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, if (openSessionForMachine(data, dom->uuid, &iid, &machine, false) < 0) goto cleanup; - if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name))) + if (!(snap = vboxDomainSnapshotGet(dom, machine, snapshot->name))) goto cleanup; rc = gVBoxAPI.UIMachine.GetCurrentSnapshot(machine, ¤t); @@ -6519,7 +6502,7 @@ static int vboxDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, goto cleanup; /* Check that snapshot exists. If so, there is no metadata. */ - if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name))) + if (!(snap = vboxDomainSnapshotGet(dom, machine, snapshot->name))) goto cleanup; ret = 0; @@ -6553,7 +6536,7 @@ static int vboxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (openSessionForMachine(data, dom->uuid, &domiid, &machine, false) < 0) goto cleanup; - newSnapshot = vboxDomainSnapshotGet(data, dom, machine, snapshot->name); + newSnapshot = vboxDomainSnapshotGet(dom, machine, snapshot->name); if (!newSnapshot) goto cleanup; @@ -6605,9 +6588,7 @@ static int vboxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } static int -vboxDomainSnapshotDeleteSingle(vboxPrivate *data, - IConsole *console, - ISnapshot *snapshot) +vboxDomainSnapshotDeleteSingle(IConsole *console, ISnapshot *snapshot) { IProgress *progress = NULL; vboxIIDUnion iid; @@ -6674,7 +6655,7 @@ vboxDomainSnapshotDeleteTree(vboxPrivate *data, goto cleanup; } - ret = vboxDomainSnapshotDeleteSingle(data, console, snapshot); + ret = vboxDomainSnapshotDeleteSingle(console, snapshot); cleanup: gVBoxAPI.UArray.vboxArrayRelease(&children); @@ -6842,7 +6823,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) (unsigned)rc); goto cleanup; } - gVBoxAPI.UIID.vboxIIDToUtf8(data, &parentiid, &parentUuid); + gVBoxAPI.UIID.vboxIIDToUtf8(&parentiid, &parentUuid); vboxIIDUnalloc(&parentiid); VBOX_UTF16_FREE(locationUtf16); VBOX_UTF8_TO_UTF16("VDI", &formatUtf16); @@ -6893,7 +6874,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) VIR_FREE(disk); goto cleanup; } - gVBoxAPI.UIID.vboxIIDToUtf8(data, &iid, &uuid); + gVBoxAPI.UIID.vboxIIDToUtf8(&iid, &uuid); disk->uuid = uuid; vboxIIDUnalloc(&iid); @@ -7155,7 +7136,7 @@ static int vboxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, if (openSessionForMachine(data, dom->uuid, &domiid, &machine, false) < 0) goto cleanup; - snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name); + snap = vboxDomainSnapshotGet(dom, machine, snapshot->name); if (!snap) goto cleanup; @@ -7206,7 +7187,7 @@ static int vboxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) ret = vboxDomainSnapshotDeleteTree(data, console, snap); else - ret = vboxDomainSnapshotDeleteSingle(data, console, snap); + ret = vboxDomainSnapshotDeleteSingle(console, snap); cleanup: VBOX_RELEASE(console); diff --git a/src/vbox/vbox_common.h b/src/vbox/vbox_common.h index b178878..074cb4e 100644 --- a/src/vbox/vbox_common.h +++ b/src/vbox/vbox_common.h @@ -348,44 +348,44 @@ typedef nsISupports IKeyboard; # define RC_SUCCEEDED(rc) NS_SUCCEEDED(rc.resultCode) # define RC_FAILED(rc) NS_FAILED(rc.resultCode) -# define VBOX_UTF16_FREE(arg) \ +# define VBOX_UTF16_FREE(arg) \ do { \ if (arg) { \ - gVBoxAPI.UPFN.Utf16Free(data->pFuncs, arg); \ + gVBoxAPI.UPFN.Utf16Free(arg); \ (arg) = NULL; \ } \ } while (0) -# define VBOX_UTF8_FREE(arg) \ +# define VBOX_UTF8_FREE(arg) \ do { \ if (arg) { \ - gVBoxAPI.UPFN.Utf8Free(data->pFuncs, arg); \ + gVBoxAPI.UPFN.Utf8Free(arg); \ (arg) = NULL; \ } \ } while (0) -# define VBOX_COM_UNALLOC_MEM(arg) \ +# define VBOX_COM_UNALLOC_MEM(arg) \ do { \ if (arg) { \ - gVBoxAPI.UPFN.ComUnallocMem(data->pFuncs, arg); \ + gVBoxAPI.UPFN.ComUnallocMem(arg); \ (arg) = NULL; \ } \ } while (0) -# define VBOX_UTF16_TO_UTF8(arg1, arg2) gVBoxAPI.UPFN.Utf16ToUtf8(data->pFuncs, arg1, arg2) -# define VBOX_UTF8_TO_UTF16(arg1, arg2) gVBoxAPI.UPFN.Utf8ToUtf16(data->pFuncs, arg1, arg2) +# define VBOX_UTF16_TO_UTF8(arg1, arg2) gVBoxAPI.UPFN.Utf16ToUtf8(arg1, arg2) +# define VBOX_UTF8_TO_UTF16(arg1, arg2) gVBoxAPI.UPFN.Utf8ToUtf16(arg1, arg2) # define VBOX_ADDREF(arg) gVBoxAPI.nsUISupports.AddRef((void *)(arg)) -# define VBOX_RELEASE(arg) \ +# define VBOX_RELEASE(arg) \ do { \ if (arg) { \ - gVBoxAPI.nsUISupports.Release((void *)arg); \ + gVBoxAPI.nsUISupports.Release((void *)arg); \ (arg) = NULL; \ } \ } while (0) -# define VBOX_MEDIUM_RELEASE(arg) \ +# define VBOX_MEDIUM_RELEASE(arg) \ do { \ if (arg) { \ gVBoxAPI.UIMedium.Release(arg); \ @@ -393,13 +393,13 @@ typedef nsISupports IKeyboard; } \ } while (0) -# define vboxIIDUnalloc(iid) gVBoxAPI.UIID.vboxIIDUnalloc(data, iid) -# define vboxIIDToUUID(iid, uuid) gVBoxAPI.UIID.vboxIIDToUUID(data, iid, uuid) -# define vboxIIDFromUUID(iid, uuid) gVBoxAPI.UIID.vboxIIDFromUUID(data, iid, uuid) -# define vboxIIDIsEqual(iid1, iid2) gVBoxAPI.UIID.vboxIIDIsEqual(data, iid1, iid2) +# define vboxIIDUnalloc(iid) gVBoxAPI.UIID.vboxIIDUnalloc(iid) +# define vboxIIDToUUID(iid, uuid) gVBoxAPI.UIID.vboxIIDToUUID(iid, uuid) +# define vboxIIDFromUUID(iid, uuid) gVBoxAPI.UIID.vboxIIDFromUUID(iid, uuid) +# define vboxIIDIsEqual(iid1, iid2) gVBoxAPI.UIID.vboxIIDIsEqual(iid1, iid2) # define DEBUGIID(msg, iid) gVBoxAPI.UIID.DEBUGIID(msg, iid) # define vboxIIDFromArrayItem(iid, array, idx) \ - gVBoxAPI.UIID.vboxIIDFromArrayItem(data, iid, array, idx) + gVBoxAPI.UIID.vboxIIDFromArrayItem(iid, array, idx) # define VBOX_IID_INITIALIZE(iid) gVBoxAPI.UIID.vboxIIDInitialize(iid) diff --git a/src/vbox/vbox_network.c b/src/vbox/vbox_network.c index ec66fab..d6b4fd0 100644 --- a/src/vbox/vbox_network.c +++ b/src/vbox/vbox_network.c @@ -350,7 +350,7 @@ static virNetworkPtr vboxNetworkLookupByName(virConnectPtr conn, const char *nam } static PRUnichar * -vboxSocketFormatAddrUtf16(vboxPrivate *data, virSocketAddrPtr addr) +vboxSocketFormatAddrUtf16(virSocketAddrPtr addr) { char *utf8 = NULL; PRUnichar *utf16 = NULL; @@ -462,10 +462,10 @@ vboxNetworkDefineCreateXML(virConnectPtr conn, const char *xml, bool start) PRUnichar *toIPAddressUtf16 = NULL; PRUnichar *trunkTypeUtf16 = NULL; - ipAddressUtf16 = vboxSocketFormatAddrUtf16(data, &ipdef->address); - networkMaskUtf16 = vboxSocketFormatAddrUtf16(data, &netmask); - fromIPAddressUtf16 = vboxSocketFormatAddrUtf16(data, &ipdef->ranges[0].start); - toIPAddressUtf16 = vboxSocketFormatAddrUtf16(data, &ipdef->ranges[0].end); + ipAddressUtf16 = vboxSocketFormatAddrUtf16(&ipdef->address); + networkMaskUtf16 = vboxSocketFormatAddrUtf16(&netmask); + fromIPAddressUtf16 = vboxSocketFormatAddrUtf16(&ipdef->ranges[0].start); + toIPAddressUtf16 = vboxSocketFormatAddrUtf16(&ipdef->ranges[0].end); if (ipAddressUtf16 == NULL || networkMaskUtf16 == NULL || fromIPAddressUtf16 == NULL || toIPAddressUtf16 == NULL) { @@ -507,8 +507,8 @@ vboxNetworkDefineCreateXML(virConnectPtr conn, const char *xml, bool start) PRUnichar *ipAddressUtf16 = NULL; PRUnichar *networkMaskUtf16 = NULL; - ipAddressUtf16 = vboxSocketFormatAddrUtf16(data, &ipdef->hosts[0].ip); - networkMaskUtf16 = vboxSocketFormatAddrUtf16(data, &netmask); + ipAddressUtf16 = vboxSocketFormatAddrUtf16(&ipdef->hosts[0].ip); + networkMaskUtf16 = vboxSocketFormatAddrUtf16(&netmask); if (ipAddressUtf16 == NULL || networkMaskUtf16 == NULL) { VBOX_UTF16_FREE(ipAddressUtf16); @@ -739,8 +739,7 @@ static int vboxNetworkCreate(virNetworkPtr network) } static int -vboxSocketParseAddrUtf16(vboxPrivate *data, const PRUnichar *utf16, - virSocketAddrPtr addr) +vboxSocketParseAddrUtf16(const PRUnichar *utf16, virSocketAddrPtr addr) { int result = -1; char *utf8 = NULL; @@ -837,13 +836,13 @@ static char *vboxNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags) /* Currently virtualbox supports only one dhcp server per network * with contigious address space from start to end */ - if (vboxSocketParseAddrUtf16(data, ipAddressUtf16, + if (vboxSocketParseAddrUtf16(ipAddressUtf16, &ipdef->address) < 0 || - vboxSocketParseAddrUtf16(data, networkMaskUtf16, + vboxSocketParseAddrUtf16(networkMaskUtf16, &ipdef->netmask) < 0 || - vboxSocketParseAddrUtf16(data, fromIPAddressUtf16, + vboxSocketParseAddrUtf16(fromIPAddressUtf16, &ipdef->ranges[0].start) < 0 || - vboxSocketParseAddrUtf16(data, toIPAddressUtf16, + vboxSocketParseAddrUtf16(toIPAddressUtf16, &ipdef->ranges[0].end) < 0) { errorOccurred = true; } @@ -874,7 +873,7 @@ static char *vboxNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags) VBOX_UTF16_TO_UTF8(macAddressUtf16, &ipdef->hosts[0].mac); - if (vboxSocketParseAddrUtf16(data, ipAddressUtf16, + if (vboxSocketParseAddrUtf16(ipAddressUtf16, &ipdef->hosts[0].ip) < 0) { errorOccurred = true; } @@ -896,9 +895,9 @@ static char *vboxNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags) gVBoxAPI.UIHNInterface.GetNetworkMask(networkInterface, &networkMaskUtf16); gVBoxAPI.UIHNInterface.GetIPAddress(networkInterface, &ipAddressUtf16); - if (vboxSocketParseAddrUtf16(data, networkMaskUtf16, + if (vboxSocketParseAddrUtf16(networkMaskUtf16, &ipdef->netmask) < 0 || - vboxSocketParseAddrUtf16(data, ipAddressUtf16, + vboxSocketParseAddrUtf16(ipAddressUtf16, &ipdef->address) < 0) { errorOccurred = true; } diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 37dcd3e..df19fdc 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -110,7 +110,7 @@ static vboxGlobalData *g_pVBoxGlobalData; #define VBOX_UTF16_FREE(arg) \ do { \ if (arg) { \ - data->pFuncs->pfnUtf16Free(arg); \ + g_pVBoxGlobalData->pFuncs->pfnUtf16Free(arg); \ (arg) = NULL; \ } \ } while (0) @@ -118,13 +118,13 @@ static vboxGlobalData *g_pVBoxGlobalData; #define VBOX_UTF8_FREE(arg) \ do { \ if (arg) { \ - data->pFuncs->pfnUtf8Free(arg); \ + g_pVBoxGlobalData->pFuncs->pfnUtf8Free(arg); \ (arg) = NULL; \ } \ } while (0) -#define VBOX_UTF16_TO_UTF8(arg1, arg2) data->pFuncs->pfnUtf16ToUtf8(arg1, arg2) -#define VBOX_UTF8_TO_UTF16(arg1, arg2) data->pFuncs->pfnUtf8ToUtf16(arg1, arg2) +#define VBOX_UTF16_TO_UTF8(arg1, arg2) g_pVBoxGlobalData->pFuncs->pfnUtf16ToUtf8(arg1, arg2) +#define VBOX_UTF8_TO_UTF16(arg1, arg2) g_pVBoxGlobalData->pFuncs->pfnUtf8ToUtf16(arg1, arg2) #define VBOX_RELEASE(arg) \ do { \ @@ -302,8 +302,7 @@ vboxIIDUnalloc_v2_x_WIN32(vboxPrivate *data ATTRIBUTE_UNUSED, } static void -_vboxIIDUnalloc(vboxPrivate *data ATTRIBUTE_UNUSED, - vboxIIDUnion *iid ATTRIBUTE_UNUSED) +_vboxIIDUnalloc(vboxIIDUnion *iid ATTRIBUTE_UNUSED) { /* Nothing to free */ } @@ -315,25 +314,23 @@ vboxIIDToUUID_v2_x_WIN32(vboxIID_v2_x_WIN32 *iid, unsigned char *uuid) } static void -_vboxIIDToUUID(vboxPrivate *data ATTRIBUTE_UNUSED, vboxIIDUnion *iidu, unsigned char *uuid) +_vboxIIDToUUID(vboxIIDUnion *iidu, unsigned char *uuid) { vboxIIDToUUID_v2_x_WIN32(&iidu->vboxIID_v2_x_WIN32, uuid); } static void -vboxIIDFromUUID_v2_x_WIN32(vboxPrivate *data, vboxIID_v2_x_WIN32 *iid, - const unsigned char *uuid) +vboxIIDFromUUID_v2_x_WIN32(vboxIID_v2_x_WIN32 *iid, const unsigned char *uuid) { - vboxIIDUnalloc_v2_x_WIN32(data, iid); + vboxIIDUnalloc_v2_x_WIN32(iid); nsIDFromChar((nsID *)&iid->value, uuid); } static void -_vboxIIDFromUUID(vboxPrivate *data, vboxIIDUnion *iidu, - const unsigned char *uuid) +_vboxIIDFromUUID(vboxIIDUnion *iidu, const unsigned char *uuid) { - vboxIIDFromUUID_v2_x_WIN32(data, &iidu->vboxIID_v2_x_WIN32, uuid); + vboxIIDFromUUID_v2_x_WIN32(&iidu->vboxIID_v2_x_WIN32, uuid); } static bool @@ -343,27 +340,26 @@ vboxIIDIsEqual_v2_x_WIN32(vboxIID_v2_x_WIN32 *iid1, vboxIID_v2_x_WIN32 *iid2) } static bool -_vboxIIDIsEqual(vboxPrivate *data ATTRIBUTE_UNUSED, vboxIIDUnion *iidu1, vboxIIDUnion *iidu2) +_vboxIIDIsEqual(vboxIIDUnion *iidu1, vboxIIDUnion *iidu2) { return vboxIIDIsEqual_v2_x_WIN32(&iidu1->vboxIID_v2_x_WIN32, &iidu2->vboxIID_v2_x_WIN32); } static void -vboxIIDFromArrayItem_v2_x_WIN32(vboxPrivate *data, vboxIID_v2_x_WIN32 *iid, +vboxIIDFromArrayItem_v2_x_WIN32(vboxIID_v2_x_WIN32 *iid, vboxArray *array, int idx) { GUID *items = (GUID *)array->items; - vboxIIDUnalloc_v2_x_WIN32(data, iid); + vboxIIDUnalloc_v2_x_WIN32(iid); memcpy(&iid->value, &items[idx], sizeof(GUID)); } static void -_vboxIIDFromArrayItem(vboxPrivate *data, vboxIIDUnion *iidu, - vboxArray *array, int idx) +_vboxIIDFromArrayItem(vboxIIDUnion *iidu, vboxArray *array, int idx) { - vboxIIDFromArrayItem_v2_x_WIN32(data, &iidu->vboxIID_v2_x_WIN32, array, idx); + vboxIIDFromArrayItem_v2_x_WIN32(&iidu->vboxIID_v2_x_WIN32, array, idx); } # define vboxIIDUnalloc(iid) vboxIIDUnalloc_v2_x_WIN32(data, iid) @@ -383,21 +379,21 @@ typedef struct _vboxIID_v2_x vboxIID_v2_x; # define IID_MEMBER(name) (iidu->vboxIID_v2_x.name) static void -vboxIIDUnalloc_v2_x(vboxPrivate *data, vboxIID_v2_x *iid) +vboxIIDUnalloc_v2_x(vboxIID_v2_x *iid) { if (iid->value == NULL) return; if (iid->value != &iid->backing) - data->pFuncs->pfnComUnallocMem(iid->value); + g_pVBoxGlobalData->pFuncs->pfnComUnallocMem(iid->value); iid->value = NULL; } static void -_vboxIIDUnalloc(vboxPrivate *data, vboxIIDUnion *iidu) +_vboxIIDUnalloc(vboxIIDUnion *iidu) { - vboxIIDUnalloc_v2_x(data, &iidu->vboxIID_v2_x); + vboxIIDUnalloc_v2_x(&iidu->vboxIID_v2_x); } static void @@ -407,17 +403,15 @@ vboxIIDToUUID_v2_x(vboxIID_v2_x *iid, unsigned char *uuid) } static void -_vboxIIDToUUID(vboxPrivate *data ATTRIBUTE_UNUSED, - vboxIIDUnion *iidu, unsigned char *uuid) +_vboxIIDToUUID(vboxIIDUnion *iidu, unsigned char *uuid) { vboxIIDToUUID_v2_x(&iidu->vboxIID_v2_x, uuid); } static void -vboxIIDFromUUID_v2_x(vboxPrivate *data, vboxIID_v2_x *iid, - const unsigned char *uuid) +vboxIIDFromUUID_v2_x(vboxIID_v2_x *iid, const unsigned char *uuid) { - vboxIIDUnalloc_v2_x(data, iid); + vboxIIDUnalloc_v2_x(iid); iid->value = &iid->backing; @@ -426,10 +420,9 @@ vboxIIDFromUUID_v2_x(vboxPrivate *data, vboxIID_v2_x *iid, } static void -_vboxIIDFromUUID(vboxPrivate *data, vboxIIDUnion *iidu, - const unsigned char *uuid) +_vboxIIDFromUUID(vboxIIDUnion *iidu, const unsigned char *uuid) { - vboxIIDFromUUID_v2_x(data, &iidu->vboxIID_v2_x, uuid); + vboxIIDFromUUID_v2_x(&iidu->vboxIID_v2_x, uuid); } static bool @@ -439,17 +432,15 @@ vboxIIDIsEqual_v2_x(vboxIID_v2_x *iid1, vboxIID_v2_x *iid2) } static bool -_vboxIIDIsEqual(vboxPrivate *data ATTRIBUTE_UNUSED, - vboxIIDUnion *iidu1, vboxIIDUnion *iidu2) +_vboxIIDIsEqual(vboxIIDUnion *iidu1, vboxIIDUnion *iidu2) { return vboxIIDIsEqual_v2_x(&iidu1->vboxIID_v2_x, &iidu2->vboxIID_v2_x); } static void -vboxIIDFromArrayItem_v2_x(vboxPrivate *data, vboxIID_v2_x *iid, - vboxArray *array, int idx) +vboxIIDFromArrayItem_v2_x(vboxIID_v2_x *iid, vboxArray *array, int idx) { - vboxIIDUnalloc_v2_x(data, iid); + vboxIIDUnalloc_v2_x(iid); iid->value = &iid->backing; @@ -457,18 +448,17 @@ vboxIIDFromArrayItem_v2_x(vboxPrivate *data, vboxIID_v2_x *iid, } static void -_vboxIIDFromArrayItem(vboxPrivate *data, vboxIIDUnion *iidu, - vboxArray *array, int idx) +_vboxIIDFromArrayItem(vboxIIDUnion *iidu, vboxArray *array, int idx) { - vboxIIDFromArrayItem_v2_x(data, &iidu->vboxIID_v2_x, array, idx); + vboxIIDFromArrayItem_v2_x(&iidu->vboxIID_v2_x, array, idx); } -# define vboxIIDUnalloc(iid) vboxIIDUnalloc_v2_x(data, iid) +# define vboxIIDUnalloc(iid) vboxIIDUnalloc_v2_x(iid) # define vboxIIDToUUID(iid, uuid) vboxIIDToUUID_v2_x(iid, uuid) -# define vboxIIDFromUUID(iid, uuid) vboxIIDFromUUID_v2_x(data, iid, uuid) +# define vboxIIDFromUUID(iid, uuid) vboxIIDFromUUID_v2_x(iid, uuid) # define vboxIIDIsEqual(iid1, iid2) vboxIIDIsEqual_v2_x(iid1, iid2) # define vboxIIDFromArrayItem(iid, array, idx) \ - vboxIIDFromArrayItem_v2_x(data, iid, array, idx) + vboxIIDFromArrayItem_v2_x(iid, array, idx) # define DEBUGIID(msg, iid) DEBUGUUID(msg, iid) # endif /* !WIN32 */ @@ -482,64 +472,59 @@ typedef struct _vboxIID_v3_x vboxIID_v3_x; # define IID_MEMBER(name) (iidu->vboxIID_v3_x.name) static void -vboxIIDUnalloc_v3_x(vboxPrivate *data, vboxIID_v3_x *iid) +vboxIIDUnalloc_v3_x(vboxIID_v3_x *iid) { if (iid->value != NULL && iid->owner) - data->pFuncs->pfnUtf16Free(iid->value); + g_pVBoxGlobalData->pFuncs->pfnUtf16Free(iid->value); iid->value = NULL; iid->owner = true; } static void -_vboxIIDUnalloc(vboxPrivate *data, vboxIIDUnion *iidu) +_vboxIIDUnalloc(vboxIIDUnion *iidu) { - vboxIIDUnalloc_v3_x(data, &iidu->vboxIID_v3_x); + vboxIIDUnalloc_v3_x(&iidu->vboxIID_v3_x); } static void -vboxIIDToUUID_v3_x(vboxPrivate *data, vboxIID_v3_x *iid, - unsigned char *uuid) +vboxIIDToUUID_v3_x(vboxIID_v3_x *iid, unsigned char *uuid) { char *utf8 = NULL; - data->pFuncs->pfnUtf16ToUtf8(iid->value, &utf8); + g_pVBoxGlobalData->pFuncs->pfnUtf16ToUtf8(iid->value, &utf8); ignore_value(virUUIDParse(utf8, uuid)); - data->pFuncs->pfnUtf8Free(utf8); + g_pVBoxGlobalData->pFuncs->pfnUtf8Free(utf8); } static void -_vboxIIDToUUID(vboxPrivate *data, vboxIIDUnion *iidu, - unsigned char *uuid) +_vboxIIDToUUID(vboxIIDUnion *iidu, unsigned char *uuid) { - vboxIIDToUUID_v3_x(data, &iidu->vboxIID_v3_x, uuid); + vboxIIDToUUID_v3_x(&iidu->vboxIID_v3_x, uuid); } static void -vboxIIDFromUUID_v3_x(vboxPrivate *data, vboxIID_v3_x *iid, - const unsigned char *uuid) +vboxIIDFromUUID_v3_x(vboxIID_v3_x *iid, const unsigned char *uuid) { char utf8[VIR_UUID_STRING_BUFLEN]; - vboxIIDUnalloc_v3_x(data, iid); + vboxIIDUnalloc_v3_x(iid); virUUIDFormat(uuid, utf8); - data->pFuncs->pfnUtf8ToUtf16(utf8, &iid->value); + g_pVBoxGlobalData->pFuncs->pfnUtf8ToUtf16(utf8, &iid->value); } static void -_vboxIIDFromUUID(vboxPrivate *data, vboxIIDUnion *iidu, - const unsigned char *uuid) +_vboxIIDFromUUID(vboxIIDUnion *iidu, const unsigned char *uuid) { - vboxIIDFromUUID_v3_x(data, &iidu->vboxIID_v3_x, uuid); + vboxIIDFromUUID_v3_x(&iidu->vboxIID_v3_x, uuid); } static bool -vboxIIDIsEqual_v3_x(vboxPrivate *data, vboxIID_v3_x *iid1, - vboxIID_v3_x *iid2) +vboxIIDIsEqual_v3_x(vboxIID_v3_x *iid1, vboxIID_v3_x *iid2) { unsigned char uuid1[VIR_UUID_BUFLEN]; unsigned char uuid2[VIR_UUID_BUFLEN]; @@ -549,42 +534,39 @@ vboxIIDIsEqual_v3_x(vboxPrivate *data, vboxIID_v3_x *iid1, * or mixture of both and we don't want to fail here by * using direct string comparison. Here virUUIDParse() takes * care of these cases. */ - vboxIIDToUUID_v3_x(data, iid1, uuid1); - vboxIIDToUUID_v3_x(data, iid2, uuid2); + vboxIIDToUUID_v3_x(iid1, uuid1); + vboxIIDToUUID_v3_x(iid2, uuid2); return memcmp(uuid1, uuid2, VIR_UUID_BUFLEN) == 0; } static bool -_vboxIIDIsEqual(vboxPrivate *data, vboxIIDUnion *iidu1, - vboxIIDUnion *iidu2) +_vboxIIDIsEqual(vboxIIDUnion *iidu1, vboxIIDUnion *iidu2) { - return vboxIIDIsEqual_v3_x(data, &iidu1->vboxIID_v3_x, &iidu2->vboxIID_v3_x); + return vboxIIDIsEqual_v3_x(&iidu1->vboxIID_v3_x, &iidu2->vboxIID_v3_x); } static void -vboxIIDFromArrayItem_v3_x(vboxPrivate *data, vboxIID_v3_x *iid, - vboxArray *array, int idx) +vboxIIDFromArrayItem_v3_x(vboxIID_v3_x *iid, vboxArray *array, int idx) { - vboxIIDUnalloc_v3_x(data, iid); + vboxIIDUnalloc_v3_x(iid); iid->value = array->items[idx]; iid->owner = false; } static void -_vboxIIDFromArrayItem(vboxPrivate *data, vboxIIDUnion *iidu, - vboxArray *array, int idx) +_vboxIIDFromArrayItem(vboxIIDUnion *iidu, vboxArray *array, int idx) { - vboxIIDFromArrayItem_v3_x(data, &iidu->vboxIID_v3_x, array, idx); + vboxIIDFromArrayItem_v3_x(&iidu->vboxIID_v3_x, array, idx); } -# define vboxIIDUnalloc(iid) vboxIIDUnalloc_v3_x(data, iid) -# define vboxIIDToUUID(iid, uuid) vboxIIDToUUID_v3_x(data, iid, uuid) -# define vboxIIDFromUUID(iid, uuid) vboxIIDFromUUID_v3_x(data, iid, uuid) -# define vboxIIDIsEqual(iid1, iid2) vboxIIDIsEqual_v3_x(data, iid1, iid2) +# define vboxIIDUnalloc(iid) vboxIIDUnalloc_v3_x(iid) +# define vboxIIDToUUID(iid, uuid) vboxIIDToUUID_v3_x(iid, uuid) +# define vboxIIDFromUUID(iid, uuid) vboxIIDFromUUID_v3_x(iid, uuid) +# define vboxIIDIsEqual(iid1, iid2) vboxIIDIsEqual_v3_x(iid1, iid2) # define vboxIIDFromArrayItem(iid, array, idx) \ - vboxIIDFromArrayItem_v3_x(data, iid, array, idx) + vboxIIDFromArrayItem_v3_x(iid, array, idx) # define DEBUGIID(msg, strUtf16) DEBUGPRUnichar(msg, strUtf16) #endif /* !(VBOX_API_VERSION == 2002000) */ @@ -1404,12 +1386,11 @@ static nsresult PR_COM_METHOD vboxCallbackOnMachineStateChange(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, PRUnichar *machineId, PRUint32 state) { + VirtualBoxCallback *callback = (VirtualBoxCallback *) pThis; virDomainPtr dom = NULL; int event = 0; int detail = 0; - vboxDriverLock((vboxPrivate *) g_pVBoxGlobalData); - VIR_DEBUG("IVirtualBoxCallback: %p, State: %d", pThis, state); DEBUGPRUnichar("machineId", machineId); @@ -1420,9 +1401,10 @@ vboxCallbackOnMachineStateChange(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, g_pVBoxGlobalData->pFuncs->pfnUtf16ToUtf8(machineId, &machineIdUtf8); ignore_value(virUUIDParse(machineIdUtf8, uuid)); - dom = vboxDomainLookupByUUID(g_pVBoxGlobalData->conn, uuid); + dom = vboxDomainLookupByUUID(callback->conn, uuid); if (dom) { virObjectEventPtr ev; + vboxPrivate *data = (vboxPrivate *) callback->conn->privateData; if (state == MachineState_Starting) { event = VIR_DOMAIN_EVENT_STARTED; @@ -1456,12 +1438,10 @@ vboxCallbackOnMachineStateChange(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, ev = virDomainEventLifecycleNewFromDom(dom, event, detail); if (ev) - virObjectEventStateQueue(g_pVBoxGlobalData->domainEvents, ev); + virObjectEventStateQueue(data->domainEvents, ev); } } - vboxDriverUnlock((vboxPrivate *) g_pVBoxGlobalData); - return NS_OK; } @@ -1523,12 +1503,11 @@ static nsresult PR_COM_METHOD vboxCallbackOnMachineRegistered(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, PRUnichar *machineId, PRBool registered) { + VirtualBoxCallback *callback = (VirtualBoxCallback *) pThis; virDomainPtr dom = NULL; int event = 0; int detail = 0; - vboxDriverLock((vboxPrivate *) g_pVBoxGlobalData); - VIR_DEBUG("IVirtualBoxCallback: %p, registered: %s", pThis, registered ? "true" : "false"); DEBUGPRUnichar("machineId", machineId); @@ -1539,9 +1518,10 @@ vboxCallbackOnMachineRegistered(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, g_pVBoxGlobalData->pFuncs->pfnUtf16ToUtf8(machineId, &machineIdUtf8); ignore_value(virUUIDParse(machineIdUtf8, uuid)); - dom = vboxDomainLookupByUUID(g_pVBoxGlobalData->conn, uuid); + dom = vboxDomainLookupByUUID(callback->conn, uuid); if (dom) { virObjectEventPtr ev; + vboxPrivate *data = (vboxPrivate *) callback->conn->privateData; /* CURRENT LIMITATION: we never get the VIR_DOMAIN_EVENT_UNDEFINED * event because the when the machine is de-registered the call @@ -1560,12 +1540,10 @@ vboxCallbackOnMachineRegistered(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, ev = virDomainEventLifecycleNewFromDom(dom, event, detail); if (ev) - virObjectEventStateQueue(g_pVBoxGlobalData->domainEvents, ev); + virObjectEventStateQueue(data->domainEvents, ev); } } - vboxDriverUnlock((vboxPrivate *) g_pVBoxGlobalData); - return NS_OK; } @@ -1634,8 +1612,9 @@ static nsresult PR_COM_METHOD vboxCallbackAddRef(nsISupports *pThis ATTRIBUTE_UNUSED) { nsresult c; + VirtualBoxCallback *callback = (VirtualBoxCallback *) pThis; - c = ++g_pVBoxGlobalData->vboxCallBackRefCount; + c = ++callback->vboxCallBackRefCount; VIR_DEBUG("pThis: %p, vboxCallback AddRef: %d", pThis, c); @@ -1646,8 +1625,9 @@ static nsresult PR_COM_METHOD vboxCallbackRelease(nsISupports *pThis) { nsresult c; + VirtualBoxCallback *callback = (VirtualBoxCallback *) pThis; - c = --g_pVBoxGlobalData->vboxCallBackRefCount; + c = --callback->vboxCallBackRefCount; if (c == 0) { /* delete object */ VIR_FREE(pThis->vtbl); @@ -1662,17 +1642,17 @@ vboxCallbackRelease(nsISupports *pThis) static nsresult PR_COM_METHOD vboxCallbackQueryInterface(nsISupports *pThis, const nsID *iid, void **resultp) { - IVirtualBoxCallback *that = (IVirtualBoxCallback *)pThis; + VirtualBoxCallback *callback = (VirtualBoxCallback *) pThis; static const nsID ivirtualboxCallbackUUID = IVIRTUALBOXCALLBACK_IID; static const nsID isupportIID = NS_ISUPPORTS_IID; /* Match UUID for IVirtualBoxCallback class */ if (memcmp(iid, &ivirtualboxCallbackUUID, sizeof(nsID)) == 0 || memcmp(iid, &isupportIID, sizeof(nsID)) == 0) { - g_pVBoxGlobalData->vboxCallBackRefCount++; - *resultp = that; + callback->vboxCallBackRefCount++; + *resultp = callback; - VIR_DEBUG("pThis: %p, vboxCallback QueryInterface: %d", pThis, g_pVBoxGlobalData->vboxCallBackRefCount); + VIR_DEBUG("pThis: %p, vboxCallback QueryInterface: %d", callback, callback->vboxCallBackRefCount); return NS_OK; } @@ -1685,8 +1665,8 @@ vboxCallbackQueryInterface(nsISupports *pThis, const nsID *iid, void **resultp) } -static IVirtualBoxCallback *vboxAllocCallbackObj(void) { - IVirtualBoxCallback *vboxCallback = NULL; +static IVirtualBoxCallback *vboxAllocCallbackObj(virConnectPtr conn) { + VirtualBoxCallback *vboxCallback = NULL; /* Allocate, Initialize and return a valid * IVirtualBoxCallback object here @@ -1718,27 +1698,30 @@ static IVirtualBoxCallback *vboxAllocCallbackObj(void) { # endif /* VBOX_API_VERSION >= 3002000 */ vboxCallback->vtbl->OnSnapshotChange = &vboxCallbackOnSnapshotChange; vboxCallback->vtbl->OnGuestPropertyChange = &vboxCallbackOnGuestPropertyChange; - g_pVBoxGlobalData->vboxCallBackRefCount = 1; - + vboxCallback->vboxCallBackRefCount = 1; } - return vboxCallback; + vboxCallback->conn = conn; + + return (IVirtualBoxCallback *) vboxCallback; } static void vboxReadCallback(int watch ATTRIBUTE_UNUSED, int fd, int events ATTRIBUTE_UNUSED, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { + vboxPrivate *data = (vboxPrivate *) opaque; + if (fd >= 0) { - g_pVBoxGlobalData->vboxQueue->vtbl->ProcessPendingEvents(g_pVBoxGlobalData->vboxQueue); + data->vboxQueue->vtbl->ProcessPendingEvents(data->vboxQueue); } else { nsresult rc; PLEvent *pEvent = NULL; - rc = g_pVBoxGlobalData->vboxQueue->vtbl->WaitForEvent(g_pVBoxGlobalData->vboxQueue, &pEvent); + rc = data->vboxQueue->vtbl->WaitForEvent(data->vboxQueue, &pEvent); if (NS_SUCCEEDED(rc)) - g_pVBoxGlobalData->vboxQueue->vtbl->HandleEvent(g_pVBoxGlobalData->vboxQueue, pEvent); + data->vboxQueue->vtbl->HandleEvent(data->vboxQueue, pEvent); } } @@ -1762,7 +1745,7 @@ vboxConnectDomainEventRegister(virConnectPtr conn, vboxDriverLock(data); if (data->vboxCallback == NULL) { - data->vboxCallback = vboxAllocCallbackObj(); + data->vboxCallback = vboxAllocCallbackObj(conn); if (data->vboxCallback != NULL) { rc = data->vboxObj->vtbl->RegisterCallback(data->vboxObj, data->vboxCallback); if (NS_SUCCEEDED(rc)) @@ -1780,7 +1763,7 @@ vboxConnectDomainEventRegister(virConnectPtr conn, PRInt32 vboxFileHandle; vboxFileHandle = data->vboxQueue->vtbl->GetEventQueueSelectFD(data->vboxQueue); - data->fdWatch = virEventAddHandle(vboxFileHandle, VIR_EVENT_HANDLE_READABLE, vboxReadCallback, NULL, NULL); + data->fdWatch = virEventAddHandle(vboxFileHandle, VIR_EVENT_HANDLE_READABLE, vboxReadCallback, data, NULL); } if (data->fdWatch >= 0) { @@ -1866,7 +1849,7 @@ static int vboxConnectDomainEventRegisterAny(virConnectPtr conn, vboxDriverLock(data); if (data->vboxCallback == NULL) { - data->vboxCallback = vboxAllocCallbackObj(); + data->vboxCallback = vboxAllocCallbackObj(conn); if (data->vboxCallback != NULL) { rc = data->vboxObj->vtbl->RegisterCallback(data->vboxObj, data->vboxCallback); if (NS_SUCCEEDED(rc)) @@ -1884,7 +1867,7 @@ static int vboxConnectDomainEventRegisterAny(virConnectPtr conn, PRInt32 vboxFileHandle; vboxFileHandle = data->vboxQueue->vtbl->GetEventQueueSelectFD(data->vboxQueue); - data->fdWatch = virEventAddHandle(vboxFileHandle, VIR_EVENT_HANDLE_READABLE, vboxReadCallback, NULL, NULL); + data->fdWatch = virEventAddHandle(vboxFileHandle, VIR_EVENT_HANDLE_READABLE, vboxReadCallback, data, NULL); } if (data->fdWatch >= 0) { @@ -1971,7 +1954,7 @@ _initializeDomainEvent(vboxPrivate *data ATTRIBUTE_UNUSED) #else /* VBOX_API_VERSION > 2002000 || VBOX_API_VERSION < 4000000 */ /* Initialize the fWatch needed for Event Callbacks */ data->fdWatch = -1; - data->pFuncs->pfnGetEventQueue(&data->vboxQueue); + g_pVBoxGlobalData->pFuncs->pfnGetEventQueue(&data->vboxQueue); if (data->vboxQueue == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nsIEventQueue object is null")); @@ -2020,8 +2003,7 @@ vboxGlobalData* _registerGlobalData(void) # if VBOX_API_VERSION < 3001000 static void -_detachDevices(vboxPrivate *data ATTRIBUTE_UNUSED, - IMachine *machine, PRUnichar *hddcnameUtf16) +_detachDevices(IMachine *machine, PRUnichar *hddcnameUtf16) { /* Disconnect all the drives if present */ machine->vtbl->DetachHardDisk(machine, hddcnameUtf16, 0, 0); @@ -2030,8 +2012,7 @@ _detachDevices(vboxPrivate *data ATTRIBUTE_UNUSED, } # else /* VBOX_API_VERSION >= 3001000 */ static void -_detachDevices(vboxPrivate *data, IMachine *machine, - PRUnichar *hddcnameUtf16 ATTRIBUTE_UNUSED) +_detachDevices(IMachine *machine, PRUnichar *hddcnameUtf16 ATTRIBUTE_UNUSED) { /* get all the controller first, then the attachments and * remove them all so that the machine can be undefined @@ -2096,8 +2077,7 @@ _deleteConfig(IMachine *machine) #else /* VBOX_API_VERSION >= 4000000 */ static void -_detachDevices(vboxPrivate *data ATTRIBUTE_UNUSED, - IMachine *machine ATTRIBUTE_UNUSED, +_detachDevices(IMachine *machine ATTRIBUTE_UNUSED, PRUnichar *hddcnameUtf16 ATTRIBUTE_UNUSED) { vboxUnsupported(); @@ -2167,9 +2147,7 @@ _deleteConfig(IMachine *machine) #if VBOX_API_VERSION < 3001000 static void -_dumpIDEHDDsOld(virDomainDefPtr def, - vboxPrivate *data, - IMachine *machine) +_dumpIDEHDDsOld(virDomainDefPtr def, IMachine *machine) { PRInt32 hddNum = 0; IHardDisk *hardDiskPM = NULL; @@ -2276,9 +2254,7 @@ _dumpIDEHDDsOld(virDomainDefPtr def, } static void -_dumpDVD(virDomainDefPtr def, - vboxPrivate *data, - IMachine *machine) +_dumpDVD(virDomainDefPtr def, IMachine *machine) { IDVDDrive *dvdDrive = NULL; IDVDImage *dvdImage = NULL; @@ -2415,9 +2391,7 @@ _detachDVD(IMachine *machine) } static void -_dumpFloppy(virDomainDefPtr def, - vboxPrivate *data, - IMachine *machine) +_dumpFloppy(virDomainDefPtr def, IMachine *machine) { IFloppyDrive *floppyDrive = NULL; IFloppyImage *floppyImage = NULL; @@ -2571,7 +2545,6 @@ _detachFloppy(IMachine *machine) static void _dumpIDEHDDsOld(virDomainDefPtr def ATTRIBUTE_UNUSED, - vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine ATTRIBUTE_UNUSED) { vboxUnsupported(); @@ -2579,7 +2552,6 @@ _dumpIDEHDDsOld(virDomainDefPtr def ATTRIBUTE_UNUSED, static void _dumpDVD(virDomainDefPtr def ATTRIBUTE_UNUSED, - vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine ATTRIBUTE_UNUSED) { vboxUnsupported(); @@ -2603,7 +2575,6 @@ _detachDVD(IMachine *machine ATTRIBUTE_UNUSED) static void _dumpFloppy(virDomainDefPtr def ATTRIBUTE_UNUSED, - vboxPrivate *data ATTRIBUTE_UNUSED, IMachine *machine ATTRIBUTE_UNUSED) { vboxUnsupported(); @@ -2674,29 +2645,29 @@ static void _pfnUninitialize(vboxPrivate *data) } } -static void _pfnComUnallocMem(PCVBOXXPCOM pFuncs, void *pv) +static void _pfnComUnallocMem(void *pv) { - pFuncs->pfnComUnallocMem(pv); + g_pVBoxGlobalData->pFuncs->pfnComUnallocMem(pv); } -static void _pfnUtf16Free(PCVBOXXPCOM pFuncs, PRUnichar *pwszString) +static void _pfnUtf16Free(PRUnichar *pwszString) { - pFuncs->pfnUtf16Free(pwszString); + g_pVBoxGlobalData->pFuncs->pfnUtf16Free(pwszString); } -static void _pfnUtf8Free(PCVBOXXPCOM pFuncs, char *pszString) +static void _pfnUtf8Free(char *pszString) { - pFuncs->pfnUtf8Free(pszString); + g_pVBoxGlobalData->pFuncs->pfnUtf8Free(pszString); } -static int _pfnUtf16ToUtf8(PCVBOXXPCOM pFuncs, const PRUnichar *pwszString, char **ppszString) +static int _pfnUtf16ToUtf8(const PRUnichar *pwszString, char **ppszString) { - return pFuncs->pfnUtf16ToUtf8(pwszString, ppszString); + return g_pVBoxGlobalData->pFuncs->pfnUtf16ToUtf8(pwszString, ppszString); } -static int _pfnUtf8ToUtf16(PCVBOXXPCOM pFuncs, const char *pszString, PRUnichar **ppwszString) +static int _pfnUtf8ToUtf16(const char *pszString, PRUnichar **ppwszString) { - return pFuncs->pfnUtf8ToUtf16(pszString, ppwszString); + return g_pVBoxGlobalData->pFuncs->pfnUtf8ToUtf16(pszString, ppwszString); } #if VBOX_API_VERSION == 2002000 @@ -2731,14 +2702,13 @@ static void _DEBUGIID(const char *msg, vboxIIDUnion *iidu) #endif /* VBOX_API_VERSION != 2002000 */ static void -_vboxIIDToUtf8(vboxPrivate *data ATTRIBUTE_UNUSED, - vboxIIDUnion *iidu ATTRIBUTE_UNUSED, +_vboxIIDToUtf8(vboxIIDUnion *iidu ATTRIBUTE_UNUSED, char **utf8 ATTRIBUTE_UNUSED) { #if VBOX_API_VERSION == 2002000 vboxUnsupported(); #else /* !(VBOX_API_VERSION == 2002000) */ - data->pFuncs->pfnUtf16ToUtf8(IID_MEMBER(value), utf8); + g_pVBoxGlobalData->pFuncs->pfnUtf16ToUtf8(IID_MEMBER(value), utf8); #endif /* !(VBOX_API_VERSION == 2002000) */ } @@ -5151,12 +5121,6 @@ void NAME(InstallUniformedAPI)(vboxUniformedAPI *pVBoxAPI) pVBoxAPI->domainEventCallbacks = 1; #endif /* VBOX_API_VERSION > 2002000 || VBOX_API_VERSION < 4000000 */ -#if VBOX_API_VERSION == 2002000 - pVBoxAPI->hasStaticGlobalData = 0; -#else /* VBOX_API_VERSION > 2002000 */ - pVBoxAPI->hasStaticGlobalData = 1; -#endif /* VBOX_API_VERSION > 2002000 */ - #if VBOX_API_VERSION >= 4000000 /* Get machine for the call to VBOX_SESSION_OPEN_EXISTING */ pVBoxAPI->getMachineForSession = 1; diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h index dac44e6..1fc5fff 100644 --- a/src/vbox/vbox_uniformed_api.h +++ b/src/vbox/vbox_uniformed_api.h @@ -21,7 +21,7 @@ # include "internal.h" -/* This file may be used in three place. That is vbox_tmpl.c, +/* This file may be used in three places. That is vbox_tmpl.c, * vbox_common.c and vbox_driver.c. The vboxUniformedAPI and some * types used for vboxUniformedAPI is defined here. * @@ -29,23 +29,21 @@ * architecture of those vbox structs(vboxObj, vboxSession, * pFuncs, vboxCallback and vboxQueue). The file should be included * after the currect vbox_CAPI_v*.h, then we can use the vbox structs - * in vboxGlobalData. The vbox_tmpl.c should implement functions + * in vboxPrivate. The vbox_tmpl.c should implement functions * defined in vboxUniformedAPI. * * In vbox_driver.c, it is used to define the struct vboxUniformedAPI. * The vbox_driver.c collects vboxUniformedAPI for all versions. * Then vboxRegister calls the vboxRegisterUniformedAPI to register. - * Note: In vbox_driver.c, the vbox structs in vboxGlobalData is - * defined by vbox_CAPI_v2.2.h. * * The vbox_common.c, it is used to generate common codes for all vbox - * versions. Bacause the same member varible's offset in a vbox struct + * versions. Bacause the same member variable's offset in a vbox struct * may change between different vbox versions. The vbox_common.c - * shouldn't directly use struct's member varibles defined in - * vbox_CAPI_v*.h. To make things safety, we include the + * shouldn't directly use struct's member variables defined in + * vbox_CAPI_v*.h. To make things safe, we include the * vbox_common.h in vbox_common.c. In this case, we treat structs - * defined by vbox as a void*. The common codes don't concern about - * the inside of this structs(actually, we can't, in the common level). + * defined by vbox as a void*. The common code is not concerned about + * the inside of those structs (actually, we can't, in the common level). * With the help of vboxUniformed API, we call VirtualBox's API and * implement the vbox driver in a high level. * @@ -155,22 +153,22 @@ typedef struct { typedef struct { int (*Initialize)(vboxPrivate *data); void (*Uninitialize)(vboxPrivate *data); - void (*ComUnallocMem)(PCVBOXXPCOM pFuncs, void *pv); - void (*Utf16Free)(PCVBOXXPCOM pFuncs, PRUnichar *pwszString); - void (*Utf8Free)(PCVBOXXPCOM pFuncs, char *pszString); - int (*Utf16ToUtf8)(PCVBOXXPCOM pFuncs, const PRUnichar *pwszString, char **ppszString); - int (*Utf8ToUtf16)(PCVBOXXPCOM pFuncs, const char *pszString, PRUnichar **ppwszString); + void (*ComUnallocMem)(void *pv); + void (*Utf16Free)(PRUnichar *pwszString); + void (*Utf8Free)(char *pszString); + int (*Utf16ToUtf8)(const PRUnichar *pwszString, char **ppszString); + int (*Utf8ToUtf16)(const char *pszString, PRUnichar **ppwszString); } vboxUniformedPFN; /* Functions for vboxIID */ typedef struct { void (*vboxIIDInitialize)(vboxIIDUnion *iidu); - void (*vboxIIDUnalloc)(vboxPrivate *data, vboxIIDUnion *iidu); - void (*vboxIIDToUUID)(vboxPrivate *data, vboxIIDUnion *iidu, unsigned char *uuid); - void (*vboxIIDFromUUID)(vboxPrivate *data, vboxIIDUnion *iidu, const unsigned char *uuid); - bool (*vboxIIDIsEqual)(vboxPrivate *data, vboxIIDUnion *iidu1, vboxIIDUnion *iidu2); - void (*vboxIIDFromArrayItem)(vboxPrivate *data, vboxIIDUnion *iidu, vboxArray *array, int idx); - void (*vboxIIDToUtf8)(vboxPrivate *data, vboxIIDUnion *iidu, char **utf8); + void (*vboxIIDUnalloc)(vboxIIDUnion *iidu); + void (*vboxIIDToUUID)(vboxIIDUnion *iidu, unsigned char *uuid); + void (*vboxIIDFromUUID)(vboxIIDUnion *iidu, const unsigned char *uuid); + bool (*vboxIIDIsEqual)(vboxIIDUnion *iidu1, vboxIIDUnion *iidu2); + void (*vboxIIDFromArrayItem)(vboxIIDUnion *iidu, vboxArray *array, int idx); + void (*vboxIIDToUtf8)(vboxIIDUnion *iidu, char **utf8); void (*DEBUGIID)(const char *msg, vboxIIDUnion *iidu); } vboxUniformedIID; @@ -566,17 +564,17 @@ typedef struct { uint32_t XPCOMCVersion; /* vbox APIs */ int (*initializeDomainEvent)(vboxPrivate *data); - void (*registerGlobalData)(vboxPrivate *data); - void (*detachDevices)(vboxPrivate *data, IMachine *machine, PRUnichar *hddcnameUtf16); + vboxGlobalData* (*registerGlobalData)(void); + void (*detachDevices)(IMachine *machine, PRUnichar *hddcnameUtf16); nsresult (*unregisterMachine)(vboxPrivate *data, vboxIIDUnion *iidu, IMachine **machine); void (*deleteConfig)(IMachine *machine); void (*vboxAttachDrivesOld)(virDomainDefPtr def, vboxPrivate *data, IMachine *machine); virDomainState (*vboxConvertState)(PRUint32 state); - void (*dumpIDEHDDsOld)(virDomainDefPtr def, vboxPrivate *data, IMachine *machine); - void (*dumpDVD)(virDomainDefPtr def, vboxPrivate *data, IMachine *machine); + void (*dumpIDEHDDsOld)(virDomainDefPtr def, IMachine *machine); + void (*dumpDVD)(virDomainDefPtr def, IMachine *machine); int (*attachDVD)(vboxPrivate *data, IMachine *machine, const char *src); int (*detachDVD)(IMachine *machine); - void (*dumpFloppy)(virDomainDefPtr def, vboxPrivate *data, IMachine *machine); + void (*dumpFloppy)(virDomainDefPtr def, IMachine *machine); int (*attachFloppy)(vboxPrivate *data, IMachine *machine, const char *src); int (*detachFloppy)(IMachine *machine); int (*snapshotRestore)(virDomainPtr dom, IMachine *machine, ISnapshot *snapshot); @@ -613,7 +611,6 @@ typedef struct { uniformedMachineStateChecker machineStateChecker; /* vbox API features */ bool domainEventCallbacks; - bool hasStaticGlobalData; bool getMachineForSession; bool detachDevicesExplicitly; bool chipsetType; -- 2.7.4
On Wed, 2016-09-28 at 13:41 -0400, Dawid Zamirski wrote:
This patch series solves (at least in my testing) vbox driver thread-safety issues that were also outlined on libvirt-users ML [1] and I was affected with.
Just to give a more practical context on the issue I'm trying to solve with this series, given this simple example code: ======================= #define _GNU_SOURCE #include <stdlib.h> #include <unistd.h> #include <stdio.h> #include <libvirt/libvirt.h> #include <libvirt/virterror.h> const char *xml_tmpl = "<?xml version=\"1.0\"?>" "<domain type=\"vbox\">" " <name>%s</name>" " <memory unit=\"MiB\">3072</memory>" " <vcpu>2</vcpu>" " <os>" " <type arch=\"x86_64\">hvm</type>" " <boot dev=\"cdrom\"/>" " <boot dev=\"hd\"/>" " </os>" " <features>" " <acpi/>" " <apic/>" " <pae/>" " </features>" " <cpu mode=\"host-model\"/>" " <devices>" " <graphics type=\"rdp\" autoport=\"yes\" multiUser=\"yes\"></graphics>" " <video primary=\"yes\">" " <model type=\"vbox\" vram=\"24576\" heads=\"1\"></model>" " </video>" " </devices>" "</domain>"; int main(int argc, char *argv[]) { virConnectPtr conn; char *vm_xml; int ret = 0; if (argc != 2) { fprintf(stderr, "Please specify VM name as first argument\n"); return 1; } conn = virConnectOpen("vbox:///session"); if (conn == NULL) { fprintf(stderr, "Failed to open connection to vbox:///session\n"); return 1; } else { printf("Connected to vbox successfully...\n"); } printf("Sleeping for 5s...\n"); sleep(5); if (asprintf(&vm_xml, xml_tmpl, argv[1]) == -1) { fprintf(stderr, "Failed to allocate memory for VM XML definition string\n"); ret = 1; goto cleanup; } printf("Defining domain...\n"); virDomainPtr dom = virDomainDefineXML(conn, vm_xml); if (dom == NULL) { const char *error = virGetLastErrorMessage(); fprintf(stderr, "Failed to define domain: %s\n", error); ret = 1; goto cleanup; } printf("Domain defined successfully.\n"); sleep(1); printf("Undefining domain...\n"); if (virDomainUndefine(dom) == -1) { const char *error = virGetLastErrorMessage(); fprintf(stderr, "Failed to undefine domain %s\n", error); fprintf(stderr, "This may require cleanup of the VBOX home dir from stale template files\n"); ret = 1; goto cleanup; } printf("Domain undefined successfully.\n"); cleanup: printf("Closing vbox connection...\n"); virConnectClose(conn); printf("Done\n"); free(vm_xml); vm_xml = NULL; return ret; } ================================ After compiling (to say, vbox-test): 1. ./vbox-test vm1 # creates/destroys vbox VM named "vm1" 2. while #1 is sleeping, ./vbox-test vm2 # will attempt to do the same for "vm2" by spawning concurrent vbox connection. 3. result: the 2nd process will segfault libvirtd with those patches applied both succeed as expected. Regards, Dawid
participants (3)
-
Dawid Zamirski -
Martin Kletzander -
Michal Privoznik