4:44 p.m.
Amongst many other changes, commit id '82c1740a' added a display of the
lvs -o 'stripes' field in order to be able to determine the number of
extents listed in the 'devices' output. This was then used to generate
a regex in order to parse the devices string. As part of the generation
of that regex and various other allocations was adding a comma ","
separator for each regex. In essance quite a bit of code in order to
parse a comma separated set of strings having a specific format.
Furthermore, the 'stripes' field is described as the "Number of stripes
or mirror legs"; however, the setting of the 'nextents' value to something
other than 1 was masked behind comparing the output of the lvs -o 'segtype'
field to "striped". Thus, for a "mirror" volume (or today for raid
segtypes)
the code would assume only 1 extent (or device) in the list. This would
lead to output in a vol-dumpxml of:
<source>
<device path='lv_test_mirror_rimage_0(0),lv_test_mirror_rimage_1'>
<extent start='0' end='33554432'/>
</device>
</source>
This patch removes all the regex/regcomp logic in favor of a simpler
mechanism of splitting the devices (groups[3]) output by the comma ","
separator. Then once split, each output string is further parsed
extracting out the parenthesized starting extent number. This is
then all stored in the volume extents array.
The resulting mirror output is then:
<source>
<device path='lv_test_mirror_rimage_0'>
<extent start='0' end='33554432'/>
</device>
<device path='lv_test_mirror_rimage_1'>
<extent start='0' end='33554432'/>
</device>
</source>
Although used now, the 'segtype' field is kept for usage by a
future patch.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_logical.c | 149 ++++++++++++----------------------
tests/virstringtest.c | 11 +++
2 files changed, 62 insertions(+), 98 deletions(-)
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 7c05b6a..3232c08 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -64,8 +64,6 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
}
-#define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED "striped"
-
struct virStorageBackendLogicalPoolVolData {
virStoragePoolObjPtr pool;
virStorageVolDefPtr vol;
@@ -75,121 +73,76 @@ static int
virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
char **const groups)
{
- int nextents, ret = -1;
- const char *regex_unit = "(\\S+)\\((\\S+)\\)";
- char *regex = NULL;
- regex_t *reg = NULL;
- regmatch_t *vars = NULL;
- char *p = NULL;
- size_t i;
- int err, nvars;
+ int ret = -1;
+ char **extents = NULL;
+ char *extnum, *end;
+ size_t i, nextents = 0;
unsigned long long offset, size, length;
- nextents = 1;
- if (STREQ(groups[4], VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED)) {
- if (virStrToLong_i(groups[5], NULL, 10, &nextents) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("malformed volume extent stripes value"));
- goto cleanup;
- }
- }
+ /* The 'devices' (or extents) are split by a comma ",", so
let's split
+ * each out into a parseable string. Since our regex to generate this
+ * data is "(\\S+)", we can safely assume at least one exists. */
+ if (!(extents = virStringSplitCount(groups[3], ",", 0, &nextents)))
+ goto cleanup;
/* Allocate and fill in extents information */
if (VIR_REALLOC_N(vol->source.extents,
vol->source.nextent + nextents) < 0)
goto cleanup;
+ vol->source.nextent += nextents;
- if (virStrToLong_ull(groups[6], NULL, 10, &length) < 0) {
+ if (virStrToLong_ull(groups[5], NULL, 10, &length) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("malformed volume extent length
value"));
goto cleanup;
}
- if (virStrToLong_ull(groups[7], NULL, 10, &size) < 0) {
+ if (virStrToLong_ull(groups[6], NULL, 10, &size) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("malformed volume extent size
value"));
goto cleanup;
}
- if (VIR_STRDUP(regex, regex_unit) < 0)
- goto cleanup;
-
- for (i = 1; i < nextents; i++) {
- if (VIR_REALLOC_N(regex, strlen(regex) + strlen(regex_unit) + 2) < 0)
- goto cleanup;
- /* "," is the separator of "devices" field */
- strcat(regex, ",");
- strncat(regex, regex_unit, strlen(regex_unit));
- }
-
- if (VIR_ALLOC(reg) < 0)
- goto cleanup;
-
- /* Each extent has a "path:offset" pair, and vars[0] will
- * be the whole matched string.
+ /* The lvs 'devices' field is described as "Underlying devices used
+ * with starting extent numbers." - The format is a "path" element
+ * followed by a "(#)", such as:
+ *
+ * /dev/hda2(0) <--- linear segtype
+ * /dev/sdc1(10240),/dev/sdd1(0) <--- striped segtype
+ * test_mirror_rimage_0(0),test_mirror_rimage_1(0) <-- mirror/raid
+ * segtype
+ *
+ * Details of a mirror can be seen via 'lvs -a -o name,devices'
*/
- nvars = (nextents * 2) + 1;
- if (VIR_ALLOC_N(vars, nvars) < 0)
- goto cleanup;
-
- err = regcomp(reg, regex, REG_EXTENDED);
- if (err != 0) {
- char error[100];
- regerror(err, reg, error, sizeof(error));
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to compile regex %s"),
- error);
- goto cleanup;
- }
-
- err = regexec(reg, groups[3], nvars, vars, 0);
- regfree(reg);
- if (err != 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("malformed volume extent devices value"));
- goto cleanup;
- }
-
- p = groups[3];
-
- /* vars[0] is skipped */
for (i = 0; i < nextents; i++) {
- size_t j;
- int len;
- char *offset_str = NULL;
-
- j = (i * 2) + 1;
- len = vars[j].rm_eo - vars[j].rm_so;
- p[vars[j].rm_eo] = '\0';
-
- if (VIR_STRNDUP(vol->source.extents[vol->source.nextent].path,
- p + vars[j].rm_so, len) < 0)
+ if (!(extnum = strchr(extents[i], '(')) ||
+ !(end = strchr(extnum, ')'))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("malformed volume extent device entry:
'%s'"),
+ extents[0]);
goto cleanup;
+ }
+ *extnum++ = '\0';
+ *end = '\0';
- len = vars[j + 1].rm_eo - vars[j + 1].rm_so;
- if (VIR_STRNDUP(offset_str, p + vars[j + 1].rm_so, len) < 0)
+ if (VIR_STRDUP(vol->source.extents[i].path, extents[i]) < 0)
goto cleanup;
- if (virStrToLong_ull(offset_str, NULL, 10, &offset) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("malformed volume extent offset value"));
- VIR_FREE(offset_str);
+ if (virStrToLong_ull(extnum, NULL, 10, &offset) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("malformed volume extent offset value
'(%s)'"),
+ extnum);
goto cleanup;
}
- VIR_FREE(offset_str);
-
- vol->source.extents[vol->source.nextent].start = offset * size;
- vol->source.extents[vol->source.nextent].end = (offset * size) + length;
- vol->source.nextent++;
+ vol->source.extents[i].start = offset * size;
+ vol->source.extents[i].end = (offset * size) + length;
}
-
ret = 0;
cleanup:
- VIR_FREE(regex);
- VIR_FREE(reg);
- VIR_FREE(vars);
+ virStringFreeList(extents);
+
return ret;
}
@@ -203,7 +156,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
virStorageVolDefPtr vol = NULL;
bool is_new_vol = false;
int ret = -1;
- const char *attrs = groups[9];
+ const char *attrs = groups[8];
/* Skip inactive volume */
if (attrs[4] != 'a')
@@ -281,7 +234,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
VIR_STORAGE_VOL_OPEN_DEFAULT, 0) < 0)
goto cleanup;
- if (virStrToLong_ull(groups[8], NULL, 10, &vol->target.allocation) < 0) {
+ if (virStrToLong_ull(groups[7], NULL, 10, &vol->target.allocation) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("malformed volume allocation
value"));
goto cleanup;
@@ -308,14 +261,14 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
{
/*
* # lvs --separator # --noheadings --units b --unbuffered --nosuffix --options \
- *
"lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr"
VGNAME
+ *
"lv_name,origin,uuid,devices,segtype,seg_size,vg_extent_size,size,lv_attr"
VGNAME
*
- *
RootLV##06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky#/dev/hda2(0)#linear#1#5234491392#33554432#5234491392#-wi-ao
- *
SwapLV##oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M#/dev/hda2(156)#linear#1#1040187392#33554432#1040187392#-wi-ao
- *
Test2##3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR#/dev/hda2(219)#linear#1#1073741824#33554432#1073741824#owi-a-
- *
Test3##UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(251)#linear#1#2181038080#33554432#2181038080#-wi-a-
- *
Test3#Test2#UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(187)#linear#1#1040187392#33554432#1040187392#swi-a-
- *
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#/dev/sdc1(10240),/dev/sdd1(0)#striped#2#42949672960#4194304#-wi-a-
+ *
RootLV##06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky#/dev/hda2(0)#linear#5234491392#33554432#5234491392#-wi-ao
+ *
SwapLV##oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M#/dev/hda2(156)#linear#1040187392#33554432#1040187392#-wi-ao
+ *
Test2##3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR#/dev/hda2(219)#linear#1073741824#33554432#1073741824#owi-a-
+ *
Test3##UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(251)#linear#2181038080#33554432#2181038080#-wi-a-
+ *
Test3#Test2#UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(187)#linear#1040187392#33554432#1040187392#swi-a-
+ *
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#/dev/sdc1(10240),/dev/sdd1(0)#striped#42949672960#4194304#-wi-a-
*
* Pull out name, origin, & uuid, device, device extent start #,
* segment size, extent size, size, attrs
@@ -332,10 +285,10 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
* striped, so "," is not a suitable separator either (rhbz 727474).
*/
const char *regexes[] = {
-
"^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#?\\s*$"
+
"^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#?\\s*$"
};
int vars[] = {
- 10
+ 9
};
int ret = -1;
virCommandPtr cmd;
@@ -351,7 +304,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
"--unbuffered",
"--nosuffix",
"--options",
-
"lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr",
+
"lv_name,origin,uuid,devices,segtype,seg_size,vg_extent_size,size,lv_attr",
pool->def->source.name,
NULL);
if (virCommandRunRegex(cmd,
diff --git a/tests/virstringtest.c b/tests/virstringtest.c
index 38d0126..faa3ce9 100644
--- a/tests/virstringtest.c
+++ b/tests/virstringtest.c
@@ -623,6 +623,17 @@ mymain(void)
const char *tokens8[] = { "gluster", "rdma", NULL };
TEST_SPLIT("gluster+rdma", "+", 2, tokens8);
+ const char *tokens9[] = { "/dev/hda2(0)", NULL };
+ TEST_SPLIT("/dev/hda2(0)", ",", 1, tokens9);
+
+ const char *tokens10[] = { "/dev/sdc1(10240)", "/dev/sdd1(0)",
NULL };
+ TEST_SPLIT("/dev/sdc1(10240),/dev/sdd1(0)", ",", 2, tokens10);
+
+ const char *tokens11[] = { "lv_test_mirror_rimage_0(0)",
+ "lv_test_mirror_rimage_1(0)", NULL };
+ TEST_SPLIT("lv_test_mirror_rimage_0(0),lv_test_mirror_rimage_1(0)",
",",
+ 2, tokens11);
+
if (virtTestRun("strdup", testStrdup, NULL) < 0)
ret = -1;
--
2.5.0