Convert Xen domain lookup driver methods to use virDomainDefPtr

Introduce use of a virDomainDefPtr in the domain lookup
APIs to simplify introduction of ACL security checks.
The virDomainPtr cannot be safely used, since the app
may have supplied mis-matching name/uuid/id fields. eg
the name points to domain X, while the uuid points to
domain Y. Resolving the virDomainPtr to a virDomainDefPtr
ensures a consistent name/uuid/id set.

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

src/conf/domain_conf.c

@@ -2061,6 +2061,30 @@ error:
     return NULL;
 }
 
+
+virDomainDefPtr virDomainDefNew(const char *name,
+                                const unsigned char *uuid,
+                                int id)
+{
+    virDomainDefPtr def;
+
+    if (VIR_ALLOC(def) < 0) {
+        virReportOOMError();
+        return NULL;
+    }
+
+    if (VIR_STRDUP(def->name, name) < 0) {
+        VIR_FREE(def);
+        return NULL;
+    }
+
+    memcpy(def->uuid, uuid, VIR_UUID_BUFLEN);
+    def->id = id;
+
+    return def;
+}
+
+
 void virDomainObjAssignDef(virDomainObjPtr domain,
                            const virDomainDefPtr def,
                            bool live,

src/conf/domain_conf.h

@@ -2172,6 +2172,10 @@ void virDomainDefFree(virDomainDefPtr vm);
 
 virDomainChrDefPtr virDomainChrDefNew(void);
 
+virDomainDefPtr virDomainDefNew(const char *name,
+                                const unsigned char *uuid,
+                                int id);
+
 enum {
     VIR_DOMAIN_OBJ_LIST_ADD_LIVE = (1 << 0),
     VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE = (1 << 1),

src/libvirt_private.syms

@@ -117,6 +117,7 @@ virDomainDefGenSecurityLabelDef;
 virDomainDefGetDefaultEmulator;
 virDomainDefGetSecurityLabelDef;
 virDomainDefMaybeAddController;
+virDomainDefNew;
 virDomainDefParseFile;
 virDomainDefParseNode;
 virDomainDefParseString;

src/xen/xen_driver.c

@@ -82,6 +82,60 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom,
 
 static bool is_privileged = false;
 
+static virDomainDefPtr xenGetDomainDefForID(virConnectPtr conn, int id)
+{
+    virDomainDefPtr ret;
+
+    ret = xenHypervisorLookupDomainByID(conn, id);
+
+    if (!ret && virGetLastError() == NULL)
+        virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+    return ret;
+}
+
+
+static virDomainDefPtr xenGetDomainDefForName(virConnectPtr conn, const char *name)
+{
+    xenUnifiedPrivatePtr priv = conn->privateData;
+    virDomainDefPtr ret;
+
+    ret = xenDaemonLookupByName(conn, name);
+
+    /* Try XM for inactive domains. */
+    if (!ret &&
+        priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3)
+        ret = xenXMDomainLookupByName(conn, name);
+
+    if (!ret && virGetLastError() == NULL)
+        virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+    return ret;
+}
+
+
+static virDomainDefPtr xenGetDomainDefForUUID(virConnectPtr conn, const unsigned char *uuid)
+{
+    xenUnifiedPrivatePtr priv = conn->privateData;
+    virDomainDefPtr ret;
+
+    ret = xenHypervisorLookupDomainByUUID(conn, uuid);
+
+    /* Try XM for inactive domains. */
+    if (!ret) {
+        if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3)
+            ret = xenXMDomainLookupByUUID(conn, uuid);
+        else
+            ret = xenDaemonLookupByUUID(conn, uuid);
+    }
+
+    if (!ret && virGetLastError() == NULL)
+        virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+
+    return ret;
+}
+
+
 /**
  * xenNumaInit:
  * @conn: pointer to the hypervisor connection
@@ -597,12 +651,18 @@ static virDomainPtr
 xenUnifiedDomainLookupByID(virConnectPtr conn, int id)
 {
     virDomainPtr ret = NULL;
+    virDomainDefPtr def = NULL;
 
-    ret = xenHypervisorLookupDomainByID(conn, id);
+    if (!(def = xenGetDomainDefForID(conn, id)))
+        goto cleanup;
 
-    if (!ret && virGetLastError() == NULL)
+    if (!(ret = virGetDomain(conn, def->name, def->uuid)))
-        virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
+        goto cleanup;
 
+    ret->id = def->id;
+
+cleanup:
+    virDomainDefFree(def);
     return ret;
 }
 
@@ -610,22 +670,19 @@ static virDomainPtr
 xenUnifiedDomainLookupByUUID(virConnectPtr conn,
                              const unsigned char *uuid)
 {
-    xenUnifiedPrivatePtr priv = conn->privateData;
+    virDomainPtr ret = NULL;
-    virDomainPtr ret;
+    virDomainDefPtr def = NULL;
 
-    ret = xenHypervisorLookupDomainByUUID(conn, uuid);
+    if (!(def = xenGetDomainDefForUUID(conn, uuid)))
+        goto cleanup;
 
-    /* Try XM for inactive domains. */
+    if (!(ret = virGetDomain(conn, def->name, def->uuid)))
-    if (!ret) {
+        goto cleanup;
-        if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3)
-            ret = xenXMDomainLookupByUUID(conn, uuid);
-        else
-            ret = xenDaemonLookupByUUID(conn, uuid);
-    }
 
-    if (!ret && virGetLastError() == NULL)
+    ret->id = def->id;
-        virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
 
+cleanup:
+    virDomainDefFree(def);
     return ret;
 }
 
@@ -633,18 +690,19 @@ static virDomainPtr
 xenUnifiedDomainLookupByName(virConnectPtr conn,
                              const char *name)
 {
-    xenUnifiedPrivatePtr priv = conn->privateData;
+    virDomainPtr ret = NULL;
-    virDomainPtr ret;
+    virDomainDefPtr def = NULL;
 
-    ret = xenDaemonLookupByName(conn, name);
+    if (!(def = xenGetDomainDefForName(conn, name)))
+        goto cleanup;
 
-    /* Try XM for inactive domains. */
+    if (!(ret = virGetDomain(conn, def->name, def->uuid)))
-    if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3)
+        goto cleanup;
-        ret = xenXMDomainLookupByName(conn, name);
 
-    if (!ret && virGetLastError() == NULL)
+    ret->id = def->id;
-        virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
 
+cleanup:
+    virDomainDefFree(def);
     return ret;
 }
 
@@ -652,28 +710,16 @@ xenUnifiedDomainLookupByName(virConnectPtr conn,
 static int
 xenUnifiedDomainIsActive(virDomainPtr dom)
 {
-    xenUnifiedPrivatePtr priv = dom->conn->privateData;
+    virDomainDefPtr def;
-    virDomainPtr currdom;
     int ret = -1;
 
-    /* ID field in dom may be outdated, so re-lookup */
+    if (!(def = xenGetDomainDefForUUID(dom->conn, dom->uuid)))
-    currdom = xenHypervisorLookupDomainByUUID(dom->conn, dom->uuid);
+        goto cleanup;
 
-    /* Try XM for inactive domains. */
+    ret = def->id == -1 ? 0 : 1;
-    if (!currdom) {
-        if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3)
-            currdom = xenXMDomainLookupByUUID(dom->conn, dom->uuid);
-        else
-            currdom = xenDaemonLookupByUUID(dom->conn, dom->uuid);
-    }
-
-    if (currdom) {
-        ret = currdom->id == -1 ? 0 : 1;
-        virDomainFree(currdom);
-    } else if (virGetLastError() == NULL) {
-        virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__);
-    }
 
+cleanup:
+    virDomainDefFree(def);
     return ret;
 }
 
@@ -681,22 +727,22 @@ static int
 xenUnifiedDomainIsPersistent(virDomainPtr dom)
 {
     xenUnifiedPrivatePtr priv = dom->conn->privateData;
-    virDomainPtr currdom = NULL;
+    virDomainDefPtr def = NULL;
     int ret = -1;
 
     if (priv->opened[XEN_UNIFIED_XM_OFFSET]) {
         /* Old Xen, pre-inactive domain management.
          * If the XM driver can see the guest, it is definitely persistent */
-        currdom = xenXMDomainLookupByUUID(dom->conn, dom->uuid);
+        def = xenXMDomainLookupByUUID(dom->conn, dom->uuid);
-        if (currdom)
+        if (def)
             ret = 1;
         else
             ret = 0;
     } else {
         /* New Xen with inactive domain management */
-        currdom = xenDaemonLookupByUUID(dom->conn, dom->uuid);
+        def = xenDaemonLookupByUUID(dom->conn, dom->uuid);
-        if (currdom) {
+        if (def) {
-            if (currdom->id == -1) {
+            if (def->id == -1) {
                 /* If its inactive, then trivially, it must be persistent */
                 ret = 1;
             } else {
@@ -708,7 +754,7 @@ xenUnifiedDomainIsPersistent(virDomainPtr dom)
                 virUUIDFormat(dom->uuid, uuidstr);
                 if (virAsprintf(&path, "%s/%s", XEND_DOMAINS_DIR, uuidstr) < 0) {
                     virReportOOMError();
-                    goto done;
+                    goto cleanup;
                 }
                 if (access(path, R_OK) == 0)
                     ret = 1;
@@ -718,9 +764,8 @@ xenUnifiedDomainIsPersistent(virDomainPtr dom)
         }
     }
 
-done:
+cleanup:
-    if (currdom)
+    virDomainDefFree(def);
-        virDomainFree(currdom);
 
     return ret;
 }

src/xen/xen_hypervisor.c

@@ -2562,12 +2562,13 @@ xenHypervisorHasDomain(virConnectPtr conn, int id)
     return 1;
 }
 
-virDomainPtr
+
+virDomainDefPtr
 xenHypervisorLookupDomainByID(virConnectPtr conn, int id)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
     xen_getdomaininfo dominfo;
-    virDomainPtr ret;
+    virDomainDefPtr ret;
     char *name;
 
     XEN_GETDOMAININFO_CLEAR(dominfo);
@@ -2584,20 +2585,20 @@ xenHypervisorLookupDomainByID(virConnectPtr conn, int id)
     if (!name)
         return NULL;
 
-    ret = virGetDomain(conn, name, XEN_GETDOMAININFO_UUID(dominfo));
+    ret = virDomainDefNew(name,
-    if (ret)
+                          XEN_GETDOMAININFO_UUID(dominfo),
-        ret->id = id;
+                          id);
     VIR_FREE(name);
     return ret;
 }
 
 
-virDomainPtr
+virDomainDefPtr
 xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid)
 {
     xen_getdomaininfolist dominfos;
     xenUnifiedPrivatePtr priv = conn->privateData;
-    virDomainPtr ret;
+    virDomainDefPtr ret;
     char *name;
     int maxids = 100, nids, i, id;
 
@@ -2648,7 +2649,7 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid)
     if (!name)
         return NULL;
 
-    ret = virGetDomain(conn, name, uuid);
+    ret = virDomainDefNew(name, uuid, id);
     if (ret)
         ret->id = id;
     VIR_FREE(name);

src/xen/xen_hypervisor.h

@@ -27,6 +27,7 @@
 # include "capabilities.h"
 # include "driver.h"
 # include "viruri.h"
+# include "domain_conf.h"
 
 /* See xenHypervisorInit() for details. */
 struct xenHypervisorVersions {
@@ -43,10 +44,9 @@ virCapsPtr xenHypervisorMakeCapabilities (virConnectPtr conn);
 int
         xenHypervisorHasDomain(virConnectPtr conn,
                                int id);
-virDomainPtr
+virDomainDefPtr
-        xenHypervisorLookupDomainByID   (virConnectPtr conn,
+        xenHypervisorLookupDomainByID   (virConnectPtr conn, int id);
-                                         int id);
+virDomainDefPtr
-virDomainPtr
         xenHypervisorLookupDomainByUUID (virConnectPtr conn,
                                          const unsigned char *uuid);
 char *

src/xen/xen_inotify.c

@@ -76,7 +76,7 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn,
                                unsigned char *uuid)
 {
     int i;
-    virDomainPtr dom;
+    virDomainDefPtr def;
     const char *uuid_str;
     unsigned char rawuuid[VIR_UUID_BUFLEN];
     xenUnifiedPrivatePtr priv = conn->privateData;
@@ -96,8 +96,8 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn,
        be set during open while we are building our
        initial list of domains */
     VIR_DEBUG("Looking for dom with uuid: %s", uuid_str);
-    /* XXX Should not have to go via a virDomainPtr obj instance */
+
-    if (!(dom = xenDaemonLookupByUUID(conn, rawuuid))) {
+    if (!(def = xenDaemonLookupByUUID(conn, rawuuid))) {
         /* If we are here, the domain has gone away.
            search for, and create a domain from the stored
            list info */
@@ -118,13 +118,13 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn,
         return -1;
     }
 
-    if (!(*name = strdup(dom->name))) {
+    if (!(*name = strdup(def->name))) {
         virReportOOMError();
-        virDomainFree(dom);
+        virDomainDefFree(def);
         return -1;
     }
-    memcpy(uuid, dom->uuid, VIR_UUID_BUFLEN);
+    memcpy(uuid, def->uuid, VIR_UUID_BUFLEN);
-    virDomainFree(dom);
+    virDomainDefFree(def);
     /* succeeded too find domain by uuid */
     return 0;
 }

src/xen/xend_internal.c

@@ -1121,15 +1121,16 @@ sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps)
  *
  * Internal routine returning the associated virDomainPtr for this domain
  *
- * Returns the domain pointer or NULL in case of error.
+ * Returns the domain def pointer or NULL in case of error.
  */
-static virDomainPtr
+static virDomainDefPtr
 sexpr_to_domain(virConnectPtr conn, const struct sexpr *root)
 {
-    virDomainPtr ret = NULL;
+    virDomainDefPtr ret = NULL;
     unsigned char uuid[VIR_UUID_BUFLEN];
     const char *name;
     const char *tmp;
+    int id = -1;
     xenUnifiedPrivatePtr priv = conn->privateData;
 
     if (sexpr_uuid(uuid, root, "domain/uuid") < 0)
@@ -1138,9 +1139,6 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr *root)
     if (name == NULL)
         goto error;
 
-    ret = virGetDomain(conn, name, uuid);
-    if (ret == NULL) return NULL;
-
     tmp = sexpr_node(root, "domain/domid");
     /* New 3.0.4 XenD will not report a domid for inactive domains,
      * so only error out for old XenD
@@ -1149,11 +1147,9 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr *root)
         goto error;
 
     if (tmp)
-        ret->id = sexpr_int(root, "domain/domid");
+        id = sexpr_int(root, "domain/domid");
-    else
-        ret->id = -1; /* An inactive domain */
 
-    return ret;
+    return virDomainDefNew(name, uuid, id);
 
 error:
     virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1704,13 +1700,13 @@ xenDaemonDomainGetState(virDomainPtr domain,
  * it in the form of a struct xend_domain.  This should be
  * free()'d when no longer needed.
  *
- * Returns domain info on success; NULL (with errno) on error
+ * Returns domain def pointer on success; NULL on error
  */
-virDomainPtr
+virDomainDefPtr
 xenDaemonLookupByName(virConnectPtr conn, const char *domname)
 {
     struct sexpr *root;
-    virDomainPtr ret = NULL;
+    virDomainDefPtr ret = NULL;
 
     root = sexpr_get(conn, "/xend/domain/%s?detail=1", domname);
     if (root == NULL)
@@ -2058,12 +2054,12 @@ xenDaemonDomainGetVcpus(virDomainPtr domain,
  *
  * Try to lookup a domain on xend based on its UUID.
  *
- * Returns a new domain object or NULL in case of failure
+ * Returns domain def pointer on success; NULL on error
  */
-virDomainPtr
+virDomainDefPtr
 xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid)
 {
-    virDomainPtr ret;
+    virDomainDefPtr ret;
     char *name = NULL;
     int id = -1;
     xenUnifiedPrivatePtr priv = conn->privateData;
@@ -2123,12 +2119,8 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid)
     if (name == NULL)
         return NULL;
 
-    ret = virGetDomain(conn, name, uuid);
+    ret = virDomainDefNew(name, uuid, id);
-    if (ret == NULL) goto cleanup;
 
-    ret->id = id;
-
-  cleanup:
     VIR_FREE(name);
     return ret;
 }

src/xen/xend_internal.h

@@ -156,8 +156,8 @@ int xenDaemonDomainSetAutostart          (virDomainPtr domain,
                                           int autostart);
 
 virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc);
-virDomainPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid);
+virDomainDefPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid);
-virDomainPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname);
+virDomainDefPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname);
 int xenDaemonDomainMigratePrepare (virConnectPtr dconn, char **cookie, int *cookielen, const char *uri_in, char **uri_out, unsigned long flags, const char *dname, unsigned long resource);
 int xenDaemonDomainMigratePerform (virDomainPtr domain, const char *cookie, int cookielen, const char *uri, unsigned long flags, const char *dname, unsigned long resource);
 

