11:35 a.m.
The following series refactors the storage pool parsing code to better
facilitate FindPoolSources (both for the test driver, and future pool backend
implementations). The last patch implements a FindPoolSources for the test
driver, with hardcoded results for test:///default.
Thanks,
Cole
Cole Robinson (4):
storage: Break out pool source parsing to a separate function.
storage: Break out function to add pool source to a SourceList.
storage: Add ParseSourceString function for use with FindPoolSources.
test: Support virStorageFindPoolSources
src/conf/storage_conf.c | 259 +++++++++++++++++++++++----------
src/conf/storage_conf.h | 7 +
src/libvirt_private.syms | 2 +
src/storage/storage_backend_fs.c | 65 ++++-----
src/storage/storage_backend_logical.c | 9 +-
src/test/test_driver.c | 75 +++++++++-
6 files changed, 291 insertions(+), 126 deletions(-)
11:35 a.m.
New subject: [libvirt] [PATCH 1/4] storage: Break out pool source parsing to a separate function.
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(a)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
9:21 a.m.
New subject: [libvirt] [PATCH 1/4] storage: Break out pool source parsing to a separate function.
On Fri, Oct 16, 2009 at 12:35:11PM -0400, Cole Robinson wrote:
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(a)redhat.com>
Refactoring looks fine to me, it seems changing the context node is
fine as it's properly restaured and XPath expressions are changed to
be made relative to the new node.
ACK, thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
11:35 a.m.
New subject: [libvirt] [PATCH 2/4] storage: Break out function to add pool source to a SourceList.
Similar in theory to *AssignDef type functions, this duplicate functionality
will be used by an future FindPoolSources implementations.
Signed-off-by: Cole Robinson <crobinso(a)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
9:24 a.m.
New subject: [libvirt] [PATCH 2/4] storage: Break out function to add pool source to a SourceList.
On Fri, Oct 16, 2009 at 12:35:12PM -0400, Cole Robinson wrote:
Similar in theory to *AssignDef type functions, this duplicate
functionality
will be used by an future FindPoolSources implementations.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
ACK, looks fine !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
11:35 a.m.
New subject: [libvirt] [PATCH 3/4] storage: Add ParseSourceString function for use with FindPoolSources.
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(a)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
9:27 a.m.
New subject: [libvirt] [PATCH 3/4] storage: Add ParseSourceString function for use with FindPoolSources.
On Fri, Oct 16, 2009 at 12:35:13PM -0400, Cole Robinson wrote:
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(a)redhat.com>
ACK, this looks right and similar to other parsing functions
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
11:35 a.m.
New subject: [libvirt] [PATCH 4/4] test: Support virStorageFindPoolSources
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(a)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
9:29 a.m.
New subject: [libvirt] [PATCH 4/4] test: Support virStorageFindPoolSources
On Fri, Oct 16, 2009 at 12:35:14PM -0400, Cole Robinson wrote:
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(a)redhat.com>
---
src/test/test_driver.c | 75 +++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 71 insertions(+), 4 deletions(-)
ACK, that's a good first step.
Out of curiosity did you check make valgrind for leaks ?
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
10:34 a.m.
New subject: [libvirt] [PATCH 4/4] test: Support virStorageFindPoolSources
On 10/28/2009 10:29 AM, Daniel Veillard wrote:
On Fri, Oct 16, 2009 at 12:35:14PM -0400, Cole Robinson wrote:
> 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(a)redhat.com>
> ---
> src/test/test_driver.c | 75 +++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 71 insertions(+), 4 deletions(-)
ACK, that's a good first step.
Out of curiosity did you check make valgrind for leaks ?
Actually, I completely forgot we have 'make -C tests valgrind'. Running
it revealed a leak, but not in any code I touched, so I pushed this
series. I'll post a patch for the preexisting leak.
Thanks,
Cole
5505
days inactive
5517
days old
9 comments
2 participants
participants (2)
-
Cole Robinson -
Daniel Veillard