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

[libvirt] [PATCH 05/11] Convert Xen domain start/migration APIs to use virDomainDefPtr



From: "Daniel P. Berrange" <berrange redhat com>

Introduce use of a virDomainDefPtr in the domain migrate &
start 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/xen/xen_driver.c    | 127 ++++++++++++++++++++++++++++++++----------------
 src/xen/xend_internal.c |  71 +++++++++------------------
 src/xen/xend_internal.h |  22 ++++++---
 src/xen/xm_internal.c   |  49 ++++++++-----------
 src/xen/xm_internal.h   |   7 +--
 5 files changed, 148 insertions(+), 128 deletions(-)

diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 89b038c..8b7dec9 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1294,18 +1294,31 @@ static char *
 xenUnifiedDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
 {
     xenUnifiedPrivatePtr priv = dom->conn->privateData;
+    virDomainDefPtr minidef = NULL;
+    virDomainDefPtr def = NULL;
+    char *ret = NULL;
+
+    if (!(minidef = xenGetDomainDefForDom(dom)))
+        goto cleanup;
 
     if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) {
-        return xenXMDomainGetXMLDesc(dom, flags);
+        def = xenXMDomainGetXMLDesc(dom->conn, minidef);
     } else {
-        char *cpus, *res;
+        char *cpus;
         xenUnifiedLock(priv);
         cpus = xenDomainUsedCpus(dom);
         xenUnifiedUnlock(priv);
-        res = xenDaemonDomainGetXMLDesc(dom, flags, cpus);
+        def = xenDaemonDomainGetXMLDesc(dom->conn, minidef, cpus);
         VIR_FREE(cpus);
-        return res;
     }
+
+    if (def)
+        ret = virDomainDefFormat(def, flags);
+
+cleanup:
+    virDomainDefFree(def);
+    virDomainDefFree(minidef);
+    return ret;
 }
 
 
@@ -1438,10 +1451,21 @@ xenUnifiedDomainMigratePerform(virDomainPtr dom,
                                const char *dname,
                                unsigned long resource)
 {
+    virDomainDefPtr def = NULL;
+    int ret = -1;
+
     virCheckFlags(XEN_MIGRATION_FLAGS, -1);
 
-    return xenDaemonDomainMigratePerform(dom, cookie, cookielen, uri,
-                                         flags, dname, resource);
+    if (!(def = xenGetDomainDefForDom(dom)))
+        goto cleanup;
+
+    ret = xenDaemonDomainMigratePerform(dom->conn, def,
+                                        cookie, cookielen, uri,
+                                        flags, dname, resource);
+
+cleanup:
+    virDomainDefFree(def);
+    return ret;
 }
 
 static virDomainPtr
@@ -1452,45 +1476,37 @@ xenUnifiedDomainMigrateFinish(virConnectPtr dconn,
                               const char *uri ATTRIBUTE_UNUSED,
                               unsigned long flags)
 {
-    virDomainPtr dom = NULL;
-    char *domain_xml = NULL;
-    virDomainPtr dom_new = NULL;
+    xenUnifiedPrivatePtr priv = dconn->privateData;
+    virDomainPtr ret = NULL;
+    virDomainDefPtr minidef = NULL;
+    virDomainDefPtr def = NULL;
 
     virCheckFlags(XEN_MIGRATION_FLAGS, NULL);
 
-    if (!(dom = xenUnifiedDomainLookupByName(dconn, dname)))
-        return NULL;
+    if (!(minidef = xenGetDomainDefForName(dconn, dname)))
+        goto cleanup;
 
     if (flags & VIR_MIGRATE_PERSIST_DEST) {
-        domain_xml = xenDaemonDomainGetXMLDesc(dom, 0, NULL);
-        if (! domain_xml) {
-            virReportError(VIR_ERR_MIGRATE_PERSIST_FAILED,
-                           "%s", _("failed to get XML representation of migrated domain"));
-            goto error;
-        }
+        if (!(def = xenDaemonDomainGetXMLDesc(dconn, minidef, NULL)))
+            goto cleanup;
 
-        dom_new = xenDaemonDomainDefineXML(dconn, domain_xml);
-        if (! dom_new) {
-            virReportError(VIR_ERR_MIGRATE_PERSIST_FAILED,
-                           "%s", _("failed to define domain on destination host"));
-            goto error;
+        if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) {
+            if (xenXMDomainDefineXML(dconn, def) < 0)
+                goto cleanup;
+        } else {
+            if (xenDaemonDomainDefineXML(dconn, def) < 0)
+                goto cleanup;
         }
-
-        /* Free additional reference added by Define */
-        virDomainFree(dom_new);
     }
 
-    VIR_FREE(domain_xml);
-
-    return dom;
-
+    ret = virGetDomain(dconn, minidef->name, minidef->uuid);
+    if (ret)
+        ret->id = minidef->id;
 
-error:
-    virDomainFree(dom);
-
-    VIR_FREE(domain_xml);
-
-    return NULL;
+cleanup:
+    virDomainDefFree(def);
+    virDomainDefFree(minidef);
+    return ret;
 }
 
 static int
@@ -1565,23 +1581,52 @@ static virDomainPtr
 xenUnifiedDomainDefineXML(virConnectPtr conn, const char *xml)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
+    virDomainDefPtr def = NULL;
+    virDomainPtr ret = NULL;
 
-    if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
-        return xenXMDomainDefineXML(conn, xml);
-    else
-        return xenDaemonDomainDefineXML(conn, xml);
+    if (!(def = virDomainDefParseString(xml, priv->caps, priv->xmlopt,
+                                        1 << VIR_DOMAIN_VIRT_XEN,
+                                        VIR_DOMAIN_XML_INACTIVE)))
+        goto cleanup;
+
+    if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) {
+        if (xenXMDomainDefineXML(conn, def) < 0)
+            goto cleanup;
+        def = NULL; /* XM driver owns it now */
+    } else {
+        if (xenDaemonDomainDefineXML(conn, def) < 0)
+            goto cleanup;
+    }
+
+    ret = virGetDomain(conn, def->name, def->uuid);
+    if (ret)
+        ret->id = -1;
+
+cleanup:
+    virDomainDefFree(def);
+    return ret;
 }
 
 static int
 xenUnifiedDomainUndefineFlags(virDomainPtr dom, unsigned int flags)
 {
     xenUnifiedPrivatePtr priv = dom->conn->privateData;
+    virDomainDefPtr def = NULL;
+    int ret = -1;
 
     virCheckFlags(0, -1);
+
+    if (!(def = xenGetDomainDefForDom(dom)))
+        goto cleanup;
+
     if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
-        return xenXMDomainUndefine(dom);
+        ret = xenXMDomainUndefine(dom->conn, def);
     else
-        return xenDaemonDomainUndefine(dom);
+        ret = xenDaemonDomainUndefine(dom->conn, def);
+
+cleanup:
+    virDomainDefFree(def);
+    return ret;
 }
 
 static int
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 9555b33..77c4dec 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1604,7 +1604,6 @@ cleanup:
 /**
  * xenDaemonDomainGetXMLDesc:
  * @domain: a domain object
- * @flags: potential dump flags
  * @cpus: list of cpu the domain is pinned to.
  *
  * Provide an XML description of the domain.
@@ -1612,27 +1611,15 @@ cleanup:
  * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
  *         the caller must free() the returned value.
  */
-char *
-xenDaemonDomainGetXMLDesc(virDomainPtr domain,
-                          unsigned int flags,
+virDomainDefPtr
+xenDaemonDomainGetXMLDesc(virConnectPtr conn,
+                          virDomainDefPtr minidef,
                           const char *cpus)
 {
-    virDomainDefPtr def;
-    char *xml;
-
-    /* Flags checked by virDomainDefFormat */
-
-    if (!(def = xenDaemonDomainFetch(domain->conn,
-                                     domain->id,
-                                     domain->name,
-                                     cpus)))
-        return NULL;
-
-    xml = virDomainDefFormat(def, flags);
-
-    virDomainDefFree(def);
-
-    return xml;
+    return xenDaemonDomainFetch(conn,
+                                minidef->id,
+                                minidef->name,
+                                cpus);
 }
 
 
@@ -2657,7 +2644,8 @@ xenDaemonDomainMigratePrepare(virConnectPtr dconn ATTRIBUTE_UNUSED,
 }
 
 int
-xenDaemonDomainMigratePerform(virDomainPtr domain,
+xenDaemonDomainMigratePerform(virConnectPtr conn,
+                              virDomainDefPtr def,
                               const char *cookie ATTRIBUTE_UNUSED,
                               int cookielen ATTRIBUTE_UNUSED,
                               const char *uri,
@@ -2801,7 +2789,7 @@ xenDaemonDomainMigratePerform(virDomainPtr domain,
      * to our advantage since all parameters supported and required
      * by current xend can be included without breaking older xend.
      */
-    ret = xend_op(domain->conn, domain->name,
+    ret = xend_op(conn, def->name,
                   "op", "migrate",
                   "destination", hostname,
                   "live", live,
@@ -2814,34 +2802,24 @@ xenDaemonDomainMigratePerform(virDomainPtr domain,
     VIR_FREE(hostname);
 
     if (ret == 0 && undefined_source)
-        xenDaemonDomainUndefine(domain);
+        xenDaemonDomainUndefine(conn, def);
 
     VIR_DEBUG("migration done");
 
     return ret;
 }
 
-virDomainPtr
-xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc)
+int
+xenDaemonDomainDefineXML(virConnectPtr conn, virDomainDefPtr def)
 {
-    int ret;
+    int ret = -1;
     char *sexpr;
-    virDomainPtr dom;
     xenUnifiedPrivatePtr priv = conn->privateData;
-    virDomainDefPtr def;
-
-    if (!(def = virDomainDefParseString(xmlDesc, priv->caps, priv->xmlopt,
-                                        1 << VIR_DOMAIN_VIRT_XEN,
-                                        VIR_DOMAIN_XML_INACTIVE))) {
-        virReportError(VIR_ERR_XML_ERROR,
-                       "%s", _("failed to parse domain description"));
-        return NULL;
-    }
 
     if (!(sexpr = xenFormatSxpr(conn, def, priv->xendConfigVersion))) {
         virReportError(VIR_ERR_XML_ERROR,
                        "%s", _("failed to build sexpr"));
-        goto error;
+        goto cleanup;
     }
 
     ret = xend_op(conn, "", "op", "new", "config", sexpr, NULL);
@@ -2849,20 +2827,15 @@ xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc)
     if (ret != 0) {
         virReportError(VIR_ERR_XEN_CALL,
                        _("Failed to create inactive domain %s"), def->name);
-        goto error;
+        goto cleanup;
     }
 
-    dom = virDomainLookupByName(conn, def->name);
-    if (dom == NULL) {
-        goto error;
-    }
-    virDomainDefFree(def);
-    return dom;
+    ret = 0;
 
-  error:
-    virDomainDefFree(def);
-    return NULL;
+cleanup:
+    return ret;
 }
+
 int
 xenDaemonDomainCreate(virConnectPtr conn,
                       virDomainDefPtr def)
@@ -2882,9 +2855,9 @@ xenDaemonDomainCreate(virConnectPtr conn,
 }
 
 int
-xenDaemonDomainUndefine(virDomainPtr domain)
+xenDaemonDomainUndefine(virConnectPtr conn, virDomainDefPtr def)
 {
-    return xend_op(domain->conn, domain->name, "op", "delete", NULL);
+    return xend_op(conn, def->name, "op", "delete", NULL);
 }
 
 /**
diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
index 01d321f..1284db3 100644
--- a/src/xen/xend_internal.h
+++ b/src/xen/xend_internal.h
@@ -111,8 +111,9 @@ int xenDaemonDomainGetState(virConnectPtr conn,
                             virDomainDefPtr def,
                             int *state,
                             int *reason);
-char *xenDaemonDomainGetXMLDesc(virDomainPtr domain, unsigned int flags,
-                                const char *cpus);
+virDomainDefPtr xenDaemonDomainGetXMLDesc(virConnectPtr conn,
+                                          virDomainDefPtr def,
+                                          const char *cpus);
 unsigned long long xenDaemonDomainGetMaxMemory(virConnectPtr conn,
                                                virDomainDefPtr def);
 char **xenDaemonListDomainsOld(virConnectPtr xend);
@@ -132,10 +133,12 @@ int xenDaemonDetachDeviceFlags(virDomainPtr domain,
                                const char *xml,
                                unsigned int flags);
 
-virDomainPtr xenDaemonDomainDefineXML(virConnectPtr xend, const char *sexpr);
+int xenDaemonDomainDefineXML(virConnectPtr conn,
+                             virDomainDefPtr def);
 int xenDaemonDomainCreate(virConnectPtr conn,
                           virDomainDefPtr def);
-int xenDaemonDomainUndefine(virDomainPtr domain);
+int xenDaemonDomainUndefine(virConnectPtr conn,
+                            virDomainDefPtr def);
 
 int	xenDaemonDomainSetVcpus		(virDomainPtr domain,
                                          unsigned int vcpus);
@@ -163,8 +166,15 @@ int xenDaemonDomainSetAutostart          (virDomainPtr domain,
 virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc);
 virDomainDefPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid);
 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);
+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 (virConnectPtr conn,
+                                   virDomainDefPtr def,
+                                   const char *cookie, int cookielen,
+                                   const char *uri, unsigned long flags,
+                                   const char *dname, unsigned long resource);
 
 int xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer);
 
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 6c884d9..79f773d 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -500,28 +500,29 @@ error:
  * Turn a config record into a lump of XML describing the
  * domain, suitable for later feeding for virDomainCreateXML
  */
-char *
-xenXMDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
+virDomainDefPtr
+xenXMDomainGetXMLDesc(virConnectPtr conn,
+                      virDomainDefPtr def)
 {
-    xenUnifiedPrivatePtr priv = domain->conn->privateData;
+    xenUnifiedPrivatePtr priv = conn->privateData;
     const char *filename;
     xenXMConfCachePtr entry;
-    char *ret = NULL;
+    virDomainDefPtr ret = NULL;
 
     /* Flags checked by virDomainDefFormat */
 
-    if (domain->id != -1)
-        return NULL;
-
     xenUnifiedLock(priv);
 
-    if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
+    if (!(filename = virHashLookup(priv->nameConfigMap, def->name)))
         goto cleanup;
 
     if (!(entry = virHashLookup(priv->configCache, filename)))
         goto cleanup;
 
-    ret = virDomainDefFormat(entry->def, flags);
+    ret = virDomainDefCopy(entry->def,
+                           priv->caps,
+                           priv->xmlopt,
+                           false);
 
 cleanup:
     xenUnifiedUnlock(priv);
@@ -945,13 +946,11 @@ xenXMDomainCreate(virConnectPtr conn,
  * Create a config file for a domain, based on an XML
  * document describing its config
  */
-virDomainPtr
-xenXMDomainDefineXML(virConnectPtr conn, const char *xml)
+int
+xenXMDomainDefineXML(virConnectPtr conn, virDomainDefPtr def)
 {
-    virDomainPtr ret;
     char *filename = NULL;
     const char *oldfilename;
-    virDomainDefPtr def = NULL;
     virConfPtr conf = NULL;
     xenXMConfCachePtr entry = NULL;
     xenUnifiedPrivatePtr priv = conn->privateData;
@@ -960,14 +959,7 @@ xenXMDomainDefineXML(virConnectPtr conn, const char *xml)
 
     if (!xenInotifyActive(conn) && xenXMConfigCacheRefresh(conn) < 0) {
         xenUnifiedUnlock(priv);
-        return NULL;
-    }
-
-    if (!(def = virDomainDefParseString(xml, priv->caps, priv->xmlopt,
-                                        1 << VIR_DOMAIN_VIRT_XEN,
-                                        VIR_DOMAIN_XML_INACTIVE))) {
-        xenUnifiedUnlock(priv);
-        return NULL;
+        return -1;
     }
 
     if (!(conf = xenFormatXM(conn, def, priv->xendConfigVersion)))
@@ -1061,10 +1053,9 @@ xenXMDomainDefineXML(virConnectPtr conn, const char *xml)
         goto error;
     }
 
-    ret = virGetDomain(conn, def->name, def->uuid);
     xenUnifiedUnlock(priv);
     VIR_FREE(filename);
-    return ret;
+    return 0;
 
  error:
     VIR_FREE(filename);
@@ -1072,25 +1063,25 @@ xenXMDomainDefineXML(virConnectPtr conn, const char *xml)
         VIR_FREE(entry->filename);
     VIR_FREE(entry);
     virConfFree(conf);
-    virDomainDefFree(def);
     xenUnifiedUnlock(priv);
-    return NULL;
+    return -1;
 }
 
 /*
  * Delete a domain from disk
  */
 int
-xenXMDomainUndefine(virDomainPtr domain)
+xenXMDomainUndefine(virConnectPtr conn,
+                    virDomainDefPtr def)
 {
-    xenUnifiedPrivatePtr priv = domain->conn->privateData;
+    xenUnifiedPrivatePtr priv = conn->privateData;
     const char *filename;
     xenXMConfCachePtr entry;
     int ret = -1;
 
     xenUnifiedLock(priv);
 
-    if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
+    if (!(filename = virHashLookup(priv->nameConfigMap, def->name)))
         goto cleanup;
 
     if (!(entry = virHashLookup(priv->configCache, filename)))
@@ -1100,7 +1091,7 @@ xenXMDomainUndefine(virDomainPtr domain)
         goto cleanup;
 
     /* Remove the name -> filename mapping */
-    if (virHashRemoveEntry(priv->nameConfigMap, domain->name) < 0)
+    if (virHashRemoveEntry(priv->nameConfigMap, def->name) < 0)
         goto cleanup;
 
     /* Remove the config record itself */
diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h
index 0521625..5a434b9 100644
--- a/src/xen/xm_internal.h
+++ b/src/xen/xm_internal.h
@@ -44,7 +44,8 @@ int xenXMDomainGetState(virConnectPtr conn,
                         virDomainDefPtr def,
                         int *state,
                         int *reason);
-char *xenXMDomainGetXMLDesc(virDomainPtr domain, unsigned int flags);
+virDomainDefPtr xenXMDomainGetXMLDesc(virConnectPtr conn,
+                                      virDomainDefPtr def);
 int xenXMDomainSetMemory(virConnectPtr conn,
                          virDomainDefPtr def,
                          unsigned long memory);
@@ -67,8 +68,8 @@ int xenXMNumOfDefinedDomains(virConnectPtr conn);
 
 int xenXMDomainCreate(virConnectPtr conn,
                       virDomainDefPtr def);
-virDomainPtr xenXMDomainDefineXML(virConnectPtr con, const char *xml);
-int xenXMDomainUndefine(virDomainPtr domain);
+int xenXMDomainDefineXML(virConnectPtr con, virDomainDefPtr def);
+int xenXMDomainUndefine(virConnectPtr conn, virDomainDefPtr def);
 
 int xenXMDomainBlockPeek (virDomainPtr dom, const char *path, unsigned long long offset, size_t size, void *buffer);
 
-- 
1.8.1.4


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