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@xxxxxxxxxx>
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list