Simplify opening of Xen drivers

Since the Xen driver was changed to only execute inside libvirtd,
there is no scenario in which it will be opened from a non-privileged
context. Thus all the code dealing with opening the sub-drivers can
be simplified to assume that they are always privileged.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

src/xen/xen_driver.c

@@ -86,14 +86,9 @@ static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = {
     [XEN_UNIFIED_XEND_OFFSET] = &xenDaemonDriver,
     [XEN_UNIFIED_XS_OFFSET] = &xenStoreDriver,
     [XEN_UNIFIED_XM_OFFSET] = &xenXMDriver,
-#if WITH_XEN_INOTIFY
-    [XEN_UNIFIED_INOTIFY_OFFSET] = &xenInotifyDriver,
-#endif
 };
 
-#if defined WITH_LIBVIRTD || defined __sun
+static bool is_privileged = false;
-static bool inside_daemon = false;
-#endif
 
 /**
  * xenNumaInit:
@@ -200,14 +195,14 @@ done:
     return res;
 }
 
-#ifdef WITH_LIBVIRTD
-
 static int
-xenUnifiedStateInitialize(bool privileged ATTRIBUTE_UNUSED,
+xenUnifiedStateInitialize(bool privileged,
                           virStateInhibitCallback callback ATTRIBUTE_UNUSED,
                           void *opaque ATTRIBUTE_UNUSED)
 {
-    inside_daemon = true;
+    /* Don't allow driver to work in non-root libvirtd */
+    if (privileged)
+        is_privileged = true;
     return 0;
 }
 
@@ -216,8 +211,6 @@ static virStateDriver state_driver = {
     .stateInitialize = xenUnifiedStateInitialize,
 };
 
-#endif
-
 /*----- Dispatch functions. -----*/
 
 /* These dispatch functions follow the model used historically
@@ -298,18 +291,15 @@ xenDomainXMLConfInit(void)
 static virDrvOpenStatus
 xenUnifiedConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags)
 {
-    int i, ret = VIR_DRV_OPEN_DECLINED;
     xenUnifiedPrivatePtr priv;
     char ebuf[1024];
 
-#ifdef __sun
     /*
      * Only the libvirtd instance can open this driver.
      * Everything else falls back to the remote driver.
      */
-    if (!inside_daemon)
+    if (!is_privileged)
         return VIR_DRV_OPEN_DECLINED;
-#endif
 
     if (conn->uri == NULL) {
         if (!xenUnifiedProbe())
@@ -379,110 +369,108 @@ xenUnifiedConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int f
     priv->xshandle = NULL;
 
 
-    /* Hypervisor is only run with privilege & required to succeed */
+    /* Hypervisor required to succeed */
-    if (xenHavePrivilege()) {
+    VIR_DEBUG("Trying hypervisor sub-driver");
-        VIR_DEBUG("Trying hypervisor sub-driver");
+    if (xenHypervisorOpen(conn, auth, flags) < 0)
-        if (xenHypervisorOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) {
+        goto error;
-            VIR_DEBUG("Activated hypervisor sub-driver");
+    VIR_DEBUG("Activated hypervisor sub-driver");
-            priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET] = 1;
+    priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET] = 1;
-        } else {
-            goto fail;
-        }
-    }
 
-    /* XenD is required to succeed if privileged */
+    /* XenD is required to succeed */
     VIR_DEBUG("Trying XenD sub-driver");
-    if (xenDaemonOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) {
+    if (xenDaemonOpen(conn, auth, flags) < 0)
-        VIR_DEBUG("Activated XenD sub-driver");
+        goto error;
-        priv->opened[XEN_UNIFIED_XEND_OFFSET] = 1;
+    VIR_DEBUG("Activated XenD sub-driver");
+    priv->opened[XEN_UNIFIED_XEND_OFFSET] = 1;
 
-        /* XenD is active, so try the xm & xs drivers too, both requird to
+    /* For old XenD, the XM driver is required to succeed */
-         * succeed if root, optional otherwise */
+    if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) {
-        if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) {
+        VIR_DEBUG("Trying XM sub-driver");
-            VIR_DEBUG("Trying XM sub-driver");
+        if (xenXMOpen(conn, auth, flags) < 0)
-            if (xenXMOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) {
+            goto error;
-                VIR_DEBUG("Activated XM sub-driver");
+        VIR_DEBUG("Activated XM sub-driver");
-                priv->opened[XEN_UNIFIED_XM_OFFSET] = 1;
+        priv->opened[XEN_UNIFIED_XM_OFFSET] = 1;
-            }
-        }
-        VIR_DEBUG("Trying XS sub-driver");
-        if (xenStoreOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) {
-            VIR_DEBUG("Activated XS sub-driver");
-            priv->opened[XEN_UNIFIED_XS_OFFSET] = 1;
-        } else {
-            if (xenHavePrivilege())
-                goto fail; /* XS is mandatory when privileged */
-        }
-    } else {
-        if (xenHavePrivilege()) {
-            goto fail; /* XenD is mandatory when privileged */
-        } else {
-            VIR_DEBUG("Handing off for remote driver");
-            ret = VIR_DRV_OPEN_DECLINED; /* Let remote_driver try instead */
-            goto clean;
-        }
     }
 
