Re: [libvirt] [PATCH] Bind to ip_addr support.

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.

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

> + > + 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.