[PATCH v2] perf python: Set error messages on call failure

[Jinli Xiao]

Updates the perf python binding to provide more informative error
messages to the user when there is a call failure. The changes include
setting error messages on several different scenarios when the package
needs to return -1 or NULL.

Resolves the FIXME in evsel__init, which previously raises segmentation
fault on failure. Now it could end more gracefully.

The shell tests provided also includes the import test, previously
written in c (https://marc.info/?l=linux-kernel&m=135911158512404). In
order to run the test, python3 and setting PYTHONPATH (to include the
perf python package) is needed.

Signed-off-by: Jinli Xiao <jinli.xiao@xxxxxxx>
---
 tools/perf/tests/shell/test_python_binding.sh | 113 ++++++++++++++++++
 tools/perf/util/python.c                      |  35 ++++--
 2 files changed, 141 insertions(+), 7 deletions(-)
 create mode 100755 tools/perf/tests/shell/test_python_binding.sh

diff --git a/tools/perf/tests/shell/test_python_binding.sh b/tools/perf/tests/shell/test_python_binding.sh
new file mode 100755
index 000000000000..99d0033ff4c5
--- /dev/null
+++ b/tools/perf/tests/shell/test_python_binding.sh
@@ -0,0 +1,113 @@
+#!/bin/bash
+# perf python binding test
+# SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+
+set -e
+
+if [ "x$PYTHON" == "x" ]
+then
+	if which python3 > /dev/null
+	then
+		PYTHON=python3
+	elif which python > /dev/null
+	then
+		PYTHON=python
+	else
+		echo Skipping test, python not detected please set environment variable PYTHON.
+		exit 2
+	fi
+fi
+
+if [ "x$PYTHONPATH" == "x" ]
+then
+    echo Skipping test, PYTHONPATH not set.
+    exit 2
+fi
+
+test_python_import()
+{
+    echo -n "Checking python binding: import perf "
+    $PYTHON -c "import perf"
+    echo "[Success]"
+}
+
+test_thread_map_exception_handling()
+{
+    echo -n "Checking python binding: thread map exception handling "
+    MAX_PID=$(cat /proc/sys/kernel/pid_max)
+    INVALID_PID=$((MAX_PID + 1))
+    $PYTHON -c "
+import perf
+try:
+    perf.thread_map($INVALID_PID)
+    raise
+except FileNotFoundError as e:
+    pass
+"
+    echo "[Success]"
+}
+
+test_cpu_map_exception_handling()
+{
+    echo -n "Checking python binding: cpu map exception handling "
+    MAX_CPU=$(lscpu | awk '/^CPU\(s\)/ {print $2}')
+    INVALID_CPU_ID="$((MAX_CPU + 1))"
+    $PYTHON -c "
+import perf
+try:
+    perf.cpu_map('$INVALID_CPU_ID')
+    raise
+except RuntimeError as e:
+    pass
+"
+    echo "[Success]"
+}
+
+test_evsel_exception_handling()
+{
+    echo -n "Checking python binding: evsel exception handling "
+    $PYTHON -c "
+import perf
+
+perf.evsel()
+
+try:
+    perf.evsel(a=1) # invalid args
+    raise
+except TypeError as e:
+    pass
+
+try:
+    perf.evsel(sample_freq=1, sample_period=1)
+except ValueError as e:
+    pass
+"
+    echo "[Success]"
+}
+
+test_evlist_read_on_cpu_exception_handling()
+{
+    echo -n "Checking python binding: evlist read_on_cpu exception handling "
+    $PYTHON -c "
+import perf
+
+cpus = perf.cpu_map()
+threads = perf.thread_map(-1)
+evlist = perf.evlist(cpus, threads)
+
+try:
+    evlist.read_on_cpu(0)
+    raise
+except RuntimeError as e:
+    pass
+"
+    echo "[Success]"
+}
+
+test_python_import
+test_thread_map_exception_handling
+test_cpu_map_exception_handling
+test_evsel_exception_handling
+test_evlist_read_on_cpu_exception_handling
+
+exit 0
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 42e8b813d010..2d01a03fa40a 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -639,8 +639,13 @@ static int pyrf_cpu_map__init(struct pyrf_cpu_map *pcpus,
 		return -1;
 
 	pcpus->cpus = perf_cpu_map__new(cpustr);
-	if (pcpus->cpus == NULL)
+	if (pcpus->cpus == NULL) {
+		if (errno)
+			PyErr_SetFromErrno(PyExc_OSError);
+		else
+			PyErr_SetString(PyExc_ValueError, "perf: invalid cpu string");
 		return -1;
+	}
 	return 0;
 }
 
@@ -708,8 +713,13 @@ static int pyrf_thread_map__init(struct pyrf_thread_map *pthreads,
 		return -1;
 
 	pthreads->threads = thread_map__new(pid, tid, uid);
-	if (pthreads->threads == NULL)
+	if (pthreads->threads == NULL) {
+		if (errno)
+			PyErr_SetFromErrno(PyExc_OSError);
+		else
+			PyErr_SetString(PyExc_ValueError, "perf: fails to create thread map");
 		return -1;
+	}
 	return 0;
 }
 
@@ -763,6 +773,7 @@ static int pyrf_thread_map__setup_types(void)
 struct pyrf_evsel {
 	PyObject_HEAD
 
+	bool initialized;
 	struct evsel evsel;
 };
 
@@ -839,13 +850,19 @@ static int pyrf_evsel__init(struct pyrf_evsel *pevsel,
 					 &enable_on_exec, &task, &watermark,
 					 &precise_ip, &mmap_data, &sample_id_all,
 					 &attr.wakeup_events, &attr.bp_type,
-					 &attr.bp_addr, &attr.bp_len, &idx))
+					 &attr.bp_addr, &attr.bp_len, &idx)) {
+		pevsel->initialized = false;
 		return -1;
+	}
 
 	/* union... */
 	if (sample_period != 0) {
-		if (attr.sample_freq != 0)
-			return -1; /* FIXME: throw right exception */
+		if (attr.sample_freq != 0) {
+			pevsel->initialized = false;
+			PyErr_SetString(PyExc_ValueError,
+					"perf: sample_freq and sample_period are mutually exclusive");
+			return -1;
+		}
 		attr.sample_period = sample_period;
 	}
 
@@ -872,12 +889,14 @@ static int pyrf_evsel__init(struct pyrf_evsel *pevsel,
 	attr.size	    = sizeof(attr);
 
 	evsel__init(&pevsel->evsel, &attr, idx);
+	pevsel->initialized = true;
 	return 0;
 }
 
 static void pyrf_evsel__delete(struct pyrf_evsel *pevsel)
 {
-	evsel__exit(&pevsel->evsel);
+	if (pevsel->initialized)
+		evsel__exit(&pevsel->evsel);
 	Py_TYPE(pevsel)->tp_free((PyObject*)pevsel);
 }
 
@@ -1098,7 +1117,7 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 
 	md = get_md(evlist, cpu);
 	if (!md)
-		return NULL;
+		return PyErr_Format(PyExc_RuntimeError, "perf: no mmap for cpu %d", cpu);
 
 	if (perf_mmap__read_init(&md->core) < 0)
 		goto end;
@@ -1324,6 +1343,8 @@ static PyObject *pyrf__tracepoint(struct pyrf_evsel *pevsel,
 				  PyObject *args, PyObject *kwargs)
 {
 #ifndef HAVE_LIBTRACEEVENT
+	PyErr_SetString(PyExc_OSError,
+			"perf: tracepoint support not compiled in");
 	return NULL;
 #else
 	struct tep_event *tp_format;
-- 
2.39.2