[libvirt] [PATCH] daemon: Always advertise libvirtd service

[Daniel P. Berrange]

On Tue, Oct 11, 2011 at 07:46:49PM +0800, Osier Yang wrote:
> This is a regression introduced by new RPC codes, previously
> we advertise the service via ssh even if the daemon doesn't
> listen on TLS port (TCP is not choosed). Now the service is
> only advertised when it listens on TLS or TCP port. This breaks
> upper layer apps which intends to discover the service, such
> as virt-manager.
> ---
>  daemon/libvirtd.c      |    6 ++++++
>  src/rpc/virnetserver.c |   22 ++++++++++++++--------
>  2 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index d1bc3dd..aa58121 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -531,6 +531,12 @@ static int daemonSetupNetworking(virNetServerPtr srv,
>  
>              virNetTLSContextFree(ctxt);
>          }
> +    } else {
> +        if (virNetServerAddService(srv, NULL,
> +                                   config->mdns_adv ?
> +                                   "_libvirt._tcp" :
> +                                   NULL) < 0)
> +            goto error;
>      }

This is a very nasty hack. If we want to advertise even when no TCP
socket is configured, then set the mdns entry name when we call
virNetServerAddService for the UNIX domain socket instead.


>  #if HAVE_SASL
> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index f739743..3cf36bc 100644
> --- a/src/rpc/virnetserver.c
> +++ b/src/rpc/virnetserver.c
> @@ -582,12 +582,16 @@ int virNetServerAddService(virNetServerPtr srv,
>  {
>      virNetServerLock(srv);
>  
> -    if (VIR_EXPAND_N(srv->services, srv->nservices, 1) < 0)
> -        goto no_memory;
> +    if (svc) {
> +        if (VIR_EXPAND_N(srv->services, srv->nservices, 1) < 0)
> +            goto no_memory;
> +    }
>  
>  #if HAVE_AVAHI
>      if (mdnsEntryName) {
> -        int port = virNetServerServiceGetPort(svc);
> +        int port = 0;
> +        if (svc)
> +            port = virNetServerServiceGetPort(svc);
>  
>          if (!virNetServerMDNSAddEntry(srv->mdnsGroup,
>                                        mdnsEntryName,
> @@ -596,12 +600,14 @@ int virNetServerAddService(virNetServerPtr srv,
>      }
>  #endif
>  
> -    srv->services[srv->nservices-1] = svc;
> -    virNetServerServiceRef(svc);
> +    if (svc) {
> +        srv->services[srv->nservices-1] = svc;
> +        virNetServerServiceRef(svc);
>  
> -    virNetServerServiceSetDispatcher(svc,
> -                                     virNetServerDispatchNewClient,
> -                                     srv);
> +        virNetServerServiceSetDispatcher(svc,
> +                                         virNetServerDispatchNewClient,
> +                                         srv);
> +    }
>  
>      virNetServerUnlock(srv);
>      return 0;

Ewww, NACK. Allowing NULL 'svc' is a horrible abuse of this API, whose
entire purpose is to register a non-NULL 'svc'.

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