On Tue, Mar 27, 2007 at 02:44:13PM +0100, Daniel P. Berrange wrote:
On Tue, Mar 27, 2007 at 12:58:08PM +0100, Richard W.M. Jones wrote:
> virDomainGetUUIDString,
> virDomainPinVcpu,
> virNetworkGetUUIDString:
>
> These functions don't seem to check whether the parameters are
> correct. For example, virDomainGetUUIDString doesn't check if the
> buffer parameter passed from Python is large enough to take the UUID string.
>
> virDomainPinVcpu is just plain strange (but I guess that strangeness
> eminates from the Xen implementation), but it seems possible for the
> Python code to be wrong about the length of the Vcpu map (string).
> Shouldn't the length be taken from the string itself?
I'm pretty sure all 3 of those methods will need to be hand-written rather
than letting the generator do its thing. For the GetUUIDString() functions
Agreed with the analysis and suggested way to change this. This mean
adding the function names in the functions_skipped in generator.py,
extend libvirt-python-api.xml to add local descriptions for those
new bindings and add the bindings C implementation in libvir.c (I usually
start with the existing autogenerated bindings found in libvir-py.c
for the functions before rerunning the generator).
the C prototype has the caller pass in a pre-allocate char * - which
is
getting translated in Python as the caller passing in a string object. This
is dumb from the python POV where we have garbage collection. The Python
wrapper code should allocate the correct sized char * when calling the
GetUUIDString methods & then return a python string object. For the PingVCPU
method things are more complicated - in C its a large bit field, although it
is represented as a char *. In python I think we need to represent it as a
list of VCPU numbers.
> virDomainUndefine,
> virNetworkUndefine:
>
> It's unclear from the libvirt documentation, but it sounds as if
> these functions invalidate (free) the virDomainPtr / virNetworkPtr
> object passed to them. (In fact, the Xen implementation of virDomainPtr
> at least _doesn't_ free it - is that a bug?) If this is the case, then
> the Python bindings ought to set self._o = None.
Yes, the virDomainPtr objects ought to be free'd by these two functions,
so the python should clear the _o member after the calls complete. It
see function_post array in generator.py
should also clear it after virDomainDestroy if it doesn't
already.
yep, it does it's in function_post[] same for virDomainDestroy,
Daniel
--
Red Hat Virtualization group
http://redhat.com/virtualization/
Daniel Veillard | virtualization library
http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit
http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine
http://rpmfind.net/