[libvirt] [PATCH] Domain Events Python Bindings
Ben Guthro
bguthro at virtualiron.com
Wed Oct 29 17:33:35 UTC 2008
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
More information about the libvir-list
mailing list