Re: [kvm-unit-tests PATCH v4 13/13] lib/report: Add stamps to reports

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxx>
>>> ---
>>>  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

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux