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@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/