+    VIR_DEBUG("Trying XS sub-driver");
+    if (xenStoreOpen(conn, auth, flags) < 0)
+        goto error;
+    VIR_DEBUG("Activated XS sub-driver");
+    priv->opened[XEN_UNIFIED_XS_OFFSET] = 1;
+
     xenNumaInit(conn);
 
     if (!(priv->caps = xenHypervisorMakeCapabilities(conn))) {
         VIR_DEBUG("Failed to make capabilities");
-        goto fail;
+        goto error;
     }
 
     if (!(priv->xmlopt = xenDomainXMLConfInit()))
-        goto fail;
+        goto error;
 
 #if WITH_XEN_INOTIFY
-    if (xenHavePrivilege()) {
+    VIR_DEBUG("Trying Xen inotify sub-driver");
-        VIR_DEBUG("Trying Xen inotify sub-driver");
+    if (xenInotifyOpen(conn, auth, flags) < 0)
-        if (xenInotifyOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) {
+        goto error;
-            VIR_DEBUG("Activated Xen inotify sub-driver");
+    VIR_DEBUG("Activated Xen inotify sub-driver");
-            priv->opened[XEN_UNIFIED_INOTIFY_OFFSET] = 1;
+    priv->opened[XEN_UNIFIED_INOTIFY_OFFSET] = 1;
-        }
-    }
 #endif
 
     if (!(priv->saveDir = strdup(XEN_SAVE_DIR))) {
         virReportOOMError();
-        goto fail;
+        goto error;
     }
 
     if (virFileMakePath(priv->saveDir) < 0) {
-        VIR_ERROR(_("Failed to create save dir '%s': %s"), priv->saveDir,
+        VIR_ERROR(_("Errored to create save dir '%s': %s"), priv->saveDir,
                   virStrerror(errno, ebuf, sizeof(ebuf)));
-        goto fail;
+        goto error;
     }
 
     return VIR_DRV_OPEN_SUCCESS;
 
-fail:
+error:
-    ret = VIR_DRV_OPEN_ERROR;
-clean:
     VIR_DEBUG("Failed to activate a mandatory sub-driver");
-    for (i = 0 ; i < XEN_UNIFIED_NR_DRIVERS ; i++)
+#if WITH_XEN_INOTIFY
-        if (priv->opened[i])
+    if (priv->opened[XEN_UNIFIED_INOTIFY_OFFSET])
-            drivers[i]->xenClose(conn);
+        xenInotifyClose(conn);
+#endif
+    if (priv->opened[XEN_UNIFIED_XM_OFFSET])
+        xenXMClose(conn);
+    if (priv->opened[XEN_UNIFIED_XS_OFFSET])
+        xenStoreClose(conn);
+    if (priv->opened[XEN_UNIFIED_XEND_OFFSET])
+        xenDaemonClose(conn);
+    if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET])
+        xenHypervisorClose(conn);
     virMutexDestroy(&priv->lock);
     VIR_FREE(priv->saveDir);
     VIR_FREE(priv);
     conn->privateData = NULL;
-    return ret;
+    return VIR_DRV_OPEN_ERROR;
 }
 
 static int
 xenUnifiedConnectClose(virConnectPtr conn)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
-    int i;
 
     virObjectUnref(priv->caps);
     virObjectUnref(priv->xmlopt);
     virDomainEventStateFree(priv->domainEvents);
 
-    for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
+#if WITH_XEN_INOTIFY
-        if (priv->opened[i])
+    if (priv->opened[XEN_UNIFIED_INOTIFY_OFFSET])
-            drivers[i]->xenClose(conn);
+        xenInotifyClose(conn);
+#endif
+    if (priv->opened[XEN_UNIFIED_XM_OFFSET])
+        xenXMClose(conn);
+    if (priv->opened[XEN_UNIFIED_XS_OFFSET])
+        xenStoreClose(conn);
+    if (priv->opened[XEN_UNIFIED_XEND_OFFSET])
+        xenDaemonClose(conn);
+    if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET])
+        xenHypervisorClose(conn);
 
     VIR_FREE(priv->saveDir);
     virMutexDestroy(&priv->lock);
@@ -2485,9 +2473,7 @@ static virDriver xenUnifiedDriver = {
 int
 xenRegister(void)
 {
-#ifdef WITH_LIBVIRTD
     if (virRegisterStateDriver(&state_driver) == -1) return -1;
-#endif
 
     return virRegisterDriver(&xenUnifiedDriver);
 }

src/xen/xen_driver.h

@@ -93,7 +93,6 @@ extern int xenRegister (void);
  * structure with direct calls in xen_unified.c.
  */
 struct xenUnifiedDriver {
-    virDrvConnectClose xenClose; /* Only mandatory callback; all others may be NULL */
     virDrvConnectGetVersion  xenVersion;
     virDrvConnectGetHostname xenGetHostname;
     virDrvDomainSuspend xenDomainSuspend;

src/xen/xen_hypervisor.c

@@ -880,7 +880,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
 static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
 
 struct xenUnifiedDriver xenHypervisorDriver = {
-    .xenClose = xenHypervisorClose,
     .xenVersion = xenHypervisorGetVersion,
     .xenDomainSuspend = xenHypervisorPauseDomain,
     .xenDomainResume = xenHypervisorResumeDomain,
@@ -2184,7 +2183,7 @@ VIR_ONCE_GLOBAL_INIT(xenHypervisor)
  *
  * Returns 0 or -1 in case of error.
  */
-virDrvOpenStatus
+int
 xenHypervisorOpen(virConnectPtr conn,
                   virConnectAuthPtr auth ATTRIBUTE_UNUSED,
                   unsigned int flags)
@@ -2192,10 +2191,10 @@ xenHypervisorOpen(virConnectPtr conn,
     int ret;
     xenUnifiedPrivatePtr priv = conn->privateData;
 
-    virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+    virCheckFlags(VIR_CONNECT_RO, -1);
 
     if (xenHypervisorInitialize() < 0)
-        return VIR_DRV_OPEN_ERROR;
+        return -1;
 
     priv->handle = -1;
 
@@ -2207,7 +2206,7 @@ xenHypervisorOpen(virConnectPtr conn,
 
     priv->handle = ret;
 
-    return VIR_DRV_OPEN_SUCCESS;
+    return 0;
 }
 
 /**

src/xen/xen_hypervisor.h

@@ -53,7 +53,7 @@ virDomainPtr
 char *
         xenHypervisorDomainGetOSType    (virDomainPtr dom);
 
-virDrvOpenStatus
+int
         xenHypervisorOpen               (virConnectPtr conn,
                                          virConnectAuthPtr auth,
                                          unsigned int flags);

src/xen/xen_inotify.c

@@ -44,10 +44,6 @@
 
 #define VIR_FROM_THIS VIR_FROM_XEN_INOTIFY
 
-struct xenUnifiedDriver xenInotifyDriver = {
-    .xenClose = xenInotifyClose,
-};
-
 static int
 xenInotifyXenCacheLookup(virConnectPtr conn,
                          const char *filename,
@@ -349,7 +345,7 @@ cleanup:
  *
  * Returns 0 or -1 in case of error.
  */
-virDrvOpenStatus
+int
 xenInotifyOpen(virConnectPtr conn,
                virConnectAuthPtr auth ATTRIBUTE_UNUSED,
                unsigned int flags)
@@ -359,7 +355,7 @@ xenInotifyOpen(virConnectPtr conn,
     char *path;
     xenUnifiedPrivatePtr priv = conn->privateData;
 
-    virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+    virCheckFlags(VIR_CONNECT_RO, -1);
 
     if (priv->configDir) {
         priv->useXenConfigCache = 1;

src/xen/xen_inotify.h

@@ -24,13 +24,10 @@
 # define __VIR_XEN_INOTIFY_H__
 
 # include "internal.h"
-# include "driver.h"
 
-extern struct xenUnifiedDriver xenInotifyDriver;
+int xenInotifyOpen(virConnectPtr conn,
-
+                   virConnectAuthPtr auth,
-virDrvOpenStatus	xenInotifyOpen	(virConnectPtr conn,
+                   unsigned int flags);
-                                         virConnectAuthPtr auth,
+int xenInotifyClose(virConnectPtr conn);
-                                         unsigned int flags);
-int		xenInotifyClose		(virConnectPtr conn);
 
 #endif

src/xen/xend_internal.c

@@ -1231,15 +1231,15 @@ error:
  *
  * Returns 0 in case of success, -1 in case of error.
  */
-virDrvOpenStatus
+int
 xenDaemonOpen(virConnectPtr conn,
               virConnectAuthPtr auth ATTRIBUTE_UNUSED,
               unsigned int flags)
 {
     char *port = NULL;
-    int ret = VIR_DRV_OPEN_ERROR;
+    int ret = -1;
 
-    virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+    virCheckFlags(VIR_CONNECT_RO, -1);
 
     /* Switch on the scheme, which we expect to be NULL (file),
      * "http" or "xen".
@@ -1286,7 +1286,7 @@ xenDaemonOpen(virConnectPtr conn,
     }
 
  done:
-    ret = VIR_DRV_OPEN_SUCCESS;
+    ret = 0;
 
 failed:
     VIR_FREE(port);
@@ -3652,7 +3652,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
 }
 
 struct xenUnifiedDriver xenDaemonDriver = {
-    .xenClose = xenDaemonClose,
     .xenVersion = xenDaemonGetVersion,
     .xenDomainSuspend = xenDaemonDomainSuspend,
     .xenDomainResume = xenDaemonDomainResume,

src/xen/xend_internal.h

@@ -95,8 +95,8 @@ xenDaemonDomainFetch(virConnectPtr xend,
 
 
 /* refactored ones */
-virDrvOpenStatus xenDaemonOpen(virConnectPtr conn, virConnectAuthPtr auth,
+int xenDaemonOpen(virConnectPtr conn, virConnectAuthPtr auth,
-                               unsigned int flags);
+                  unsigned int flags);
 int xenDaemonClose(virConnectPtr conn);
 int xenDaemonGetVersion(virConnectPtr conn, unsigned long *hvVer);
 int xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info);

src/xen/xm_internal.c

@@ -81,7 +81,6 @@ static int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml,
 #define XM_XML_ERROR "Invalid xml"
 
 struct xenUnifiedDriver xenXMDriver = {
-    .xenClose = xenXMClose,
     .xenDomainGetMaxMemory = xenXMDomainGetMaxMemory,
     .xenDomainSetMaxMemory = xenXMDomainSetMaxMemory,
     .xenDomainSetMemory = xenXMDomainSetMemory,
@@ -419,14 +418,14 @@ xenXMConfigCacheRefresh(virConnectPtr conn)
  * us watch for changes (see separate driver), otherwise we poll
  * every few seconds
  */
-virDrvOpenStatus
+int
 xenXMOpen(virConnectPtr conn,
           virConnectAuthPtr auth ATTRIBUTE_UNUSED,
           unsigned int flags)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
 
-    virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+    virCheckFlags(VIR_CONNECT_RO, -1);
 
     priv->configDir = XM_CONFIG_DIR;
 

src/xen/xm_internal.h

@@ -36,8 +36,7 @@ int xenXMConfigCacheRefresh (virConnectPtr conn);
 int xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename);
 int xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename);
 
-virDrvOpenStatus xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth,
+int xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags);
-                           unsigned int flags);
 int xenXMClose(virConnectPtr conn);
 const char *xenXMGetType(virConnectPtr conn);
 int xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info);

src/xen/xs_internal.c

@@ -58,7 +58,6 @@ static void xenStoreWatchEvent(int watch, int fd, int events, void *data);
 static void xenStoreWatchListFree(xenStoreWatchListPtr list);
 
 struct xenUnifiedDriver xenStoreDriver = {
-    .xenClose = xenStoreClose,
     .xenDomainShutdown = xenStoreDomainShutdown,
     .xenDomainReboot = xenStoreDomainReboot,
     .xenDomainGetOSType = xenStoreDomainGetOSType,
@@ -218,14 +217,14 @@ virDomainGetVMInfo(virDomainPtr domain, const char *vm, const char *name)
  *
  * Returns 0 or -1 in case of error.
  */
-virDrvOpenStatus
+int
 xenStoreOpen(virConnectPtr conn,
              virConnectAuthPtr auth ATTRIBUTE_UNUSED,
              unsigned int flags)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
 
-    virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+    virCheckFlags(VIR_CONNECT_RO, -1);
 
     if (flags & VIR_CONNECT_RO)
         priv->xshandle = xs_daemon_open_readonly();

src/xen/xs_internal.h

@@ -29,7 +29,7 @@
 extern struct xenUnifiedDriver xenStoreDriver;
 int xenStoreInit (void);
 
-virDrvOpenStatus	xenStoreOpen	(virConnectPtr conn,
+int		xenStoreOpen		(virConnectPtr conn,
                                          virConnectAuthPtr auth,
                                          unsigned int flags);
 int		xenStoreClose		(virConnectPtr conn);