On Tue, Sep 18, 2007 at 04:13:46AM -0400, Daniel Veillard wrote:
On Tue, Sep 18, 2007 at 03:50:18AM +0100, Daniel P. Berrange wrote:
This is a serious patch at supporting Avahi advertisement of the libvirtd service.
- configure by default will probe to see if avahi is available and if found will enable appropriate code.
--with-avahi will force error if not found --without-avahi will disable it with no checking
- HAVE_AVAHI is defined in config.h if avahi is found & used to conditionally enable some code in qemud/qemud.c
- HAVE_AVAHI is also a Makefile conditional to enable compilation of the mdns.c and mdns.h files. A little makefile rearrangement was needed to make sure variables like EXTRA_DIST were defined before we appended to them with +=
- The code in mdns.c contains all the support for dealing with the Avahi APIs.
- The primary Avahi API is horribily complicated for day-to-day use in application code, exposing far too much of the event loop and state machine. So we expose a simplified API to libvirt in mdns.h
Heh, did you tell the Avahi devels ?
No, but perhaps I should. I don;t know if glib has a higher level API for accessing the Avahi stuff. I know there's a out-of-box glib event loop impl for it.
- The Avahi client library is basically a shim which talks to the Avahi daemon using DBus system daemon. The DBus stuff doesn't leak out of the Avahi APIs - it is loosely couple - all we need do is provide Avahi with an event loop implementation which was surprisingly easy. The libvirtd daemon does now indirectly link with DBus, but I don't see any problem with this. Don't want it - then use --without-avahi
That's fine, as long as the extensions don't decrease portability I don't see this as a problem.
Ok I tweaked the configure.in checks I did for avahi slightly. Now, if you don't have 'pkg-config' available, it'll simply disable the avahi feature instead of failing the entire script. It'll auto-detect by default, and can be overriden with the --with/--without-avahi flags.
- We advertise a service name of _libvirt._tcp The IETF draft recommends use of the name from /etc/services associated with your app. There is a way to register official Avahi services names. We don't have an /etc/service name registered either though.
I rememember we looked at the IANA stuff for registering a port number http://www.redhat.com/archives/libvir-list/2007-June/msg00204.html I suggested using "10 " What SHORT name (14 CHARACTER MAXIMUM) do you want associated with this port number?"
libvirt"
But we never did that registration step.
Ok, well our mDNS name matches. So guess we should look at actually doing a application to register both the mDNS & service name.
- This patch does not advertise any per-VM VNC server instances, but I have prepared the APIs in mdns.h to be ready to support that with minimal effort. A pre-requisite for this is an extension to the driver API to get async signals when VMs start & stop, since making the daemon poll hypervisors will suck big time.
When implemented each VM will be its own mdns 'group' and the VNC server associated with it will be the 'service' advertised in that group.
Having applied this patch & started the daemon, if /etc/init.d/avahi-daemon is running, you should see the service advertised on the LAN. As mentioned earlier if you start Avahi daemon after libvirt it should detect this too.
Sounds excellent !
I've now tested it on Fedora 7, 8 and RHEL-5 & its working very nicely.
These are all mildy abusing mdns / zeroconf, but then x509 certificates don't really fit into the model of 'zero conf' in the first place. If people want true zero-conf then the (SSH) tunnel is better (and always available), but if they've setup certificates they should still be allowed to use zero-conf to at least locate hosts. So mildly abusing the rules is reasonable IMHO.
Maybe suggesting that application developpers default to SSH when using a server autodetected with Avahi is the most practical ATM assuming we don't find a way to advertise the FQDN. Unless we can find the domain from the locally installed certificate, after all if people want to use the certificate they should have some installed locally and then we can probably guess the right domain name, no ?
Yeah, I reckon recommending SSH is a good general rule for this. I'll keep thinking about a better way to handle x509 validation by the client - we can easily add more TXT records to the mDNS stuff increemntally if needed.
+dnl Avahi library +AC_ARG_WITH(avahi, + [ --with-avahi use avahi to advertise remote daemon], + [], + [with_avahi=check]) + +if test "$with_avahi" = "check"; then + PKG_CHECK_EXISTS(avahi-client >= $AVAHI_REQUIRED, [with_avahi=yes], [with_avahi=no]) +fi + +if test "$with_avahi" = "yes"; then + PKG_CHECK_MODULES(AVAHI, avahi-client >= $AVAHI_REQUIRED) + AC_DEFINE_UNQUOTED(HAVE_AVAHI, 1, [whether Avahi is used to broadcast server presense]) +else + AVAHI_CFLAGS= + AVAHI_LIBS= +fi +AM_CONDITIONAL(HAVE_AVAHI, [test "$with_avahi" = "yes"]) +AC_SUBST(AVAHI_CFLAGS) +AC_SUBST(AVAHI_LIBS)
I assume that if an OS has Avahi, then it has pkg-check, in that case it really should not be a problem.
As mentioned above, I also tweaked it to disable Avahi if pkg-config is missing completely.
+ +if HAVE_AVAHI +libvirtd_SOURCES += mdns.c mdns.h +libvirtd_CFLAGS += $(AVAHI_CFLAGS) +libvirtd_LDADD += $(AVAHI_LIBS) +else +EXTRA_DIST += mdns.c mdns.h +endif
Wouldn't adding them to EXTRA_DIST in any case be good enough ? if SOURCES and EXTRA_DIST carry the same is that really a problem when building the archive ?
Yep, I made this change - adding them to EXTRA_DIST unconditionally.
Patch is surprizingly small, looks good, I would commit to CVS, no pending issue looks like a potential real problem for users, and more testing would be good.
Ok, its committed! Also added the BuildRequires to the spec file & docs on the two new config file options. 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 -=|