[Libvir] [PATCH] Don't throw away errors in virConnectOpen (at least not so often)
Error messages, I like them. I don't like them to be thrown away. A nice feature of virterror is that it'll throw away errors under the following conditions: (1) You are in virConnectOpen, and (2) You pass a non-NULL virConnectPtr to __virRaiseError. libvirt has a lot of errors which meet those conditions - the attached patch fixes the ones I could find. It also fixes qemuOpenConnection so that it doesn't try to open a Unix socket with random stack data. It also adds error messages in some useful places where previously there was an error, but no message. Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 Directors: Michael Cunningham (USA), Charlie Peters (USA) and David Owens (Ireland)
On Tue, May 01, 2007 at 03:04:32PM +0100, Richard W.M. Jones wrote:
Error messages, I like them. I don't like them to be thrown away.
A nice feature of virterror is that it'll throw away errors under the following conditions: (1) You are in virConnectOpen, and (2) You pass a non-NULL virConnectPtr to __virRaiseError.
libvirt has a lot of errors which meet those conditions - the attached patch fixes the ones I could find.
It also fixes qemuOpenConnection so that it doesn't try to open a Unix socket with random stack data.
It also adds error messages in some useful places where previously there was an error, but no message.
In general that looks okay, but instead of passing NULL as the first argument can't we do a selection in __virRaiseError instead based on the type of error and just avoid virConnectOpen errors which are not related to unavailability of the virtualization ? 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/
On Wed, May 02, 2007 at 08:15:11AM -0400, Daniel Veillard wrote:
On Tue, May 01, 2007 at 03:04:32PM +0100, Richard W.M. Jones wrote:
Error messages, I like them. I don't like them to be thrown away.
A nice feature of virterror is that it'll throw away errors under the following conditions: (1) You are in virConnectOpen, and (2) You pass a non-NULL virConnectPtr to __virRaiseError.
libvirt has a lot of errors which meet those conditions - the attached patch fixes the ones I could find.
It also fixes qemuOpenConnection so that it doesn't try to open a Unix socket with random stack data.
It also adds error messages in some useful places where previously there was an error, but no message.
In general that looks okay, but instead of passing NULL as the first argument can't we do a selection in __virRaiseError instead based on the type of error and just avoid virConnectOpen errors which are not related to unavailability of the virtualization ?
That doesn't make sense to me. The issue here is that libvirt has two places where it stores error details for the caller: - The global error object - The per-connection error object If the virConnectOpen call fails, then the caller has no virConnectPtr object available. So every codepath in virConnectOpen should be using the global error object & hence always be passing NULL to __virRaiseError. So I think Rich's patch is already doing the correct thing AFAICT 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 -=|
This patch has been committed. Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 Directors: Michael Cunningham (USA), Charlie Peters (USA) and David Owens (Ireland)
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones