[kvm-unit-tests,v4,13/13] lib/report: Add stamps to reports
diff mbox series

Message ID 20190103100806.9039-14-frankja@linux.ibm.com
State New
Headers show
Series
  • 390x: Add cross hypervisor and disk boot
Related show

Commit Message

Janosch Frank Jan. 3, 2019, 10:08 a.m. UTC
Lets add a function to get stamps for our reports. It'll help with
finding the test case that was causing problems in dumps.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/libcflat.h |  1 +
 lib/report.c   | 10 ++++++++++
 2 files changed, 11 insertions(+)

Comments

Thomas Huth Jan. 3, 2019, 1:55 p.m. UTC | #1
On 2019-01-03 11:08, Janosch Frank wrote:
> Lets add a function to get stamps for our reports. It'll help with
> finding the test case that was causing problems in dumps.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/libcflat.h |  1 +
>  lib/report.c   | 10 ++++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 7529958..4d462f8 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -98,6 +98,7 @@ extern int vsnprintf(char *buf, int size, const char *fmt, va_list va)
>  extern int vprintf(const char *fmt, va_list va)
>  					__attribute__((format(printf, 1, 0)));
>  
> +extern void report_register_stamp(char * (*func)(void));
>  void report_prefix_pushf(const char *prefix_fmt, ...)
>  					__attribute__((format(printf, 1, 2)));
>  extern void report_prefix_push(const char *prefix);
> diff --git a/lib/report.c b/lib/report.c
> index ca9b4fd..586db63 100644
> --- a/lib/report.c
> +++ b/lib/report.c
> @@ -16,9 +16,17 @@
>  static unsigned int tests, failures, xfailures, skipped;
>  static char prefixes[256];
>  static struct spinlock lock;
> +static char * (*stamp_func)(void);
>  
>  #define PREFIX_DELIMITER ": "
>  
> +void report_register_stamp(char * (*func)(void))
> +{
> +	spin_lock(&lock);
> +	stamp_func = func;
> +	spin_unlock(&lock);
> +}
> +
>  void report_pass(void)
>  {
>  	spin_lock(&lock);
> @@ -90,6 +98,8 @@ static void va_report(const char *msg_fmt,
>  	spin_lock(&lock);
>  
>  	tests++;
> +	if (stamp_func)
> +		printf("%s ", stamp_func());
>  	printf("%s: ", prefix);
>  	puts(prefixes);
>  	vprintf(msg_fmt, va);
> 

Looks fine to me, but I think you should rather defer this patch to the
point in time when you also enable it for the s390x tests?

 Thomas
Janosch Frank Jan. 3, 2019, 2:32 p.m. UTC | #2
On 03.01.19 14:55, Thomas Huth wrote:
> On 2019-01-03 11:08, Janosch Frank wrote:
>> Lets add a function to get stamps for our reports. It'll help with
>> finding the test case that was causing problems in dumps.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  lib/libcflat.h |  1 +
>>  lib/report.c   | 10 ++++++++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/lib/libcflat.h b/lib/libcflat.h
>> index 7529958..4d462f8 100644
>> --- a/lib/libcflat.h
>> +++ b/lib/libcflat.h
>> @@ -98,6 +98,7 @@ extern int vsnprintf(char *buf, int size, const char *fmt, va_list va)
>>  extern int vprintf(const char *fmt, va_list va)
>>  					__attribute__((format(printf, 1, 0)));
>>  
>> +extern void report_register_stamp(char * (*func)(void));
>>  void report_prefix_pushf(const char *prefix_fmt, ...)
>>  					__attribute__((format(printf, 1, 2)));
>>  extern void report_prefix_push(const char *prefix);
>> diff --git a/lib/report.c b/lib/report.c
>> index ca9b4fd..586db63 100644
>> --- a/lib/report.c
>> +++ b/lib/report.c
>> @@ -16,9 +16,17 @@
>>  static unsigned int tests, failures, xfailures, skipped;
>>  static char prefixes[256];
>>  static struct spinlock lock;
>> +static char * (*stamp_func)(void);
>>  
>>  #define PREFIX_DELIMITER ": "
>>  
>> +void report_register_stamp(char * (*func)(void))
>> +{
>> +	spin_lock(&lock);
>> +	stamp_func = func;
>> +	spin_unlock(&lock);
>> +}
>> +
>>  void report_pass(void)
>>  {
>>  	spin_lock(&lock);
>> @@ -90,6 +98,8 @@ static void va_report(const char *msg_fmt,
>>  	spin_lock(&lock);
>>  
>>  	tests++;
>> +	if (stamp_func)
>> +		printf("%s ", stamp_func());
>>  	printf("%s: ", prefix);
>>  	puts(prefixes);
>>  	vprintf(msg_fmt, va);
>>
> 
> Looks fine to me, but I think you should rather defer this patch to the
> point in time when you also enable it for the s390x tests?
> 
>  Thomas
> 

This series came up because of some internal testing need which I can't
upstream right now. So internally I already had it enabled but mostly
hard coded.

If there's absolutely no upstream need I'll put it on my bench for latter :)
Thomas Huth Jan. 3, 2019, 2:37 p.m. UTC | #3
On 2019-01-03 15:32, Janosch Frank wrote:
> On 03.01.19 14:55, Thomas Huth wrote:
>> On 2019-01-03 11:08, Janosch Frank wrote:
>>> Lets add a function to get stamps for our reports. It'll help with
>>> finding the test case that was causing problems in dumps.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>  lib/libcflat.h |  1 +
>>>  lib/report.c   | 10 ++++++++++
>>>  2 files changed, 11 insertions(+)
>>>
>>> diff --git a/lib/libcflat.h b/lib/libcflat.h
>>> index 7529958..4d462f8 100644
>>> --- a/lib/libcflat.h
>>> +++ b/lib/libcflat.h
>>> @@ -98,6 +98,7 @@ extern int vsnprintf(char *buf, int size, const char *fmt, va_list va)
>>>  extern int vprintf(const char *fmt, va_list va)
>>>  					__attribute__((format(printf, 1, 0)));
>>>  
>>> +extern void report_register_stamp(char * (*func)(void));
>>>  void report_prefix_pushf(const char *prefix_fmt, ...)
>>>  					__attribute__((format(printf, 1, 2)));
>>>  extern void report_prefix_push(const char *prefix);
>>> diff --git a/lib/report.c b/lib/report.c
>>> index ca9b4fd..586db63 100644
>>> --- a/lib/report.c
>>> +++ b/lib/report.c
>>> @@ -16,9 +16,17 @@
>>>  static unsigned int tests, failures, xfailures, skipped;
>>>  static char prefixes[256];
>>>  static struct spinlock lock;
>>> +static char * (*stamp_func)(void);
>>>  
>>>  #define PREFIX_DELIMITER ": "
>>>  
>>> +void report_register_stamp(char * (*func)(void))
>>> +{
>>> +	spin_lock(&lock);
>>> +	stamp_func = func;
>>> +	spin_unlock(&lock);
>>> +}
>>> +
>>>  void report_pass(void)
>>>  {
>>>  	spin_lock(&lock);
>>> @@ -90,6 +98,8 @@ static void va_report(const char *msg_fmt,
>>>  	spin_lock(&lock);
>>>  
>>>  	tests++;
>>> +	if (stamp_func)
>>> +		printf("%s ", stamp_func());
>>>  	printf("%s: ", prefix);
>>>  	puts(prefixes);
>>>  	vprintf(msg_fmt, va);
>>>
>>
>> Looks fine to me, but I think you should rather defer this patch to the
>> point in time when you also enable it for the s390x tests?
>>
>>  Thomas
>>
> 
> This series came up because of some internal testing need which I can't
> upstream right now. So internally I already had it enabled but mostly
> hard coded.
> 
> If there's absolutely no upstream need I'll put it on my bench for latter :)

Generally, it does not make much sense to include functions in upstream
if they are not used yet. E.g. if you later forget to send your
patch(es) that use it, we just have dead code in the upstream repository
that nobody uses.  So either also send your patch that adds this to the
s390x code, or please delay it to a later point in time.

 Thanks,
  Thomas

Patch
diff mbox series

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 7529958..4d462f8 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -98,6 +98,7 @@  extern int vsnprintf(char *buf, int size, const char *fmt, va_list va)
 extern int vprintf(const char *fmt, va_list va)
 					__attribute__((format(printf, 1, 0)));
 
+extern void report_register_stamp(char * (*func)(void));
 void report_prefix_pushf(const char *prefix_fmt, ...)
 					__attribute__((format(printf, 1, 2)));
 extern void report_prefix_push(const char *prefix);
diff --git a/lib/report.c b/lib/report.c
index ca9b4fd..586db63 100644
--- a/lib/report.c
+++ b/lib/report.c
@@ -16,9 +16,17 @@ 
 static unsigned int tests, failures, xfailures, skipped;
 static char prefixes[256];
 static struct spinlock lock;
+static char * (*stamp_func)(void);
 
 #define PREFIX_DELIMITER ": "
 
+void report_register_stamp(char * (*func)(void))
+{
+	spin_lock(&lock);
+	stamp_func = func;
+	spin_unlock(&lock);
+}
+
 void report_pass(void)
 {
 	spin_lock(&lock);
@@ -90,6 +98,8 @@  static void va_report(const char *msg_fmt,
 	spin_lock(&lock);
 
 	tests++;
+	if (stamp_func)
+		printf("%s ", stamp_func());
 	printf("%s: ", prefix);
 	puts(prefixes);
 	vprintf(msg_fmt, va);