diff mbox series

[kvmtool,v1,1/2] Propagate the error code from any VCPU thread

Message ID 20250120161800.30270-2-alexandru.elisei@arm.com (mailing list archive)
State New
Headers show
Series Error handling fixes | expand

Commit Message

Alexandru Elisei Jan. 20, 2025, 4:17 p.m. UTC
kvm_cmd_run_work() doesn't capture the return code for VCPU 0 when calling
pthread_join(). Then kvm_cpu__exit() waits for the threads for VCPUs 1..N
to terminate, and it sets the return value to 0 if at least one of the VCPU
threads returns 0.

This approach creates several issues:

1. An error code from a previous VCPU is overwritten with 0 (success).
2. kvmtool will return 0 even if VCPU 0 encountered an error, as long as
   at least one of the other VCPUs exited successfully.
3. kvm_cpu__exit() will return an uninitialized value if all VCPUs 1..N
   exited with an error, or if there is only one VCPU, VCPU 0.

Fix all issues issues by propagating the return code from VCPU 0, and
saving the error code for each of the remaining VCPUs. If multiple VCPU
threads exit with an error, the exit status is set to the first error.

With this change, kvmtool will terminate with a non-zero exit code if there
was an unhandled error in KVM_RUN, as expected.

While on the subject of return values, apply the more common pattern of
using PTR_ERR(<return value>) in kvm_cpu_thread() instead of double
casting the return value.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 builtin-run.c         |  9 +++++----
 include/kvm/kvm-cpu.h |  2 +-
 kvm-cpu.c             | 17 ++++++++++-------
 3 files changed, 16 insertions(+), 12 deletions(-)
diff mbox series

Patch

diff --git a/builtin-run.c b/builtin-run.c
index ebff9d5da49d..4b9a3917f006 100644
--- a/builtin-run.c
+++ b/builtin-run.c
@@ -293,7 +293,7 @@  static void *kvm_cpu_thread(void *arg)
 	if (kvm_cpu__start(current_kvm_cpu))
 		goto panic_kvm;
 
-	return (void *) (intptr_t) 0;
+	return ERR_PTR(0);
 
 panic_kvm:
 	pr_err("KVM exit reason: %u (\"%s\")",
@@ -310,7 +310,7 @@  panic_kvm:
 	kvm_cpu__show_code(current_kvm_cpu);
 	kvm_cpu__show_page_tables(current_kvm_cpu);
 
-	return (void *) (intptr_t) 1;
+	return ERR_PTR(1);
 }
 
 static char kernel[PATH_MAX];
@@ -830,6 +830,7 @@  static struct kvm *kvm_cmd_run_init(int argc, const char **argv)
 
 static int kvm_cmd_run_work(struct kvm *kvm)
 {
+	void *vcpu0_ret;
 	int i;
 
 	for (i = 0; i < kvm->nrcpus; i++) {
@@ -838,10 +839,10 @@  static int kvm_cmd_run_work(struct kvm *kvm)
 	}
 
 	/* Only VCPU #0 is going to exit by itself when shutting down */
-	if (pthread_join(kvm->cpus[0]->thread, NULL) != 0)
+	if (pthread_join(kvm->cpus[0]->thread, &vcpu0_ret) != 0)
 		die("unable to join with vcpu 0");
 
-	return kvm_cpu__exit(kvm);
+	return kvm_cpu__exit(kvm, PTR_ERR(vcpu0_ret));
 }
 
 static void kvm_cmd_run_exit(struct kvm *kvm, int guest_ret)
diff --git a/include/kvm/kvm-cpu.h b/include/kvm/kvm-cpu.h
index 0f16f8d6e872..8f76f8a1123a 100644
--- a/include/kvm/kvm-cpu.h
+++ b/include/kvm/kvm-cpu.h
@@ -10,7 +10,7 @@  struct kvm_cpu_task {
 };
 
 int kvm_cpu__init(struct kvm *kvm);
-int kvm_cpu__exit(struct kvm *kvm);
+int kvm_cpu__exit(struct kvm *kvm, int vcpu0_ret);
 struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id);
 void kvm_cpu__delete(struct kvm_cpu *vcpu);
 void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu);
diff --git a/kvm-cpu.c b/kvm-cpu.c
index f66dcd07220c..7362f2e99261 100644
--- a/kvm-cpu.c
+++ b/kvm-cpu.c
@@ -1,3 +1,5 @@ 
+#include "linux/err.h"
+
 #include "kvm/kvm-cpu.h"
 
 #include "kvm/symbol.h"
@@ -303,10 +305,11 @@  fail_alloc:
 }
 base_init(kvm_cpu__init);
 
-int kvm_cpu__exit(struct kvm *kvm)
+int kvm_cpu__exit(struct kvm *kvm, int vcpu0_ret)
 {
-	int i, r;
-	void *ret = NULL;
+	int ret = vcpu0_ret;
+	void *vcpu_ret;
+	int i;
 
 	kvm_cpu__delete(kvm->cpus[0]);
 	kvm->cpus[0] = NULL;
@@ -315,12 +318,12 @@  int kvm_cpu__exit(struct kvm *kvm)
 	for (i = 1; i < kvm->nrcpus; i++) {
 		if (kvm->cpus[i]->is_running) {
 			pthread_kill(kvm->cpus[i]->thread, SIGKVMEXIT);
-			if (pthread_join(kvm->cpus[i]->thread, &ret) != 0)
+			if (pthread_join(kvm->cpus[i]->thread, &vcpu_ret) != 0)
 				die("pthread_join");
 			kvm_cpu__delete(kvm->cpus[i]);
+			if (!ret && PTR_ERR(vcpu_ret))
+				ret = PTR_ERR(vcpu_ret);
 		}
-		if (ret == NULL)
-			r = 0;
 	}
 	kvm__continue(kvm);
 
@@ -330,5 +333,5 @@  int kvm_cpu__exit(struct kvm *kvm)
 
 	close(task_eventfd);
 
-	return r;
+	return ret;
 }