src/xen/xm_internal.c

@@ -815,13 +815,13 @@ xenXMDomainPinVcpu(virDomainPtr domain,
 /*
  * Find an inactive domain based on its name
  */
-virDomainPtr
+virDomainDefPtr
 xenXMDomainLookupByName(virConnectPtr conn, const char *domname)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
     const char *filename;
     xenXMConfCachePtr entry;
-    virDomainPtr ret = NULL;
+    virDomainDefPtr ret = NULL;
 
     xenUnifiedLock(priv);
 
@@ -834,12 +834,7 @@ xenXMDomainLookupByName(virConnectPtr conn, const char *domname)
     if (!(entry = virHashLookup(priv->configCache, filename)))
         goto cleanup;
 
-    if (!(ret = virGetDomain(conn, domname, entry->def->uuid)))
+    ret = virDomainDefNew(domname, entry->def->uuid, -1);
-        goto cleanup;
-
-    /* Ensure its marked inactive, because may be cached
-       handle to a previously active domain */
-    ret->id = -1;
 
 cleanup:
     xenUnifiedUnlock(priv);
@@ -867,12 +862,12 @@ xenXMDomainSearchForUUID(const void *payload,
 /*
  * Find an inactive domain based on its UUID
  */
-virDomainPtr
+virDomainDefPtr
 xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
     xenXMConfCachePtr entry;
-    virDomainPtr ret = NULL;
+    virDomainDefPtr ret = NULL;
 
     xenUnifiedLock(priv);
 
@@ -882,12 +877,7 @@ xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid)
     if (!(entry = virHashSearch(priv->configCache, xenXMDomainSearchForUUID, (const void *)uuid)))
         goto cleanup;
 
-    if (!(ret = virGetDomain(conn, entry->def->name, uuid)))
+    ret = virDomainDefNew(entry->def->name, uuid, -1);
-        goto cleanup;
-
-    /* Ensure its marked inactive, because may be cached
-       handle to a previously active domain */
-    ret->id = -1;
 
 cleanup:
     xenUnifiedUnlock(priv);
@@ -1130,7 +1120,7 @@ struct xenXMListIteratorContext {
 static void
 xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) {
     struct xenXMListIteratorContext *ctx = data;
-    virDomainPtr dom = NULL;
+    virDomainDefPtr def = NULL;
 
     if (ctx->oom)
         return;
@@ -1138,14 +1128,14 @@ xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data)
     if (ctx->count == ctx->max)
         return;
 
-    dom = xenDaemonLookupByName(ctx->conn, name);
+    def = xenDaemonLookupByName(ctx->conn, name);
-    if (!dom) {
+    if (!def) {
         if (!(ctx->names[ctx->count] = strdup(name)))
             ctx->oom = 1;
         else
             ctx->count++;
     } else {
-        virDomainFree(dom);
+        virDomainDefFree(def);
     }
 }
 

src/xen/xm_internal.h

@@ -51,9 +51,8 @@ int xenXMDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus,
 int xenXMDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags);
 int xenXMDomainPinVcpu(virDomainPtr domain, unsigned int vcpu,
                        unsigned char *cpumap, int maplen);
-virDomainPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname);
+virDomainDefPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname);
-virDomainPtr xenXMDomainLookupByUUID(virConnectPtr conn,
+virDomainDefPtr xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid);
-                                     const unsigned char *uuid);
 
 int xenXMListDefinedDomains(virConnectPtr conn, char ** const names, int maxnames);
 int xenXMNumOfDefinedDomains(virConnectPtr conn);