11:56 a.m.
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
11:56 a.m.
New subject: [libvirt] [PATCH 1/4] conf: Refactor virDomainDiskSourcePoolDefParse
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
3:32 a.m.
New subject: [libvirt] [PATCH 1/4] conf: Refactor virDomainDiskSourcePoolDefParse
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
4:14 a.m.
New subject: [libvirt] [PATCH 1/4] conf: Refactor virDomainDiskSourcePoolDefParse
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
11:56 a.m.
New subject: [libvirt] [PATCH 2/4] conf: Split out code to parse the source of a disk definition
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
1:41 p.m.
New subject: [libvirt] [PATCH 2/4] conf: Split out code to parse the source of a disk definition
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);
11:56 a.m.
New subject: [libvirt] [PATCH 3/4] conf: Rename virDomainDiskHostDefFree to virDomainDiskHostDefClear
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
1:41 p.m.
New subject: [libvirt] [PATCH 3/4] conf: Rename virDomainDiskHostDefFree to virDomainDiskHostDefClear
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
11:56 a.m.
New subject: [libvirt] [PATCH 4/4] conf: Refactor virDomainDiskSourceDefParse
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
4035
days inactive
4036
days old
8 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa