Re: [libvirt] [PATCH 1/1] Support for Callbacks and VirtualBox Version 3.

> I think your .gitconfig needs email addr fixing :-) thanks did it ;) > 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); Did this. > > + 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. No it also occurs when the machine is being restored from a saved state, but then again the machine was paused while saving it so I guess it is safe to consider it occurs only after unpausing the machine. > > + (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; done. > > + > > + /* 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 :-( the main problem here is that since we are in the callback of the machine which was just de-registered, we can't get back to virtualbox and ask for the name of the machine cause the machine is no more :( and thus the problem. i am trying to update this part so that the callback also gives the machine name, but it will take some time. > > + if (vboxRet == 0) { > > + PRInt32 vboxFileHandle; > > + vboxFileHandle = > > g_pVBoxGlobalData->vboxQueue->vtbl->GetEventQueueSelectFD(g_pVBoxGlobalDa > >ta->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. fixing this.. > > + > > + virEventRemoveHandle(0); > > .....here you can pass a real handle ID, instead of '0' which will > unregister some random callback that isn't neccessarily yours. and this.. > 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. will surely try this, but not sure if it'd be easy, too many #if's :( the patch with above changes is attached below: