[Libvir] PATCH: Fix xen unified driver open logic
Daniel P. Berrange
berrange at redhat.com
Mon Mar 17 16:02:06 UTC 2008
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 :|
More information about the libvir-list
mailing list