[PATCH] KVM: selftests: Drop newline from __TEST_REQUIRE

[PATCH] KVM: selftests: Drop newline from __TEST_REQUIRE

[Andrew Jones]

A few __TEST_REQUIRE callers are appending their own newline, resulting
in an extra one being output. Rather than remove the newlines from
those callers, remove it from __TEST_REQUIRE and add newlines to all
the other callers, as __TEST_REQUIRE was the only output function
appending newlines and consistency is a good thing.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---

Applies to kvm-x86/selftests (I chose that branch to ensure I got the
MAGIC_TOKEN change)

 tools/testing/selftests/kvm/aarch64/arch_timer.c          | 4 ++--
 tools/testing/selftests/kvm/aarch64/debug-exceptions.c    | 4 ++--
 tools/testing/selftests/kvm/aarch64/vgic_irq.c            | 2 +-
 tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c | 2 +-
 tools/testing/selftests/kvm/access_tracking_perf_test.c   | 4 ++--
 tools/testing/selftests/kvm/include/test_util.h           | 4 ++--
 tools/testing/selftests/kvm/lib/kvm_util.c                | 2 +-
 tools/testing/selftests/kvm/lib/x86_64/processor.c        | 4 ++--
 tools/testing/selftests/kvm/rseq_test.c                   | 2 +-
 tools/testing/selftests/kvm/system_counter_offset_test.c  | 2 +-
 tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c   | 2 +-
 11 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
index 274b8465b42a..89b93e342654 100644
--- a/tools/testing/selftests/kvm/aarch64/arch_timer.c
+++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
@@ -392,7 +392,7 @@ static struct kvm_vm *test_vm_create(void)
 
 	test_init_timer_irq(vm);
 	gic_fd = vgic_v3_setup(vm, nr_vcpus, 64, GICD_BASE_GPA, GICR_BASE_GPA);
-	__TEST_REQUIRE(gic_fd >= 0, "Failed to create vgic-v3");
+	__TEST_REQUIRE(gic_fd >= 0, "Failed to create vgic-v3\n");
 
 	/* Make all the test's cmdline args visible to the guest */
 	sync_global_to_guest(vm, test_args);
@@ -470,7 +470,7 @@ int main(int argc, char *argv[])
 		exit(KSFT_SKIP);
 
 	__TEST_REQUIRE(!test_args.migration_freq_ms || get_nprocs() >= 2,
-		       "At least two physical CPUs needed for vCPU migration");
+		       "At least two physical CPUs needed for vCPU migration\n");
 
 	vm = test_vm_create();
 	test_run(vm);
diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
index 866002917441..19088a971b1f 100644
--- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
+++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
@@ -540,7 +540,7 @@ void test_guest_debug_exceptions_all(uint64_t aa64dfr0)
 
 	/* Number of breakpoints */
 	brp_num = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_BRPs), aa64dfr0) + 1;
-	__TEST_REQUIRE(brp_num >= 2, "At least two breakpoints are required");
+	__TEST_REQUIRE(brp_num >= 2, "At least two breakpoints are required\n");
 
 	/* Number of watchpoints */
 	wrp_num = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_WRPs), aa64dfr0) + 1;
@@ -585,7 +585,7 @@ int main(int argc, char *argv[])
 	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
 	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), &aa64dfr0);
 	__TEST_REQUIRE(debug_version(aa64dfr0) >= 6,
-		       "Armv8 debug architecture not supported.");
+		       "Armv8 debug architecture not supported.\n");
 	kvm_vm_free(vm);
 
 	while ((opt = getopt(argc, argv, "i:")) != -1) {
diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
index 2e64b4856e38..e4452c4b60c2 100644
--- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c
+++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
@@ -766,7 +766,7 @@ static void test_vgic(uint32_t nr_irqs, bool level_sensitive, bool eoi_split)
 
 	gic_fd = vgic_v3_setup(vm, 1, nr_irqs,
 			GICD_BASE_GPA, GICR_BASE_GPA);
-	__TEST_REQUIRE(gic_fd >= 0, "Failed to create vgic-v3, skipping");
+	__TEST_REQUIRE(gic_fd >= 0, "Failed to create vgic-v3, skipping\n");
 
 	vm_install_exception_handler(vm, VECTOR_IRQ_CURRENT,
 		guest_irq_handlers[args.eoi_split][args.level_sensitive]);
diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
index 5ea78986e665..3c5b83e0e776 100644
--- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
+++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
@@ -458,7 +458,7 @@ static void create_vpmu_vm(void *guest_code)
 	vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
 					GICD_BASE_GPA, GICR_BASE_GPA);
 	__TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
-		       "Failed to create vgic-v3, skipping");
+		       "Failed to create vgic-v3, skipping\n");
 
 	/* Make sure that PMUv3 support is indicated in the ID register */
 	vcpu_get_reg(vpmu_vm.vcpu,
diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
index 3c7defd34f56..65efec6f4f90 100644
--- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
+++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
@@ -103,7 +103,7 @@ static uint64_t lookup_pfn(int pagemap_fd, struct kvm_vm *vm, uint64_t gva)
 		return 0;
 
 	pfn = entry & PAGEMAP_PFN_MASK;
-	__TEST_REQUIRE(pfn, "Looking up PFNs requires CAP_SYS_ADMIN");
+	__TEST_REQUIRE(pfn, "Looking up PFNs requires CAP_SYS_ADMIN\n");
 
 	return pfn;
 }
