[libvirt] [PATCH 1/1] Support for Callbacks and VirtualBox Version 3.
Daniel P. Berrange
berrange at redhat.com
Tue Jul 14 22:34:08 UTC 2009
On Mon, Jul 13, 2009 at 06:55:47PM +0200, Pritesh Kothari wrote:
> Hi All,
>
> I have added support for callbacks and VirtualBox version 3.
> I have attached the patch for the same, it is made against the head as of
> today.
>
> To check if the callbacks work you can run the example from examples/domain-
> events/events-c directory and then start a domain to see if you get any
> callback events.
>
> Regards,
> Pritesh
> commit 1766765df5384cae45202c34ab20164d7c1544fe
> Author: pk221555 <pk221555 at krishna.(none)>
> Date: Mon Jul 13 18:29:43 2009 +0200
I think your .gitconfig needs email addr fixing :-)
> @@ -710,8 +863,13 @@ static virDomainPtr vboxDomainLookupByName(virConnectPtr conn, const char *name)
> IMachine **machines = NULL;
> PRUint32 machineCnt = 0;
> virDomainPtr dom = NULL;
> +#if VBOX_API_VERSION == 2002
> nsID *iid = NULL;
> - char *machineName = NULL;
> +#else
> + PRUnichar *iidUtf16 = NULL;
> + char *iidUtf8 = NULL;
> +#endif
> + char *machineNameUtf8 = NULL;
> PRUnichar *machineNameUtf16 = NULL;
> unsigned char iidl[VIR_UUID_BUFLEN];
> int i, matched = 0;
> @@ -735,16 +893,25 @@ static virDomainPtr vboxDomainLookupByName(virConnectPtr conn, const char *name)
> if (isAccessible) {
>
> machine->vtbl->GetName(machine, &machineNameUtf16);
> - data->pFuncs->pfnUtf16ToUtf8(machineNameUtf16, &machineName);
> + data->pFuncs->pfnUtf16ToUtf8(machineNameUtf16, &machineNameUtf8);
>
> - if (machineName && (STREQ(name, machineName))) {
> + if (STREQ(name, machineNameUtf8)) {
>
> PRUint32 state;
>
> matched = 1;
>
> +#if VBOX_API_VERSION == 2002
> machine->vtbl->GetId(machine, &iid);
> nsIDtoChar(iidl, iid);
> + data->pFuncs->pfnComUnallocMem(iid);
> +#else
> + machine->vtbl->GetId(machine, &iidUtf16);
> + data->pFuncs->pfnUtf16ToUtf8(iidUtf16, &iidUtf8);
> + virUUIDParse(iidUtf8, iidl);
> + data->pFuncs->pfnUtf8Free(iidUtf8);
> + data->pFuncs->pfnUtf16Free(iidUtf16);
> +#endif
[snip]
> @@ -790,21 +961,34 @@ static int vboxDomainSuspend(virDomainPtr dom) {
> nsresult rc;
> vboxGlobalData *data = dom->conn->privateData;
> IMachine *machine = NULL;
> +#if VBOX_API_VERSION == 2002
> nsID *iid = NULL;
> +#else
> + PRUnichar *iidUtf16 = NULL;
> + char iidUtf8[VIR_UUID_STRING_BUFLEN];
> +#endif
> IConsole *console = NULL;
> PRUint32 state;
> int ret = -1;
>
> +#if VBOX_API_VERSION == 2002
> if (VIR_ALLOC(iid) < 0) {
> virReportOOMError(dom->conn);
> goto cleanup;
> }
> +#endif
>
> if (data->vboxObj) {
> PRBool isAccessible = PR_FALSE;
>
> +#if VBOX_API_VERSION == 2002
> nsIDFromChar(iid, dom->uuid);
> rc = data->vboxObj->vtbl->GetMachine(data->vboxObj, iid, &machine);
> +#else
> + virUUIDFormat(dom->uuid, iidUtf8);
> + data->pFuncs->pfnUtf8ToUtf16(iidUtf8, &iidUtf16);
> + rc = data->vboxObj->vtbl->GetMachine(data->vboxObj, iidUtf16, &machine);
> +#endif
> if (NS_FAILED(rc)) {
> vboxError(dom->conn, VIR_ERR_INVALID_DOMAIN,
> "no domain with matching id %d", dom->id);
> @@ -820,7 +1004,11 @@ static int vboxDomainSuspend(virDomainPtr dom) {
>
> if (state == MachineState_Running) {
> /* set state pause */
> +#if VBOX_API_VERSION == 2002
> data->vboxObj->vtbl->OpenExistingSession(data->vboxObj, data->vboxSession, iid);
> +#else
> + data->vboxObj->vtbl->OpenExistingSession(data->vboxObj, data->vboxSession, iidUtf16);
> +#endif
> data->vboxSession->vtbl->GetConsole(data->vboxSession, &console);
> if (console) {
> console->vtbl->Pause(console);
> @@ -844,7 +1032,11 @@ cleanup:
> if (machine)
> machine->vtbl->nsisupports.Release((nsISupports *)machine);
>
> +#if VBOX_API_VERSION == 2002
> VIR_FREE(iid);
> +#else
> + data->pFuncs->pfnUtf16Free(iidUtf16);
> +#endif
> return ret;
> }
I'm thinking it is going to get rather painful to #if/else/endif this
stuff throughout the file.
Perhaps it wouldbe better to define some wrapper datatypes / functions
#if VBOX_API_VERSION == 2002
typedef vboxIID nsID;
void vboxIIDFromDom(virDomainPtr dom, vboxIID *iid) {
nsIDFromChar(iid, dom->uuid);
}
void vboxIIDFree(vboxIID *iid) {
VIR_FREE(iid);
}
#else
typedef vboxIID PRUnichar;
void vboxIIDFromDom(virDomainPtr dom, vboxIID *iid) {
char iidUtf8[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom->uuid, iidUtf8);
data->pFuncs->pfnUtf8ToUtf16(iidUtf8, iid);
}
void vboxIIDFree(vboxIID *iid) {
data->pFuncs->pfnUtf16Free(iidUtf16);
}
#endif
So, then the code could simply do
vboxIIDFromDom(dom, &iid);
rc = data->vboxObj->vtbl->GetMachine(data->vboxObj, iid, &machine);
vboxIIDRelease(iid);
> +#if VBOX_API_VERSION == 2002
> + /* No Callback support for VirtualBox 2.2.* series */
> +#else /* !(VBOX_API_VERSION == 2002) */
> +
> +/* Functions needed for Callbacks */
> +static nsresult vboxCallbackOnMachineStateChange (IVirtualBoxCallback *pThis,
> + PRUnichar * machineId,
> + PRUint32 state) {
> + virDomainPtr dom = NULL;
> + int event = 0;
> + int detail = 0;
> +
> + g_pVBoxGlobalData->domainEventDispatching = 1;
> + vboxDriverLock(g_pVBoxGlobalData);
> +
> + DEBUG("IVirtualBoxCallback: %p, State: %d", pThis, state);
> + DEBUGPRUnichar(machineId);
> +
> + if (machineId) {
> + char *machineIdUtf8 = NULL;
> + unsigned char uuid[VIR_UUID_BUFLEN];
> +
> + g_pVBoxGlobalData->pFuncs->pfnUtf16ToUtf8(machineId, &machineIdUtf8);
> + virUUIDParse(machineIdUtf8, uuid);
> +
> + dom = vboxDomainLookupByUUID(g_pVBoxGlobalData->conn, uuid);
> + if (dom) {
> + int i = 0;
> +
> + if (state == MachineState_Starting) {
> + event = VIR_DOMAIN_EVENT_STARTED;
> + detail = VIR_DOMAIN_EVENT_STARTED_BOOTED;
> + } else if (state == MachineState_Restoring) {
> + event = VIR_DOMAIN_EVENT_STARTED;
> + detail = VIR_DOMAIN_EVENT_STARTED_RESTORED;
> + } else if (state == MachineState_Paused) {
> + event = VIR_DOMAIN_EVENT_SUSPENDED;
> + detail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
> + } else if (state == MachineState_Running) {
> + event = VIR_DOMAIN_EVENT_RESUMED;
> + detail = VIR_DOMAIN_EVENT_RESUMED_UNPAUSED;
Does 'MachineState_Running' only occur upon unpausing the VM ?
The 'RESUMED' even is only intended to occur in that case.
> + } else if (state == MachineState_PoweredOff) {
> + event = VIR_DOMAIN_EVENT_STOPPED;
> + detail = VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN;
> + } else if (state == MachineState_Stopping) {
> + event = VIR_DOMAIN_EVENT_STOPPED;
> + detail = VIR_DOMAIN_EVENT_STOPPED_DESTROYED;
> + } else if (state == MachineState_Aborted) {
> + event = VIR_DOMAIN_EVENT_STOPPED;
> + detail = VIR_DOMAIN_EVENT_STOPPED_CRASHED;
> + } else if (state == MachineState_Saving) {
> + event = VIR_DOMAIN_EVENT_STOPPED;
> + detail = VIR_DOMAIN_EVENT_STOPPED_SAVED;
> + } else {
> + event = VIR_DOMAIN_EVENT_STOPPED;
> + detail = VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN;
> + }
> +
> +
> +static nsresult vboxCallbackOnExtraDataCanChange (IVirtualBoxCallback *pThis,
> + PRUnichar * machineId,
> + PRUnichar * key,
> + PRUnichar * value,
> + PRUnichar * * error,
> + PRBool * allowChange) {
> + DEBUG("IVirtualBoxCallback: %p, allowChange: %s", pThis, *allowChange ? "true" : "false");
> + DEBUGPRUnichar(machineId);
> + DEBUGPRUnichar(key);
> + DEBUGPRUnichar(value);
> + (void)error; /* so that the compiler doesn't complain about unsed variables */
> +
> + return NS_OK;
> +}
Just add ATTRIBUTE_UNUSED to the parameter declaration instead of (void)error;
> +
> +static nsresult vboxCallbackOnMachineRegistered (IVirtualBoxCallback *pThis,
> + PRUnichar * machineId,
> + PRBool registered) {
> + virDomainPtr dom = NULL;
> + int event = 0;
> + int detail = 0;
> +
> + g_pVBoxGlobalData->domainEventDispatching = 1;
> + vboxDriverLock(g_pVBoxGlobalData);
> +
> + DEBUG("IVirtualBoxCallback: %p, registered: %s", pThis, registered ? "true" : "false");
> + DEBUGPRUnichar(machineId);
> +
> + if (machineId) {
> + char *machineIdUtf8 = NULL;
> + unsigned char uuid[VIR_UUID_BUFLEN];
> +
> + g_pVBoxGlobalData->pFuncs->pfnUtf16ToUtf8(machineId, &machineIdUtf8);
> + virUUIDParse(machineIdUtf8, uuid);
> +
> + dom = vboxDomainLookupByUUID(g_pVBoxGlobalData->conn, uuid);
> + if (dom) {
> + int i = 0;
> +
> + /* CURRENT LIMITATION: we never get the VIR_DOMAIN_EVENT_UNDEFINED
> + * event becuase the when the machine is de-registered the call
> + * to vboxDomainLookupByUUID fails and thus we don't get any
> + * dom pointer which is necessary (null dom pointer doesn't work)
> + * to show the VIR_DOMAIN_EVENT_UNDEFINED event
> + */
Hmm, that's a little annoying. You already have the UUID, and 'id' is obviously
-1 for inactive guests. So only missing thing is the name :-(
> +
> +static nsresult vboxCallbackQueryInterface(nsISupports *pThis, const nsID *iid, void **resultp) {
> + IVirtualBoxCallback *that = (IVirtualBoxCallback *)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;
> +
> + DEBUG("pThis: %p, vboxCallback QueryInterface: %d", pThis, g_pVBoxGlobalData->vboxCallBackRefCount);
> +
> + return NS_OK;
> + }
> +
> +
> + DEBUG("pThis: %p, vboxCallback QueryInterface didn't find a matching interface", pThis);
> + DEBUGUUID(iid);
> + DEBUGUUID(&ivirtualboxCallbackUUID);
> + return NS_NOINTERFACE;
> +}
> +
> +
> +static IVirtualBoxCallback *vboxAllocCallbackObj (virConnectPtr conn) {
> + IVirtualBoxCallback *vboxCallback = NULL;
> +
> + /* Allocate, Initialize and return a validi
> + * IVirtualBoxCallback object here
> + */
> + if ((VIR_ALLOC(vboxCallback) < 0) || (VIR_ALLOC(vboxCallback->vtbl) < 0)) {
> + virReportOOMError(conn);
> + return NULL;
> + }
> +
> + {
> + vboxCallback->vtbl->nsisupports.AddRef = &vboxCallbackAddRef;
> + vboxCallback->vtbl->nsisupports.Release = &vboxCallbackRelease;
> + vboxCallback->vtbl->nsisupports.QueryInterface = &vboxCallbackQueryInterface;
> + vboxCallback->vtbl->OnMachineStateChange = &vboxCallbackOnMachineStateChange;
> + vboxCallback->vtbl->OnMachineDataChange = &vboxCallbackOnMachineDataChange;
> + vboxCallback->vtbl->OnExtraDataCanChange = &vboxCallbackOnExtraDataCanChange;
> + vboxCallback->vtbl->OnExtraDataChange = &vboxCallbackOnExtraDataChange;
> + vboxCallback->vtbl->OnMediaRegistered = &vboxCallbackOnMediaRegistered;
> + vboxCallback->vtbl->OnMachineRegistered = &vboxCallbackOnMachineRegistered;
> + vboxCallback->vtbl->OnSessionStateChange = &vboxCallbackOnSessionStateChange;
> + vboxCallback->vtbl->OnSnapshotTaken = &vboxCallbackOnSnapshotTaken;
> + vboxCallback->vtbl->OnSnapshotDiscarded = &vboxCallbackOnSnapshotDiscarded;
> + vboxCallback->vtbl->OnSnapshotChange = &vboxCallbackOnSnapshotChange;
> + vboxCallback->vtbl->OnGuestPropertyChange = &vboxCallbackOnGuestPropertyChange;
> + g_pVBoxGlobalData->vboxCallBackRefCount = 1;
> +
> + }
> +
> + return vboxCallback;
> +}
> +
> +static void vboxReadCallback(int watch ATTRIBUTE_UNUSED,
> + int fd,
> + int events ATTRIBUTE_UNUSED,
> + void *opaque ATTRIBUTE_UNUSED) {
> + if (fd >= 0) {
> + g_pVBoxGlobalData->vboxQueue->vtbl->ProcessPendingEvents(g_pVBoxGlobalData->vboxQueue);
> + } else {
> + nsresult rc;
> + PLEvent *pEvent = NULL;
> +
> + rc = g_pVBoxGlobalData->vboxQueue->vtbl->WaitForEvent(g_pVBoxGlobalData->vboxQueue, &pEvent);
> + if (NS_SUCCEEDED(rc))
> + g_pVBoxGlobalData->vboxQueue->vtbl->HandleEvent(g_pVBoxGlobalData->vboxQueue, pEvent);
> + }
> +}
> +
> +static int vboxDomainEventRegister (virConnectPtr conn,
> + virConnectDomainEventCallback callback,
> + void *opaque,
> + virFreeCallback freecb) {
> + nsresult rc;
> + vboxGlobalData *data = conn->privateData;
> + int vboxRet = -1;
> + int eventRet = -1;
> + int ret = -1;
> +
> + /* Locking has to be there as callbacks are not
> + * really fully thread safe
> + */
> + vboxDriverLock(data);
> +
> + if(data->vboxObj) {
> + if (data->vboxCallback == NULL) {
> + data->vboxCallback = vboxAllocCallbackObj(conn);
> + if (data->vboxCallback != NULL) {
> + rc = data->vboxObj->vtbl->RegisterCallback(data->vboxObj, data->vboxCallback);
> + if (NS_SUCCEEDED(rc)) {
> + vboxRet = 0;
> + }
> + }
> + } else {
> + vboxRet = 0;
> + }
> +
> + /* Get the vbox file handle and add a event handle to it
> + * so that the events can be passed down to the user
> + */
> + if (vboxRet == 0) {
> + PRInt32 vboxFileHandle;
> + vboxFileHandle = g_pVBoxGlobalData->vboxQueue->vtbl->GetEventQueueSelectFD(g_pVBoxGlobalData->vboxQueue);
> +
> + eventRet = virEventAddHandle(vboxFileHandle, VIR_EVENT_HANDLE_READABLE, vboxReadCallback, NULL, NULL);
> +
> + if (eventRet >= 0) {
You need to storage 'eventRet' in your virConnectPtr's privateData
so that later......
I'm also surprised you can pass in 'NULL' to the 'opaque' parameter of
virEventAddHandle(), because I'd expect your vboxReadCallback()
function to need to have a reference to the your 'data' object
or virConnectPtr object later.
> + /* Once a callback is registered with virtualbox, use a list
> + * to store the callbacks registered with libvirt so that
> + * later you can iterate over them
> + */
> +
> + ret = virDomainEventCallbackListAdd(conn, data->domainEventCallbacks,
> + callback, opaque, freecb);
> + DEBUG("virDomainEventCallbackListAdd (ret = %d) ( conn: %p, "
> + "data->domainEventCallbacks: %p, callback: %p, opaque: %p, "
> + "freecb: %p )", ret, conn, data->domainEventCallbacks, callback,
> + opaque, freecb);
> + }
> + }
> + }
> +
> + vboxDriverUnlock(data);
> +
> + if (ret >= 0) {
> + return ret;
> + } else {
> + if (data->vboxObj && data->vboxCallback) {
> + data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback);
> + }
> + return -1;
> + }
> +}
> +
> +static int vboxDomainEventDeregister (virConnectPtr conn,
> + virConnectDomainEventCallback callback) {
> + vboxGlobalData *data = conn->privateData;
> + int ret = -1;
> +
> + /* Locking has to be there as callbacks are not
> + * really fully thread safe
> + */
> + vboxDriverLock(data);
> +
> + if (data->domainEventDispatching)
> + ret = virDomainEventCallbackListMarkDelete(conn, data->domainEventCallbacks,
> + callback);
> + else
> + ret = virDomainEventCallbackListRemove(conn, data->domainEventCallbacks,
> + callback);
> +
> + virEventRemoveHandle(0);
.....here you can pass a real handle ID, instead of '0' which will
unregister some random callback that isn't neccessarily yours.
> +
> + if(data->vboxObj && data->vboxCallback) {
> + /* check count here of how many times register was called
> + * and only on the last de-register do the un-register call
> + */
> + if (data->domainEventCallbacks && (data->domainEventCallbacks->count <= 0)) {
> + int i = 0;
> +
> + data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback);
> + data->vboxCallback->vtbl->nsisupports.Release((nsISupports *)data->vboxCallback);
> +
> + /* iterate and free all the opaque objects using the
> + * freecb callback provided in vboxDomainEventRegister()
> + */
> + for (i = 0; i < data->domainEventCallbacks->count; i++) {
> + if (data->domainEventCallbacks->callbacks[i]->freecb) {
> + data->domainEventCallbacks->callbacks[i]->freecb(data->domainEventCallbacks->callbacks[i]->opaque);
> + }
> + }
> + }
> + }
> +
> + vboxDriverUnlock(data);
> +
> + return ret;
> +}
I don't know how hard it'd be to unpick this now, but it'd be nice to
have this in 2 patches, one adding support for version 3, and the 2nd
then implementing the event callbacks.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
More information about the libvir-list
mailing list