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

23 May 2013

On 05/09/2013 08:59 AM, Daniel P. Berrange wrote:
...
From: "Daniel P. Berrange" <berrange@redhat.com>
[...snip...]
...

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;
+
Coverity complains

1666 	

(7) Event var_deref_op: 	Dereferencing null pointer "def".
Also see events: 	[assign_zero]

Because of :

1660 	            goto cleanup;

(4) Event assign_zero: 	Assigning: "def" = "NULL".
Also see events: 	[var_deref_op]

1661 	        def = NULL; /* XM driver owns it now */

(5) Event if_fallthrough: 	Falling through to end of if statement

1662 	    } else {
...
+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
[...snip...]

    

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

John Ferlan