[libvirt] [PATCH 0/5] macvtap support for Qemu/KVM VMs via libvirt

[Daniel Veillard]

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 at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/