We need to parse a source XML block for FindPoolSources, so this is a step in sharing the parsing. The new storage pool XML 2 XML tests cover this area pretty well to ensure we aren't causing regressions. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/storage_conf.c | 185 +++++++++++++++++++++++++++-------------------- 1 files changed, 108 insertions(+), 77 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index c975b65..6575fe0 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -366,14 +366,14 @@ static int virStoragePoolDefParseAuthChap(virConnectPtr conn, xmlXPathContextPtr ctxt, virStoragePoolAuthChapPtr auth) { - auth->login = virXPathString(conn, "string(./source/auth/@login)", ctxt); + auth->login = virXPathString(conn, "string(./auth/@login)", ctxt); if (auth->login == NULL) { virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", _("missing auth host attribute")); return -1; } - auth->passwd = virXPathString(conn, "string(./source/auth/@passwd)", ctxt); + auth->passwd = virXPathString(conn, "string(./auth/@passwd)", ctxt); if (auth->passwd == NULL) { virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", _("missing auth passwd attribute")); @@ -383,6 +383,96 @@ virStoragePoolDefParseAuthChap(virConnectPtr conn, return 0; } +static int +virStoragePoolDefParseSource(virConnectPtr conn, + xmlXPathContextPtr ctxt, + virStoragePoolSourcePtr source, + int pool_type, + xmlNodePtr node) { + int ret = -1; + xmlNodePtr relnode, *nodeset = NULL; + char *authType = NULL; + int nsource, i; + virStoragePoolOptionsPtr options; + + relnode = ctxt->node; + ctxt->node = node; + + if ((options = virStoragePoolOptionsForPoolType(pool_type)) == NULL) { + goto cleanup; + } + + source->name = virXPathString(conn, "string(./name)", ctxt); + + if (options->formatFromString) { + char *format = virXPathString(conn, "string(./format/@type)", ctxt); + if (format == NULL) + source->format = options->defaultFormat; + else + source->format = options->formatFromString(format); + + if (source->format < 0) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + _("unknown pool format type %s"), format); + VIR_FREE(format); + goto cleanup; + } + VIR_FREE(format); + } + + source->host.name = virXPathString(conn, "string(./host/@name)", ctxt); + + nsource = virXPathNodeSet(conn, "./device", ctxt, &nodeset); + if (nsource > 0) { + if (VIR_ALLOC_N(source->devices, nsource) < 0) { + VIR_FREE(nodeset); + virReportOOMError(conn); + goto cleanup; + } + + for (i = 0 ; i < nsource ; i++) { + xmlChar *path = xmlGetProp(nodeset[i], BAD_CAST "path"); + if (path == NULL) { + VIR_FREE(nodeset); + virStorageReportError(conn, VIR_ERR_XML_ERROR, + "%s", _("missing storage pool source device path")); + goto cleanup; + } + source->devices[i].path = (char *)path; + } + source->ndevice = nsource; + } + + source->dir = virXPathString(conn, "string(./dir/@path)", ctxt); + source->adapter = virXPathString(conn, "string(./adapter/@name)", ctxt); + + authType = virXPathString(conn, "string(./auth/@type)", ctxt); + if (authType == NULL) { + source->authType = VIR_STORAGE_POOL_AUTH_NONE; + } else { + if (STREQ(authType, "chap")) { + source->authType = VIR_STORAGE_POOL_AUTH_CHAP; + } else { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + _("unknown auth type '%s'"), + (const char *)authType); + goto cleanup; + } + } + + if (source->authType == VIR_STORAGE_POOL_AUTH_CHAP) { + if (virStoragePoolDefParseAuthChap(conn, ctxt, &source->auth.chap) < 0) + goto cleanup; + } + + ret = 0; +cleanup: + ctxt->node = relnode; + + VIR_FREE(authType); + VIR_FREE(nodeset); + return ret; +} static int virStorageDefParsePerms(virConnectPtr conn, @@ -460,9 +550,9 @@ virStoragePoolDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) { virStoragePoolOptionsPtr options; virStoragePoolDefPtr ret; + xmlNodePtr source_node; char *type = NULL; char *uuid = NULL; - char *authType = NULL; if (VIR_ALLOC(ret) < 0) { virReportOOMError(conn); @@ -483,10 +573,17 @@ virStoragePoolDefParseXML(virConnectPtr conn, goto cleanup; } + source_node = virXPathNode(conn, "./source", ctxt); + if (source_node) { + if (virStoragePoolDefParseSource(conn, ctxt, &ret->source, ret->type, + source_node) < 0) + goto cleanup; + } + ret->name = virXPathString(conn, "string(./name)", ctxt); if (ret->name == NULL && options->flags & VIR_STORAGE_POOL_SOURCE_NAME) - ret->name = virXPathString(conn, "string(./source/name)", ctxt); + ret->name = ret->source.name; if (ret->name == NULL) { virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", _("missing pool source name element")); @@ -509,66 +606,23 @@ virStoragePoolDefParseXML(virConnectPtr conn, VIR_FREE(uuid); } - if (options->formatFromString) { - char *format = virXPathString(conn, "string(./source/format/@type)", ctxt); - if (format == NULL) - ret->source.format = options->defaultFormat; - else - ret->source.format = options->formatFromString(format); - - if (ret->source.format < 0) { - virStorageReportError(conn, VIR_ERR_XML_ERROR, - _("unknown pool format type %s"), format); - VIR_FREE(format); - goto cleanup; - } - VIR_FREE(format); - } - if (options->flags & VIR_STORAGE_POOL_SOURCE_HOST) { - if ((ret->source.host.name = virXPathString(conn, "string(./source/host/@name)", ctxt)) == NULL) { + if (!ret->source.host.name) { virStorageReportError(conn, VIR_ERR_XML_ERROR, - "%s", _("missing storage pool source host name")); + "%s", + _("missing storage pool source host name")); goto cleanup; } } - if (options->flags & VIR_STORAGE_POOL_SOURCE_DEVICE) { - xmlNodePtr *nodeset = NULL; - int nsource, i; - if ((nsource = virXPathNodeSet(conn, "./source/device", ctxt, &nodeset)) < 0) { - virStorageReportError(conn, VIR_ERR_XML_ERROR, - "%s", _("cannot extract storage pool source devices")); - goto cleanup; - } - if (VIR_ALLOC_N(ret->source.devices, nsource) < 0) { - VIR_FREE(nodeset); - virReportOOMError(conn); - goto cleanup; - } - for (i = 0 ; i < nsource ; i++) { - xmlChar *path = xmlGetProp(nodeset[i], BAD_CAST "path"); - if (path == NULL) { - VIR_FREE(nodeset); - virStorageReportError(conn, VIR_ERR_XML_ERROR, - "%s", _("missing storage pool source device path")); - goto cleanup; - } - ret->source.devices[i].path = (char *)path; - } - VIR_FREE(nodeset); - ret->source.ndevice = nsource; - } if (options->flags & VIR_STORAGE_POOL_SOURCE_DIR) { - if ((ret->source.dir = virXPathString(conn, "string(./source/dir/@path)", ctxt)) == NULL) { + if (!ret->source.dir) { virStorageReportError(conn, VIR_ERR_XML_ERROR, - "%s", _("missing storage pool source path")); + "%s", _("missing storage pool source path")); goto cleanup; } } if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) { - ret->source.name = virXPathString(conn, "string(./source/name)", - ctxt); if (ret->source.name == NULL) { /* source name defaults to pool name */ ret->source.name = strdup(ret->name); @@ -580,36 +634,13 @@ virStoragePoolDefParseXML(virConnectPtr conn, } if (options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) { - if ((ret->source.adapter = virXPathString(conn, - "string(./source/adapter/@name)", - ctxt)) == NULL) { + if (!ret->source.adapter) { virStorageReportError(conn, VIR_ERR_XML_ERROR, - "%s", _("missing storage pool source adapter name")); + "%s", _("missing storage pool source adapter name")); goto cleanup; } } - authType = virXPathString(conn, "string(./source/auth/@type)", ctxt); - if (authType == NULL) { - ret->source.authType = VIR_STORAGE_POOL_AUTH_NONE; - } else { - if (STREQ(authType, "chap")) { - ret->source.authType = VIR_STORAGE_POOL_AUTH_CHAP; - } else { - virStorageReportError(conn, VIR_ERR_XML_ERROR, - _("unknown auth type '%s'"), - (const char *)authType); - VIR_FREE(authType); - goto cleanup; - } - VIR_FREE(authType); - } - - if (ret->source.authType == VIR_STORAGE_POOL_AUTH_CHAP) { - if (virStoragePoolDefParseAuthChap(conn, ctxt, &ret->source.auth.chap) < 0) - goto cleanup; - } - if ((ret->target.path = virXPathString(conn, "string(./target/path)", ctxt)) == NULL) { virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", _("missing storage pool target path")); -- 1.6.0.6

