On Tue, Mar 11, 2008 at 06:47:47AM -0400, Daniel Veillard wrote:
On Mon, Mar 10, 2008 at 07:09:36PM +0000, Daniel P. Berrange wrote:
> When adding PolicyKit support we disabled the proxy driver, but did not
> correctly fix up the Xen unified driver. The result is that it is still
> trying to run the proxy setuid helper which doesn't exist and thus it fails
> the open operation before the remote driver gets the opportunity to process
> the URI. I attempted to fix this by just disabling the proxy driver in the
> unified driver, but came to the conclusion the logic of the current code is
> just not flexible enough for what we need to be able todo these days.
>
> THe core problem is the 'for(;;)' loop iterating over the drivers - it
> already has several special cases in the loop body to skip drivers, or
> ignore errors and adding more special cases is making my mind hurt trying
> to trace the logic.
>
> So I have removed the loop, and encode the desired logic explicitly. The
> diff a little unpleasant to read, so to summarize the logic is thus:
>
> - If root only, try open the hypervisor driver
> -> Failure to open is fatal, do not try other drivers
hum, I'm not 100% sure of that, an old libvirt version might still be
able to work though xend in face of an hypervisor change it can't handle,
we had the problem for example with 0.4.0 on xen-3.2, there was side effects
but it was basically working without hypervisor access...
This attached patch adds that use case too. Failure of the HV driver is
now non-fatal. The other rules below are unchagned
> - Try to open the XenD driver
> - If XenD suceeds
> -> If XenD < 3.0.4, then open the XM driver for inactive domains
> -> Try to open the XS driver
> => Failure to open is fatal if root
> - Else XenD fails
> ->.If proxy is compiled in, try to open proxy
> => Failure to open is fatal
>
>
> This should result in one of the following combinations of drivers being
> activated:
>
> root: (HV + XenD + XS)
> root: (HV + XenD + XS + XM)
root: (XenD + XS [+XM]) should still be allowed IMHO,
Dan.
Index: configure.in
===================================================================
RCS file: /data/cvs/libvirt/configure.in,v
retrieving revision 1.134
diff -u -p -r1.134 configure.in
--- configure.in 11 Mar 2008 14:49:04 -0000 1.134
+++ configure.in 17 Mar 2008 15:52:58 -0000
@@ -869,6 +869,9 @@ fi
AC_MSG_RESULT([$with_xen_proxy])
AM_CONDITIONAL(WITH_PROXY,[test "$with_xen_proxy" = "yes"])
+if test "$with_xen_proxy" = "yes"; then
+ AC_DEFINE(WITH_PROXY, 1, [Whether Xen proxy is enabled])
+fi
dnl Enable building libvirtd?
AM_CONDITIONAL(WITH_LIBVIRTD,[test "x$with_libvirtd" = "xyes"])
Index: src/remote_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/remote_internal.c,v
retrieving revision 1.62
diff -u -p -r1.62 remote_internal.c
--- src/remote_internal.c 17 Mar 2008 10:27:32 -0000 1.62
+++ src/remote_internal.c 17 Mar 2008 15:52:58 -0000
@@ -835,6 +835,14 @@ remoteOpen (virConnectPtr conn,
}
}
#endif
+#if WITH_XEN
+ if (uri &&
+ uri->scheme && STREQ (uri->scheme, "xen") &&
+ (!uri->server || STREQ (uri->server, "")) &&
+ (!uri->path || STREQ(uri->path, "/"))) {
+ rflags |= VIR_DRV_OPEN_REMOTE_UNIX;
+ }
+#endif
priv->magic = DEAD;
priv->sock = -1;
Index: src/xen_unified.c
===================================================================
RCS file: /data/cvs/libvirt/src/xen_unified.c,v
retrieving revision 1.38
diff -u -p -r1.38 xen_unified.c
--- src/xen_unified.c 27 Feb 2008 10:37:19 -0000 1.38
+++ src/xen_unified.c 17 Mar 2008 15:52:58 -0000
@@ -42,6 +42,7 @@
#include "util.h"
#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt,__VA_ARGS__)
+#define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg)
static int
xenUnifiedNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info);
@@ -239,7 +240,7 @@ xenUnifiedProbe (void)
static int
xenUnifiedOpen (virConnectPtr conn, xmlURIPtr uri, virConnectAuthPtr auth, int flags)
{
- int i, j;
+ int i;
xenUnifiedPrivatePtr priv;
/* Refuse any scheme which isn't "xen://" or "http://". */
@@ -276,41 +277,73 @@ xenUnifiedOpen (virConnectPtr conn, xmlU
priv->xshandle = NULL;
priv->proxy = -1;
- for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) {
- priv->opened[i] = 0;
- /* Only use XM driver for Xen <= 3.0.3 (ie xendConfigVersion <= 2) */
- if (drivers[i] == &xenXMDriver &&
- priv->xendConfigVersion > 2)
- continue;
-
- /* Ignore proxy for root */
- if (i == XEN_UNIFIED_PROXY_OFFSET && getuid() == 0)
- continue;
-
- if (drivers[i]->open) {
- DEBUG("trying Xen sub-driver %d", i);
- if (drivers[i]->open (conn, uri, auth, flags) == VIR_DRV_OPEN_SUCCESS)
- priv->opened[i] = 1;
- DEBUG("Xen sub-driver %d open %s\n",
- i, priv->opened[i] ? "ok" : "failed");
+ /* Hypervisor is only run as root & required to succeed */
+ if (getuid() == 0) {
+ DEBUG0("Trying hypervisor sub-driver");
+ if (drivers[XEN_UNIFIED_HYPERVISOR_OFFSET]->open(conn, uri, auth, flags) ==
+ VIR_DRV_OPEN_SUCCESS) {
+ DEBUG0("Activated hypervisor sub-driver");
+ priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET] = 1;
}
+ }
- /* If as root, then all drivers must succeed.
- If non-root, then only proxy must succeed */
- if (!priv->opened[i] &&
- (getuid() == 0 || i == XEN_UNIFIED_PROXY_OFFSET)) {
- for (j = 0; j < i; ++j)
- if (priv->opened[j]) drivers[j]->close (conn);
- free (priv);
- /* The assumption is that one of the underlying drivers
- * has set virterror already.
- */
- return VIR_DRV_OPEN_ERROR;
+ /* XenD is required to suceed if root.
+ * If it fails as non-root, then the proxy driver may take over
+ */
+ DEBUG0("Trying XenD sub-driver");
+ if (drivers[XEN_UNIFIED_XEND_OFFSET]->open(conn, uri, auth, flags) ==
+ VIR_DRV_OPEN_SUCCESS) {
+ DEBUG0("Activated XenD sub-driver");
+ priv->opened[XEN_UNIFIED_XEND_OFFSET] = 1;
+
+ /* XenD is active, so try the xm & xs drivers too, both requird to
+ * succeed if root, optional otherwise */
+ if (priv->xendConfigVersion <= 2) {
+ DEBUG0("Trying XM sub-driver");
+ if (drivers[XEN_UNIFIED_XM_OFFSET]->open(conn, uri, auth, flags) ==
+ VIR_DRV_OPEN_SUCCESS) {
+ DEBUG0("Activated XM sub-driver");
+ priv->opened[XEN_UNIFIED_XM_OFFSET] = 1;
+ }
+ }
+ DEBUG0("Trying XS sub-driver");
+ if (drivers[XEN_UNIFIED_XS_OFFSET]->open(conn, uri, auth, flags) ==
+ VIR_DRV_OPEN_SUCCESS) {
+ DEBUG0("Activated XS sub-driver");
+ priv->opened[XEN_UNIFIED_XS_OFFSET] = 1;
+ } else {
+ if (getuid() == 0)
+ goto fail; /* XS is mandatory as root */
+ }
+ } else {
+ if (getuid() == 0) {
+ goto fail; /* XenD is mandatory as root */
+ } else {
+#if WITH_PROXY
+ DEBUG0("Trying proxy sub-driver");
+ if (drivers[XEN_UNIFIED_PROXY_OFFSET]->open(conn, uri, auth, flags) ==
+ VIR_DRV_OPEN_SUCCESS) {
+ DEBUG0("Activated proxy sub-driver");
+ priv->opened[XEN_UNIFIED_PROXY_OFFSET] = 1;
+ } else {
+ goto fail; /* Proxy is mandatory if XenD failed */
+ }
+#else
+ DEBUG0("Handing off for remote driver");
+ return VIR_DRV_OPEN_DECLINED; /* Let remote_driver try instead */
+#endif
}
}
return VIR_DRV_OPEN_SUCCESS;
+
+ fail:
+ DEBUG0("Failed to activate a mandatory sub-driver");
+ for (i = 0 ; i < XEN_UNIFIED_NR_DRIVERS ; i++)
+ if (priv->opened[i]) drivers[i]->close(conn);
+ free(priv);
+ return VIR_DRV_OPEN_ERROR;
}
#define GET_PRIVATE(conn) \
Index: src/xend_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xend_internal.c,v
retrieving revision 1.174
diff -u -p -r1.174 xend_internal.c
--- src/xend_internal.c 17 Mar 2008 10:27:32 -0000 1.174
+++ src/xend_internal.c 17 Mar 2008 15:52:59 -0000
@@ -234,14 +234,13 @@ do_connect(virConnectPtr xend)
close(s);
errno = serrno;
s = -1;
- /*
- * not being able to connect via the socket as a normal user
- * is rather normal, this should fallback to the proxy (or
- * remote) mechanism.
- */
- if ((getuid() == 0) || (xend->flags & VIR_CONNECT_RO)) {
- virXendError(xend, VIR_ERR_INTERNAL_ERROR,
- _("failed to connect to xend"));
+
+ /*
+ * Connecting to XenD as root is mandatory, so log this error
+ */
+ if (getuid() == 0) {
+ virXendError(xend, VIR_ERR_INTERNAL_ERROR,
+ _("failed to connect to xend"));
}
}
--
|: Red Hat, Engineering, Boston -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|