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

[Libvir] PATCH: Fix xen unified driver open logic



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

 - 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)
 non-root: (XenD)
 non-root: (XenD + XS)
 non-root: (proxy)

If non-root, and the proxy is not compiled in, we'll hand off to the remote
driver. Any other scenario will result in an explicit fail.

Dan.

? docs/apibuild.pyc
Index: configure.in
===================================================================
RCS file: /data/cvs/libvirt/configure.in,v
retrieving revision 1.133
diff -u -p -r1.133 configure.in
--- configure.in	3 Mar 2008 14:42:37 -0000	1.133
+++ configure.in	10 Mar 2008 18:59:23 -0000
@@ -873,6 +873,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.61
diff -u -p -r1.61 remote_internal.c
--- src/remote_internal.c	26 Feb 2008 07:05:18 -0000	1.61
+++ src/remote_internal.c	10 Mar 2008 18:59:24 -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	10 Mar 2008 18:59:24 -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 +242,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 +279,69 @@ 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)
+            goto fail;
+
+        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) {
+        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) {
+                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) {
+            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) {
+                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 mandatory 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.173
diff -u -p -r1.173 xend_internal.c
--- src/xend_internal.c	7 Mar 2008 09:23:30 -0000	1.173
+++ src/xend_internal.c	10 Mar 2008 18:59:24 -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 :|


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