[kvm-unit-tests PATCH v4 00/13] Improve the output of test runners

[kvm-unit-tests PATCH v4 00/13] Improve the output of test runners

[Radim Krčmář]

v3: http://www.spinics.net/lists/kvm/msg130911.html

Changes are described in each changed patch.

The output differs a bit from v3, because SKIP when QEMU didn't have a
chance to run the test now prints the last line of stdout + stderr in
summary.

  PASS apic (14 tests)
  PASS emulator (131 tests, 1 skipped)
  PASS pmu (67 tests, 1 expected failures)
  SKIP svm (0 tests)
  SKIP taskswitch (i386 only)
  PASS kvmclock_test 
  SKIP hyperv_synic (qemu-kvm: Property '.hv-synic' not found)


Radim Krčmář (13):
  lib/report: allow test skipping
  x86/*: report skipped tests
  x86/unittests: remove svm-disabled
  lib/string: add strncmp
  lib/util: add args_parse_keyval
  x86/pmu: expect failure with nmi_watchdog
  lib/report: don't print 0 failed tests
  scripts/runtime: consolidate summary tags
  run_tests: print SUMMARY line in summary
  run_tests: log stderr
  scripts/runtime: skip tests that cannot run
  scripts/arch-run: change QEMU failure detection
  scripts/run: generalize check

 arm/run                 |  2 +-
 lib/libcflat.h          |  1 +
 lib/report.c            | 49 ++++++++++++++++++++++++-----------
 lib/string.c            | 17 ++++++++-----
 lib/string.h            |  1 +
 lib/util.c              | 17 +++++++++++++
 lib/util.h              | 10 ++++++++
 powerpc/run             |  2 +-
 run_tests.sh            | 15 ++++++-----
 scripts/arch-run.bash   |  2 +-
 scripts/mkstandalone.sh |  4 +++
 scripts/runtime.bash    | 68 ++++++++++++++++++++++++++++++++++++-------------
 x86/Makefile.common     |  1 +
 x86/apic.c              |  7 +++--
 x86/emulator.c          |  2 +-
 x86/hyperv_synic.c      |  2 +-
 x86/pku.c               |  2 +-
 x86/pmu.c               |  8 ++++--
 x86/smap.c              |  2 +-
 x86/svm.c               |  2 +-
 x86/tsc.c               |  2 +-
 x86/unittests.cfg       |  9 +------
 22 files changed, 156 insertions(+), 69 deletions(-)

-- 
2.8.2

[kvm-unit-tests PATCH v4 01/13] lib/report: allow test skipping

[Radim Krčmář]

We can now explicitly mark a unit-test as skipped.
If all unit-tests were skipped, the whole test is reported as skipped as
well.  This also includes the case where no tests were run, but still
ended with report_summary().

When the whole test is skipped, ./run_tests.sh prints yellow "SKIP"
instead of green "PASS".

Return value of 77 is used to please Autotools.  I also renamed few
things in reporting code and chose to refactor a logic while at it.

Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 lib/libcflat.h       |  1 +
 lib/report.c         | 45 +++++++++++++++++++++++++++++++--------------
 scripts/runtime.bash |  2 ++
 3 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index df50615b4366..582e3fc60e28 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -79,6 +79,7 @@ extern void report_prefix_pop(void);
 extern void report(const char *msg_fmt, bool pass, ...);
 extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
 extern void report_abort(const char *msg_fmt, ...);
+extern void report_skip(const char *msg_fmt, ...);
 extern int report_summary(void);
 
 extern void dump_stack(void);
diff --git a/lib/report.c b/lib/report.c
index 9e45781ee2ab..430b2aeaecbd 100644
--- a/lib/report.c
+++ b/lib/report.c
@@ -13,7 +13,7 @@
 #include "libcflat.h"
 #include "asm/spinlock.h"
 
-static unsigned int tests, failures, xfailures;
+static unsigned int tests, failures, xfailures, skipped;
 static char prefixes[256];
 static struct spinlock lock;
 
@@ -43,23 +43,25 @@ void report_prefix_pop(void)
 	spin_unlock(&lock);
 }
 
-void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, va_list va)
+static void va_report(const char *msg_fmt,
+		bool pass, bool xfail, bool skip, va_list va)
 {
-	char *pass = xfail ? "XPASS" : "PASS";
-	char *fail = xfail ? "XFAIL" : "FAIL";
+	char *prefix = skip ? "SKIP"
+	                    : xfail ? (pass ? "XPASS" : "XFAIL")
+	                            : (pass ? "PASS"  : "FAIL");
 
 	spin_lock(&lock);
 
 	tests++;
-	printf("%s: ", cond ? pass : fail);
+	printf("%s: ", prefix);
 	puts(prefixes);
 	vprintf(msg_fmt, va);
 	puts("\n");
-	if (xfail && cond)
-		failures++;
-	else if (xfail)
+	if (skip)
+		skipped++;
+	else if (xfail && !pass)
 		xfailures++;
-	else if (!cond)
+	else if (xfail || !pass)
 		failures++;
 
 	spin_unlock(&lock);
@@ -69,7 +71,7 @@ void report(const char *msg_fmt, bool pass, ...)
 {
 	va_list va;
 	va_start(va, pass);
-	va_report_xfail(msg_fmt, false, pass, va);
+	va_report(msg_fmt, pass, false, false, va);
 	va_end(va);
 }
 
@@ -77,7 +79,15 @@ void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...)
 {
 	va_list va;
 	va_start(va, pass);
-	va_report_xfail(msg_fmt, xfail, pass, va);
+	va_report(msg_fmt, pass, xfail, false, va);
+	va_end(va);
+}
+
+void report_skip(const char *msg_fmt, ...)
+{
+	va_list va;
+	va_start(va, msg_fmt);
+	va_report(msg_fmt, false, false, true, va);
 	va_end(va);
 }
 
@@ -87,9 +97,16 @@ int report_summary(void)
 
 	printf("\nSUMMARY: %d tests, %d unexpected failures", tests, failures);
 	if (xfailures)
-		printf(", %d expected failures\n", xfailures);
-	else
-		printf("\n");
+		printf(", %d expected failures", xfailures);
+	if (skipped)
+		printf(", %d skipped", skipped);
+	printf("\n");
+
+	if (tests == skipped)
+		/* Blame AUTOTOOLS for using 77 for skipped test and QEMU for
+		 * mangling error codes in a way that gets 77 if we ... */
+		return 77 >> 1;
+
 	return failures > 0 ? 1 : 0;
 
 	spin_unlock(&lock);
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 0e055f0dddc2..ed073721216c 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -51,6 +51,8 @@ function run()
 
     if [ $ret -eq 0 ]; then
         echo -e "\e[32mPASS\e[0m $1"
+    elif [ $ret -eq 77 ]; then
+        echo -e "\e[33mSKIP\e[0m $1"
     elif [ $ret -eq 124 ]; then
         echo -e "\e[31mFAIL\e[0m $1 (timeout; duration=$timeout)"
     else
-- 
2.8.2

