[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] build: drop conditional use of mdns code



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]