[libvirt] [PATCH] Bind to ip_addr support.

[Stefan de Konink]

On Tue, 13 May 2008, Daniel P. Berrange wrote:

> On Sat, May 10, 2008 at 07:42:51PM +0200, Stefan de Konink wrote:
> > About mdns, do you think it would be a good thing to *not* follow the
> > Avahi advise and explicitly set the host the service is running on? In my
> > humble opinion I think that would be a wise decision.
> >
> > host 	The host this services is residing on. We recommend to pass NULL
> > here, the daemon will than automatically insert the local host name in
> > that case
> >
> > I wonder how avahi will decide if the service doesn't run on 0.0.0.0 but
> > on a specific address. Then we still have domain left, but with a bit
> > smart implementation upstream that could just work.
>
> Well the  'host' parameter of the avahi_entry_group_add_service() method
> does not control which interfaces Avahi broadcasts on. It merely controls
> the hostname included in the advertisement. So if a machine had 2 nics and
> libvirt was only listening on one NIC, by setting this explicitly it means
> we'd be broadcasting on both NICs still, but the advertisment will have an
> IP address that's not reachable via one of the NICs.

I have have send a patch to avahi to prevent this. In avahi you are now
able to set the actual interface that is allowed to be broadcasted on.
allow-interfaces and deny-interfaces.


> > The attached patch explicitly sets the host of the mDNS advertisement.
> >
> >
> > Because of some 'unexpected' behavior... I hoped strdup(NULL) would just
> > work, but it didn't, I placed it inside an if loop. The debug message
> > doesn't mind '(null)' but I think 0.0.0.0 is more appropriate.
>
> NB, libvirt is explicitly written to be protocol independant - so avoid
> refering to '0.0.0.0' which is IPv4 specific. We fully support IPv6 and
> it is enabled by default if the host OS has IPv6 enabled (which all
> Fedora since FC6 do)
>
> > @@ -440,18 +441,28 @@
> >      }
> >  }
> >
> > -struct libvirtd_mdns_entry *libvirtd_mdns_add_entry(struct libvirtd_mdns_group *group, const char *type, int port) {
> > +struct libvirtd_mdns_entry *libvirtd_mdns_add_entry(struct libvirtd_mdns_group *group, const char *type, const char *host, int port) {
> >      struct libvirtd_mdns_entry *entry = malloc(sizeof(*entry));
> >
> > -    AVAHI_DEBUG("Adding entry %s %d to group %s", type, port, group->name);
> > +    AVAHI_DEBUG("Adding entry %s %s %d to group %s", type, (host == NULL ? "0.0.0.0" : host), port, group->name);
>
> This is misleading - NULL indicates bind to all addresses which includes
> IPv6 if enabled - 0.0.0.0 is IPv4 only. Since this is only a debug message
> its not too troubling. Just pass in 'host' unchanged - sprintf will output
> it as '(null)' which is more accurate.

Oki.

> > +
> > +    if (host != NULL) {
> > +        if (!(entry->host = strdup(host))) {
> > +	    free(entry->type);
> > +            free(entry);
>
> There is indentation whitespace damage here. Please check the HACKING
> file for details of how to setup vim and emacs to comply with libvirt
> indentation rules automatically.

Update and resubmit?



Stefan