* [PATCH 1/3] selftests: KVM: Check stat name before other fields
2022-07-19 14:31 [PATCH 0/3] selftests: KVM: Improvements to binary stats test Oliver Upton
@ 2022-07-19 14:31 ` Oliver Upton
2022-07-19 14:31 ` [PATCH 2/3] selftests: KVM: Provide descriptive assertions in kvm_binary_stats_test Oliver Upton
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Oliver Upton @ 2022-07-19 14:31 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, seanjc, Oliver Upton
From: Oliver Upton <oupton@google.com>
In order to provide more useful test assertions that describe the broken
stats descriptor, perform sanity check on the stat name before any other
descriptor field. While at it, avoid dereferencing the name field if the
sanity check fails as it is more likely to contain garbage.
Signed-off-by: Oliver Upton <oupton@google.com>
---
tools/testing/selftests/kvm/kvm_binary_stats_test.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/kvm_binary_stats_test.c b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
index b01e8b0851e7..40227ad9ba0c 100644
--- a/tools/testing/selftests/kvm/kvm_binary_stats_test.c
+++ b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
@@ -73,6 +73,10 @@ static void stats_test(int stats_fd)
for (i = 0; i < header.num_desc; ++i) {
pdesc = get_stats_descriptor(stats_desc, i, &header);
+ /* Check name string */
+ TEST_ASSERT(strlen(pdesc->name) < header.name_size,
+ "KVM stats name (index: %d) too long", i);
+
/* Check type,unit,base boundaries */
TEST_ASSERT((pdesc->flags & KVM_STATS_TYPE_MASK) <= KVM_STATS_TYPE_MAX,
"Unknown KVM stats type");
@@ -99,9 +103,7 @@ static void stats_test(int stats_fd)
TEST_ASSERT(pdesc->exponent <= 0, "Unsupported KVM stats unit");
break;
}
- /* Check name string */
- TEST_ASSERT(strlen(pdesc->name) < header.name_size,
- "KVM stats name(%s) too long", pdesc->name);
+
/* Check size field, which should not be zero */
TEST_ASSERT(pdesc->size,
"KVM descriptor(%s) with size of 0", pdesc->name);
--
2.37.0.170.g444d1eabd0-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] selftests: KVM: Provide descriptive assertions in kvm_binary_stats_test
2022-07-19 14:31 [PATCH 0/3] selftests: KVM: Improvements to binary stats test Oliver Upton
2022-07-19 14:31 ` [PATCH 1/3] selftests: KVM: Check stat name before other fields Oliver Upton
@ 2022-07-19 14:31 ` Oliver Upton
2022-07-19 14:31 ` [PATCH 3/3] selftests: KVM: Add exponent check for boolean stats Oliver Upton
2022-07-19 16:09 ` [PATCH 0/3] selftests: KVM: Improvements to binary stats test Andrew Jones
3 siblings, 0 replies; 6+ messages in thread
From: Oliver Upton @ 2022-07-19 14:31 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, seanjc, Oliver Upton
From: Oliver Upton <oupton@google.com>
As it turns out, tests sometimes fail. When that is the case, packing
the test assertion with as much relevant information helps track down
the problem more quickly.
Sharpen up the stat descriptor assertions in kvm_binary_stats_test to
more precisely describe the reason for the test assertion and which
stat is to blame.
Signed-off-by: Oliver Upton <oupton@google.com>
---
.../selftests/kvm/kvm_binary_stats_test.c | 24 ++++++++++++-------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/kvm/kvm_binary_stats_test.c b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
index 40227ad9ba0c..3237c7c94bf0 100644
--- a/tools/testing/selftests/kvm/kvm_binary_stats_test.c
+++ b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
@@ -31,6 +31,7 @@ static void stats_test(int stats_fd)
struct kvm_stats_desc *stats_desc;
u64 *stats_data;
struct kvm_stats_desc *pdesc;
+ u32 type, unit, base;
/* Read kvm stats header */
read_stats_header(stats_fd, &header);
@@ -72,18 +73,21 @@ static void stats_test(int stats_fd)
/* Sanity check for fields in descriptors */
for (i = 0; i < header.num_desc; ++i) {
pdesc = get_stats_descriptor(stats_desc, i, &header);
+ type = pdesc->flags & KVM_STATS_TYPE_MASK;
+ unit = pdesc->flags & KVM_STATS_UNIT_MASK;
+ base = pdesc->flags & KVM_STATS_BASE_MASK;
/* Check name string */
TEST_ASSERT(strlen(pdesc->name) < header.name_size,
"KVM stats name (index: %d) too long", i);
/* Check type,unit,base boundaries */
- TEST_ASSERT((pdesc->flags & KVM_STATS_TYPE_MASK) <= KVM_STATS_TYPE_MAX,
- "Unknown KVM stats type");
- TEST_ASSERT((pdesc->flags & KVM_STATS_UNIT_MASK) <= KVM_STATS_UNIT_MAX,
- "Unknown KVM stats unit");
- TEST_ASSERT((pdesc->flags & KVM_STATS_BASE_MASK) <= KVM_STATS_BASE_MAX,
- "Unknown KVM stats base");
+ TEST_ASSERT(type <= KVM_STATS_TYPE_MAX,
+ "Unknown KVM stats (%s) type: %u", pdesc->name, type);
+ TEST_ASSERT(unit <= KVM_STATS_UNIT_MAX,
+ "Unknown KVM stats (%s) unit: %u", pdesc->name, unit);
+ TEST_ASSERT(base <= KVM_STATS_BASE_MAX,
+ "Unknown KVM stats (%s) base: %u", pdesc->name, base);
/*
* Check exponent for stats unit
@@ -97,10 +101,14 @@ static void stats_test(int stats_fd)
case KVM_STATS_UNIT_NONE:
case KVM_STATS_UNIT_BYTES:
case KVM_STATS_UNIT_CYCLES:
- TEST_ASSERT(pdesc->exponent >= 0, "Unsupported KVM stats unit");
+ TEST_ASSERT(pdesc->exponent >= 0,
+ "Unsupported KVM stats (%s) exponent: %i",
+ pdesc->name, pdesc->exponent);
break;
case KVM_STATS_UNIT_SECONDS:
- TEST_ASSERT(pdesc->exponent <= 0, "Unsupported KVM stats unit");
+ TEST_ASSERT(pdesc->exponent <= 0,
+ "Unsupported KVM stats (%s) exponent: %i",
+ pdesc->name, pdesc->exponent);
break;
}
--
2.37.0.170.g444d1eabd0-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] selftests: KVM: Add exponent check for boolean stats
2022-07-19 14:31 [PATCH 0/3] selftests: KVM: Improvements to binary stats test Oliver Upton
2022-07-19 14:31 ` [PATCH 1/3] selftests: KVM: Check stat name before other fields Oliver Upton
2022-07-19 14:31 ` [PATCH 2/3] selftests: KVM: Provide descriptive assertions in kvm_binary_stats_test Oliver Upton
@ 2022-07-19 14:31 ` Oliver Upton
2022-07-19 16:09 ` [PATCH 0/3] selftests: KVM: Improvements to binary stats test Andrew Jones
3 siblings, 0 replies; 6+ messages in thread
From: Oliver Upton @ 2022-07-19 14:31 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, seanjc, Oliver Upton
From: Oliver Upton <oupton@google.com>
The only sensible exponent for a boolean stat is 0. Add a test assertion
requiring all boolean statistics to have an exponent of 0.
Signed-off-by: Oliver Upton <oupton@google.com>
---
tools/testing/selftests/kvm/kvm_binary_stats_test.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/testing/selftests/kvm/kvm_binary_stats_test.c b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
index 3237c7c94bf0..0b45ac593387 100644
--- a/tools/testing/selftests/kvm/kvm_binary_stats_test.c
+++ b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
@@ -96,6 +96,7 @@ static void stats_test(int stats_fd)
* Exponent for unit seconds should be less than or equal to 0
* Exponent for unit clock cycles should be greater than or
* equal to 0
+ * Exponent for unit boolean should be 0
*/
switch (pdesc->flags & KVM_STATS_UNIT_MASK) {
case KVM_STATS_UNIT_NONE:
@@ -110,6 +111,11 @@ static void stats_test(int stats_fd)
"Unsupported KVM stats (%s) exponent: %i",
pdesc->name, pdesc->exponent);
break;
+ case KVM_STATS_UNIT_BOOLEAN:
+ TEST_ASSERT(pdesc->exponent == 0,
+ "Unsupported KVM stats (%s) exponent: %d",
+ pdesc->name, pdesc->exponent);
+ break;
}
/* Check size field, which should not be zero */
--
2.37.0.170.g444d1eabd0-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] selftests: KVM: Improvements to binary stats test
2022-07-19 14:31 [PATCH 0/3] selftests: KVM: Improvements to binary stats test Oliver Upton
` (2 preceding siblings ...)
2022-07-19 14:31 ` [PATCH 3/3] selftests: KVM: Add exponent check for boolean stats Oliver Upton
@ 2022-07-19 16:09 ` Andrew Jones
2022-07-19 17:53 ` Paolo Bonzini
3 siblings, 1 reply; 6+ messages in thread
From: Andrew Jones @ 2022-07-19 16:09 UTC (permalink / raw)
To: Oliver Upton; +Cc: kvm, pbonzini, seanjc, Oliver Upton
On Tue, Jul 19, 2022 at 02:31:31PM +0000, Oliver Upton wrote:
> From: Oliver Upton <oupton@google.com>
>
> Small series to improve the debuggability of the binary stats test w/
> more descriptive test assertions + add coverage for boolean stats.
>
> Applies to kvm/queue, with the following patches applied:
>
> - 1b870fa5573e ("kvm: stats: tell userspace which values are boolean")
> - https://lore.kernel.org/kvm/20220719125229.2934273-1-oupton@google.com/
>
> First time sending patches from my new inbox, apologies if I've screwed
> something up.
>
> Oliver Upton (3):
> selftests: KVM: Check stat name before other fields
> selftests: KVM: Provide descriptive assertions in
> kvm_binary_stats_test
> selftests: KVM: Add exponent check for boolean stats
>
> .../selftests/kvm/kvm_binary_stats_test.c | 38 +++++++++++++------
> 1 file changed, 27 insertions(+), 11 deletions(-)
>
> --
> 2.37.0.170.g444d1eabd0-goog
>
For the series,
Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] selftests: KVM: Improvements to binary stats test
2022-07-19 16:09 ` [PATCH 0/3] selftests: KVM: Improvements to binary stats test Andrew Jones
@ 2022-07-19 17:53 ` Paolo Bonzini
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2022-07-19 17:53 UTC (permalink / raw)
To: Andrew Jones, Oliver Upton; +Cc: kvm, seanjc, Oliver Upton
On 7/19/22 18:09, Andrew Jones wrote:
> On Tue, Jul 19, 2022 at 02:31:31PM +0000, Oliver Upton wrote:
>> From: Oliver Upton <oupton@google.com>
>>
>> Small series to improve the debuggability of the binary stats test w/
>> more descriptive test assertions + add coverage for boolean stats.
>>
>> Applies to kvm/queue, with the following patches applied:
>>
>> - 1b870fa5573e ("kvm: stats: tell userspace which values are boolean")
>> - https://lore.kernel.org/kvm/20220719125229.2934273-1-oupton@google.com/
>>
>> First time sending patches from my new inbox, apologies if I've screwed
>> something up.
>>
>> Oliver Upton (3):
>> selftests: KVM: Check stat name before other fields
>> selftests: KVM: Provide descriptive assertions in
>> kvm_binary_stats_test
>> selftests: KVM: Add exponent check for boolean stats
>>
>> .../selftests/kvm/kvm_binary_stats_test.c | 38 +++++++++++++------
>> 1 file changed, 27 insertions(+), 11 deletions(-)
>>
>> --
>> 2.37.0.170.g444d1eabd0-goog
>>
>
> For the series,
>
> Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread