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

Re: [libvirt] [PATCH] Fix build when using polkit0



On Mon, Jul 11, 2011 at 04:16:55PM -0600, Jim Fehlig wrote:
> Daniel P. Berrange wrote:
> > I'd like the virNetServer stuff to not have any mention of policy
> > kit in it. So rather than saying 'bool usePolkit', have a
> > 'bool connectDBus', and just remove those HAVE_POLKIT0 conditionals
> > so that the DBus API is always available in virNetServer. The changes
> > you made under daemon/ are all fine
> >   
> 
> The attached patch introduces a HAVE_DBUS conditional, which AFAICT is
> only used by polkit0 so is only set when polkit0 is detected during
> configure.  Is this what you had in mind?
> 
> Regards,
> Jim
> 

> From c21f206a8196cef7657e30b9ee830bcc4bbfc3ff Mon Sep 17 00:00:00 2001
> From: Jim Fehlig <jfehlig novell com>
> Date: Thu, 7 Jul 2011 15:12:26 -0600
> Subject: [V2] Fix build when using polkit0
> 
> V2: Remove policy kit references from virNetServer and use DBus APIs
>     directly, if available.
> ---
>  configure.ac           |    5 +++++
>  daemon/libvirtd.c      |   24 ++++--------------------
>  daemon/remote.c        |   21 ++++++++++-----------
>  src/Makefile.am        |    4 +++-
>  src/rpc/virnetserver.c |   41 ++++++++++++++++++++++++++++++++++++++++-
>  src/rpc/virnetserver.h |    8 ++++++++
>  6 files changed, 70 insertions(+), 33 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index ae747fb..e9d5be4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1010,6 +1010,7 @@ AC_ARG_WITH([polkit],
>    [with_polkit=check])
>  
>  with_polkit0=no
> +with_dbus=no
>  with_polkit1=no
>  if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then
>    dnl Check for new polkit first - just a binary
> @@ -1038,6 +1039,8 @@ if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then
>          [use PolicyKit for UNIX socket access checks])
>        AC_DEFINE_UNQUOTED([HAVE_POLKIT0], 1,
>          [use PolicyKit for UNIX socket access checks])
> +      AC_DEFINE_UNQUOTED([HAVE_DBUS], 1,
> +        [use DBus for PolicyKit])
>  
>        old_CFLAGS=$CFLAGS
>        old_LIBS=$LIBS
> @@ -1052,11 +1055,13 @@ if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then
>          AC_DEFINE_UNQUOTED([POLKIT_AUTH],["$POLKIT_AUTH"],[Location of polkit-auth program])
>        fi
>        with_polkit0="yes"
> +      with_dbus="yes"
>      fi
>    fi
>  fi
>  AM_CONDITIONAL([HAVE_POLKIT], [test "x$with_polkit" = "xyes"])
>  AM_CONDITIONAL([HAVE_POLKIT0], [test "x$with_polkit0" = "xyes"])
> +AM_CONDITIONAL([HAVE_DBUS], [test "x$with_dbus" = "xyes"])
>  AM_CONDITIONAL([HAVE_POLKIT1], [test "x$with_polkit1" = "xyes"])
>  AC_SUBST([POLKIT_CFLAGS])
>  AC_SUBST([POLKIT_LIBS])
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index a4198d9..c7ee605 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -576,26 +576,6 @@ static int daemonSetupNetworking(virNetServerPtr srv,
>      }
>  #endif
>  
> -#if HAVE_POLKIT0
> -    if (auth_unix_rw == REMOTE_AUTH_POLKIT ||
> -        auth_unix_ro == REMOTE_AUTH_POLKIT) {
> -        DBusError derr;
> -
> -        dbus_connection_set_change_sigpipe(FALSE);
> -        dbus_threads_init_default();
> -
> -        dbus_error_init(&derr);
> -        server->sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr);
> -        if (!(server->sysbus)) {
> -            VIR_ERROR(_("Failed to connect to system bus for PolicyKit auth: %s"),
> -                      derr.message);
> -            dbus_error_free(&derr);
> -            goto error;
> -        }
> -        dbus_connection_set_exit_on_disconnect(server->sysbus, FALSE);
> -    }
> -#endif
> -
>      return 0;
>  
>  error:
> @@ -1285,6 +1265,7 @@ int main(int argc, char **argv) {
>      struct daemonConfig *config;
>      bool privileged = geteuid() == 0 ? true : false;
>      bool implicit_conf = false;
> +    bool use_polkit_dbus;
>  
>      struct option opts[] = {
>          { "verbose", no_argument, &verbose, 1},
> @@ -1445,10 +1426,13 @@ int main(int argc, char **argv) {
>          umask(old_umask);
>      }
>  
> +    use_polkit_dbus = config->auth_unix_rw == REMOTE_AUTH_POLKIT ||
> +            config->auth_unix_ro == REMOTE_AUTH_POLKIT;
>      if (!(srv = virNetServerNew(config->min_workers,
>                                  config->max_workers,
>                                  config->max_clients,
>                                  config->mdns_adv ? config->mdns_name : NULL,
> +                                use_polkit_dbus,
>                                  remoteClientInitHook))) {
>          ret = VIR_DAEMON_ERR_INIT;
>          goto cleanup;
> diff --git a/daemon/remote.c b/daemon/remote.c
> index a2e79ef..0172626 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -43,6 +43,7 @@
>  #include "command.h"
>  #include "intprops.h"
>  #include "virnetserverservice.h"
> +#include "virnetserver.h"
>  
>  #include "remote_protocol.h"
>  #include "qemu_protocol.h"
> @@ -2115,7 +2116,7 @@ authdeny:
>  }
>  #elif HAVE_POLKIT0
>  static int
> -remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
> +remoteDispatchAuthPolkit(virNetServerPtr server,
>                           virNetServerClientPtr client,
>                           virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED,
>                           virNetMessageErrorPtr rerr,
> @@ -2137,21 +2138,19 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
>  
>      memset(ident, 0, sizeof ident);
>  
> -    virMutexLock(&server->lock);
> -    virMutexLock(&client->lock);
> -    virMutexUnlock(&server->lock);
> +    virMutexLock(&priv->lock);
>  
> -    action = client->readonly ?
> +    action = virNetServerClientGetReadonly(client) ?
>          "org.libvirt.unix.monitor" :
>          "org.libvirt.unix.manage";
>  
>      VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client));
> -    if (client->auth != REMOTE_AUTH_POLKIT) {
> +    if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) {
>          VIR_ERROR(_("client tried invalid PolicyKit init request"));
>          goto authfail;
>      }
>  
> -    if (qemudGetSocketIdentity(virNetServerClientGetFD(client), &callerUid, &callerPid) < 0) {
> +    if (virNetServerClientGetLocalIdentity(client, &callerUid, &callerPid) < 0) {
>          VIR_ERROR(_("cannot get peer socket identity"));
>          goto authfail;
>      }
> @@ -2164,7 +2163,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
>  
>      VIR_INFO("Checking PID %d running as %d", callerPid, callerUid);
>      dbus_error_init(&err);
> -    if (!(pkcaller = polkit_caller_new_from_pid(server->sysbus,
> +    if (!(pkcaller = polkit_caller_new_from_pid(virNetServerGetDBusConn(server),
>                                                  callerPid, &err))) {
>          VIR_ERROR(_("Failed to lookup policy kit caller: %s"), err.message);
>          dbus_error_free(&err);
> @@ -2226,9 +2225,9 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
>               action, callerPid, callerUid,
>               polkit_result_to_string_representation(pkresult));
>      ret->complete = 1;
> -    client->auth = REMOTE_AUTH_NONE;
> +    virNetServerClientSetIdentity(client, ident);
>  
> -    virMutexUnlock(&client->lock);
> +    virMutexUnlock(&priv->lock);
>      return 0;
>  
>  error:
> @@ -2236,7 +2235,7 @@ error:
>      virNetError(VIR_ERR_AUTH_FAILED, "%s",
>                  _("authentication failed"));
>      virNetMessageSaveError(rerr);
> -    virMutexUnlock(&client->lock);
> +    virMutexUnlock(&priv->lock);
>      return -1;
>  
>  authfail:
> diff --git a/src/Makefile.am b/src/Makefile.am
> index cd8a7e9..a3ee8ba 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1268,10 +1268,12 @@ EXTRA_DIST += \
>  endif
>  libvirt_net_rpc_server_la_CFLAGS = \
>  			$(AVAHI_CFLAGS) \
> -			$(AM_CFLAGS)
> +			$(AM_CFLAGS) \
> +			$(POLKIT_CFLAGS)
>  libvirt_net_rpc_server_la_LDFLAGS = \
>  			$(AM_LDFLAGS) \
>  			$(AVAHI_LIBS) \
> +			$(POLKIT_LIBS) \
>  			$(CYGWIN_EXTRA_LDFLAGS) \
>  			$(MINGW_EXTRA_LDFLAGS)
>  libvirt_net_rpc_server_la_LIBADD = \
> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index 5e1719b..94d46f6 100644
> --- a/src/rpc/virnetserver.c
> +++ b/src/rpc/virnetserver.c
> @@ -39,6 +39,9 @@
>  #if HAVE_AVAHI
>  # include "virnetservermdns.h"
>  #endif
> +#if HAVE_DBUS
> +# include <dbus/dbus.h>
> +#endif
>  
>  #define VIR_FROM_THIS VIR_FROM_RPC
>  #define virNetError(code, ...)                                    \
> @@ -84,6 +87,10 @@ struct _virNetServer {
>      virNetServerMDNSGroupPtr mdnsGroup;
>  #endif
>  
> +#if HAVE_DBUS
> +    DBusConnection *sysbus;
> +#endif
> +
>      size_t nservices;
>      virNetServerServicePtr *services;
>  
> @@ -270,6 +277,7 @@ virNetServerPtr virNetServerNew(size_t min_workers,
>                                  size_t max_workers,
>                                  size_t max_clients,
>                                  const char *mdnsGroupName,
> +                                bool connectDBus,
>                                  virNetServerClientInitHook clientInitHook)
>  {
>      virNetServerPtr srv;
> @@ -306,6 +314,25 @@ virNetServerPtr virNetServerNew(size_t min_workers,
>      }
>  #endif
>  
> +#if HAVE_DBUS
> +    if (connectDBus) {
> +        DBusError derr;
> +
> +        dbus_connection_set_change_sigpipe(FALSE);
> +        dbus_threads_init_default();
> +
> +        dbus_error_init(&derr);
> +        srv->sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr);
> +        if (!(srv->sysbus)) {
> +            VIR_ERROR(_("Failed to connect to system bus for PolicyKit auth: %s"),
> +                      derr.message);
> +            dbus_error_free(&derr);
> +            goto error;
> +        }
> +        dbus_connection_set_exit_on_disconnect(srv->sysbus, FALSE);
> +    }
> +#endif
> +
>      if (virMutexInit(&srv->lock) < 0) {
>          virNetError(VIR_ERR_INTERNAL_ERROR, "%s",
>                      _("cannot initialize mutex"));
> @@ -363,6 +390,14 @@ bool virNetServerIsPrivileged(virNetServerPtr srv)
>  }
>  
>  
> +#if HAVE_DBUS
> +DBusConnection* virNetServerGetDBusConn(virNetServerPtr srv)
> +{
> +    return srv->sysbus;
> +}
> +#endif
> +
> +
>  void virNetServerAutoShutdown(virNetServerPtr srv,
>                                unsigned int timeout,
>                                virNetServerAutoShutdownFunc func,
> @@ -377,7 +412,6 @@ void virNetServerAutoShutdown(virNetServerPtr srv,
>      virNetServerUnlock(srv);
>  }
>  
> -
>  static sig_atomic_t sigErrors = 0;
>  static int sigLastErrno = 0;
>  static int sigWrite = -1;
> @@ -747,6 +781,11 @@ void virNetServerFree(virNetServerPtr srv)
>  
>      VIR_FREE(srv->mdnsGroupName);
>  
> +#if HAVE_DBUS
> +    if (srv->sysbus)
> +        dbus_connection_unref(srv->sysbus);
> +#endif
> +
>      virNetServerUnlock(srv);
>      virMutexDestroy(&srv->lock);
>      VIR_FREE(srv);
> diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
> index 6e7a21b..810d8a3 100644
> --- a/src/rpc/virnetserver.h
> +++ b/src/rpc/virnetserver.h
> @@ -25,6 +25,9 @@
>  # define __VIR_NET_SERVER_H__
>  
>  # include <signal.h>
> +# if HAVE_DBUS
> +#  include <dbus/dbus.h>
> +# endif
>  
>  # include "virnettlscontext.h"
>  # include "virnetserverprogram.h"
> @@ -38,6 +41,7 @@ virNetServerPtr virNetServerNew(size_t min_workers,
>                                  size_t max_workers,
>                                  size_t max_clients,
>                                  const char *mdnsGroupName,
> +                                bool connectDBus,
>                                  virNetServerClientInitHook clientInitHook);
>  
>  typedef int (*virNetServerAutoShutdownFunc)(virNetServerPtr srv, void *opaque);
> @@ -46,6 +50,10 @@ void virNetServerRef(virNetServerPtr srv);
>  
>  bool virNetServerIsPrivileged(virNetServerPtr srv);
>  
> +# if HAVE_DBUS
> +DBusConnection* virNetServerGetDBusConn(virNetServerPtr srv);
> +# endif
> +
>  void virNetServerAutoShutdown(virNetServerPtr srv,
>                                unsigned int timeout,
>                                virNetServerAutoShutdownFunc func,

ACK, this looks fine now

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]