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

[John Ferlan]

On 05/09/2013 08:59 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at 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...]