[Libvir] PATCH: Avahi advertisement for libvirtd

[Daniel P. Berrange]

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  -=|