On Fri, Aug 03, 2012 at 04:33:17PM -0600, Eric Blake wrote:

Commit 1f6f723 missed a step. At first I was worried that scrubbing the conditionals would lead to a runtime failure when compiled without avahi, but my testing makes it appear that the runtime error will only occur if the .conf files in /etc request mdns advertisement; and the old behavior was to silently ignore the request, so this is actually a better behavior of explicitly failing only when the config requests the impossible.

* src/rpc/virnetserver.c: Drop HAVE_AVAHI conditionals; all callers already passed NULL if mdns_adv was not configured. ---

In response to: https://www.redhat.com/archives/libvir-list/2012-August/msg00130.html I originally thought it would be harder than this, but my testing seems to show that this works. Caveat: my testing without avahi was done in a VM and not on bare metal, and since nested virt is slower, I may have inadvertently cut an important corner and missed a flaw in my above reasoning about why this is safe. So a close review, including checking all call sites, would be appreciated.

src/rpc/virnetserver.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 248ad9f..295e8fd 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -36,9 +36,7 @@ #include "util.h" #include "virfile.h" #include "event.h" -#if HAVE_AVAHI -# include "virnetservermdns.h" -#endif +#include "virnetservermdns.h"

#ifndef SA_SIGINFO # define SA_SIGINFO 0 @@ -81,10 +79,8 @@ struct _virNetServer { int sigwatch;

char *mdnsGroupName; -#if HAVE_AVAHI virNetServerMDNSPtr mdns; virNetServerMDNSGroupPtr mdnsGroup; -#endif

size_t nservices; virNetServerServicePtr *services; @@ -364,7 +360,6 @@ virNetServerPtr virNetServerNew(size_t min_workers, virReportOOMError(); goto error; } -#if HAVE_AVAHI if (srv->mdnsGroupName) { if (!(srv->mdns = virNetServerMDNSNew())) goto error; @@ -372,7 +367,6 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv->mdnsGroupName))) goto error; } -#endif

if (virMutexInit(&srv->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -592,14 +586,13 @@ error:

int virNetServerAddService(virNetServerPtr srv, virNetServerServicePtr svc, - const char *mdnsEntryName ATTRIBUTE_UNUSED) + const char *mdnsEntryName) { virNetServerLock(srv);

if (VIR_EXPAND_N(srv->services, srv->nservices, 1) < 0) goto no_memory;

-#if HAVE_AVAHI if (mdnsEntryName) { int port = virNetServerServiceGetPort(svc);

@@ -608,7 +601,6 @@ int virNetServerAddService(virNetServerPtr srv, port)) goto error; } -#endif

srv->services[srv->nservices-1] = svc; virNetServerServiceRef(svc); @@ -622,9 +614,7 @@ int virNetServerAddService(virNetServerPtr srv,

no_memory: virReportOOMError(); -#if HAVE_AVAHI error: -#endif virNetServerUnlock(srv); return -1; } @@ -694,11 +684,9 @@ void virNetServerRun(virNetServerPtr srv)

virNetServerLock(srv);

-#if HAVE_AVAHI if (srv->mdns && virNetServerMDNSStart(srv->mdns) < 0) goto cleanup; -#endif

srv->quit = 0;

@@ -826,9 +814,7 @@ void virNetServerFree(virNetServerPtr srv) VIR_FREE(srv->clients);

VIR_FREE(srv->mdnsGroupName); -#if HAVE_AVAHI virNetServerMDNSFree(srv->mdns); -#endif

virMutexDestroy(&srv->lock); VIR_FREE(srv);

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