Similar in theory to *AssignDef type functions, this duplicate functionality will be used by an future FindPoolSources implementations. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/storage_conf.c | 22 ++++++++++++++++++++++ src/conf/storage_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/storage/storage_backend_fs.c | 31 ++++++++++++++++++------------- src/storage/storage_backend_logical.c | 9 ++------- 5 files changed, 46 insertions(+), 20 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 6575fe0..13056e8 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1633,6 +1633,28 @@ virStoragePoolObjDeleteDef(virConnectPtr conn, return 0; } +virStoragePoolSourcePtr +virStoragePoolSourceListNewSource(virConnectPtr conn, + virStoragePoolSourceListPtr list) +{ + virStoragePoolSourcePtr source; + + if (VIR_ALLOC(source) < 0) { + virReportOOMError(conn); + return NULL; + } + + if (VIR_REALLOC_N(list->sources, list->nsources+1) < 0) { + virReportOOMError(conn); + return NULL; + } + + source = &list->sources[list->nsources++]; + memset(source, 0, sizeof(*source)); + + return source; +} + char *virStoragePoolSourceListFormat(virConnectPtr conn, virStoragePoolSourceListPtr def) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index a22ac5e..652448a 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -375,6 +375,9 @@ void virStoragePoolObjListFree(virStoragePoolObjListPtr pools); void virStoragePoolObjRemove(virStoragePoolObjListPtr pools, virStoragePoolObjPtr pool); +virStoragePoolSourcePtr +virStoragePoolSourceListNewSource(virConnectPtr conn, + virStoragePoolSourceListPtr list); char *virStoragePoolSourceListFormat(virConnectPtr conn, virStoragePoolSourceListPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 45d1069..ec3c7d9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -376,6 +376,7 @@ virStoragePoolObjListFree; virStoragePoolObjRemove; virStoragePoolObjSaveDef; virStoragePoolSourceFree; +virStoragePoolSourceListNewSource; virStoragePoolSourceListFormat; virStorageVolDefFindByKey; virStorageVolDefFindByName; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 6816da8..2b7f083 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -136,14 +136,15 @@ struct _virNetfsDiscoverState { typedef struct _virNetfsDiscoverState virNetfsDiscoverState; static int -virStorageBackendFileSystemNetFindPoolSourcesFunc(virConnectPtr conn ATTRIBUTE_UNUSED, +virStorageBackendFileSystemNetFindPoolSourcesFunc(virConnectPtr conn, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, char **const groups, void *data) { virNetfsDiscoverState *state = data; const char *name, *path; - virStoragePoolSource *src; + virStoragePoolSource *src = NULL; + int ret = -1; path = groups[0]; @@ -151,30 +152,34 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(virConnectPtr conn ATTRIBUTE_U if (name == NULL) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, _("invalid netfs path (no /): %s"), path); - return -1; + goto cleanup; } name += 1; if (*name == '\0') { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, _("invalid netfs path (ends in /): %s"), path); - return -1; + goto cleanup; } - if (VIR_REALLOC_N(state->list.sources, state->list.nsources+1) < 0) { - virReportOOMError(conn); - return -1; - } - memset(state->list.sources + state->list.nsources, 0, sizeof(*state->list.sources)); + if (!(src = virStoragePoolSourceListNewSource(conn, &state->list))) + goto cleanup; - src = state->list.sources + state->list.nsources++; if (!(src->host.name = strdup(state->host)) || - !(src->dir = strdup(path))) - return -1; + !(src->dir = strdup(path))) { + virReportOOMError(conn); + goto cleanup; + } src->format = VIR_STORAGE_POOL_NETFS_NFS; - return 0; + src = NULL; + ret = 0; +cleanup: + if (src) + virStoragePoolSourceFree(src); + return ret; } + static char * virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn, const char *srcSpec, diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4389120..91726cd 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -278,15 +278,10 @@ virStorageBackendLogicalFindPoolSourcesFunc(virConnectPtr conn, } if (thisSource == NULL) { - if (VIR_REALLOC_N(sourceList->sources, sourceList->nsources + 1) != 0) { - virReportOOMError(conn); + if (!(thisSource = virStoragePoolSourceListNewSource(conn, + sourceList))) goto err_no_memory; - } - - thisSource = &sourceList->sources[sourceList->nsources]; - sourceList->nsources++; - memset(thisSource, 0, sizeof(*thisSource)); thisSource->name = vgname; } else -- 1.6.0.6

This will simplify adding FindPoolSources support to more pool backends in the future (as well as the test driver). Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/storage_conf.c | 52 ++++++++++++++++++++++++++++++++++++++ src/conf/storage_conf.h | 4 +++ src/libvirt_private.syms | 1 + src/storage/storage_backend_fs.c | 34 ++++++------------------ 4 files changed, 66 insertions(+), 25 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 13056e8..2924a0d 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -474,6 +474,58 @@ cleanup: return ret; } +virStoragePoolSourcePtr +virStoragePoolDefParseSourceString(virConnectPtr conn, + const char *srcSpec, + int pool_type) +{ + xmlDocPtr doc = NULL; + xmlNodePtr node = NULL; + xmlXPathContextPtr xpath_ctxt = NULL; + virStoragePoolSourcePtr def = NULL, ret = NULL; + + doc = xmlReadDoc((const xmlChar *)srcSpec, "srcSpec.xml", NULL, + XML_PARSE_NOENT | XML_PARSE_NONET | + XML_PARSE_NOERROR | XML_PARSE_NOWARNING); + + if (doc == NULL) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + "%s", _("bad <source> spec")); + goto cleanup; + } + + xpath_ctxt = xmlXPathNewContext(doc); + if (xpath_ctxt == NULL) { + virReportOOMError(conn); + goto cleanup; + } + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(conn); + goto cleanup; + } + + node = virXPathNode(conn, "/source", xpath_ctxt); + if (!node) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + "%s", _("root element was not source")); + goto cleanup; + } + + if (virStoragePoolDefParseSource(conn, xpath_ctxt, def, pool_type, + node) < 0) + goto cleanup; + + ret = def; + def = NULL; +cleanup: + if (def) + virStoragePoolSourceFree(def); + xmlFreeDoc(doc); + xmlXPathFreeContext(xpath_ctxt); + + return ret; +} static int virStorageDefParsePerms(virConnectPtr conn, xmlXPathContextPtr ctxt, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 652448a..a795981 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -376,6 +376,10 @@ void virStoragePoolObjRemove(virStoragePoolObjListPtr pools, virStoragePoolObjPtr pool); virStoragePoolSourcePtr +virStoragePoolDefParseSourceString(virConnectPtr conn, + const char *srcSpec, + int pool_type); +virStoragePoolSourcePtr virStoragePoolSourceListNewSource(virConnectPtr conn, virStoragePoolSourceListPtr list); char *virStoragePoolSourceListFormat(virConnectPtr conn, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ec3c7d9..be1c870 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -376,6 +376,7 @@ virStoragePoolObjListFree; virStoragePoolObjRemove; virStoragePoolObjSaveDef; virStoragePoolSourceFree; +virStoragePoolDefParseSourceString; virStoragePoolSourceListNewSource; virStoragePoolSourceListFormat; virStorageVolDefFindByKey; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 2b7f083..16e4bd9 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -199,8 +199,6 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn, int vars[] = { 1 }; - xmlDocPtr doc = NULL; - xmlXPathContextPtr xpath_ctxt = NULL; virNetfsDiscoverState state = { .host = NULL, .list = { @@ -210,31 +208,18 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn, } }; const char *prog[] = { SHOWMOUNT, "--no-headers", "--exports", NULL, NULL }; + virStoragePoolSourcePtr source = NULL; int exitstatus; char *retval = NULL; unsigned int i; - doc = xmlReadDoc((const xmlChar *)srcSpec, "srcSpec.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOERROR | XML_PARSE_NOWARNING); - if (doc == NULL) { - virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", _("bad <source> spec")); + source = virStoragePoolDefParseSourceString(conn, srcSpec, + VIR_STORAGE_POOL_NETFS); + if (!source) goto cleanup; - } - xpath_ctxt = xmlXPathNewContext(doc); - if (xpath_ctxt == NULL) { - virReportOOMError(conn); - goto cleanup; - } - - state.host = virXPathString(conn, "string(/source/host/@name)", xpath_ctxt); - if (!state.host || !state.host[0]) { - virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", - _("missing <host> in <source> spec")); - goto cleanup; - } - prog[3] = state.host; + state.host = source->host.name; + prog[3] = source->host.name; if (virStorageBackendRunProgRegex(conn, NULL, prog, 1, regexes, vars, virStorageBackendFileSystemNetFindPoolSourcesFunc, @@ -251,11 +236,10 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn, for (i = 0; i < state.list.nsources; i++) virStoragePoolSourceFree(&state.list.sources[i]); - VIR_FREE(state.list.sources); - VIR_FREE(state.host); + if (source) + virStoragePoolSourceFree(source); - xmlFreeDoc(doc); - xmlXPathFreeContext(xpath_ctxt); + VIR_FREE(state.list.sources); return retval; } -- 1.6.0.6

The results are hardcoded into the test driver, no option to read from a testfile is implemented at this time. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/test/test_driver.c | 75 +++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 888bc9c..aabad9a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -225,6 +225,29 @@ static const char *defaultPoolXML = " </target>" "</pool>"; +static const char *defaultPoolSourcesLogicalXML = +"<sources>\n" +" <source>\n" +" <device path='/dev/sda20'/>\n" +" <name>testvg1</name>\n" +" <format type='lvm2'/>\n" +" </source>\n" +" <source>\n" +" <device path='/dev/sda21'/>\n" +" <name>testvg2</name>\n" +" <format type='lvm2'/>\n" +" </source>\n" +"</sources>\n"; + +static const char *defaultPoolSourcesNetFSXML = +"<sources>\n" +" <source>\n" +" <host name='%s'/>\n" +" <dir path='/testshare'/>\n" +" <format type='nfs'/>\n" +" </source>\n" +"</sources>\n"; + static const char *defaultNodeXML = "<device>" " <name>computer</name>" @@ -3210,12 +3233,56 @@ cleanup: } static char * -testStorageFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *type ATTRIBUTE_UNUSED, - const char *srcSpec ATTRIBUTE_UNUSED, +testStorageFindPoolSources(virConnectPtr conn, + const char *type, + const char *srcSpec, unsigned int flags ATTRIBUTE_UNUSED) { - return NULL; + virStoragePoolSourcePtr source = NULL; + int pool_type; + char *ret = NULL; + + pool_type = virStoragePoolTypeFromString(type); + if (!pool_type) { + testError(conn, VIR_ERR_INTERNAL_ERROR, + _("unknown storage pool type %s"), type); + goto cleanup; + } + + if (srcSpec) { + source = virStoragePoolDefParseSourceString(conn, srcSpec, pool_type); + if (!source) + goto cleanup; + } + + switch (pool_type) { + + case VIR_STORAGE_POOL_LOGICAL: + ret = strdup(defaultPoolSourcesLogicalXML); + if (!ret) + virReportOOMError(conn); + break; + + case VIR_STORAGE_POOL_NETFS: + if (!source || !source->host.name) { + testError(conn, VIR_ERR_INVALID_ARG, + "%s", "hostname must be specified for netfs sources"); + goto cleanup; + } + + if (virAsprintf(&ret, defaultPoolSourcesNetFSXML, + source->host.name) < 0) + virReportOOMError(conn); + break; + + default: + testError(conn, VIR_ERR_NO_SUPPORT, + _("pool type '%s' does not support source discovery"), type); + } + +cleanup: + virStoragePoolSourceFree(source); + return ret; } -- 1.6.0.6