@@ -385,7 +385,7 @@ int main(int argc, char *argv[])
 
 	page_idle_fd = open("/sys/kernel/mm/page_idle/bitmap", O_RDWR);
 	__TEST_REQUIRE(page_idle_fd >= 0,
-		       "CONFIG_IDLE_PAGE_TRACKING is not enabled");
+		       "CONFIG_IDLE_PAGE_TRACKING is not enabled\n");
 	close(page_idle_fd);
 
 	for_each_guest_mode(run_test, &params);
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index a0c7dd3a5b30..88763f0e1b78 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -37,10 +37,10 @@ void __printf(1, 2) print_skip(const char *fmt, ...);
 #define __TEST_REQUIRE(f, fmt, ...)				\
 do {								\
 	if (!(f))						\
-		ksft_exit_skip("- " fmt "\n", ##__VA_ARGS__);	\
+		ksft_exit_skip("- " fmt, ##__VA_ARGS__);	\
 } while (0)
 
-#define TEST_REQUIRE(f) __TEST_REQUIRE(f, "Requirement not met: %s", #f)
+#define TEST_REQUIRE(f) __TEST_REQUIRE(f, "Requirement not met: %s\n", #f)
 
 ssize_t test_write(int fd, const void *buf, size_t count);
 ssize_t test_read(int fd, void *buf, size_t count);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 17a978b8a2c4..7b40990d9b08 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -27,7 +27,7 @@ int open_path_or_exit(const char *path, int flags)
 	int fd;
 
 	fd = open(path, flags);
-	__TEST_REQUIRE(fd >= 0, "%s not available (errno: %d)", path, errno);
+	__TEST_REQUIRE(fd >= 0, "%s not available (errno: %d)\n", path, errno);
 
 	return fd;
 }
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index d8288374078e..abd19059b236 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -719,12 +719,12 @@ void __vm_xsave_require_permission(uint64_t xfeature, const char *name)
 	close(kvm_fd);
 
 	if (rc == -1 && (errno == ENXIO || errno == EINVAL))
-		__TEST_REQUIRE(0, "KVM_X86_XCOMP_GUEST_SUPP not supported");
+		__TEST_REQUIRE(0, "KVM_X86_XCOMP_GUEST_SUPP not supported\n");
 
 	TEST_ASSERT(rc == 0, "KVM_GET_DEVICE_ATTR(0, KVM_X86_XCOMP_GUEST_SUPP) error: %ld", rc);
 
 	__TEST_REQUIRE(bitmask & xfeature,
-		       "Required XSAVE feature '%s' not supported", name);
+		       "Required XSAVE feature '%s' not supported\n", name);
 
 	TEST_REQUIRE(!syscall(SYS_arch_prctl, ARCH_REQ_XCOMP_GUEST_PERM, ilog2(xfeature)));
 
diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c
index f74e76d03b7e..e825fe6807fa 100644
--- a/tools/testing/selftests/kvm/rseq_test.c
+++ b/tools/testing/selftests/kvm/rseq_test.c
@@ -183,7 +183,7 @@ static void calc_min_max_cpu(void)
 	}
 
 	__TEST_REQUIRE(cnt >= 2,
-		       "Only one usable CPU, task migration not possible");
+		       "Only one usable CPU, task migration not possible\n");
 }
 
 int main(int argc, char *argv[])
diff --git a/tools/testing/selftests/kvm/system_counter_offset_test.c b/tools/testing/selftests/kvm/system_counter_offset_test.c
index 7f5b330b6a1b..46e4ca694e6d 100644
--- a/tools/testing/selftests/kvm/system_counter_offset_test.c
+++ b/tools/testing/selftests/kvm/system_counter_offset_test.c
@@ -30,7 +30,7 @@ static void check_preconditions(struct kvm_vcpu *vcpu)
 {
 	__TEST_REQUIRE(!__vcpu_has_device_attr(vcpu, KVM_VCPU_TSC_CTRL,
 					       KVM_VCPU_TSC_OFFSET),
-		       "KVM_VCPU_TSC_OFFSET not supported; skipping test");
+		       "KVM_VCPU_TSC_OFFSET not supported; skipping test\n");
 }
 
 static void setup_system_counter(struct kvm_vcpu *vcpu, struct test_case *test)
diff --git a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
index 83e25bccc139..a3f4edbd9add 100644
--- a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
+++ b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
@@ -259,7 +259,7 @@ int main(int argc, char **argv)
 	__TEST_REQUIRE(token == MAGIC_TOKEN,
 		       "This test must be run with the magic token %d.\n"
 		       "This is done by nx_huge_pages_test.sh, which\n"
-		       "also handles environment setup for the test.", MAGIC_TOKEN);
+		       "also handles environment setup for the test.\n", MAGIC_TOKEN);
 
 	run_test(reclaim_period_ms, false, reboot_permissions);
 	run_test(reclaim_period_ms, true, reboot_permissions);
-- 
2.43.0

Re: [PATCH] KVM: selftests: Drop newline from __TEST_REQUIRE

[Sean Christopherson]

On Thu, Nov 30, 2023, Andrew Jones wrote:
> A few __TEST_REQUIRE callers are appending their own newline, resulting
> in an extra one being output. Rather than remove the newlines from
> those callers, remove it from __TEST_REQUIRE and add newlines to all
> the other callers, as __TEST_REQUIRE was the only output function
> appending newlines and consistency is a good thing.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> 
> Applies to kvm-x86/selftests (I chose that branch to ensure I got the
> MAGIC_TOKEN change)

Heh, and then I went and created a conflict anyways :-)

https://lore.kernel.org/all/20231129224042.530798-1-seanjc@google.com

If there are no objections, I'll grab this in kvm-x86/selftests and sort out the
MAGIC_TOKEN conflict.

Re: [PATCH] KVM: selftests: Drop newline from __TEST_REQUIRE

[Andrew Jones]

On Thu, Nov 30, 2023 at 07:16:28PM +0000, Sean Christopherson wrote:
> On Thu, Nov 30, 2023, Andrew Jones wrote:
> > A few __TEST_REQUIRE callers are appending their own newline, resulting
> > in an extra one being output. Rather than remove the newlines from
> > those callers, remove it from __TEST_REQUIRE and add newlines to all
> > the other callers, as __TEST_REQUIRE was the only output function
> > appending newlines and consistency is a good thing.
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> > 
> > Applies to kvm-x86/selftests (I chose that branch to ensure I got the
> > MAGIC_TOKEN change)
> 
> Heh, and then I went and created a conflict anyways :-)
> 
> https://lore.kernel.org/all/20231129224042.530798-1-seanjc@google.com
> 
> If there are no objections, I'll grab this in kvm-x86/selftests and sort out the
> MAGIC_TOKEN conflict.

No objections from me.

Thanks,
drew

Re: [PATCH] KVM: selftests: Drop newline from __TEST_REQUIRE

[Sean Christopherson]

On Thu, Nov 30, 2023, Sean Christopherson wrote:
> On Thu, Nov 30, 2023, Andrew Jones wrote:
> > A few __TEST_REQUIRE callers are appending their own newline, resulting
> > in an extra one being output. Rather than remove the newlines from
> > those callers, remove it from __TEST_REQUIRE and add newlines to all
> > the other callers, as __TEST_REQUIRE was the only output function
> > appending newlines and consistency is a good thing.
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> > 
> > Applies to kvm-x86/selftests (I chose that branch to ensure I got the
> > MAGIC_TOKEN change)
> 
> Heh, and then I went and created a conflict anyways :-)
> 
> https://lore.kernel.org/all/20231129224042.530798-1-seanjc@google.com
> 
> If there are no objections, I'll grab this in kvm-x86/selftests and sort out the
> MAGIC_TOKEN conflict.

Actually, I misread the patch.  I thought you were removing newlines, not adding
them.  My thinking for TEST_REQUIRE() is that it should look and behave like
TEST_ASSERT(), not like a raw printf().  I.e. the caller provides the raw message,
and the framework handles formatting the final output.

Re: [PATCH] KVM: selftests: Drop newline from __TEST_REQUIRE

[Andrew Jones]

On Fri, Dec 01, 2023 at 08:08:31AM -0800, Sean Christopherson wrote:
> On Thu, Nov 30, 2023, Sean Christopherson wrote:
> > On Thu, Nov 30, 2023, Andrew Jones wrote:
> > > A few __TEST_REQUIRE callers are appending their own newline, resulting
> > > in an extra one being output. Rather than remove the newlines from
> > > those callers, remove it from __TEST_REQUIRE and add newlines to all
> > > the other callers, as __TEST_REQUIRE was the only output function
> > > appending newlines and consistency is a good thing.
> > > 
> > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > > ---
> > > 
> > > Applies to kvm-x86/selftests (I chose that branch to ensure I got the
> > > MAGIC_TOKEN change)
> > 
> > Heh, and then I went and created a conflict anyways :-)
> > 
> > https://lore.kernel.org/all/20231129224042.530798-1-seanjc@google.com
> > 
> > If there are no objections, I'll grab this in kvm-x86/selftests and sort out the
> > MAGIC_TOKEN conflict.
> 
> Actually, I misread the patch.  I thought you were removing newlines, not adding
> them.  My thinking for TEST_REQUIRE() is that it should look and behave like
> TEST_ASSERT(), not like a raw printf().  I.e. the caller provides the raw message,
> and the framework handles formatting the final output.

Darn. I'm not sure how I forgot about the other TEST_* functions. We
indeed want them to be consistent. I'll reverse the patch. Also, it
looks like when errno == EACCES, test_assert() will be missing a newline.
I'll do a quick audit of TEST_* functions and their callers this time
around.

Thanks,
drew