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....
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|