[libvirt] [PATCH 3/5] storage_conf: Various fixes or improvements on pool def parsing
Osier Yang
jyang at redhat.com
Thu May 16 12:40:52 UTC 2013
virStorageDefParsePerms:
* Use uid_t/gid_t to do casting
virStoragePoolDefParseSource:
* Improve the error message for source "name" parsing
* Remove the useless casting (const char *)
virStoragePoolDefParseAuthChap:
* Fix the wrong error message
virStoragePoolDefParseAuthCephx:
* Don't leak "uuid"
virStoragePoolDefParseXML:
* Remove the useless casting "(const char *)"
* Use VIR_ERR_XML_ERROR for pool 'type' parsing
* Remove the xmlFree()
* s/tmppath/path/,
* Create "virStoragePoolDefPtr def", and use ret to track the
return value; frees the strings at "cleanup" label instead
if freeing them in the middle.
* Other small changes.
---
src/conf/storage_conf.c | 169 ++++++++++++++++++++++++------------------------
1 file changed, 85 insertions(+), 84 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 8fa805b..dd55d2c 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -446,15 +446,15 @@ virStoragePoolDefParseAuthChap(xmlXPathContextPtr ctxt,
{
auth->login = virXPathString("string(./auth/@login)", ctxt);
if (auth->login == NULL) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing auth host attribute"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing auth login attribute"));
return -1;
}
auth->passwd = virXPathString("string(./auth/@passwd)", ctxt);
if (auth->passwd == NULL) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing auth passwd attribute"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing auth passwd attribute"));
return -1;
}
@@ -466,10 +466,12 @@ virStoragePoolDefParseAuthCephx(xmlXPathContextPtr ctxt,
virStoragePoolAuthCephxPtr auth)
{
char *uuid = NULL;
+ int ret = -1;
+
auth->username = virXPathString("string(./auth/@username)", ctxt);
if (auth->username == NULL) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing auth username attribute"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing auth username attribute"));
return -1;
}
@@ -485,19 +487,22 @@ virStoragePoolDefParseAuthCephx(xmlXPathContextPtr ctxt,
if (auth->secret.usage != NULL) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("either auth secret uuid or usage expected"));
- return -1;
+ goto cleanup;
}
if (virUUIDParse(uuid, auth->secret.uuid) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("invalid auth secret uuid"));
- return -1;
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("invalid auth secret uuid"));
+ goto cleanup;
}
auth->secret.uuidUsable = true;
} else {
auth->secret.uuidUsable = false;
}
- return 0;
+ ret = 0;
+cleanup:
+ VIR_FREE(uuid);
+ return ret;
}
static int
@@ -526,7 +531,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
source->name = virXPathString("string(./name)", ctxt);
if (pool_type == VIR_STORAGE_POOL_RBD && source->name == NULL) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing mandatory 'name' field for RBD pool name"));
+ _("element 'name' is mandatory for RBD pool name"));
goto cleanup;
}
@@ -559,8 +564,8 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
for (i = 0; i < source->nhost; i++) {
name = virXMLPropString(nodeset[i], "name");
if (name == NULL) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing storage pool host name"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing storage pool host name"));
goto cleanup;
}
source->hosts[i].name = name;
@@ -595,8 +600,8 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
char *path = virXMLPropString(nodeset[i], "path");
if (path == NULL) {
VIR_FREE(nodeset);
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing storage pool source device path"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing storage pool source device path"));
goto cleanup;
}
source->devices[i].path = path;
@@ -667,7 +672,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
} else {
virReportError(VIR_ERR_XML_ERROR,
_("unknown auth type '%s'"),
- (const char *)authType);
+ authType);
goto cleanup;
}
}
@@ -768,8 +773,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
if (virStrToLong_i(mode, NULL, 8, &tmp) < 0 || (tmp & ~0777)) {
VIR_FREE(mode);
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("malformed octal mode"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("malformed octal mode"));
goto error;
}
perms->mode = tmp;
@@ -780,22 +785,22 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
perms->uid = (uid_t) -1;
} else {
if (virXPathLong("number(./owner)", ctxt, &v) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("malformed owner element"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("malformed owner element"));
goto error;
}
- perms->uid = (int)v;
+ perms->uid = (uid_t)v;
}
if (virXPathNode("./group", ctxt) == NULL) {
perms->gid = (gid_t) -1;
} else {
if (virXPathLong("number(./group)", ctxt, &v) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("malformed group element"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("malformed group element"));
goto error;
}
- perms->gid = (int)v;
+ perms->gid = (gid_t)v;
}
/* NB, we're ignoring missing labels here - they'll simply inherit */
@@ -811,85 +816,81 @@ static virStoragePoolDefPtr
virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
{
virStoragePoolOptionsPtr options;
- virStoragePoolDefPtr ret;
+ virStoragePoolDefPtr ret = NULL;
+ virStoragePoolDefPtr def;
xmlNodePtr source_node;
char *type = NULL;
char *uuid = NULL;
- char *tmppath;
+ char *target_path = NULL;
- if (VIR_ALLOC(ret) < 0) {
+ if (VIR_ALLOC(def) < 0) {
virReportOOMError();
return NULL;
}
type = virXPathString("string(./@type)", ctxt);
- if ((ret->type = virStoragePoolTypeFromString((const char *)type)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown storage pool type %s"), (const char*)type);
+ if ((def->type = virStoragePoolTypeFromString(type)) < 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unknown storage pool type %s"), type);
goto cleanup;
}
- xmlFree(type);
- type = NULL;
-
- if ((options = virStoragePoolOptionsForPoolType(ret->type)) == NULL) {
+ if ((options = virStoragePoolOptionsForPoolType(def->type)) == NULL)
goto cleanup;
- }
source_node = virXPathNode("./source", ctxt);
if (source_node) {
- if (virStoragePoolDefParseSource(ctxt, &ret->source, ret->type,
+ if (virStoragePoolDefParseSource(ctxt, &def->source, def->type,
source_node) < 0)
goto cleanup;
}
- ret->name = virXPathString("string(./name)", ctxt);
- if (ret->name == NULL &&
+ def->name = virXPathString("string(./name)", ctxt);
+ if (def->name == NULL &&
options->flags & VIR_STORAGE_POOL_SOURCE_NAME)
- ret->name = ret->source.name;
- if (ret->name == NULL) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing pool source name element"));
+ def->name = def->source.name;
+ if (def->name == NULL) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing pool source name element"));
goto cleanup;
}
uuid = virXPathString("string(./uuid)", ctxt);
if (uuid == NULL) {
- if (virUUIDGenerate(ret->uuid) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("unable to generate uuid"));
+ if (virUUIDGenerate(def->uuid) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("unable to generate uuid"));
goto cleanup;
}
} else {
- if (virUUIDParse(uuid, ret->uuid) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("malformed uuid element"));
+ if (virUUIDParse(uuid, def->uuid) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("malformed uuid element"));
goto cleanup;
}
- VIR_FREE(uuid);
}
if (options->flags & VIR_STORAGE_POOL_SOURCE_HOST) {
- if (!ret->source.nhost) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s",
+ if (!def->source.nhost) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing storage pool source host name"));
goto cleanup;
}
}
if (options->flags & VIR_STORAGE_POOL_SOURCE_DIR) {
- if (!ret->source.dir) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing storage pool source path"));
+ if (!def->source.dir) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing storage pool source path"));
goto cleanup;
}
}
+
if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) {
- if (ret->source.name == NULL) {
+ if (def->source.name == NULL) {
/* source name defaults to pool name */
- ret->source.name = strdup(ret->name);
- if (ret->source.name == NULL) {
+ def->source.name = strdup(def->name);
+ if (def->source.name == NULL) {
virReportOOMError();
goto cleanup;
}
@@ -897,30 +898,30 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
}
if (options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) {
- if (!ret->source.adapter.type) {
+ if (!def->source.adapter.type) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing storage pool source adapter"));
goto cleanup;
}
- if (ret->source.adapter.type ==
+ if (def->source.adapter.type ==
VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
- if (!ret->source.adapter.data.fchost.wwnn ||
- !ret->source.adapter.data.fchost.wwpn) {
+ if (!def->source.adapter.data.fchost.wwnn ||
+ !def->source.adapter.data.fchost.wwpn) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("'wwnn' and 'wwpn' must be specified for adapter "
"type 'fchost'"));
goto cleanup;
}
- if (!virValidateWWN(ret->source.adapter.data.fchost.wwnn) ||
- !virValidateWWN(ret->source.adapter.data.fchost.wwpn))
+ if (!virValidateWWN(def->source.adapter.data.fchost.wwnn) ||
+ !virValidateWWN(def->source.adapter.data.fchost.wwpn))
goto cleanup;
- } else if (ret->source.adapter.type ==
+ } else if (def->source.adapter.type ==
VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
- if (!ret->source.adapter.data.name) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing storage pool source adapter name"));
+ if (!def->source.adapter.data.name) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing storage pool source adapter name"));
goto cleanup;
}
}
@@ -928,9 +929,9 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
/* If DEVICE is the only source type, then its required */
if (options->flags == VIR_STORAGE_POOL_SOURCE_DEVICE) {
- if (!ret->source.ndevice) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing storage pool source device name"));
+ if (!def->source.ndevice) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing storage pool source device name"));
goto cleanup;
}
}
@@ -938,29 +939,29 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
/* When we are working with a virtual disk we can skip the target
* path and permissions */
if (!(options->flags & VIR_STORAGE_POOL_SOURCE_NETWORK)) {
- if ((tmppath = virXPathString("string(./target/path)", ctxt)) == NULL) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing storage pool target path"));
+ if (!(target_path = virXPathString("string(./target/path)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing storage pool target path"));
goto cleanup;
}
- ret->target.path = virFileSanitizePath(tmppath);
- VIR_FREE(tmppath);
- if (!ret->target.path)
+ def->target.path = virFileSanitizePath(target_path);
+ if (!def->target.path)
goto cleanup;
- if (virStorageDefParsePerms(ctxt, &ret->target.perms,
+ if (virStorageDefParsePerms(ctxt, &def->target.perms,
"./target/permissions",
DEFAULT_POOL_PERM_MODE) < 0)
goto cleanup;
}
- return ret;
-
+ ret = def;
cleanup:
VIR_FREE(uuid);
- xmlFree(type);
- virStoragePoolDefFree(ret);
- return NULL;
+ VIR_FREE(type);
+ VIR_FREE(target_path);
+ if (!ret)
+ virStoragePoolDefFree(ret);
+ return ret;
}
virStoragePoolDefPtr
--
1.8.1.4
More information about the libvir-list
mailing list