8:23 a.m.
Fix the broken XPath and add tests.
Peter Krempa (3):
storage: util: Split out the gluster volume extraction code into new
function
test: Introduce testing of virStorageUtilGlusterExtractPoolSources
storage: Fix XPath for looking up gluster volume name
src/storage/storage_util.c | 93 +++++++++++--------
src/storage/storage_util.h | 4 +
tests/Makefile.am | 15 ++-
.../virstorageutildata/gluster-parse-basic-dst.xml | 6 ++
.../virstorageutildata/gluster-parse-basic-src.xml | 47 ++++++++++
.../gluster-parse-multivol-dst.xml | 14 +++
.../gluster-parse-multivol-src.xml | 32 +++++++
tests/virstorageutiltest.c | 103 +++++++++++++++++++++
8 files changed, 272 insertions(+), 42 deletions(-)
create mode 100644 tests/virstorageutildata/gluster-parse-basic-dst.xml
create mode 100644 tests/virstorageutildata/gluster-parse-basic-src.xml
create mode 100644 tests/virstorageutildata/gluster-parse-multivol-dst.xml
create mode 100644 tests/virstorageutildata/gluster-parse-multivol-src.xml
create mode 100644 tests/virstorageutiltest.c
--
2.12.1
8:23 a.m.
New subject: [libvirt] [PATCH 1/3] storage: util: Split out the gluster volume extraction code into new function
To allow testing of the algorithm, split out the extractor into a
separate helper.
---
src/storage/storage_util.c | 93 +++++++++++++++++++++++++++-------------------
src/storage/storage_util.h | 4 ++
2 files changed, 58 insertions(+), 39 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 7687eb89a..8459e9d5b 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2836,6 +2836,59 @@ virStorageBackendDeleteLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
}
+int
+virStorageUtilGlusterExtractPoolSources(const char *host,
+ const char *xml,
+ int pooltype,
+ virStoragePoolSourceListPtr list)
+{
+ xmlDocPtr doc = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlNodePtr *nodes = NULL;
+ virStoragePoolSource *src = NULL;
+ size_t i;
+ int nnodes;
+ int ret = -1;
+
+ if (!(doc = virXMLParseStringCtxt(xml, _("(gluster_cli_output)"),
&ctxt)))
+ goto cleanup;
+
+ if ((nnodes = virXPathNodeSet("//volumes/volume", ctxt, &nodes)) <
0)
+ goto cleanup;
+
+ for (i = 0; i < nnodes; i++) {
+ ctxt->node = nodes[i];
+
+ if (!(src = virStoragePoolSourceListNewSource(list)))
+ goto cleanup;
+
+ if (!(src->dir = virXPathString("string(//name)", ctxt))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to extract gluster volume name"));
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC_N(src->hosts, 1) < 0)
+ goto cleanup;
+ src->nhost = 1;
+
+ if (VIR_STRDUP(src->hosts[0].name, host) < 0)
+ goto cleanup;
+
+ src->format = pooltype;
+ }
+
+ ret = nnodes;
+
+ cleanup:
+ VIR_FREE(nodes);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(doc);
+
+ return ret;
+}
+
+
/**
* virStorageBackendFindGlusterPoolSources:
* @host: host to detect volumes on
@@ -2856,12 +2909,6 @@ virStorageBackendFindGlusterPoolSources(const char *host,
char *glusterpath = NULL;
char *outbuf = NULL;
virCommandPtr cmd = NULL;
- xmlDocPtr doc = NULL;
- xmlXPathContextPtr ctxt = NULL;
- xmlNodePtr *nodes = NULL;
- virStoragePoolSource *src = NULL;
- size_t i;
- int nnodes;
int rc;
int ret = -1;
@@ -2892,41 +2939,9 @@ virStorageBackendFindGlusterPoolSources(const char *host,
goto cleanup;
}
- if (!(doc = virXMLParseStringCtxt(outbuf, _("(gluster_cli_output)"),
- &ctxt)))
- goto cleanup;
-
- if ((nnodes = virXPathNodeSet("//volumes/volume", ctxt, &nodes)) <
0)
- goto cleanup;
-
- for (i = 0; i < nnodes; i++) {
- ctxt->node = nodes[i];
-
- if (!(src = virStoragePoolSourceListNewSource(list)))
- goto cleanup;
-
- if (!(src->dir = virXPathString("string(//name)", ctxt))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("failed to extract gluster volume name"));
- goto cleanup;
- }
-
- if (VIR_ALLOC_N(src->hosts, 1) < 0)
- goto cleanup;
- src->nhost = 1;
-
- if (VIR_STRDUP(src->hosts[0].name, host) < 0)
- goto cleanup;
-
- src->format = pooltype;
- }
-
- ret = nnodes;
+ ret = virStorageUtilGlusterExtractPoolSources(host, outbuf, pooltype, list);
cleanup:
- VIR_FREE(nodes);
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(doc);
VIR_FREE(outbuf);
virCommandFree(cmd);
VIR_FREE(glusterpath);
diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h
index fa3b6522c..bf8424808 100644
--- a/src/storage/storage_util.h
+++ b/src/storage/storage_util.h
@@ -93,6 +93,10 @@ int virStorageBackendDeleteLocal(virConnectPtr conn,
int virStorageBackendRefreshLocal(virConnectPtr conn,
virStoragePoolObjPtr pool);
+int virStorageUtilGlusterExtractPoolSources(const char *host,
+ const char *xml,
+ int pooltype,
+ virStoragePoolSourceListPtr list);
int virStorageBackendFindGlusterPoolSources(const char *host,
int pooltype,
virStoragePoolSourceListPtr list,
--
2.12.1
8:23 a.m.
New subject: [libvirt] [PATCH 2/3] test: Introduce testing of virStorageUtilGlusterExtractPoolSources
Add a test program called virstorageutiltest and test the gluster pool
detection code.
---
tests/Makefile.am | 15 ++-
.../virstorageutildata/gluster-parse-basic-dst.xml | 6 ++
.../virstorageutildata/gluster-parse-basic-src.xml | 47 ++++++++++
tests/virstorageutiltest.c | 102 +++++++++++++++++++++
4 files changed, 167 insertions(+), 3 deletions(-)
create mode 100644 tests/virstorageutildata/gluster-parse-basic-dst.xml
create mode 100644 tests/virstorageutildata/gluster-parse-basic-src.xml
create mode 100644 tests/virstorageutiltest.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a6f189b8b..254c4c0ad 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -160,7 +160,9 @@ EXTRA_DIST = \
xlconfigdata \
xmconfigdata \
xml2sexprdata \
- xml2vmxdata
+ xml2vmxdata \
+ virstorageutildata \
+ $(NULL)
test_helpers = commandhelper ssh
test_programs = virshtest sockettest \
@@ -352,7 +354,7 @@ test_programs += nwfilterxml2firewalltest
endif WITH_NWFILTER
if WITH_STORAGE
-test_programs += storagevolxml2argvtest
+test_programs += storagevolxml2argvtest virstorageutiltest
endif WITH_STORAGE
if WITH_STORAGE_FS
@@ -859,6 +861,13 @@ genericxml2xmltest_LDADD = $(LDADDS)
if WITH_STORAGE
+virstorageutiltest_SOURCES = \
+ virstorageutiltest.c testutils.c testutils.h
+virstorageutiltest_LDADD = \
+ ../src/libvirt_driver_storage_impl.la \
+ $(LDADDS) \
+ $(NULL)
+
storagevolxml2argvtest_SOURCES = \
storagevolxml2argvtest.c \
testutils.c testutils.h
@@ -867,7 +876,7 @@ storagevolxml2argvtest_LDADD = \
../src/libvirt_driver_storage_impl.la $(LDADDS)
else ! WITH_STORAGE
-EXTRA_DIST += storagevolxml2argvtest.c
+EXTRA_DIST += storagevolxml2argvtest.c virstorageutiltest.c
endif ! WITH_STORAGE
storagevolxml2xmltest_SOURCES = \
diff --git a/tests/virstorageutildata/gluster-parse-basic-dst.xml
b/tests/virstorageutildata/gluster-parse-basic-dst.xml
new file mode 100644
index 000000000..fbde06f3b
--- /dev/null
+++ b/tests/virstorageutildata/gluster-parse-basic-dst.xml
@@ -0,0 +1,6 @@
+<sources>
+ <source>
+ <host name='testhost'/>
+ <dir path='vol0'/>
+ </source>
+</sources>
diff --git a/tests/virstorageutildata/gluster-parse-basic-src.xml
b/tests/virstorageutildata/gluster-parse-basic-src.xml
new file mode 100644
index 000000000..08f97cb72
--- /dev/null
+++ b/tests/virstorageutildata/gluster-parse-basic-src.xml
@@ -0,0 +1,47 @@
+<?xml version="1.0" encoding="UTF-8"
standalone="yes"?>
+<cliOutput>
+ <opRet>0</opRet>
+ <opErrno>0</opErrno>
+ <opErrstr/>
+ <volInfo>
+ <volumes>
+ <volume>
+ <name>vol0</name>
+ <id>ac14dfa5-0b98-4593-a2aa-9fe2bb9b9ce3</id>
+ <status>1</status>
+ <statusStr>Started</statusStr>
+ <snapshotCount>0</snapshotCount>
+ <brickCount>2</brickCount>
+ <distCount>2</distCount>
+ <stripeCount>1</stripeCount>
+ <replicaCount>2</replicaCount>
+ <arbiterCount>0</arbiterCount>
+ <disperseCount>0</disperseCount>
+ <redundancyCount>0</redundancyCount>
+ <type>2</type>
+ <typeStr>Replicate</typeStr>
+ <transport>0</transport>
+ <bricks>
+ <brick
uuid="a6f5ddea-bc6a-44db-ae1d-5aa1db743490">virt-gluster-node1:/bricks/brick1/brick<name>virt-gluster-node1:/bricks/brick1/brick</name><hostUuid>a6f5ddea-bc6a-44db-ae1d-5aa1db743490</hostUuid><isArbiter>0</isArbiter></brick>
+ <brick
uuid="f4ab9fb1-44ec-443b-8783-e5f70ed78da3">virt-gluster-node2:/bricks/brick1/brick<name>virt-gluster-node2:/bricks/brick1/brick</name><hostUuid>f4ab9fb1-44ec-443b-8783-e5f70ed78da3</hostUuid><isArbiter>0</isArbiter></brick>
+ </bricks>
+ <optCount>3</optCount>
+ <options>
+ <option>
+ <name>nfs.disable</name>
+ <value>on</value>
+ </option>
+ <option>
+ <name>performance.readdir-ahead</name>
+ <value>on</value>
+ </option>
+ <option>
+ <name>transport.address-family</name>
+ <value>inet</value>
+ </option>
+ </options>
+ </volume>
+ <count>1</count>
+ </volumes>
+ </volInfo>
+</cliOutput>
diff --git a/tests/virstorageutiltest.c b/tests/virstorageutiltest.c
new file mode 100644
index 000000000..0d4ede6c2
--- /dev/null
+++ b/tests/virstorageutiltest.c
@@ -0,0 +1,102 @@
+/*
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ */
+
+#include <config.h>
+
+#include <stdlib.h>
+
+#include "testutils.h"
+#include "virerror.h"
+#include "virfile.h"
+#include "virlog.h"
+#include "virstring.h"
+
+#include "storage/storage_util.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("tests.storageutiltest");
+
+
+static int
+testGlusterLookupParse(const void *data)
+{
+ virStoragePoolSourceList list = { .type = VIR_STORAGE_POOL_GLUSTER,
+ .nsources = 0,
+ .sources = NULL
+ };
+ const char *testname = data;
+ size_t i;
+ char *srcxml = NULL;
+ char *srcxmldata = NULL;
+ char *destxml = NULL;
+ char *actual = NULL;
+ int ret = -1;
+
+ if (virAsprintf(&srcxml,
+ "%s/virstorageutildata/gluster-parse-%s-src.xml",
+ abs_srcdir, testname) < 0 ||
+ virAsprintf(&destxml,
+ "%s/virstorageutildata/gluster-parse-%s-dst.xml",
+ abs_srcdir, testname) < 0)
+ goto cleanup;
+
+ if (virTestLoadFile(srcxml, &srcxmldata) < 0)
+ goto cleanup;
+
+ if (virStorageUtilGlusterExtractPoolSources("testhost", srcxmldata,
+ VIR_STORAGE_POOL_GLUSTER,
+ &list) < 0)
+ goto cleanup;
+
+ if (!(actual = virStoragePoolSourceListFormat(&list)))
+ goto cleanup;
+
+ ret = virTestCompareToFile(actual, destxml);
+
+ cleanup:
+ VIR_FREE(srcxml);
+ VIR_FREE(srcxmldata);
+ VIR_FREE(destxml);
+ VIR_FREE(actual);
+
+ for (i = 0; i < list.nsources; i++)
+ virStoragePoolSourceClear(&list.sources[i]);
+ VIR_FREE(list.sources);
+
+
+ return ret;
+}
+
+
+static int
+mymain(void)
+{
+ int ret = 0;
+
+#define DO_TEST_GLUSTER_LOOKUP(testname) \
+ if (virTestRun("gluster lookup " testname, testGlusterLookupParse,
\
+ testname) < 0) \
+ ret = -1
+
+ DO_TEST_GLUSTER_LOOKUP("basic");
+
+#undef DO_TEST_GLUSTER_LOOKUP
+
+ return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+VIRT_TEST_MAIN(mymain)
--
2.12.1
8:23 a.m.
New subject: [libvirt] [PATCH 3/3] storage: Fix XPath for looking up gluster volume name
Use the relative lookup specifier rather than the global one. Otherwise
only the first name would be looked up. Add a test case to cover the
scenario.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1436574
---
src/storage/storage_util.c | 2 +-
.../gluster-parse-multivol-dst.xml | 14 ++++++++++
.../gluster-parse-multivol-src.xml | 32 ++++++++++++++++++++++
tests/virstorageutiltest.c | 1 +
4 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 tests/virstorageutildata/gluster-parse-multivol-dst.xml
create mode 100644 tests/virstorageutildata/gluster-parse-multivol-src.xml
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 8459e9d5b..e949fc3d4 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2862,7 +2862,7 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
if (!(src = virStoragePoolSourceListNewSource(list)))
goto cleanup;
- if (!(src->dir = virXPathString("string(//name)", ctxt))) {
+ if (!(src->dir = virXPathString("string(./name)", ctxt))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("failed to extract gluster volume name"));
goto cleanup;
diff --git a/tests/virstorageutildata/gluster-parse-multivol-dst.xml
b/tests/virstorageutildata/gluster-parse-multivol-dst.xml
new file mode 100644
index 000000000..d2d8fefc6
--- /dev/null
+++ b/tests/virstorageutildata/gluster-parse-multivol-dst.xml
@@ -0,0 +1,14 @@
+<sources>
+ <source>
+ <host name='testhost'/>
+ <dir path='aaa'/>
+ </source>
+ <source>
+ <host name='testhost'/>
+ <dir path='test'/>
+ </source>
+ <source>
+ <host name='testhost'/>
+ <dir path='test1'/>
+ </source>
+</sources>
diff --git a/tests/virstorageutildata/gluster-parse-multivol-src.xml
b/tests/virstorageutildata/gluster-parse-multivol-src.xml
new file mode 100644
index 000000000..0c1f9d10e
--- /dev/null
+++ b/tests/virstorageutildata/gluster-parse-multivol-src.xml
@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"
standalone="yes"?>
+<!--- note that the XML file is truncated -->
+<cliOutput>
+ <opRet>0</opRet>
+ <opErrno>0</opErrno>
+ <opErrstr/>
+ <volInfo>
+ <volumes>
+ <volume>
+ <name>aaa</name>
+ <id>d0b219d4-4169-4907-8994-d2e2434854ed</id>
+ <status>0</status>
+ <statusStr>Created</statusStr>
+ <snapshotCount>0</snapshotCount>
+ </volume>
+ <volume>
+ <name>test</name>
+ <id>32826068-2320-4b62-a825-2554edb7f020</id>
+ <status>1</status>
+ <statusStr>Started</statusStr>
+ <snapshotCount>0</snapshotCount>
+ </volume>
+ <volume>
+ <name>test1</name>
+ <id>dfa070f4-b12f-4166-8d68-041b73127abc</id>
+ <status>0</status>
+ <statusStr>Created</statusStr>
+ </volume>
+ <count>3</count>
+ </volumes>
+ </volInfo>
+</cliOutput>
diff --git a/tests/virstorageutiltest.c b/tests/virstorageutiltest.c
index 0d4ede6c2..90f86233d 100644
--- a/tests/virstorageutiltest.c
+++ b/tests/virstorageutiltest.c
@@ -93,6 +93,7 @@ mymain(void)
ret = -1
DO_TEST_GLUSTER_LOOKUP("basic");
+ DO_TEST_GLUSTER_LOOKUP("multivol");
#undef DO_TEST_GLUSTER_LOOKUP
--
2.12.1
7:59 a.m.
On Wed, Mar 29, 2017 at 15:23:37 +0200, Peter Krempa wrote:
Fix the broken XPath and add tests.
Peter Krempa (3):
storage: util: Split out the gluster volume extraction code into new
function
test: Introduce testing of virStorageUtilGlusterExtractPoolSources
storage: Fix XPath for looking up gluster volume name
I'll post another version of this since I found other problem in the
code. I would require changing some code in this patch, so a reorder can
be nicer.
2824
days inactive
2825
days old
4 comments
1 participants
participants (1)
-
Peter Krempa