On Tue, Jan 08, 2013 at 04:30:49PM -0700, Eric Blake wrote:
On 01/08/2013 01:47 PM, Daniel P. Berrange wrote:
On Mon, Jan 07, 2013 at 05:37:30PM -0700, Eric Blake wrote:
Touches quite a bit, but hopefully for the better. What platform are you targeting where you were unwilling to require gnutls as a prereq?
No specific platform as such, just that if you build with --without-remote and --without-libvirtd we should not be mandating use of gnutls. Various people have asked for this feature over the years, so I think it is worth it.
Overall, your patch looks sane, and you have a 'weak ACK' - that is, I'm willing to look the other way and let this patch go in, if you don't think it is worth even more refactoring to avoid quite so much leaky #ifdef throughout the code base.
Basically I'm following the approach used for SASL. It would be nice to try and adapt virnet{tls,sasl}context.c so that all the functions still exist, but have no-op impls, but that's much more work - I've tried it before with SASL but never got a satisfactory result
As it is, with your patch, I just got this failure on RHEL 5:
/usr/bin/perl ./check-symfile.pl l ibvirt.yms \ .libs/libvirt.so Expected symbol virNetServerClientGetTLSKeySize is not in ELF library ...
I still need to do more investigation, but it makes me wonder if we got the conditional symfile manipulation correct?
Yeah, actually I think that's something I forgot to handle. That said on RHEL5, GNUTLS should be present so that symbol ought to have been built, unless you were testing with --without-gnutls perhaps ? Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|