From 7650460024a828774440d45e8497b21350cb206d Mon Sep 17 00:00:00 2001

Message-Id: <7650460024a828774440d45e8497b21350cb206d@dist-git>

From: Peter Krempa <pkrempa@redhat.com>

Date: Wed, 26 Feb 2014 14:54:13 +0100

Subject: [PATCH] conf: Refactor virDomainDiskSourceDefParse

https://bugzilla.redhat.com/show_bug.cgi?id=1032370

Now that the function is separate clean out a few ugly places and fix up

error messages.

(cherry picked from commit c2986ff0d3da47f26e356fd12310fc032be1c7a5)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/conf/domain_conf.c | 119 +++++++++++++++++++++++--------------------------

 1 file changed, 56 insertions(+), 63 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index eef2176..409fcda 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -4837,17 +4837,18 @@ virDomainDiskSourceDefParse(xmlNodePtr node,

                             int type,

                             char **source,

                             int *proto,

-                            size_t *ndefhosts,

-                            virDomainDiskHostDefPtr *defhosts,

+                            size_t *nhosts,

+                            virDomainDiskHostDefPtr *hosts,

                             virDomainDiskSourcePoolDefPtr *srcpool)

 {

     char *protocol = NULL;

     char *transport = NULL;

-    virDomainDiskHostDefPtr hosts = NULL;

-    int nhosts = 0;

+    virDomainDiskHostDef host;

     xmlNodePtr child;

     int ret = -1;

+    memset(&host, 0, sizeof(host));

+

     switch (type) {

     case VIR_DOMAIN_DISK_TYPE_FILE:

         *source = virXMLPropString(node, "file");

@@ -4859,110 +4860,102 @@ virDomainDiskSourceDefParse(xmlNodePtr node,

         *source = virXMLPropString(node, "dir");

         break;

     case VIR_DOMAIN_DISK_TYPE_NETWORK:

-        protocol = virXMLPropString(node, "protocol");

-        if (protocol == NULL) {

-            virReportError(VIR_ERR_INTERNAL_ERROR,

-                           "%s", _("missing protocol type"));

-            goto error;

+        if (!(protocol = virXMLPropString(node, "protocol"))) {

+            virReportError(VIR_ERR_XML_ERROR, "%s",

+                           _("missing network source protocol type"));

+            goto cleanup;

         }

-        *proto = virDomainDiskProtocolTypeFromString(protocol);

-        if (*proto < 0) {

-            virReportError(VIR_ERR_INTERNAL_ERROR,

-                           _("unknown protocol type '%s'"),

-                           protocol);

-            goto error;

+

+        if ((*proto = virDomainDiskProtocolTypeFromString(protocol)) < 0){

+            virReportError(VIR_ERR_XML_ERROR,

+                           _("unknown protocol type '%s'"), protocol);

+            goto cleanup;

         }

         if (!(*source = virXMLPropString(node, "name")) &&

             *proto != VIR_DOMAIN_DISK_PROTOCOL_NBD) {

-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

+            virReportError(VIR_ERR_XML_ERROR, "%s",

                            _("missing name for disk source"));

-            goto error;

+            goto cleanup;

         }

+

         child = node->children;

         while (child != NULL) {

             if (child->type == XML_ELEMENT_NODE &&

                 xmlStrEqual(child->name, BAD_CAST "host")) {

-                if (VIR_REALLOC_N(hosts, nhosts + 1) < 0)

-                    goto error;

-                hosts[nhosts].name = NULL;

-                hosts[nhosts].port = NULL;

-                hosts[nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;

-                hosts[nhosts].socket = NULL;

-                nhosts++;

+

+                host.transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;

                 /* transport can be tcp (default), unix or rdma.  */

-                transport = virXMLPropString(child, "transport");

-                if (transport != NULL) {

-                    hosts[nhosts - 1].transport = virDomainDiskProtocolTransportTypeFromString(transport);

-                    if (hosts[nhosts - 1].transport < 0) {

+                if ((transport = virXMLPropString(child, "transport"))) {

+                    host.transport = virDomainDiskProtocolTransportTypeFromString(transport);

+                    if (host.transport < 0) {

                         virReportError(VIR_ERR_XML_ERROR,

                                        _("unknown protocol transport type '%s'"),

                                        transport);

-                        goto error;

+                        goto cleanup;

                     }

                 }

-                hosts[nhosts - 1].socket = virXMLPropString(child, "socket");

-                if (hosts[nhosts - 1].transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX &&

-                    hosts[nhosts - 1].socket == NULL) {

-                    virReportError(VIR_ERR_XML_ERROR,

-                                   "%s", _("missing socket for unix transport"));

-                    goto error;

+

+                host.socket = virXMLPropString(child, "socket");

+

+                if (host.transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX &&

+                    host.socket == NULL) {

+                    virReportError(VIR_ERR_XML_ERROR, "%s",

+                                   _("missing socket for unix transport"));

+                    goto cleanup;

                 }

-                if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX &&

-                    hosts[nhosts - 1].socket != NULL) {

+

+                if (host.transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX &&

+                    host.socket != NULL) {

                     virReportError(VIR_ERR_XML_ERROR,

-                                   _("transport %s does not support socket attribute"),

+                                   _("transport '%s' does not support "

+                                     "socket attribute"),

                                    transport);

-                    goto error;

+                    goto cleanup;

                 }

+

                 VIR_FREE(transport);

-                if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {

-                    hosts[nhosts - 1].name = virXMLPropString(child, "name");

-                    if (!hosts[nhosts - 1].name) {

-                        virReportError(VIR_ERR_XML_ERROR,

-                                       "%s", _("missing name for host"));

-                        goto error;

+

+                if (host.transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {

+                    if (!(host.name = virXMLPropString(child, "name"))) {

+                        virReportError(VIR_ERR_XML_ERROR, "%s",

+                                       _("missing name for host"));

+                        goto cleanup;

                     }

-                    hosts[nhosts - 1].port = virXMLPropString(child, "port");

+

+                    host.port = virXMLPropString(child, "port");

                 }

+

+                if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host) < 0)

+                    goto cleanup;

             }

             child = child->next;

         }

         break;

     case VIR_DOMAIN_DISK_TYPE_VOLUME:

         if (virDomainDiskSourcePoolDefParse(node, srcpool) < 0)

-            goto error;

+            goto cleanup;

         break;

     default:

         virReportError(VIR_ERR_INTERNAL_ERROR,

                        _("unexpected disk type %s"),

                        virDomainDiskTypeToString(type));

-        goto error;

+        goto cleanup;

     }

-    /* People sometimes pass a bogus '' source path

-       when they mean to omit the source element

-       completely (e.g. CDROM without media). This is

-       just a little compatibility check to help

-       those broken apps */

+    /* People sometimes pass a bogus '' source path when they mean to omit the

+     * source element completely (e.g. CDROM without media). This is just a

+     * little compatibility check to help those broken apps */

     if (*source && STREQ(*source, ""))

         VIR_FREE(*source);

-    *ndefhosts = nhosts;

-    *defhosts = hosts;

-    nhosts = 0;

-

     ret = 0;

-error:

+cleanup:

+    virDomainDiskHostDefClear(&host);

     VIR_FREE(protocol);

     VIR_FREE(transport);

-    while (nhosts > 0) {

-        virDomainDiskHostDefClear(&hosts[nhosts - 1]);

-        nhosts--;

-    }

-

     return ret;

 }

-- 

1.9.0