[Libvir] A couple of questions about the Python bindings
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? 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. Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
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 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 should also clear it after virDomainDestroy if it doesn't already. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
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@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones