[libvirt] [PATCH 0/4] Split out disk source parsing code
To avoid code duplication when I'll later attempt to add support for snapshots on gluster I'll need to split out the parsing code for disk source. Peter Krempa (4): conf: Refactor virDomainDiskSourcePoolDefParse conf: Split out code to parse the source of a disk definition conf: Rename virDomainDiskHostDefFree to virDomainDiskHostDefClear conf: Refactor virDomainDiskSourceDefParse src/conf/domain_conf.c | 297 +++++++++++++++++++++++++---------------------- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_command.c | 4 +- 4 files changed, 165 insertions(+), 140 deletions(-) -- 1.8.4.2
For some strange reason virDomainDiskSourcePoolDefParse accessed def of the disk and allocated the pool object in it. To avoid the need to carry over the disk definition object, refactor this function to return the allocated object instead. --- src/conf/domain_conf.c | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7849693..50ce44d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4683,49 +4683,48 @@ cleanup: static int virDomainDiskSourcePoolDefParse(xmlNodePtr node, - virDomainDiskDefPtr def) + virDomainDiskSourcePoolDefPtr *srcpool) { - char *pool = NULL; - char *volume = NULL; char *mode = NULL; + virDomainDiskSourcePoolDefPtr source; int ret = -1; - pool = virXMLPropString(node, "pool"); - volume = virXMLPropString(node, "volume"); + *srcpool = NULL; + + if (VIR_ALLOC(source) < 0) + return -1; + + source->pool = virXMLPropString(node, "pool"); + source->volume = virXMLPropString(node, "volume"); mode = virXMLPropString(node, "mode"); /* CD-ROM and Floppy allows no source */ - if (!pool && !volume) - return 0; + if (!source->pool && !source->volume) { + ret = 0; + goto cleanup; + } - if (!pool || !volume) { + if (!source->pool || !source->volume) { virReportError(VIR_ERR_XML_ERROR, "%s", _("'pool' and 'volume' must be specified together " "for 'pool' type source")); goto cleanup; } - if (VIR_ALLOC(def->srcpool) < 0) - goto cleanup; - - if (mode && (def->srcpool->mode = - virDomainDiskSourcePoolModeTypeFromString(mode)) <= 0) { + if (mode && + (source->mode = virDomainDiskSourcePoolModeTypeFromString(mode)) <= 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown source mode '%s' for volume type disk"), mode); goto cleanup; } - def->srcpool->pool = pool; - pool = NULL; - def->srcpool->volume = volume; - volume = NULL; - + *srcpool = source; + source = NULL; ret = 0; cleanup: - VIR_FREE(pool); - VIR_FREE(volume); + virDomainDiskSourcePoolDefFree(source); VIR_FREE(mode); return ret; } @@ -4916,7 +4915,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } break; case VIR_DOMAIN_DISK_TYPE_VOLUME: - if (virDomainDiskSourcePoolDefParse(cur, def) < 0) + if (virDomainDiskSourcePoolDefParse(cur, &def->srcpool) < 0) goto error; break; default: -- 1.8.4.2
On 11/06/2013 06:56 PM, Peter Krempa wrote:
For some strange reason virDomainDiskSourcePoolDefParse accessed def of the disk and allocated the pool object in it. To avoid the need to carry over the disk definition object, refactor this function to return the allocated object instead. --- src/conf/domain_conf.c | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-)
ACK
On 11/07/13 10:32, Ján Tomko wrote:
On 11/06/2013 06:56 PM, Peter Krempa wrote:
For some strange reason virDomainDiskSourcePoolDefParse accessed def of the disk and allocated the pool object in it. To avoid the need to carry over the disk definition object, refactor this function to return the allocated object instead. --- src/conf/domain_conf.c | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-)
ACK
Pushed; Thanks. Peter
To avoid code duplication between snapshot configuration code that parses the disk source too we need to split out this code that will be reused later on. This patch tries to be code movement, some aspects of this function will be refactored later. --- src/conf/domain_conf.c | 261 ++++++++++++++++++++++++++++--------------------- 1 file changed, 147 insertions(+), 114 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 50ce44d..a6e7047 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4729,6 +4729,140 @@ cleanup: return ret; } + +static int +virDomainDiskSourceDefParse(xmlNodePtr node, + int type, + char **source, + int *proto, + size_t *ndefhosts, + virDomainDiskHostDefPtr *defhosts, + virDomainDiskSourcePoolDefPtr *srcpool) +{ + char *protocol = NULL; + char *transport = NULL; + virDomainDiskHostDefPtr hosts = NULL; + int nhosts = 0; + xmlNodePtr child; + int ret = -1; + + switch (type) { + case VIR_DOMAIN_DISK_TYPE_FILE: + *source = virXMLPropString(node, "file"); + break; + case VIR_DOMAIN_DISK_TYPE_BLOCK: + *source = virXMLPropString(node, "dev"); + break; + case VIR_DOMAIN_DISK_TYPE_DIR: + *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; + } + *proto = virDomainDiskProtocolTypeFromString(protocol); + if (*proto < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown protocol type '%s'"), + protocol); + goto error; + } + + if (!(*source = virXMLPropString(node, "name")) && + *proto != VIR_DOMAIN_DISK_PROTOCOL_NBD) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing name for disk source")); + goto error; + } + 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++; + + /* 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) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown protocol transport type '%s'"), + transport); + goto error; + } + } + 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; + } + if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && + hosts[nhosts - 1].socket != NULL) { + virReportError(VIR_ERR_XML_ERROR, + _("transport %s does not support socket attribute"), + transport); + goto error; + } + 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; + } + hosts[nhosts - 1].port = virXMLPropString(child, "port"); + } + } + child = child->next; + } + break; + case VIR_DOMAIN_DISK_TYPE_VOLUME: + if (virDomainDiskSourcePoolDefParse(node, srcpool) < 0) + goto error; + break; + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected disk type %s"), + virDomainDiskTypeToString(type)); + goto error; + } + + /* 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: + while (nhosts > 0) { + virDomainDiskHostDefFree(&hosts[nhosts - 1]); + nhosts--; + } + + return ret; +} + + #define VENDOR_LEN 8 #define PRODUCT_LEN 16 @@ -4760,8 +4894,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, char *protocol = NULL; char *protocol_transport = NULL; char *trans = NULL; - virDomainDiskHostDefPtr hosts = NULL; - int nhosts = 0; char *bus = NULL; char *cachetag = NULL; char *error_policy = NULL; @@ -4824,116 +4956,26 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { - if (!source && !hosts && !def->srcpool && + if (!source && !def->hosts && !def->srcpool && xmlStrEqual(cur->name, BAD_CAST "source")) { sourceNode = cur; - switch (def->type) { - case VIR_DOMAIN_DISK_TYPE_FILE: - source = virXMLPropString(cur, "file"); - break; - case VIR_DOMAIN_DISK_TYPE_BLOCK: - source = virXMLPropString(cur, "dev"); - break; - case VIR_DOMAIN_DISK_TYPE_DIR: - source = virXMLPropString(cur, "dir"); - break; - case VIR_DOMAIN_DISK_TYPE_NETWORK: - protocol = virXMLPropString(cur, "protocol"); - if (protocol == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing protocol type")); - goto error; - } - def->protocol = virDomainDiskProtocolTypeFromString(protocol); - if (def->protocol < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown protocol type '%s'"), - protocol); - goto error; - } - if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { - expected_secret_usage = VIR_SECRET_USAGE_TYPE_ISCSI; - } else if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { - expected_secret_usage = VIR_SECRET_USAGE_TYPE_CEPH; - } - if (!(source = virXMLPropString(cur, "name")) && - def->protocol != VIR_DOMAIN_DISK_PROTOCOL_NBD) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing name for disk source")); - goto error; - } - child = cur->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++; - - /* transport can be tcp (default), unix or rdma. */ - protocol_transport = virXMLPropString(child, "transport"); - if (protocol_transport != NULL) { - hosts[nhosts - 1].transport = virDomainDiskProtocolTransportTypeFromString(protocol_transport); - if (hosts[nhosts - 1].transport < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown protocol transport type '%s'"), - protocol_transport); - goto error; - } - } - 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; - } - if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && - hosts[nhosts - 1].socket != NULL) { - virReportError(VIR_ERR_XML_ERROR, - _("transport %s does not support socket attribute"), - protocol_transport); - goto error; - } - VIR_FREE(protocol_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; - } - hosts[nhosts - 1].port = virXMLPropString(child, "port"); - } - } - child = child->next; - } - break; - case VIR_DOMAIN_DISK_TYPE_VOLUME: - if (virDomainDiskSourcePoolDefParse(cur, &def->srcpool) < 0) - goto error; - break; - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected disk type %s"), - virDomainDiskTypeToString(def->type)); + if (virDomainDiskSourceDefParse(cur, def->type, + &source, + &def->protocol, + &def->nhosts, + &def->hosts, + &def->srcpool) < 0) goto error; + + if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { + expected_secret_usage = VIR_SECRET_USAGE_TYPE_ISCSI; + } else if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + expected_secret_usage = VIR_SECRET_USAGE_TYPE_CEPH; } startupPolicy = virXMLPropString(cur, "startupPolicy"); - /* 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); } else if (!target && xmlStrEqual(cur->name, BAD_CAST "target")) { target = virXMLPropString(cur, "dev"); @@ -5229,7 +5271,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, /* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present. LUN is for raw access CD-ROMs * that are not attached to a physical device presently */ - if (source == NULL && hosts == NULL && !def->srcpool && + if (source == NULL && def->hosts == NULL && !def->srcpool && def->device != VIR_DOMAIN_DISK_DEVICE_CDROM && def->device != VIR_DOMAIN_DISK_DEVICE_LUN && def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { @@ -5544,10 +5586,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, source = NULL; def->dst = target; target = NULL; - def->hosts = hosts; - hosts = NULL; - def->nhosts = nhosts; - nhosts = 0; def->auth.username = authUsername; authUsername = NULL; def->driverName = driverName; @@ -5601,11 +5639,6 @@ cleanup: VIR_FREE(tray); VIR_FREE(removable); VIR_FREE(trans); - while (nhosts > 0) { - virDomainDiskHostDefFree(&hosts[nhosts - 1]); - nhosts--; - } - VIR_FREE(hosts); VIR_FREE(protocol); VIR_FREE(protocol_transport); VIR_FREE(device); -- 1.8.4.2
On 11/06/2013 06:56 PM, Peter Krempa wrote:
To avoid code duplication between snapshot configuration code that parses the disk source too we need to split out this code that will be reused later on.
This patch tries to be code movement, some aspects of this function will be refactored later. --- src/conf/domain_conf.c | 261 ++++++++++++++++++++++++++++--------------------- 1 file changed, 147 insertions(+), 114 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 50ce44d..a6e7047 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
+ if (virDomainDiskSourceDefParse(cur, def->type, + &source, + &def->protocol, + &def->nhosts, + &def->hosts, + &def->srcpool) < 0) goto error; +
+ if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { + expected_secret_usage = VIR_SECRET_USAGE_TYPE_ISCSI; + } else if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + expected_secret_usage = VIR_SECRET_USAGE_TYPE_CEPH; }
def->protocol was only filled if def->type is TYPE_NETWORK
startupPolicy = virXMLPropString(cur, "startupPolicy");
- /* 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); } else if (!target && xmlStrEqual(cur->name, BAD_CAST "target")) { target = virXMLPropString(cur, "dev");
@@ -5601,11 +5639,6 @@ cleanup: VIR_FREE(tray); VIR_FREE(removable); VIR_FREE(trans); - while (nhosts > 0) { - virDomainDiskHostDefFree(&hosts[nhosts - 1]); - nhosts--; - } - VIR_FREE(hosts);
VIR_FREE(protocol); VIR_FREE(protocol_transport);
You forgot to move these two.
VIR_FREE(device);
The function destroys only the contents not the object itself thus it should be called Clear. --- src/conf/domain_conf.c | 6 +++--- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_command.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a6e7047..d74dccc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1216,13 +1216,13 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) } for (i = 0; i < def->nhosts; i++) - virDomainDiskHostDefFree(&def->hosts[i]); + virDomainDiskHostDefClear(&def->hosts[i]); VIR_FREE(def->hosts); VIR_FREE(def); } -void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def) +void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def) { if (!def) return; @@ -4855,7 +4855,7 @@ virDomainDiskSourceDefParse(xmlNodePtr node, error: while (nhosts > 0) { - virDomainDiskHostDefFree(&hosts[nhosts - 1]); + virDomainDiskHostDefClear(&hosts[nhosts - 1]); nhosts--; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4520ae4..458be97 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2208,7 +2208,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); void virDomainInputDefFree(virDomainInputDefPtr def); void virDomainDiskDefFree(virDomainDiskDefPtr def); void virDomainLeaseDefFree(virDomainLeaseDefPtr def); -void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def); +void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def); int virDomainDeviceFindControllerModel(virDomainDefPtr def, virDomainDeviceInfoPtr info, int controllerType); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ad8a79d..a00c82d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -197,7 +197,7 @@ virDomainDiskErrorPolicyTypeToString; virDomainDiskFindByBusAndDst; virDomainDiskGeometryTransTypeFromString; virDomainDiskGeometryTransTypeToString; -virDomainDiskHostDefFree; +virDomainDiskHostDefClear; virDomainDiskIndexByName; virDomainDiskInsert; virDomainDiskInsertPreAlloced; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e48c9c2..4922845 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3509,7 +3509,7 @@ cleanup: return ret; error: - virDomainDiskHostDefFree(def->hosts); + virDomainDiskHostDefClear(def->hosts); VIR_FREE(def->hosts); goto cleanup; } @@ -3611,7 +3611,7 @@ qemuParseNBDString(virDomainDiskDefPtr disk) return 0; error: - virDomainDiskHostDefFree(h); + virDomainDiskHostDefClear(h); VIR_FREE(h); return -1; } -- 1.8.4.2
On 11/06/2013 06:56 PM, Peter Krempa wrote:
The function destroys only the contents not the object itself thus it should be called Clear. --- src/conf/domain_conf.c | 6 +++--- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_command.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-)
ACK
Now that the function is separate clean out a few ugly places and fix up error messages. --- 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 d74dccc..8ce9361 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4735,17 +4735,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"); @@ -4757,108 +4758,100 @@ 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)) + 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: - while (nhosts > 0) { - virDomainDiskHostDefClear(&hosts[nhosts - 1]); - nhosts--; - } - +cleanup: + virDomainDiskHostDefClear(&host); return ret; } -- 1.8.4.2
participants (2)
-
Ján Tomko -
Peter Krempa