[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.
--- 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
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
On Thu, Apr 05, 2018 at 08:17:00AM -0500, Eric Blake wrote:
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?
Yes I can change that. The consequence of multiple refactorings while I was trying to work out how to convert PyObject to char * (you'd think that would be easy ...)
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.
I guess I'll change this one too, it's not a case where we actually want to avoid a side effect. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
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 :|
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 care
It'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
participants (3)
-
Daniel P. Berrangé -
Eric Blake -
Richard W.M. Jones