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(a)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 :|