[kvm-unit-tests PATCH v4 02/13] x86/*: report skipped tests

[Radim Krčmář]

No care to consistency or exhaustivity was given.

Drew pointed out that tscdeadline_latency.c doesn't report skipped
tests.  tscdeadline_latency needs complete refactoring and is not run
with ./run_tests, so I ignored it.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v4: explained the reason behind ignoring v3 review

 x86/apic.c         | 7 +++----
 x86/emulator.c     | 2 +-
 x86/hyperv_synic.c | 2 +-
 x86/pku.c          | 2 +-
 x86/pmu.c          | 2 +-
 x86/smap.c         | 2 +-
 x86/svm.c          | 2 +-
 x86/tsc.c          | 2 +-
 8 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/x86/apic.c b/x86/apic.c
index 61a0a7659805..8b08a950a0c7 100644
--- a/x86/apic.c
+++ b/x86/apic.c
@@ -27,7 +27,7 @@ static void tsc_deadline_timer_isr(isr_regs_t *regs)
     ++tdt_count;
 }
 
-static void start_tsc_deadline_timer(void)
+static void __test_tsc_deadline_timer(void)
 {
     handle_irq(TSC_DEADLINE_TIMER_VECTOR, tsc_deadline_timer_isr);
     irq_enable();
@@ -45,7 +45,6 @@ static int enable_tsc_deadline_timer(void)
     if (cpuid(1).c & (1 << 24)) {
         lvtt = TSC_DEADLINE_TIMER_MODE | TSC_DEADLINE_TIMER_VECTOR;
         apic_write(APIC_LVTT, lvtt);
-        start_tsc_deadline_timer();
         return 1;
     } else {
         return 0;
@@ -55,9 +54,9 @@ static int enable_tsc_deadline_timer(void)
 static void test_tsc_deadline_timer(void)
 {
     if(enable_tsc_deadline_timer()) {
-        printf("tsc deadline timer enabled\n");
+        __test_tsc_deadline_timer();
     } else {
-        printf("tsc deadline timer not detected\n");
+        report_skip("tsc deadline timer not detected");
     }
 }
 
diff --git a/x86/emulator.c b/x86/emulator.c
index 3730721258bc..8d262d832112 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -1082,7 +1082,7 @@ static void illegal_movbe_handler(struct ex_regs *regs)
 static void test_illegal_movbe(void)
 {
 	if (!(cpuid(1).c & (1 << 22))) {
-		printf("SKIP: illegal movbe\n");
+		report_skip("illegal movbe");
 		return;
 	}
 
diff --git a/x86/hyperv_synic.c b/x86/hyperv_synic.c
index 6e088944be1f..f5eb82bb7336 100644
--- a/x86/hyperv_synic.c
+++ b/x86/hyperv_synic.c
@@ -202,7 +202,7 @@ int main(int ac, char **av)
 
         report("Hyper-V SynIC test", ok);
     } else {
-        report("Hyper-V SynIC is not supported", true);
+        report_skip("Hyper-V SynIC is not supported");
     }
 
     return report_summary();
diff --git a/x86/pku.c b/x86/pku.c
index df51d1b59048..6214f0bc151f 100644
--- a/x86/pku.c
+++ b/x86/pku.c
@@ -68,7 +68,7 @@ int main(int ac, char **av)
 
     if (!(cpuid_indexed(7, 0).c & (1 << X86_FEATURE_PKU))) {
         printf("PKU not enabled, aborting\n");
-        abort();
+        return report_summary();
     }
 
     setup_vm();
diff --git a/x86/pmu.c b/x86/pmu.c
index 03f80190bb25..c68980044dee 100644
--- a/x86/pmu.c
+++ b/x86/pmu.c
@@ -387,7 +387,7 @@ int main(int ac, char **av)
 
 	if (!eax.split.version_id) {
 		printf("No pmu is detected!\n");
-		return 1;
+		return report_summary();
 	}
 	printf("PMU version:         %d\n", eax.split.version_id);
 	printf("GP counters:         %d\n", eax.split.num_counters);
diff --git a/x86/smap.c b/x86/smap.c
index 69e71864dc9b..c9de081a474b 100644
--- a/x86/smap.c
+++ b/x86/smap.c
@@ -93,7 +93,7 @@ int main(int ac, char **av)
 
 	if (!(cpuid_indexed(7, 0).b & (1 << X86_FEATURE_SMAP))) {
 		printf("SMAP not enabled, aborting\n");
-		abort();
+		return report_summary();
 	}
 
 	setup_vm();
diff --git a/x86/svm.c b/x86/svm.c
index 934b2ae91fa8..301cf8cb2d6b 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -1064,7 +1064,7 @@ int main(int ac, char **av)
 
     if (!(cpuid(0x80000001).c & 4)) {
         printf("SVM not availble\n");
-        return 0;
+        return report_summary();
     }
 
     setup_svm();
diff --git a/x86/tsc.c b/x86/tsc.c
index 6f89c911c2bc..62450e71725c 100644
--- a/x86/tsc.c
+++ b/x86/tsc.c
@@ -43,5 +43,5 @@ int main()
 		test_rdtscp(0x100);
 	} else
 		printf("rdtscp not supported\n");
-	return 0;
+	return report_summary();
 }
-- 
2.8.2

Re: [kvm-unit-tests PATCH v4 02/13] x86/*: report skipped tests

[Paolo Bonzini]

On 11/05/2016 18:12, Radim Krčmář wrote:
>  
>      if (!(cpuid_indexed(7, 0).c & (1 << X86_FEATURE_PKU))) {
>          printf("PKU not enabled, aborting\n");
> -        abort();
> +        return report_summary();

Needs report_skip?

>      }
>  
>      setup_vm();
> diff --git a/x86/pmu.c b/x86/pmu.c
> index 03f80190bb25..c68980044dee 100644
> --- a/x86/pmu.c
> +++ b/x86/pmu.c
> @@ -387,7 +387,7 @@ int main(int ac, char **av)
>  
>  	if (!eax.split.version_id) {
>  		printf("No pmu is detected!\n");
> -		return 1;
> +		return report_summary();

Needs report_skip?

>  	}
>  	printf("PMU version:         %d\n", eax.split.version_id);
>  	printf("GP counters:         %d\n", eax.split.num_counters);
> diff --git a/x86/smap.c b/x86/smap.c
> index 69e71864dc9b..c9de081a474b 100644
> --- a/x86/smap.c
> +++ b/x86/smap.c
> @@ -93,7 +93,7 @@ int main(int ac, char **av)
>  
>  	if (!(cpuid_indexed(7, 0).b & (1 << X86_FEATURE_SMAP))) {
>  		printf("SMAP not enabled, aborting\n");
> -		abort();
> +		return report_summary();

Needs report_skip?

>  	}
>  
>  	setup_vm();
> diff --git a/x86/svm.c b/x86/svm.c
> index 934b2ae91fa8..301cf8cb2d6b 100644
> --- a/x86/svm.c
> +++ b/x86/svm.c
> @@ -1064,7 +1064,7 @@ int main(int ac, char **av)
>  
>      if (!(cpuid(0x80000001).c & 4)) {
>          printf("SVM not availble\n");
> -        return 0;
> +        return report_summary();

Needs report_skip?

>      }
>  
>      setup_svm();
> diff --git a/x86/tsc.c b/x86/tsc.c
> index 6f89c911c2bc..62450e71725c 100644
> --- a/x86/tsc.c
> +++ b/x86/tsc.c
> @@ -43,5 +43,5 @@ int main()
>  		test_rdtscp(0x100);
>  	} else
>  		printf("rdtscp not supported\n");

Needs report_skip?

> -	return 0;
> +	return report_summary();

Just sending a fixup patch is enough.

Paolo

Re: [kvm-unit-tests PATCH v4 02/13] x86/*: report skipped tests

[Radim Krčmář]

2016-05-17 14:19+0200, Paolo Bonzini:
> On 11/05/2016 18:12, Radim Krčmář wrote:
>>      if (!(cpuid_indexed(7, 0).c & (1 << X86_FEATURE_PKU))) {
>>          printf("PKU not enabled, aborting\n");
>> -        abort();
>> +        return report_summary();
> 
> Needs report_skip?
> 
>> diff --git a/x86/pmu.c b/x86/pmu.c
>> @@ -387,7 +387,7 @@ int main(int ac, char **av)
>>  	if (!eax.split.version_id) {
>>  		printf("No pmu is detected!\n");
>> -		return 1;
>> +		return report_summary();
> 
> Needs report_skip?
> 
>> diff --git a/x86/smap.c b/x86/smap.c
>> @@ -93,7 +93,7 @@ int main(int ac, char **av)
>>  	if (!(cpuid_indexed(7, 0).b & (1 << X86_FEATURE_SMAP))) {
>>  		printf("SMAP not enabled, aborting\n");
>> -		abort();
>> +		return report_summary();
> 
> Needs report_skip?
> 
>> diff --git a/x86/svm.c b/x86/svm.c
>>      if (!(cpuid(0x80000001).c & 4)) {
>>          printf("SVM not availble\n");
>> -        return 0;
>> +        return report_summary();
> 
> Needs report_skip?
> 
>> diff --git a/x86/tsc.c b/x86/tsc.c
>>  		printf("rdtscp not supported\n");
> 
> Needs report_skip?
> 
>> -	return 0;
>> +	return report_summary();
> 
> Just sending a fixup patch is enough.

report_skip is not needed, because the a test that ended in
report_summary without any report is considered as skipped.

The fixup below turns printfs to report_skip is below.  The difference
is that summary will now print

  SKIP pku (1 tests, 1 skipped)

instead of previous

  SKIP pku (0 tests)

I like "0 tests" summary better and only x86/hyperv_synic does the
opposite, so a fix could go both ways.

---8<---
diff --git a/x86/pku.c b/x86/pku.c
index 6214f0bc151f..4b418fdc8a20 100644
--- a/x86/pku.c
+++ b/x86/pku.c
@@ -67,7 +67,7 @@ int main(int ac, char **av)
     unsigned int pkru_wd = 0x20;
 
     if (!(cpuid_indexed(7, 0).c & (1 << X86_FEATURE_PKU))) {
-        printf("PKU not enabled, aborting\n");
+        report_skip("PKU not enabled\n");
         return report_summary();
     }
 
diff --git a/x86/pmu.c b/x86/pmu.c
index b0f9f81af68e..308274761336 100644
--- a/x86/pmu.c
+++ b/x86/pmu.c
@@ -390,7 +390,7 @@ int main(int ac, char **av)
 	host_nmi_watchdog = atol_keyval(ac, av, "host_nmi_watchdog");
 
 	if (!eax.split.version_id) {
-		printf("No pmu is detected!\n");
+		report_skip("No pmu is detected!\n");
 		return report_summary();
 	}
 	printf("PMU version:         %d\n", eax.split.version_id);
diff --git a/x86/smap.c b/x86/smap.c
index c9de081a474b..23722beb00f5 100644
--- a/x86/smap.c
+++ b/x86/smap.c
@@ -92,7 +92,7 @@ int main(int ac, char **av)
 	unsigned long i;
 
 	if (!(cpuid_indexed(7, 0).b & (1 << X86_FEATURE_SMAP))) {
-		printf("SMAP not enabled, aborting\n");
+		report_skip("SMAP not enabled\n");
 		return report_summary();
 	}
 
diff --git a/x86/svm.c b/x86/svm.c
index 301cf8cb2d6b..005cbf7c3352 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -1063,7 +1063,7 @@ int main(int ac, char **av)
     smp_init();
 
     if (!(cpuid(0x80000001).c & 4)) {
-        printf("SVM not availble\n");
+        report_skip("SVM not availble\n");
         return report_summary();
     }
 
diff --git a/x86/tsc.c b/x86/tsc.c
index 62450e71725c..33fdc114c04c 100644
--- a/x86/tsc.c
+++ b/x86/tsc.c
@@ -42,6 +42,6 @@ int main()
 		test_rdtscp(10);
 		test_rdtscp(0x100);
 	} else
-		printf("rdtscp not supported\n");
+		report_skip("rdtscp not supported\n");
 	return report_summary();
 }

Re: [kvm-unit-tests PATCH v4 02/13] x86/*: report skipped tests

[Radim Krčmář]

2016-05-25 19:19+0200, Radim Krčmář:
> is that summary will now print
> 
>   SKIP pku (1 tests, 1 skipped)
> 
> instead of previous
> 
>   SKIP pku (0 tests)
> 
> I like "0 tests" summary better and only x86/hyperv_synic does the
> opposite, so a fix could go both ways.

The fixup below does the other way.  And the commit message could read
"Some care to consistency [...]" with either of them. :)

---8<---
diff --git a/x86/hyperv_synic.c b/x86/hyperv_synic.c
index f5eb82bb7336..5a06fb724a87 100644
--- a/x86/hyperv_synic.c
+++ b/x86/hyperv_synic.c
@@ -202,7 +202,7 @@ int main(int ac, char **av)
 
         report("Hyper-V SynIC test", ok);
     } else {
-        report_skip("Hyper-V SynIC is not supported");
+        printf("Hyper-V SynIC is not supported");
     }
 
     return report_summary();
diff --git a/x86/pku.c b/x86/pku.c
index 6214f0bc151f..488cce3c6fdf 100644
--- a/x86/pku.c
+++ b/x86/pku.c
@@ -67,7 +67,7 @@ int main(int ac, char **av)
     unsigned int pkru_wd = 0x20;
 
     if (!(cpuid_indexed(7, 0).c & (1 << X86_FEATURE_PKU))) {
-        printf("PKU not enabled, aborting\n");
+        printf("PKU not enabled\n");
         return report_summary();
     }
 
diff --git a/x86/smap.c b/x86/smap.c
index c9de081a474b..69dd80915c4a 100644
--- a/x86/smap.c
+++ b/x86/smap.c
@@ -92,7 +92,7 @@ int main(int ac, char **av)
 	unsigned long i;
 
 	if (!(cpuid_indexed(7, 0).b & (1 << X86_FEATURE_SMAP))) {
-		printf("SMAP not enabled, aborting\n");
+		printf("SMAP not enabled\n");
 		return report_summary();
 	}
 

Re: [kvm-unit-tests PATCH v4 02/13] x86/*: report skipped tests

[Paolo Bonzini]

On 25/05/2016 19:23, Radim Krčmář wrote:
> diff --git a/x86/hyperv_synic.c b/x86/hyperv_synic.c
> index f5eb82bb7336..5a06fb724a87 100644
> --- a/x86/hyperv_synic.c
> +++ b/x86/hyperv_synic.c
> @@ -202,7 +202,7 @@ int main(int ac, char **av)
>  
>          report("Hyper-V SynIC test", ok);
>      } else {
> -        report_skip("Hyper-V SynIC is not supported");
> +        printf("Hyper-V SynIC is not supported");
>      }
>  
>      return report_summary();
> diff --git a/x86/pku.c b/x86/pku.c
> index 6214f0bc151f..488cce3c6fdf 100644
> --- a/x86/pku.c
> +++ b/x86/pku.c
> @@ -67,7 +67,7 @@ int main(int ac, char **av)
>      unsigned int pkru_wd = 0x20;
>  
>      if (!(cpuid_indexed(7, 0).c & (1 << X86_FEATURE_PKU))) {
> -        printf("PKU not enabled, aborting\n");
> +        printf("PKU not enabled\n");
>          return report_summary();
>      }
>  
> diff --git a/x86/smap.c b/x86/smap.c
> index c9de081a474b..69dd80915c4a 100644
> --- a/x86/smap.c
> +++ b/x86/smap.c
> @@ -92,7 +92,7 @@ int main(int ac, char **av)
>  	unsigned long i;
>  
>  	if (!(cpuid_indexed(7, 0).b & (1 << X86_FEATURE_SMAP))) {
> -		printf("SMAP not enabled, aborting\n");
> +		printf("SMAP not enabled\n");
>  		return report_summary();
>  	}
>  

This one looks good.

Thanks,

Paolo

[kvm-unit-tests PATCH v4 03/13] x86/unittests: remove svm-disabled

[Radim Krčmář]

svm-disabled says SKIP after the last patch.  It's better than the FAIL
it returned before (QEMU warns, so return 0 is treated as an error), but
I don't think that the test is useful enough to be fixed to return PASS.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 x86/unittests.cfg | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index fcee6f98c598..60747cfca94e 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -159,12 +159,6 @@ smp = 2
 extra_params = -cpu qemu64,+svm
 arch = x86_64
 
-[svm-disabled]
-file = svm.flat
-smp = 2
-extra_params = -cpu qemu64,-svm
-arch = x86_64
-
 [taskswitch]
 file = taskswitch.flat
 arch = i386
-- 
2.8.2

[kvm-unit-tests PATCH v4 04/13] lib/string: add strncmp

[Radim Krčmář]

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v4: new

 lib/string.c | 17 ++++++++++-------
 lib/string.h |  1 +
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index e7bcfe945fcf..ee93e25e9821 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -26,15 +26,18 @@ char *strcpy(char *dest, const char *src)
     return strcat(dest, src);
 }
 
+int strncmp(const char *a, const char *b, size_t n)
+{
+    for (; n--; ++a, ++b)
+        if (*a != *b || *a == '\0')
+            return *a - *b;
+
+    return 0;
+}
+
 int strcmp(const char *a, const char *b)
 {
-    while (*a == *b) {
-	if (*a == '\0') {
-	    break;
-	}
-	++a, ++b;
-    }
-    return *a - *b;
+    return strncmp(a, b, SIZE_MAX);
 }
 
 char *strchr(const char *s, int c)
diff --git a/lib/string.h b/lib/string.h
index 4e24f54d9e23..2391013ad2b1 100644
--- a/lib/string.h
+++ b/lib/string.h
@@ -5,6 +5,7 @@ extern unsigned long strlen(const char *buf);
 extern char *strcat(char *dest, const char *src);
 extern char *strcpy(char *dest, const char *src);
 extern int strcmp(const char *a, const char *b);
+extern int strncmp(const char *a, const char *b, size_t n);
 extern char *strchr(const char *s, int c);
 extern char *strstr(const char *haystack, const char *needle);
 extern void *memset(void *s, int c, size_t n);
-- 
2.8.2

Re: [kvm-unit-tests PATCH v4 04/13] lib/string: add strncmp

[Andrew Jones]

On Wed, May 11, 2016 at 06:12:46PM +0200, Radim Krčmář wrote:
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v4: new
> 
>  lib/string.c | 17 ++++++++++-------
>  lib/string.h |  1 +
>  2 files changed, 11 insertions(+), 7 deletions(-)

Reviewed-by: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/lib/string.c b/lib/string.c
> index e7bcfe945fcf..ee93e25e9821 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -26,15 +26,18 @@ char *strcpy(char *dest, const char *src)
>      return strcat(dest, src);
>  }
>  
> +int strncmp(const char *a, const char *b, size_t n)
> +{
> +    for (; n--; ++a, ++b)
> +        if (*a != *b || *a == '\0')
> +            return *a - *b;
> +
> +    return 0;
> +}
> +
>  int strcmp(const char *a, const char *b)
>  {
> -    while (*a == *b) {
> -	if (*a == '\0') {
> -	    break;
> -	}
> -	++a, ++b;
> -    }
> -    return *a - *b;
> +    return strncmp(a, b, SIZE_MAX);
>  }
>  
>  char *strchr(const char *s, int c)
> diff --git a/lib/string.h b/lib/string.h
> index 4e24f54d9e23..2391013ad2b1 100644
> --- a/lib/string.h
> +++ b/lib/string.h
> @@ -5,6 +5,7 @@ extern unsigned long strlen(const char *buf);
>  extern char *strcat(char *dest, const char *src);
>  extern char *strcpy(char *dest, const char *src);
>  extern int strcmp(const char *a, const char *b);
> +extern int strncmp(const char *a, const char *b, size_t n);
>  extern char *strchr(const char *s, int c);
>  extern char *strstr(const char *haystack, const char *needle);
>  extern void *memset(void *s, int c, size_t n);
> -- 
> 2.8.2
> 

[kvm-unit-tests PATCH v4 05/13] lib/util: add args_parse_keyval

[Radim Krčmář]

The function parses command line arguments in "key=val" format, and
treats "key" as "key=1".

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v4: new

 lib/util.c | 17 +++++++++++++++++
 lib/util.h | 10 ++++++++++
 2 files changed, 27 insertions(+)

diff --git a/lib/util.c b/lib/util.c
index 69b18100c972..8b33d474f4c0 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -16,3 +16,20 @@ int parse_keyval(char *s, long *val)
 	*val = atol(p+1);
 	return p - s;
 }
+
+long args_parse_keyval(int argc, char **argv, char *key)
+{
+	int i;
+	size_t keylen = strlen(key);
+
+	for (i = 1; i < argc; i++) {
+		if (keylen > 0 && strncmp(argv[i], key, keylen - 1))
+			continue;
+		if (argv[i][keylen] == '\0')
+			return 1;
+		if (argv[i][keylen] == '=')
+			return atol(argv[i] + keylen + 1);
+	}
+
+	return 0;
+}
diff --git a/lib/util.h b/lib/util.h
index 4c4b44132277..d475b058526b 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -20,4 +20,14 @@
  */
 extern int parse_keyval(char *s, long *val);
 
+/*
+ * argc and argv are standard C arguments to main().
+ * args_parse_keyval looks for an element of argv that matches the key.
+ * Returns val interpreted as a long if the element is in key=val format.
+ * Returns 1 if the element is key.
+ * Returns 0 otherwise.
+ *
+ */
+long args_parse_keyval(int argc, char **argv, char *key);
+
 #endif
-- 
2.8.2

Re: [kvm-unit-tests PATCH v4 05/13] lib/util: add args_parse_keyval

[Andrew Jones]

On Wed, May 11, 2016 at 06:12:47PM +0200, Radim Krčmář wrote:
> The function parses command line arguments in "key=val" format, and
> treats "key" as "key=1".
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v4: new
> 
>  lib/util.c | 17 +++++++++++++++++
>  lib/util.h | 10 ++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/lib/util.c b/lib/util.c
> index 69b18100c972..8b33d474f4c0 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -16,3 +16,20 @@ int parse_keyval(char *s, long *val)
>  	*val = atol(p+1);
>  	return p - s;
>  }
> +
> +long args_parse_keyval(int argc, char **argv, char *key)
> +{

I like this better than parse_keyval, except for...

> +	int i;
> +	size_t keylen = strlen(key);
> +
> +	for (i = 1; i < argc; i++) {
> +		if (keylen > 0 && strncmp(argv[i], key, keylen - 1))
> +			continue;
> +		if (argv[i][keylen] == '\0')
> +			return 1;
> +		if (argv[i][keylen] == '=')
> +			return atol(argv[i] + keylen + 1);
> +	}
> +
> +	return 0;

...this. Here we have ambiguous results. Either key was there
and had a value of 0, or wasn't there, and we still get zero.
How about merging the two, and then fixing-up the arm and powerpc
uses of the old one. I'd drop the "args_" from the name then
too.

Thanks,
drew

> +}
> diff --git a/lib/util.h b/lib/util.h
> index 4c4b44132277..d475b058526b 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -20,4 +20,14 @@
>   */
>  extern int parse_keyval(char *s, long *val);
>  
> +/*
> + * argc and argv are standard C arguments to main().
> + * args_parse_keyval looks for an element of argv that matches the key.
> + * Returns val interpreted as a long if the element is in key=val format.
> + * Returns 1 if the element is key.
> + * Returns 0 otherwise.
> + *
> + */
> +long args_parse_keyval(int argc, char **argv, char *key);
> +
>  #endif
> -- 
> 2.8.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH v4 05/13] lib/util: add args_parse_keyval

[Radim Krčmář]

2016-05-11 18:58+0200, Andrew Jones:
> On Wed, May 11, 2016 at 06:12:47PM +0200, Radim Krčmář wrote:
>> The function parses command line arguments in "key=val" format, and
>> treats "key" as "key=1".
>> 
>> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
>> ---
>>  v4: new
>> 
>>  lib/util.c | 17 +++++++++++++++++
>>  lib/util.h | 10 ++++++++++
>>  2 files changed, 27 insertions(+)
>> 
>> diff --git a/lib/util.c b/lib/util.c
>> index 69b18100c972..8b33d474f4c0 100644
>> --- a/lib/util.c
>> +++ b/lib/util.c
>> @@ -16,3 +16,20 @@ int parse_keyval(char *s, long *val)
>>  	*val = atol(p+1);
>>  	return p - s;
>>  }
>> +
>> +long args_parse_keyval(int argc, char **argv, char *key)
>> +{
> 
> I like this better than parse_keyval, except for...
> 
>> +	int i;
>> +	size_t keylen = strlen(key);
>> +
>> +	for (i = 1; i < argc; i++) {
>> +		if (keylen > 0 && strncmp(argv[i], key, keylen - 1))
>> +			continue;
>> +		if (argv[i][keylen] == '\0')
>> +			return 1;
>> +		if (argv[i][keylen] == '=')
>> +			return atol(argv[i] + keylen + 1);
>> +	}
>> +
>> +	return 0;
> 
> ...this. Here we have ambiguous results. Either key was there
> and had a value of 0, or wasn't there, and we still get zero.

Yes, I implied a boolean key.

> How about merging the two, and then fixing-up the arm and powerpc
> uses of the old one. I'd drop the "args_" from the name then
> too.

Seems reasonable.  I'll make a new series:
 [1/3] add strncmp
 [2/3] add a slightly modified args_parse_keyval,
       int __parse_keyval(int argc, char **argv, char *key, long *val)
       that will return -1/0/1 on !key/key/key=val
 [3/3] replace parse_keyval with renamed __parse_keyval

([2/3] and [3/3] may be squished) The series will depend on your "make
argv[0] the program name" and v5 of mine will depend on it.

Re: [kvm-unit-tests PATCH v4 05/13] lib/util: add args_parse_keyval

[Andrew Jones]

On Wed, May 11, 2016 at 06:12:47PM +0200, Radim Krčmář wrote:
> The function parses command line arguments in "key=val" format, and
> treats "key" as "key=1".
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v4: new
> 
>  lib/util.c | 17 +++++++++++++++++
>  lib/util.h | 10 ++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/lib/util.c b/lib/util.c
> index 69b18100c972..8b33d474f4c0 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -16,3 +16,20 @@ int parse_keyval(char *s, long *val)
>  	*val = atol(p+1);
>  	return p - s;
>  }
> +
> +long args_parse_keyval(int argc, char **argv, char *key)
> +{
> +	int i;
> +	size_t keylen = strlen(key);
> +
> +	for (i = 1; i < argc; i++) {

Forgot to mention that I like this 'i = 1' here because it's
motivation to merge a series[*] I have in flight :-)

[*] http://www.spinics.net/lists/kvm/msg131243.html

> +		if (keylen > 0 && strncmp(argv[i], key, keylen - 1))
> +			continue;
> +		if (argv[i][keylen] == '\0')
> +			return 1;
> +		if (argv[i][keylen] == '=')
> +			return atol(argv[i] + keylen + 1);
> +	}
> +
> +	return 0;
> +}
> diff --git a/lib/util.h b/lib/util.h
> index 4c4b44132277..d475b058526b 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -20,4 +20,14 @@
>   */
>  extern int parse_keyval(char *s, long *val);
>  
> +/*
> + * argc and argv are standard C arguments to main().
> + * args_parse_keyval looks for an element of argv that matches the key.
> + * Returns val interpreted as a long if the element is in key=val format.
> + * Returns 1 if the element is key.
> + * Returns 0 otherwise.
> + *
> + */
> +long args_parse_keyval(int argc, char **argv, char *key);
> +
>  #endif
> -- 
> 2.8.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kvm-unit-tests PATCH v4 06/13] x86/pmu: expect failure with nmi_watchdog

[Radim Krčmář]

Host's nmi_watchdog takes one slot, making the "all counters" unit-test
fail.  We know exactly what happens, mark it as expected failure.

PMU test is now executed regardless of host_nmi_watchdog.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v4: improved command line parsing [Drew]

 x86/Makefile.common | 1 +
 x86/pmu.c           | 6 +++++-
 x86/unittests.cfg   | 3 +--
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/x86/Makefile.common b/x86/Makefile.common
index 356d879a986b..4951210f3331 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -3,6 +3,7 @@
 all: test_cases
 
 cflatobjs += lib/pci.o
+cflatobjs += lib/util.o
 cflatobjs += lib/x86/io.o
 cflatobjs += lib/x86/smp.o
 cflatobjs += lib/x86/vm.o
diff --git a/x86/pmu.c b/x86/pmu.c
index c68980044dee..a4b88b410904 100644
--- a/x86/pmu.c
+++ b/x86/pmu.c
@@ -8,6 +8,7 @@
 #include "x86/vm.h"
 
 #include "libcflat.h"
+#include <util.h>
 #include <stdint.h>
 
 #define FIXED_CNT_INDEX 32
