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
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list