Richard W.M. Jones
2018-Apr-05 12:47 UTC
[Libguestfs] [PATCH nbdkit] python: Turn python exceptions into nbdkit errors
Much more annoying that it needs to be, but I have tested it and it works on Python 2 & 3. Note this will not work on Python 3.0 - 3.2, but I guess we don't care about those versions. Rich.
Richard W.M. Jones
2018-Apr-05 12:47 UTC
[Libguestfs] [PATCH nbdkit] python: Turn python exceptions into nbdkit errors properly.
---
configure.ac | 8 +++++++
plugins/python/python.c | 51 +++++++++++++++++++++++++++++++++++++++---
tests/Makefile.am | 1 +
tests/python-exception.py | 45 +++++++++++++++++++++++++++++++++++++
tests/test-python-exception.sh | 42 ++++++++++++++++++++++++++++++++++
5 files changed, 144 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac
index be38c84..c4c04d3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -270,6 +270,14 @@ AS_IF([test "x$PYTHON" != "xno"
&& test "x$enable_python" != "xno"],[
[AC_DEFINE([HAVE_PYSTRING_FROMSTRING],1,
[Found PyString_FromString in libpython.])],
[],[$PYTHON_BLDLIBRARY])
+ AC_CHECK_LIB([c],[PyString_AsString],
+ [AC_DEFINE([HAVE_PYSTRING_ASSTRING],1,
+ [Found PyString_AsString in libpython.])],
+ [],[$PYTHON_BLDLIBRARY])
+ AC_CHECK_LIB([c],[PyUnicode_AsUTF8],
+ [AC_DEFINE([HAVE_PYUNICODE_ASUTF8],1,
+ [Found PyUnicode_AsUTF8 in libpython.])],
+ [],[$PYTHON_BLDLIBRARY])
LIBS="$old_LIBS"
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 83a32ea..02f0d4c 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -103,13 +103,58 @@ callback_defined (const char *name, PyObject **obj_rtn)
return 1;
}
+/* Convert bytes/str/unicode into a string. Caller must free. */
+static char *
+python_to_string (PyObject *str)
+{
+ char *r;
+
+ if (str) {
+#ifdef HAVE_PYUNICODE_ASUTF8
+ if (PyUnicode_Check (str)) {
+ r = PyUnicode_AsUTF8 (str);
+ r = strdup (r);
+ return r;
+ }
+ else
+#endif
+#ifdef HAVE_PYSTRING_ASSTRING
+ if (PyString_Check (str)) {
+ r = PyString_AsString (str); /* Internal pointer in PyObject. */
+ r = strdup (r);
+ return r;
+ }
+ else
+#endif
+ if (PyBytes_Check (str)) {
+ r = PyBytes_AS_STRING (str);
+ r = strdup (r);
+ return r;
+ }
+ }
+ return NULL;
+}
+
static int
check_python_failure (const char *callback)
{
if (PyErr_Occurred ()) {
- nbdkit_error ("%s: callback failed: %s", script, callback);
- /* How to turn this into a string? XXX */
- PyErr_Print ();
+ PyObject *type, *error, *traceback, *error_str;
+ char *error_cstr;
+
+ /* Convert the Python exception to a string.
+ * https://stackoverflow.com/a/1418703
+ * But forget about the traceback, it's very hard to print.
+ * https://stackoverflow.com/q/1796510
+ */
+ PyErr_Fetch (&type, &error, &traceback);
+ PyErr_NormalizeException (&type, &error, &traceback);
+ error_str = PyObject_Str (error);
+ error_cstr = python_to_string (error_str);
+ nbdkit_error ("%s: %s: error: %s",
+ script, callback, error_cstr ? :
"<unknown>");
+ Py_DECREF (error_str);
+ free (error_cstr);
return -1;
}
return 0;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 56a20f6..6cb6344 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -383,6 +383,7 @@ if HAVE_PYTHON
check_PROGRAMS += test-python
TESTS += \
test-python \
+ test-python-exception.sh \
test-shebang-python.sh
test_python_SOURCES = test-lang-plugins.c test.h
diff --git a/tests/python-exception.py b/tests/python-exception.py
new file mode 100644
index 0000000..93531cc
--- /dev/null
+++ b/tests/python-exception.py
@@ -0,0 +1,45 @@
+# nbdkit
+# Copyright (C) 2018 Red Hat Inc.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# A dummy python plugin which just raises an exception in config_complete.
+
+def config_complete():
+ raise RuntimeError("this is the test string")
+
+def open(readonly):
+ return 1
+
+def get_size(h):
+ return 0
+
+def pread(h, count, offset):
+ return ""
diff --git a/tests/test-python-exception.sh b/tests/test-python-exception.sh
new file mode 100755
index 0000000..83999af
--- /dev/null
+++ b/tests/test-python-exception.sh
@@ -0,0 +1,42 @@
+#!/bin/bash -
+# nbdkit
+# Copyright (C) 2018 Red Hat Inc.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+output=test-python-exception.out
+
+rm -f $output
+
+nbdkit -f -v python ./python-exception.py > $output 2>&1 ||:
+
+grep 'this is the test string' $output
+
+rm $output
--
2.16.2
Daniel P. Berrangé
2018-Apr-05 13:05 UTC
Re: [Libguestfs] [PATCH nbdkit] python: Turn python exceptions into nbdkit errors
On Thu, Apr 05, 2018 at 01:47:30PM +0100, Richard W.M. Jones wrote:> Much more annoying that it needs to be, but I have tested it and it > works on Python 2 & 3. Note this will not work on Python 3.0 - 3.2, > but I guess we don't care about those versions.What's the problem that impacts 3.0 -> 3.2 ? I don't particularly care, just curious really. Perhaps mention the problem in the commit message as a hint for anyone who does care Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Eric Blake
2018-Apr-05 13:17 UTC
Re: [Libguestfs] [PATCH nbdkit] python: Turn python exceptions into nbdkit errors properly.
On 04/05/2018 07:47 AM, Richard W.M. Jones wrote:> --- > configure.ac | 8 +++++++ > plugins/python/python.c | 51 +++++++++++++++++++++++++++++++++++++++--- > tests/Makefile.am | 1 + > tests/python-exception.py | 45 +++++++++++++++++++++++++++++++++++++ > tests/test-python-exception.sh | 42 ++++++++++++++++++++++++++++++++++ > 5 files changed, 144 insertions(+), 3 deletions(-) >> +/* Convert bytes/str/unicode into a string. Caller must free. */ > +static char * > +python_to_string (PyObject *str) > +{ > + char *r; > + > + if (str) { > +#ifdef HAVE_PYUNICODE_ASUTF8 > + if (PyUnicode_Check (str)) { > + r = PyUnicode_AsUTF8 (str); > + r = strdup (r); > + return r;Any simpler to just write: return strdup (PyUnicode_AsUTF8 (str)); instead of using the temporary variable r?> static int > check_python_failure (const char *callback) > { > if (PyErr_Occurred ()) { > - nbdkit_error ("%s: callback failed: %s", script, callback); > - /* How to turn this into a string? XXX */ > - PyErr_Print (); > + PyObject *type, *error, *traceback, *error_str; > + char *error_cstr; > + > + /* Convert the Python exception to a string. > + * https://stackoverflow.com/a/1418703 > + * But forget about the traceback, it's very hard to print. > + * https://stackoverflow.com/q/1796510 > + */ > + PyErr_Fetch (&type, &error, &traceback); > + PyErr_NormalizeException (&type, &error, &traceback); > + error_str = PyObject_Str (error); > + error_cstr = python_to_string (error_str); > + nbdkit_error ("%s: %s: error: %s", > + script, callback, error_cstr ? : "<unknown>");Okay, not the first use of that gcc extension. Looks reasonable to me. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2018-Apr-05 13:31 UTC
Re: [Libguestfs] [PATCH nbdkit] python: Turn python exceptions into nbdkit errors
On Thu, Apr 05, 2018 at 02:05:12PM +0100, Daniel P. Berrangé wrote:> On Thu, Apr 05, 2018 at 01:47:30PM +0100, Richard W.M. Jones wrote: > > Much more annoying that it needs to be, but I have tested it and it > > works on Python 2 & 3. Note this will not work on Python 3.0 - 3.2, > > but I guess we don't care about those versions. > > What's the problem that impacts 3.0 -> 3.2 ? I don't particularly > care, just curious really. Perhaps mention the problem in the commit > message as a hint for anyone who does careIt's just that PyUnicode_AsUTF8 was added in 3.3. (https://docs.python.org/3/c-api/unicode.html) We could add a workaround fairly easily, but I don't think we care about this range of old versions now. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v