On Tue, Feb 09, 2010 at 06:40:12PM +0000, Daniel P. Berrange wrote: > On Tue, Feb 09, 2010 at 10:21:20AM -0500, Stefan Berger wrote: > > Daniel, > > > > some of this code doesn't build anymore due to the recent changes with > > the conn parameter being removed. > > Do you want me to re-submit? > > Not at this time - there are a whole lot more patches I'm working on to > remove 'conn' from many other places which would just break your code > again. We can do any neccessary fixup to these macvtap patches at time > of commit. > > > I actually liked the conn parameter for error reporting and handling in > > the return path. Any function > > where the conn parameter was needed, I anticipated a simple return code > > for success and failure with the > > error already attached to the 'conn' parameter via one of the error > > reporting functions. Other functions > > where the conn parameter was not passed, the return value was anticipated > > to have an 'errno meaning'. Now > > that meaning seems lost. I am wondering whether I can still leave the conn > > parameter as an ATTRIBUTE_UNUSED > > for those functions where I only anticipate a success/failure return and > > error already being reported > > via a function? > > The main problem with the 'conn' parameter is that there are a huge > set of circumstances in which it will be 'NULL' and we've had too > many bugs where code assumed it would always be non-NULL. By removing > the conn parameter (nearly) everywhere in the internal APIs we can > get rid of this source of bugs. > > We've not really enumerated it anywhere, but as a general rule helper > functions should set the libvirt full error. I'd like to see the returning > of 'errno' reduced to be an exception, just used in places where the caller > needs to handle & ignore the potential errno. > > Again don't worry about updating these patches of yours yet - this error > reporting / conn cleanup is a big ongoing project.... I think the cleanup has been pushed to git now, so maybe it's worth rebasing, if you could also look at the my few comments about the patches in the process :-) thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list