10:07 a.m.
This series continues where series 0b231195 left and refactors logging filter
parser. Series 0b231195 also introduced a small bug where if a specific
testcase passes, but should fail, the original parser return value is returned
instead of -1. That way, the virlogtest as a whole still appears as passed,
even though it failed. Thus, this series also addresses this issue.
Erik Skultety (3):
tests: virlogtest: Fix testLogParseOutputs return value
tests: Add new testcases to test parsing of log filters in virlogtest
virlog: Refactor virLogParseFilters
src/util/virlog.c | 95 +++++++++++++++++++++++++++++++++++++-----------------
tests/virlogtest.c | 50 +++++++++++++++++++++++++---
2 files changed, 111 insertions(+), 34 deletions(-)
--
2.4.3
10:07 a.m.
New subject: [libvirt] [PATCH 1/3] tests: virlogtest: Fix testLogParseOutputs return value
The test can return positive value even though it should have failed. It just
returns the value parser returned, which should be flipped back to -1 if
something went wrong or the result was unexpected, but it isn't.
---
tests/virlogtest.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tests/virlogtest.c b/tests/virlogtest.c
index 4eef4fd..acb0b08 100644
--- a/tests/virlogtest.c
+++ b/tests/virlogtest.c
@@ -47,10 +47,11 @@ static int
testLogParseOutputs(const void *opaque)
{
int ret = -1;
+ int noutputs;
const struct testLogData *data = opaque;
- ret = virLogParseOutputs(data->str);
- if (ret < 0) {
+ noutputs = virLogParseOutputs(data->str);
+ if (noutputs < 0) {
if (!data->pass) {
VIR_TEST_DEBUG("Got expected error: %s\n",
virGetLastErrorMessage());
@@ -58,9 +59,9 @@ testLogParseOutputs(const void *opaque)
ret = 0;
goto cleanup;
}
- } else if (ret != data->count) {
+ } else if (noutputs != data->count) {
VIR_TEST_DEBUG("Expected number of parsed outputs is %d, "
- "but got %d\n", data->count, ret);
+ "but got %d\n", data->count, noutputs);
goto cleanup;
} else if (!data->pass) {
VIR_TEST_DEBUG("Test should have failed\n");
--
2.4.3
10:07 a.m.
New subject: [libvirt] [PATCH 2/3] tests: Add new testcases to test parsing of log filters in virlogtest
---
tests/virlogtest.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/tests/virlogtest.c b/tests/virlogtest.c
index acb0b08..332b5e4 100644
--- a/tests/virlogtest.c
+++ b/tests/virlogtest.c
@@ -75,6 +75,37 @@ testLogParseOutputs(const void *opaque)
}
static int
+testLogParseFilters(const void *opaque)
+{
+ int ret = -1;
+ int nfilters;
+ const struct testLogData *data = opaque;
+
+ nfilters = virLogParseFilters(data->str);
+ if (nfilters < 0) {
+ if (!data->pass) {
+ VIR_TEST_DEBUG("Got expected error: %s\n",
+ virGetLastErrorMessage());
+ virResetLastError();
+ ret = 0;
+ goto cleanup;
+ }
+ } else if (nfilters != data->count) {
+ VIR_TEST_DEBUG("Expected number of parsed outputs is %d, "
+ "but got %d\n", data->count, nfilters);
+ goto cleanup;
+ } else if (!data->pass) {
+ VIR_TEST_DEBUG("Test should have failed\n");
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ virLogReset();
+ return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -97,6 +128,10 @@ mymain(void)
DO_TEST_FULL("testLogParseOutputs " # str, testLogParseOutputs, str, count,
false)
#define TEST_PARSE_OUTPUTS(str, count) \
DO_TEST_FULL("testLogParseOutputs " # str, testLogParseOutputs, str, count,
true)
+#define TEST_PARSE_FILTERS_FAIL(str, count) \
+ DO_TEST_FULL("testLogParseFilters " # str, testLogParseFilters, str, count,
false)
+#define TEST_PARSE_FILTERS(str, count) \
+ DO_TEST_FULL("testLogParseFilters " # str, testLogParseFilters, str, count,
true)
TEST_LOG_MATCH("2013-10-11 15:43:43.866+0000: 28302: info : libvirt version:
1.1.3");
@@ -107,6 +142,12 @@ mymain(void)
TEST_PARSE_OUTPUTS_FAIL("foo:stderr", 1);
TEST_PARSE_OUTPUTS_FAIL("1:bar", 1);
TEST_PARSE_OUTPUTS_FAIL("1:stderr:foobar", 1);
+ TEST_PARSE_FILTERS("1:foo", 1);
+ TEST_PARSE_FILTERS("1:foo 2:bar 3:foobar", 3);
+ TEST_PARSE_FILTERS_FAIL("5:foo", 1);
+ TEST_PARSE_FILTERS_FAIL("1:", 1);
+ TEST_PARSE_FILTERS_FAIL(":foo", 1);
+ TEST_PARSE_FILTERS_FAIL("1:+", 1);
return ret;
}
--
2.4.3
3:43 p.m.
New subject: [libvirt] [PATCH 2/3] tests: Add new testcases to test parsing of log filters in virlogtest
On 18.03.2016 10:07, Erik Skultety wrote:
---
tests/virlogtest.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/tests/virlogtest.c b/tests/virlogtest.c
index acb0b08..332b5e4 100644
--- a/tests/virlogtest.c
+++ b/tests/virlogtest.c
@@ -75,6 +75,37 @@ testLogParseOutputs(const void *opaque)
}
static int
+testLogParseFilters(const void *opaque)
+{
+ int ret = -1;
+ int nfilters;
+ const struct testLogData *data = opaque;
+
+ nfilters = virLogParseFilters(data->str);
+ if (nfilters < 0) {
+ if (!data->pass) {
+ VIR_TEST_DEBUG("Got expected error: %s\n",
+ virGetLastErrorMessage());
+ virResetLastError();
+ ret = 0;
+ goto cleanup;
+ }
+ } else if (nfilters != data->count) {
+ VIR_TEST_DEBUG("Expected number of parsed outputs is %d, "
+ "but got %d\n", data->count, nfilters);
+ goto cleanup;
+ } else if (!data->pass) {
+ VIR_TEST_DEBUG("Test should have failed\n");
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ virLogReset();
+ return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -97,6 +128,10 @@ mymain(void)
DO_TEST_FULL("testLogParseOutputs " # str, testLogParseOutputs, str,
count, false)
#define TEST_PARSE_OUTPUTS(str, count) \
DO_TEST_FULL("testLogParseOutputs " # str, testLogParseOutputs, str,
count, true)
+#define TEST_PARSE_FILTERS_FAIL(str, count) \
+ DO_TEST_FULL("testLogParseFilters " # str, testLogParseFilters, str,
count, false)
+#define TEST_PARSE_FILTERS(str, count) \
One extra space.
+ DO_TEST_FULL("testLogParseFilters " # str,
testLogParseFilters, str, count, true)
TEST_LOG_MATCH("2013-10-11 15:43:43.866+0000: 28302: info : libvirt version:
1.1.3");
@@ -107,6 +142,12 @@ mymain(void)
TEST_PARSE_OUTPUTS_FAIL("foo:stderr", 1);
TEST_PARSE_OUTPUTS_FAIL("1:bar", 1);
TEST_PARSE_OUTPUTS_FAIL("1:stderr:foobar", 1);
+ TEST_PARSE_FILTERS("1:foo", 1);
+ TEST_PARSE_FILTERS("1:foo 2:bar 3:foobar", 3);
+ TEST_PARSE_FILTERS_FAIL("5:foo", 1);
+ TEST_PARSE_FILTERS_FAIL("1:", 1);
+ TEST_PARSE_FILTERS_FAIL(":foo", 1);
+ TEST_PARSE_FILTERS_FAIL("1:+", 1);
return ret;
}
Michal
10:07 a.m.
New subject: [libvirt] [PATCH 3/3] virlog: Refactor virLogParseFilters
Patch 0b231195 refactored logging output parser to make it more readable.
This patch does similar thing to logging filter parser.
---
src/util/virlog.c | 95 +++++++++++++++++++++++++++++++++++++------------------
1 file changed, 65 insertions(+), 30 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 007fc65..738eaac 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1211,6 +1211,52 @@ virLogParseOutputs(const char *src)
}
+static int
+virLogParseFilter(const char *filter)
+{
+ int ret = -1;
+ size_t count = 0;
+ virLogPriority prio;
+ char **tokens = NULL;
+ unsigned int flags = 0;
+ char *ref = NULL;
+
+ if (!filter)
+ return -1;
+
+ VIR_DEBUG("filter=%s", filter);
+
+ if (!(tokens = virStringSplitCount(filter, ":", 0, &count)))
+ return -1;
+
+ if (count != 2)
+ goto cleanup;
+
+ if (virStrToLong_uip(tokens[0], NULL, 10, &prio) < 0 ||
+ (prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR))
+ goto cleanup;
+
+ ref = tokens[1];
+ if (ref[0] == '+') {
+ flags |= VIR_LOG_STACK_TRACE;
+ ref++;
+ }
+
+ if (!*ref)
+ goto cleanup;
+
+ if (virLogDefineFilter(ref, prio, flags) < 0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ if (ret < 0)
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to parse and define log filter %s"), filter);
+ virStringFreeList(tokens);
+ return ret;
+}
+
/**
* virLogParseFilters:
* @filters: string defining a (set of) filter(s)
@@ -1227,49 +1273,38 @@ virLogParseOutputs(const char *src)
* Multiple filter can be defined in a single @filters, they just need to be
* separated by spaces.
*
- * Returns the number of filter parsed and installed or -1 in case of error
+ * Returns the number of filter parsed or -1 in case of error.
*/
int
virLogParseFilters(const char *filters)
{
- const char *cur = filters, *str;
- char *name;
- virLogPriority prio;
int ret = -1;
int count = 0;
+ size_t i;
+ char **strings = NULL;
- if (cur == NULL)
+ if (!filters)
return -1;
- virSkipSpaces(&cur);
- while (*cur != 0) {
- unsigned int flags = 0;
- prio = virParseNumber(&cur);
- if ((prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR))
- goto cleanup;
- if (*cur != ':')
- goto cleanup;
- cur++;
- if (*cur == '+') {
- flags |= VIR_LOG_STACK_TRACE;
- cur++;
- }
- str = cur;
- while ((*cur != 0) && (!IS_SPACE(cur)))
- cur++;
- if (str == cur)
- goto cleanup;
- if (VIR_STRNDUP(name, str, cur - str) < 0)
+ VIR_DEBUG("filters=%s", filters);
+
+ if (!(strings = virStringSplit(filters, " ", 0)))
+ goto cleanup;
+
+ for (i = 0; strings[i]; i++) {
+ /* virStringSplit may return empty strings */
+ if (STREQ(strings[i], ""))
+ continue;
+
+ if (virLogParseFilter(strings[i]) < 0)
goto cleanup;
- if (virLogDefineFilter(name, prio, flags) >= 0)
- count++;
- VIR_FREE(name);
- virSkipSpaces(&cur);
+
+ count++;
}
+
ret = count;
cleanup:
- if (ret == -1)
- VIR_WARN("Ignoring invalid log filter setting.");
+ virStringFreeList(strings);
return ret;
}
--
2.4.3
3:43 p.m.
On 18.03.2016 10:07, Erik Skultety wrote:
This series continues where series 0b231195 left and refactors
logging filter
parser. Series 0b231195 also introduced a small bug where if a specific
testcase passes, but should fail, the original parser return value is returned
instead of -1. That way, the virlogtest as a whole still appears as passed,
even though it failed. Thus, this series also addresses this issue.
Erik Skultety (3):
tests: virlogtest: Fix testLogParseOutputs return value
tests: Add new testcases to test parsing of log filters in virlogtest
virlog: Refactor virLogParseFilters
src/util/virlog.c | 95 +++++++++++++++++++++++++++++++++++++-----------------
tests/virlogtest.c | 50 +++++++++++++++++++++++++---
2 files changed, 111 insertions(+), 34 deletions(-)
ACK series.
Michal
4:47 p.m.
On 24/03/16 15:43, Michal Privoznik wrote:
On 18.03.2016 10:07, Erik Skultety wrote:
> This series continues where series 0b231195 left and refactors logging filter
> parser. Series 0b231195 also introduced a small bug where if a specific
> testcase passes, but should fail, the original parser return value is returned
> instead of -1. That way, the virlogtest as a whole still appears as passed,
> even though it failed. Thus, this series also addresses this issue.
>
> Erik Skultety (3):
> tests: virlogtest: Fix testLogParseOutputs return value
> tests: Add new testcases to test parsing of log filters in virlogtest
> virlog: Refactor virLogParseFilters
>
> src/util/virlog.c | 95 +++++++++++++++++++++++++++++++++++++-----------------
> tests/virlogtest.c | 50 +++++++++++++++++++++++++---
> 2 files changed, 111 insertions(+), 34 deletions(-)
>
ACK series.
Michal
Adjusted 2/3 and pushed the series.
Thank you for review.
Erik
3103
days inactive
3109
days old
6 comments
2 participants
participants (2)
-
Erik Skultety -
Michal Privoznik