Comments inline below Daniel Veillard wrote on 10/29/2008 01:09 PM: ... >> +def myDomainEventCallback1 (conn, dom, event, opaque): >> + print "myDomainEventCallback1 EVENT: Domain %s(%s) %s" % (dom.name(), dom.ID(), eventToString(event)) >> + >> +def myDomainEventCallback2 (conn, dom, event, opaque): >> + print "myDomainEventCallback2 EVENT: Domain %s(%s) %s" % (dom.name(), dom.ID(), eventToString(event)) > > Thinking out loud, maybe we should allow dom to be NULL/None in > examples, if we extend the API later to add node related events dom > would be NULL, isn't it I was under the impression that re-use of this API was undesired, and that the events API would be extended to call out each event class explicitly (IIRC, Daniel B suggested this) If we were to extend the API, there would be a virConnectNodeEventRegister/Deregister and associated callbacks, with their own signatures. So - we would not be mxing NodeEventCallbacks, and DomainEventCallbacks with the same python code. > [...] >> +########################################## >> +# Main >> +########################################## >> + >> +def usage(): >> + print "usage: "+os.path.basename(sys.argv[0])+" [uri]" >> + print " uri will default to qemu:///system" > > ideally for regression testing it would be nice to be able to provide > a test driver definition, but augmented with an emulated scenario, > things like: > <scenario> > <event type="start"> > <domain type='test2'> > <name>test2</name> > <memory>512000</memory> > <currentMemory>512000</currentMemory> > <vcpu>2</vcpu> > <os> > <type arch='i686'>hvm</type> > <boot dev='hd'/> > </os> > </domain> > </event> > <event type="pause"> > <domain name="test"/> > </event> > <event type="resume"> > <domain name="test"/> > </event> > <event type="destroy"> > <domain name="test2"/> > </event> > </scenario> I really have no knowledge of how the test driver works, or how to design a test of this code. Ad discussed in an earlier thread - since this event code depends on OS state, I made no effort to design tests to exercise the code, beyond the example app. While I agree that a regression test would be desirable, I really don't know how that would be accomplished. ... >> +static PyObject * >> +libvirt_virEventRegisterImpl(ATTRIBUTE_UNUSED PyObject * self, >> + PyObject * args) >> +{ >> + Py_XDECREF(addHandleObj); >> + Py_XDECREF(updateHandleObj); >> + Py_XDECREF(removeHandleObj); >> + Py_XDECREF(addTimeoutObj); >> + Py_XDECREF(updateTimeoutObj); >> + Py_XDECREF(removeTimeoutObj); > > hum, maybe the ParseTuple should be done before onto temporary > variables and then only DECREF, right now if the parse fails, then > the next event may lead to a crash due to garbage collected code :-) > hmmm...maybe I'm misunderstanding how XDECREF works...I thought it would not dec the ref if the object was NULL Other than the above comments, I think the other suggestions are good ones. I'll take a look. Ben -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list