@@ -92,6 +93,7 @@ struct pmu_event {
 };
 
 static int num_counters;
+static bool host_nmi_watchdog;
 
 char *buf;
 
@@ -291,7 +293,7 @@ static void check_counters_many(void)
 		if (!verify_counter(&cnt[i]))
 			break;
 
-	report("all counters", i == n);
+	report_xfail("all counters", host_nmi_watchdog, i == n);
 }
 
 static void check_counter_overflow(void)
@@ -385,6 +387,8 @@ int main(int ac, char **av)
 	ebx.full = id.b;
 	edx.full = id.d;
 
+	host_nmi_watchdog = args_parse_keyval(ac, av, "host_nmi_watchdog");
+
 	if (!eax.split.version_id) {
 		printf("No pmu is detected!\n");
 		return report_summary();
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 60747cfca94e..3852b40a2927 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -121,8 +121,7 @@ file = msr.flat
 
 [pmu]
 file = pmu.flat
-extra_params = -cpu host
-check = /proc/sys/kernel/nmi_watchdog=0
+extra_params = -cpu host -append "host_nmi_watchdog=`cat /proc/sys/kernel/nmi_watchdog`"
 
 [port80]
 file = port80.flat
-- 
2.8.2

Re: [kvm-unit-tests PATCH v4 06/13] x86/pmu: expect failure with nmi_watchdog

[Andrew Jones]

On Wed, May 11, 2016 at 06:12:48PM +0200, Radim Krčmář wrote:
> Host's nmi_watchdog takes one slot, making the "all counters" unit-test
> fail.  We know exactly what happens, mark it as expected failure.
> 
> PMU test is now executed regardless of host_nmi_watchdog.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v4: improved command line parsing [Drew]
> 
>  x86/Makefile.common | 1 +
>  x86/pmu.c           | 6 +++++-
>  x86/unittests.cfg   | 3 +--
>  3 files changed, 7 insertions(+), 3 deletions(-)

Reviewed-by: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index 356d879a986b..4951210f3331 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -3,6 +3,7 @@
>  all: test_cases
>  
>  cflatobjs += lib/pci.o
> +cflatobjs += lib/util.o
>  cflatobjs += lib/x86/io.o
>  cflatobjs += lib/x86/smp.o
>  cflatobjs += lib/x86/vm.o
> diff --git a/x86/pmu.c b/x86/pmu.c
> index c68980044dee..a4b88b410904 100644
> --- a/x86/pmu.c
> +++ b/x86/pmu.c
> @@ -8,6 +8,7 @@
>  #include "x86/vm.h"
>  
>  #include "libcflat.h"
> +#include <util.h>
>  #include <stdint.h>
>  
>  #define FIXED_CNT_INDEX 32
> @@ -92,6 +93,7 @@ struct pmu_event {
>  };
>  
>  static int num_counters;
> +static bool host_nmi_watchdog;
>  
>  char *buf;
>  
> @@ -291,7 +293,7 @@ static void check_counters_many(void)
>  		if (!verify_counter(&cnt[i]))
>  			break;
>  
> -	report("all counters", i == n);
> +	report_xfail("all counters", host_nmi_watchdog, i == n);
>  }
>  
>  static void check_counter_overflow(void)
> @@ -385,6 +387,8 @@ int main(int ac, char **av)
>  	ebx.full = id.b;
>  	edx.full = id.d;
>  
> +	host_nmi_watchdog = args_parse_keyval(ac, av, "host_nmi_watchdog");
> +
>  	if (!eax.split.version_id) {
>  		printf("No pmu is detected!\n");
>  		return report_summary();
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index 60747cfca94e..3852b40a2927 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -121,8 +121,7 @@ file = msr.flat
>  
>  [pmu]
>  file = pmu.flat
> -extra_params = -cpu host
> -check = /proc/sys/kernel/nmi_watchdog=0
> +extra_params = -cpu host -append "host_nmi_watchdog=`cat /proc/sys/kernel/nmi_watchdog`"
>  
>  [port80]
>  file = port80.flat
> -- 
> 2.8.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kvm-unit-tests PATCH v4 07/13] lib/report: don't print 0 failed tests

[Radim Krčmář]

Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 lib/report.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/report.c b/lib/report.c
index 430b2aeaecbd..4cd75534f978 100644
--- a/lib/report.c
+++ b/lib/report.c
@@ -95,7 +95,9 @@ int report_summary(void)
 {
 	spin_lock(&lock);
 
-	printf("\nSUMMARY: %d tests, %d unexpected failures", tests, failures);
+	printf("\nSUMMARY: %d tests", tests);
+	if (failures)
+		printf(", %d unexpected failures", failures);
 	if (xfailures)
 		printf(", %d expected failures", xfailures);
 	if (skipped)
-- 
2.8.2

[kvm-unit-tests PATCH v4 08/13] scripts/runtime: consolidate summary tags

[Radim Krčmář]

Turn skip into yellow SKIP and add reusable definitions of all tags.

Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 scripts/runtime.bash | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index ed073721216c..8d374103a71c 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -2,6 +2,10 @@
 : ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
 : ${TIMEOUT:=90s}
 
+PASS() { echo -ne "\e[32mPASS\e[0m"; }
+SKIP() { echo -ne "\e[33mSKIP\e[0m"; }
+FAIL() { echo -ne "\e[31mFAIL\e[0m"; }
+
 function run()
 {
     local testname="$1"
@@ -23,7 +27,7 @@ function run()
     fi
 
     if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
-        echo "skip $1 ($arch only)"
+        echo "`SKIP` $1 ($arch only)"
         return 2
     fi
 
@@ -34,7 +38,7 @@ function run()
         path=${check_param%%=*}
         value=${check_param#*=}
         if [ "$path" ] && [ "$(cat $path)" != "$value" ]; then
-            echo "skip $1 ($path not equal to $value)"
+            echo "`SKIP` $1 ($path not equal to $value)"
             return 2
         fi
     done
@@ -50,13 +54,13 @@ function run()
     ret=$?
 
     if [ $ret -eq 0 ]; then
-        echo -e "\e[32mPASS\e[0m $1"
+        echo "`PASS` $1"
     elif [ $ret -eq 77 ]; then
-        echo -e "\e[33mSKIP\e[0m $1"
+        echo "`SKIP` $1"
     elif [ $ret -eq 124 ]; then
-        echo -e "\e[31mFAIL\e[0m $1 (timeout; duration=$timeout)"
+        echo "`FAIL` $1 (timeout; duration=$timeout)"
     else
-        echo -e "\e[31mFAIL\e[0m $1"
+        echo "`FAIL` $1"
     fi
 
     return $ret
-- 
2.8.2

Re: [kvm-unit-tests PATCH v4 08/13] scripts/runtime: consolidate summary tags

[Paolo Bonzini]

On 11/05/2016 18:12, Radim Krčmář wrote:
> Turn skip into yellow SKIP and add reusable definitions of all tags.
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  scripts/runtime.bash | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index ed073721216c..8d374103a71c 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -2,6 +2,10 @@
>  : ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
>  : ${TIMEOUT:=90s}
>  
> +PASS() { echo -ne "\e[32mPASS\e[0m"; }
> +SKIP() { echo -ne "\e[33mSKIP\e[0m"; }
> +FAIL() { echo -ne "\e[31mFAIL\e[0m"; }

If you use for example $'\e[32mPASS\e[0m', then you can use variables
instead of functions.

> +
>  function run()
>  {
>      local testname="$1"
> @@ -23,7 +27,7 @@ function run()
>      fi
>  
>      if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
> -        echo "skip $1 ($arch only)"
> +        echo "`SKIP` $1 ($arch only)"
>          return 2
>      fi
>  
> @@ -34,7 +38,7 @@ function run()
>          path=${check_param%%=*}
>          value=${check_param#*=}
>          if [ "$path" ] && [ "$(cat $path)" != "$value" ]; then
> -            echo "skip $1 ($path not equal to $value)"
> +            echo "`SKIP` $1 ($path not equal to $value)"
>              return 2
>          fi
>      done
> @@ -50,13 +54,13 @@ function run()
>      ret=$?
>  
>      if [ $ret -eq 0 ]; then
> -        echo -e "\e[32mPASS\e[0m $1"
> +        echo "`PASS` $1"
>      elif [ $ret -eq 77 ]; then
> -        echo -e "\e[33mSKIP\e[0m $1"
> +        echo "`SKIP` $1"
>      elif [ $ret -eq 124 ]; then
> -        echo -e "\e[31mFAIL\e[0m $1 (timeout; duration=$timeout)"
> +        echo "`FAIL` $1 (timeout; duration=$timeout)"
>      else
> -        echo -e "\e[31mFAIL\e[0m $1"
> +        echo "`FAIL` $1"
>      fi
>  
>      return $ret
> 

Re: [kvm-unit-tests PATCH v4 08/13] scripts/runtime: consolidate summary tags

[Radim Krčmář]

2016-05-17 14:22+0200, Paolo Bonzini:
> On 11/05/2016 18:12, Radim Krčmář wrote:
>> Turn skip into yellow SKIP and add reusable definitions of all tags.
>> 
>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
>> ---
>>  scripts/runtime.bash | 16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>> 
>> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
>> index ed073721216c..8d374103a71c 100644
>> --- a/scripts/runtime.bash
>> +++ b/scripts/runtime.bash
>> @@ -2,6 +2,10 @@
>>  : ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
>>  : ${TIMEOUT:=90s}
>>  
>> +PASS() { echo -ne "\e[32mPASS\e[0m"; }
>> +SKIP() { echo -ne "\e[33mSKIP\e[0m"; }
>> +FAIL() { echo -ne "\e[31mFAIL\e[0m"; }
> 
> If you use for example $'\e[32mPASS\e[0m', then you can use variables
> instead of functions.

Great, thanks.  A fixup is below

---8<---
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 39a9072ed103..2bbdb864c889 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -2,9 +2,9 @@
 : ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
 : ${TIMEOUT:=90s}
 
-PASS() { echo -ne "\e[32mPASS\e[0m"; }
-SKIP() { echo -ne "\e[33mSKIP\e[0m"; }
-FAIL() { echo -ne "\e[31mFAIL\e[0m"; }
+PASS=$'\e[32mPASS\e[0m'
+SKIP=$'\e[33mSKIP\e[0m'
+FAIL=$'\e[31mFAIL\e[0m'
 
 extract_summary()
 {
@@ -53,18 +53,18 @@ function run()
     fi
 
     if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
-        echo "`SKIP` $1 ($arch only)"
+        echo "$SKIP $1 ($arch only)"
         return 2
     fi
 
     eval $check || {
         echo "skipped $testname (\`$check\` returned $?)" | RUNTIME_log_stderr
-        echo "`SKIP` $testname (check failed)"
+        echo "$SKIP $testname (check failed)"
         return 77
     }
 
     last_line=$(premature_failure) && {
-        echo "`SKIP` $1 ($last_line)"
+        echo "$SKIP $1 ($last_line)"
         return 77
     }
 
@@ -80,13 +80,13 @@ function run()
     ret=$?
 
     if [ $ret -eq 0 ]; then
-        echo "`PASS` $1 $summary"
+        echo "$PASS $1 $summary"
     elif [ $ret -eq 77 ]; then
-        echo "`SKIP` $1 $summary"
+        echo "$SKIP $1 $summary"
     elif [ $ret -eq 124 ]; then
-        echo "`FAIL` $1 (timeout; duration=$timeout)"
+        echo "$FAIL $1 (timeout; duration=$timeout)"
     else
-        echo "`FAIL` $1 $summary"
+        echo "$FAIL $1 $summary"
     fi
 
     return $ret

[kvm-unit-tests PATCH v4 09/13] run_tests: print SUMMARY line in summary

[Radim Krčmář]

SUMMARY line is present if the test uses lib/report.  Summary contains
some interesting information, so duplicating the output isn't that big
of a cost.  Our log redirection got more complicated, though.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v4:
 * less ev[ai]l [Paolo]
 * didn't include Drew's r-b

 run_tests.sh            | 14 ++++++++------
 scripts/mkstandalone.sh |  3 +++
 scripts/runtime.bash    | 15 ++++++++++-----
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/run_tests.sh b/run_tests.sh
index 7e0237f3aa11..b634e472a834 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -46,13 +46,15 @@ while getopts "g:hv" opt; do
     esac
 done
 
-if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
-	log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
-else
-	log_redir=">> test.log"
-fi
+RUNTIME_log_stdout () {
+    if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
+        ./scripts/pretty_print_stacks.py $1 >> test.log
+    else
+        cat >> test.log
+    fi
+}
+
 
-RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
 config=$TEST_DIR/unittests.cfg
 rm -f test.log
 printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 26094dfba373..77793fd64c76 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -68,6 +68,9 @@ generate_test ()
 	(echo "#!/bin/bash"
 	 cat scripts/arch-run.bash "$TEST_DIR/run") | temp_file RUNTIME_arch_run
 
+	echo "exec {stdout}>&1"
+	echo "RUNTIME_log_stdout () { cat >&\$stdout; }"
+
 	cat scripts/runtime.bash
 
 	echo "run ${args[@]}"
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 8d374103a71c..6d9d8bffb82f 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -6,6 +6,11 @@ PASS() { echo -ne "\e[32mPASS\e[0m"; }
 SKIP() { echo -ne "\e[33mSKIP\e[0m"; }
 FAIL() { echo -ne "\e[31mFAIL\e[0m"; }
 
+extract_summary()
+{
+    tail -1 | grep '^SUMMARY: ' | sed 's/^SUMMARY: /(/;s/$/)/'
+}
+
 function run()
 {
     local testname="$1"
@@ -49,18 +54,18 @@ function run()
     fi
 
     # extra_params in the config file may contain backticks that need to be
-    # expanded, so use eval to start qemu
-    eval $cmdline
+    # expanded, so use eval to start qemu.  Same for $RUNTIME_log_stdout.
+    summary=$(eval $cmdline > >(tee >(RUNTIME_log_stdout $kernel) | extract_summary))
     ret=$?
 
     if [ $ret -eq 0 ]; then
-        echo "`PASS` $1"
+        echo "`PASS` $1 $summary"
     elif [ $ret -eq 77 ]; then
-        echo "`SKIP` $1"
+        echo "`SKIP` $1 $summary"
     elif [ $ret -eq 124 ]; then
         echo "`FAIL` $1 (timeout; duration=$timeout)"
     else
-        echo "`FAIL` $1"
+        echo "`FAIL` $1 $summary"
     fi
 
     return $ret
-- 
2.8.2

Re: [kvm-unit-tests PATCH v4 09/13] run_tests: print SUMMARY line in summary

[Andrew Jones]

On Wed, May 11, 2016 at 06:12:51PM +0200, Radim Krčmář wrote:
> SUMMARY line is present if the test uses lib/report.  Summary contains
> some interesting information, so duplicating the output isn't that big
> of a cost.  Our log redirection got more complicated, though.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v4:
>  * less ev[ai]l [Paolo]
>  * didn't include Drew's r-b
> 
>  run_tests.sh            | 14 ++++++++------
>  scripts/mkstandalone.sh |  3 +++
>  scripts/runtime.bash    | 15 ++++++++++-----
>  3 files changed, 21 insertions(+), 11 deletions(-)

Reviewed-by: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/run_tests.sh b/run_tests.sh
> index 7e0237f3aa11..b634e472a834 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -46,13 +46,15 @@ while getopts "g:hv" opt; do
>      esac
>  done
>  
> -if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> -	log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
> -else
> -	log_redir=">> test.log"
> -fi
> +RUNTIME_log_stdout () {
> +    if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> +        ./scripts/pretty_print_stacks.py $1 >> test.log
> +    else
> +        cat >> test.log
> +    fi
> +}
> +
>  
> -RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
>  config=$TEST_DIR/unittests.cfg
>  rm -f test.log
>  printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 26094dfba373..77793fd64c76 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -68,6 +68,9 @@ generate_test ()
>  	(echo "#!/bin/bash"
>  	 cat scripts/arch-run.bash "$TEST_DIR/run") | temp_file RUNTIME_arch_run
>  
> +	echo "exec {stdout}>&1"
> +	echo "RUNTIME_log_stdout () { cat >&\$stdout; }"
> +
>  	cat scripts/runtime.bash
>  
>  	echo "run ${args[@]}"
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 8d374103a71c..6d9d8bffb82f 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -6,6 +6,11 @@ PASS() { echo -ne "\e[32mPASS\e[0m"; }
>  SKIP() { echo -ne "\e[33mSKIP\e[0m"; }
>  FAIL() { echo -ne "\e[31mFAIL\e[0m"; }
>  
> +extract_summary()
> +{
> +    tail -1 | grep '^SUMMARY: ' | sed 's/^SUMMARY: /(/;s/$/)/'
> +}
> +
>  function run()
>  {
>      local testname="$1"
> @@ -49,18 +54,18 @@ function run()
>      fi
>  
>      # extra_params in the config file may contain backticks that need to be
> -    # expanded, so use eval to start qemu
> -    eval $cmdline
> +    # expanded, so use eval to start qemu.  Same for $RUNTIME_log_stdout.
> +    summary=$(eval $cmdline > >(tee >(RUNTIME_log_stdout $kernel) | extract_summary))
>      ret=$?
>  
>      if [ $ret -eq 0 ]; then
> -        echo "`PASS` $1"
> +        echo "`PASS` $1 $summary"
>      elif [ $ret -eq 77 ]; then
> -        echo "`SKIP` $1"
> +        echo "`SKIP` $1 $summary"
>      elif [ $ret -eq 124 ]; then
>          echo "`FAIL` $1 (timeout; duration=$timeout)"
>      else
> -        echo "`FAIL` $1"
> +        echo "`FAIL` $1 $summary"
>      fi
>  
>      return $ret
> -- 
> 2.8.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH v4 09/13] run_tests: print SUMMARY line in summary

[Paolo Bonzini]

On 11/05/2016 18:12, Radim Krčmář wrote:
> +    # expanded, so use eval to start qemu.  Same for $RUNTIME_log_stdout.

Luckily the second part is not true anymore.  Fixed it myself.

Paolo

> +    summary=$(eval $cmdline > >(tee >(RUNTIME_log_stdout $kernel) | extract_summary))

Re: [kvm-unit-tests PATCH v4 09/13] run_tests: print SUMMARY line in summary

[Radim Krčmář]

2016-05-17 14:32+0200, Paolo Bonzini:
> On 11/05/2016 18:12, Radim Krčmář wrote:
>> +    # expanded, so use eval to start qemu.  Same for $RUNTIME_log_stdout.
> 
> Luckily the second part is not true anymore.  Fixed it myself.

Thanks.

[kvm-unit-tests PATCH v4 10/13] run_tests: log stderr

[Radim Krčmář]

Summary should provide all important information.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v4: new

 run_tests.sh            | 1 +
 scripts/mkstandalone.sh | 1 +
 scripts/runtime.bash    | 3 ++-
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/run_tests.sh b/run_tests.sh
index b634e472a834..254129d8723c 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -46,6 +46,7 @@ while getopts "g:hv" opt; do
     esac
 done
 
+RUNTIME_log_stderr () { cat >> test.log; }
 RUNTIME_log_stdout () {
     if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
         ./scripts/pretty_print_stacks.py $1 >> test.log
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 77793fd64c76..d2bae1922f4b 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -70,6 +70,7 @@ generate_test ()
 
 	echo "exec {stdout}>&1"
 	echo "RUNTIME_log_stdout () { cat >&\$stdout; }"
+	echo "RUNTIME_log_stderr () { cat >&2; }"
 
 	cat scripts/runtime.bash
 
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 6d9d8bffb82f..deae077e50a1 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -55,7 +55,8 @@ function run()
 
     # extra_params in the config file may contain backticks that need to be
     # expanded, so use eval to start qemu.  Same for $RUNTIME_log_stdout.
-    summary=$(eval $cmdline > >(tee >(RUNTIME_log_stdout $kernel) | extract_summary))
+    summary=$(eval $cmdline 2> >(RUNTIME_log_stderr) \
+                             > >(tee >(RUNTIME_log_stdout $kernel) | extract_summary))
     ret=$?
 
     if [ $ret -eq 0 ]; then
-- 
2.8.2

[kvm-unit-tests PATCH v4 11/13] scripts/runtime: skip tests that cannot run

[Radim Krčmář]

A case where QEMU won't run the kernel should be considered as skipped.
Hyper-V tests used to FAIL on old QEMUs.  The infamous QEMU=/dev/null
FAIL streak is covered too.  Some error messages in arch/run were
reworded, because they are now visible in summary.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v4:
 * clarified "kvm only" error message [Drew+Lukáš]
 * used _NO_FILE_4Uhere_ instead of /dev/null [Drew]
 * completely reworked output, now prints the last line

 arm/run              |  2 +-
 powerpc/run          |  2 +-
 scripts/runtime.bash | 28 +++++++++++++++++++++++++++-
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/arm/run b/arm/run
index ebf703d5757c..a2f35ef6a7e6 100755
--- a/arm/run
+++ b/arm/run
@@ -19,7 +19,7 @@ if [ -c /dev/kvm ]; then
 fi
 
 if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
-	echo "skip $TESTNAME (kvm only)"
+	echo "KVM is needed, but not available on this host"
 	exit 2
 fi
 
diff --git a/powerpc/run b/powerpc/run
index 56698e622592..14988a75b1e2 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -16,7 +16,7 @@ if [ -c /dev/kvm ]; then
 fi
 
 if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
-	echo "skip $TESTNAME (kvm only)"
+	echo "KVM is needed, but not available on this host"
 	exit 2
 fi
 
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index deae077e50a1..c9ab9ba47d81 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -11,6 +11,27 @@ extract_summary()
     tail -1 | grep '^SUMMARY: ' | sed 's/^SUMMARY: /(/;s/$/)/'
 }
 
+# We assume that QEMU is going to work if it tried to load the kernel
+premature_failure()
+{
+    local log="$(eval $(get_cmdline _NO_FILE_4Uhere_) 2>&1)"
+    local last_line=$(tail -1 <<< "$log")
+
+    echo "$last_line" | grep -qi "could not load kernel" &&
+        return 1
+
+    RUNTIME_log_stderr <<< "$log"
+
+    echo "$last_line"
+    return 0
+}
+
+get_cmdline()
+{
+    local kernel=$1
+    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
+}
+
 function run()
 {
     local testname="$1"
@@ -48,7 +69,12 @@ function run()
         fi
     done
 
-    cmdline="TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
+    last_line=$(premature_failure) && {
+        echo "`SKIP` $1 ($last_line)"
+        return 77
+    }
+
+    cmdline=$(get_cmdline $kernel)
     if [ "$verbose" = "yes" ]; then
         echo $cmdline
     fi
-- 
2.8.2

Re: [kvm-unit-tests PATCH v4 11/13] scripts/runtime: skip tests that cannot run

[Andrew Jones]

On Wed, May 11, 2016 at 06:30:41PM +0200, Radim Krčmář wrote:
> A case where QEMU won't run the kernel should be considered as skipped.
> Hyper-V tests used to FAIL on old QEMUs.  The infamous QEMU=/dev/null
> FAIL streak is covered too.  Some error messages in arch/run were
> reworded, because they are now visible in summary.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v4:
>  * clarified "kvm only" error message [Drew+Lukáš]
>  * used _NO_FILE_4Uhere_ instead of /dev/null [Drew]
>  * completely reworked output, now prints the last line
> 
>  arm/run              |  2 +-
>  powerpc/run          |  2 +-
>  scripts/runtime.bash | 28 +++++++++++++++++++++++++++-
>  3 files changed, 29 insertions(+), 3 deletions(-)

Reviewed-by: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/arm/run b/arm/run
> index ebf703d5757c..a2f35ef6a7e6 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -19,7 +19,7 @@ if [ -c /dev/kvm ]; then
>  fi
>  
>  if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
> -	echo "skip $TESTNAME (kvm only)"
> +	echo "KVM is needed, but not available on this host"
>  	exit 2
>  fi
>  
> diff --git a/powerpc/run b/powerpc/run
> index 56698e622592..14988a75b1e2 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -16,7 +16,7 @@ if [ -c /dev/kvm ]; then
>  fi
>  
>  if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ]; then
> -	echo "skip $TESTNAME (kvm only)"
> +	echo "KVM is needed, but not available on this host"
>  	exit 2
>  fi
>  
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index deae077e50a1..c9ab9ba47d81 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -11,6 +11,27 @@ extract_summary()
>      tail -1 | grep '^SUMMARY: ' | sed 's/^SUMMARY: /(/;s/$/)/'
>  }
>  
> +# We assume that QEMU is going to work if it tried to load the kernel
> +premature_failure()
> +{
> +    local log="$(eval $(get_cmdline _NO_FILE_4Uhere_) 2>&1)"
> +    local last_line=$(tail -1 <<< "$log")
> +
> +    echo "$last_line" | grep -qi "could not load kernel" &&
> +        return 1
> +
> +    RUNTIME_log_stderr <<< "$log"
> +
> +    echo "$last_line"
> +    return 0
> +}
> +
> +get_cmdline()
> +{
> +    local kernel=$1
> +    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> +}
> +
>  function run()
>  {
>      local testname="$1"
> @@ -48,7 +69,12 @@ function run()
>          fi
>      done
>  
> -    cmdline="TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> +    last_line=$(premature_failure) && {
> +        echo "`SKIP` $1 ($last_line)"
> +        return 77
> +    }
> +
> +    cmdline=$(get_cmdline $kernel)
>      if [ "$verbose" = "yes" ]; then
>          echo $cmdline
>      fi
> -- 
> 2.8.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kvm-unit-tests PATCH v4 12/13] scripts/arch-run: change QEMU failure detection

[Radim Krčmář]

We FAIL the test whenever there is an output on stderr.  QEMU or its
libraries can warn and the test could still work, so ignore any line
that has a warning keyword.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v4: new

 scripts/arch-run.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 8ef4ff5634a0..8e75c6ed6e17 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -55,7 +55,7 @@ run_qemu ()
 		# Even when ret==1 (unittest success) if we also got stderr
 		# logs, then we assume a QEMU failure. Otherwise we translate
 		# status of 1 to 0 (SUCCESS)
-		if [ -z "$errors" ]; then
+		if [ -z "$(echo "$errors" | grep -vi warning)" ]; then
 			ret=0
 		fi
 	fi
-- 
2.8.2

Re: [kvm-unit-tests PATCH v4 12/13] scripts/arch-run: change QEMU failure detection

[Andrew Jones]

On Wed, May 11, 2016 at 06:30:42PM +0200, Radim Krčmář wrote:
> We FAIL the test whenever there is an output on stderr.  QEMU or its
> libraries can warn and the test could still work, so ignore any line
> that has a warning keyword.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v4: new
> 
>  scripts/arch-run.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 8ef4ff5634a0..8e75c6ed6e17 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -55,7 +55,7 @@ run_qemu ()
>  		# Even when ret==1 (unittest success) if we also got stderr
>  		# logs, then we assume a QEMU failure. Otherwise we translate
>  		# status of 1 to 0 (SUCCESS)
> -		if [ -z "$errors" ]; then
> +		if [ -z "$(echo "$errors" | grep -vi warning)" ]; then
>  			ret=0
>  		fi
>  	fi
> -- 
> 2.8.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kvm-unit-tests PATCH v4 13/13] scripts/run: generalize check

[Radim Krčmář]

config attribute "check" is currently unused and a simpler imlementation
has better chances of being used.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v4: new

 scripts/runtime.bash | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index c9ab9ba47d81..7f9bf9a2de0e 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -57,17 +57,11 @@ function run()
         return 2
     fi
 
-    # check a file for a particular value before running a test
-    # the check line can contain multiple files to check separated by a space
-    # but each check parameter needs to be of the form <path>=<value>
-    for check_param in ${check[@]}; do
-        path=${check_param%%=*}
-        value=${check_param#*=}
-        if [ "$path" ] && [ "$(cat $path)" != "$value" ]; then
-            echo "`SKIP` $1 ($path not equal to $value)"
-            return 2
-        fi
-    done
+    eval $check || {
+        echo "skipped $testname (\`$check\` returned $?)" | RUNTIME_log_stderr
+        echo "`SKIP` $testname (check failed)"
+        return 77
+    }
 
     last_line=$(premature_failure) && {
         echo "`SKIP` $1 ($last_line)"
-- 
2.8.2

Re: [kvm-unit-tests PATCH v4 13/13] scripts/run: generalize check

[Andrew Jones]

On Wed, May 11, 2016 at 06:30:43PM +0200, Radim Krčmář wrote:
> config attribute "check" is currently unused and a simpler imlementation
> has better chances of being used.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v4: new
> 
>  scripts/runtime.bash | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index c9ab9ba47d81..7f9bf9a2de0e 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -57,17 +57,11 @@ function run()
>          return 2
>      fi
>  
> -    # check a file for a particular value before running a test
> -    # the check line can contain multiple files to check separated by a space
> -    # but each check parameter needs to be of the form <path>=<value>
> -    for check_param in ${check[@]}; do
> -        path=${check_param%%=*}
> -        value=${check_param#*=}
> -        if [ "$path" ] && [ "$(cat $path)" != "$value" ]; then
> -            echo "`SKIP` $1 ($path not equal to $value)"
> -            return 2
> -        fi
> -    done
> +    eval $check || {
> +        echo "skipped $testname (\`$check\` returned $?)" | RUNTIME_log_stderr
> +        echo "`SKIP` $testname (check failed)"
> +        return 77
> +    }
>  
>      last_line=$(premature_failure) && {
>          echo "`SKIP` $1 ($last_line)"
> -- 

Good, except you need to update all the unittests.cfg headers where
check is documented. Maybe something like

 check = <bash-expr>	# check an expression is true before running the test

Thanks,
drew

Re: [kvm-unit-tests PATCH v4 10/13] run_tests: log stderr

[Andrew Jones]

On Wed, May 11, 2016 at 06:30:40PM +0200, Radim Krčmář wrote:
> Summary should provide all important information.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v4: new
> 
>  run_tests.sh            | 1 +
>  scripts/mkstandalone.sh | 1 +
>  scripts/runtime.bash    | 3 ++-
>  3 files changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/run_tests.sh b/run_tests.sh
> index b634e472a834..254129d8723c 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -46,6 +46,7 @@ while getopts "g:hv" opt; do
>      esac
>  done
>  
> +RUNTIME_log_stderr () { cat >> test.log; }
>  RUNTIME_log_stdout () {
>      if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
>          ./scripts/pretty_print_stacks.py $1 >> test.log
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 77793fd64c76..d2bae1922f4b 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -70,6 +70,7 @@ generate_test ()
>  
>  	echo "exec {stdout}>&1"
>  	echo "RUNTIME_log_stdout () { cat >&\$stdout; }"
> +	echo "RUNTIME_log_stderr () { cat >&2; }"
>  
>  	cat scripts/runtime.bash
>  
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 6d9d8bffb82f..deae077e50a1 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -55,7 +55,8 @@ function run()
>  
>      # extra_params in the config file may contain backticks that need to be
>      # expanded, so use eval to start qemu.  Same for $RUNTIME_log_stdout.
> -    summary=$(eval $cmdline > >(tee >(RUNTIME_log_stdout $kernel) | extract_summary))
> +    summary=$(eval $cmdline 2> >(RUNTIME_log_stderr) \
> +                             > >(tee >(RUNTIME_log_stdout $kernel) | extract_summary))
>      ret=$?
>  
>      if [ $ret -eq 0 ]; then
> -- 
> 2.8.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH v4 00/13] Improve the output of test runners

[Paolo Bonzini]

On 11/05/2016 18:12, Radim Krčmář wrote:
> Radim Krčmář (13):
>   lib/report: allow test skipping
>   x86/*: report skipped tests
>   x86/unittests: remove svm-disabled
>   lib/string: add strncmp
>   lib/util: add args_parse_keyval
>   x86/pmu: expect failure with nmi_watchdog
>   lib/report: don't print 0 failed tests
>   scripts/runtime: consolidate summary tags
>   run_tests: print SUMMARY line in summary
>   run_tests: log stderr
>   scripts/runtime: skip tests that cannot run
>   scripts/arch-run: change QEMU failure detection
>   scripts/run: generalize check

Applying everything except 5, 6, 13.  It would be nice to get the fixup
for patch 3 before I push...

Paolo

>  arm/run                 |  2 +-
>  lib/libcflat.h          |  1 +
>  lib/report.c            | 49 ++++++++++++++++++++++++-----------
>  lib/string.c            | 17 ++++++++-----
>  lib/string.h            |  1 +
>  lib/util.c              | 17 +++++++++++++
>  lib/util.h              | 10 ++++++++
>  powerpc/run             |  2 +-
>  run_tests.sh            | 15 ++++++-----
>  scripts/arch-run.bash   |  2 +-
>  scripts/mkstandalone.sh |  4 +++
>  scripts/runtime.bash    | 68 ++++++++++++++++++++++++++++++++++++-------------
>  x86/Makefile.common     |  1 +
>  x86/apic.c              |  7 +++--
>  x86/emulator.c          |  2 +-
>  x86/hyperv_synic.c      |  2 +-
>  x86/pku.c               |  2 +-
>  x86/pmu.c               |  8 ++++--
>  x86/smap.c              |  2 +-
>  x86/svm.c               |  2 +-
>  x86/tsc.c               |  2 +-
>  x86/unittests.cfg       |  9 +------
>  22 files changed, 156 insertions(+), 69 deletions(-)
> 

Re: [kvm-unit-tests PATCH v4 00/13] Improve the output of test runners

[Radim Krčmář]

2016-05-17 14:29+0200, Paolo Bonzini:
> On 11/05/2016 18:12, Radim Krčmář wrote:
>> Radim Krčmář (13):
>>   lib/report: allow test skipping
>>   x86/*: report skipped tests
>>   x86/unittests: remove svm-disabled
>>   lib/string: add strncmp
>>   lib/util: add args_parse_keyval
>>   x86/pmu: expect failure with nmi_watchdog
>>   lib/report: don't print 0 failed tests
>>   scripts/runtime: consolidate summary tags
>>   run_tests: print SUMMARY line in summary
>>   run_tests: log stderr
>>   scripts/runtime: skip tests that cannot run
>>   scripts/arch-run: change QEMU failure detection
>>   scripts/run: generalize check
> 
> Applying everything except 5, 6, 13.  It would be nice to get the fixup
> for patch 3 before I push...

Sorry for the delay ... I am overloaded for the next month as well, so
improved versions of 5 and 6 seem unlikely before July.

Re: [kvm-unit-tests PATCH v4 00/13] Improve the output of test runners

[Paolo Bonzini]

On 25/05/2016 19:49, Radim Krčmář wrote:
> 2016-05-17 14:29+0200, Paolo Bonzini:
>> On 11/05/2016 18:12, Radim Krčmář wrote:
>>> Radim Krčmář (13):
>>>   lib/report: allow test skipping
>>>   x86/*: report skipped tests
>>>   x86/unittests: remove svm-disabled
>>>   lib/string: add strncmp
>>>   lib/util: add args_parse_keyval
>>>   x86/pmu: expect failure with nmi_watchdog
>>>   lib/report: don't print 0 failed tests
>>>   scripts/runtime: consolidate summary tags
>>>   run_tests: print SUMMARY line in summary
>>>   run_tests: log stderr
>>>   scripts/runtime: skip tests that cannot run
>>>   scripts/arch-run: change QEMU failure detection
>>>   scripts/run: generalize check
>>
>> Applying everything except 5, 6, 13.  It would be nice to get the fixup
>> for patch 3 before I push...
> 
> Sorry for the delay ... I am overloaded for the next month as well, so
> improved versions of 5 and 6 seem unlikely before July.

Applying the fixup to patch 2 and pushing to git.kernel.org, thanks.

Thanks,

Paolo