Richard W.M. Jones
2020-Aug-10 17:38 UTC
[Libguestfs] [PATCH nbdkit] python: Allow extents to return any iterable (which includes lists).
Thanks: Nir Soffer.
Enhances: commit c12e3cb150259f0058727a50341a2d14bb0015a3
---
plugins/python/nbdkit-python-plugin.pod | 3 +-
plugins/python/python.c | 39 +++++++++++++++----------
2 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/plugins/python/nbdkit-python-plugin.pod
b/plugins/python/nbdkit-python-plugin.pod
index d7b6033f..ddae677e 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -377,7 +377,8 @@ optionally using C<nbdkit.set_error> first.
(Optional)
def extents(h, count, offset, flags):
- # return a list of (offset, length, type) tuples:
+ # return an iterable object (eg. list) of
+ # (offset, length, type) tuples:
return [ (off1, len1, type1), (off2, len2, type2), ... ]
=back
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 46b912e2..27c5ede2 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -1035,7 +1035,8 @@ py_extents (void *handle, uint32_t count, uint64_t offset,
struct handle *h = handle;
PyObject *fn;
PyObject *r;
- Py_ssize_t i, size;
+ PyObject *iter, *t;
+ size_t size;
if (callback_defined ("extents", &fn)) {
PyErr_Clear ();
@@ -1045,29 +1046,25 @@ py_extents (void *handle, uint32_t count, uint64_t
offset,
if (check_python_failure ("extents") == -1)
return -1;
- /* We expect a list of extents to be returned. Each extent is a
- * tuple (offset, length, type). The list must not be empty.
- */
- if (!PyList_Check (r)) {
- nbdkit_error ("extents method did not return a list");
- Py_DECREF (r);
- return -1;
- }
- size = PyList_Size (r);
- if (size < 1) {
- nbdkit_error ("extents method cannot return an empty list");
+ iter = PyObject_GetIter (r);
+ if (iter == NULL) {
+ nbdkit_error ("extents method did not return "
+ "something which is iterable");
Py_DECREF (r);
return -1;
}
- for (i = 0; i < size; ++i) {
- PyObject *t, *py_offset, *py_length, *py_type;
+ size = 0;
+ while ((t = PyIter_Next (iter)) != NULL) {
+ PyObject *py_offset, *py_length, *py_type;
uint64_t extent_offset, extent_length;
uint32_t extent_type;
- t = PyList_GetItem (r, i);
+ size++;
+
if (!PyTuple_Check (t) || PyTuple_Size (t) != 3) {
- nbdkit_error ("extents method did not return a list of
3-tuples");
+ nbdkit_error ("extents method did not return an iterable of
3-tuples");
+ Py_DECREF (iter);
Py_DECREF (r);
return -1;
}
@@ -1078,16 +1075,26 @@ py_extents (void *handle, uint32_t count, uint64_t
offset,
extent_length = PyLong_AsUnsignedLongLong (py_length);
extent_type = PyLong_AsUnsignedLong (py_type);
if (check_python_failure ("PyLong") == -1) {
+ Py_DECREF (iter);
Py_DECREF (r);
return -1;
}
if (nbdkit_add_extent (extents,
extent_offset, extent_length, extent_type) == -1)
{
+ Py_DECREF (iter);
Py_DECREF (r);
return -1;
}
}
+ if (size < 1) {
+ nbdkit_error ("extents method cannot return an empty list");
+ Py_DECREF (iter);
+ Py_DECREF (r);
+ return -1;
+ }
+
+ Py_DECREF (iter);
Py_DECREF (r);
}
else {
--
2.27.0
Eric Blake
2020-Aug-10 18:32 UTC
Re: [Libguestfs] [PATCH nbdkit] python: Allow extents to return any iterable (which includes lists).
On 8/10/20 12:38 PM, Richard W.M. Jones wrote:> Thanks: Nir Soffer. > Enhances: commit c12e3cb150259f0058727a50341a2d14bb0015a3 > --- > plugins/python/nbdkit-python-plugin.pod | 3 +- > plugins/python/python.c | 39 +++++++++++++++---------- > 2 files changed, 25 insertions(+), 17 deletions(-) >> > - for (i = 0; i < size; ++i) { > - PyObject *t, *py_offset, *py_length, *py_type; > + size = 0; > + while ((t = PyIter_Next (iter)) != NULL) { > + PyObject *py_offset, *py_length, *py_type; > uint64_t extent_offset, extent_length; > uint32_t extent_type; > > - t = PyList_GetItem (r, i); > + size++; > + > if (!PyTuple_Check (t) || PyTuple_Size (t) != 3) { > - nbdkit_error ("extents method did not return a list of 3-tuples"); > + nbdkit_error ("extents method did not return an iterable of 3-tuples"); > + Py_DECREF (iter); > Py_DECREF (r); > return -1; > } > @@ -1078,16 +1075,26 @@ py_extents (void *handle, uint32_t count, uint64_t offset, > extent_length = PyLong_AsUnsignedLongLong (py_length); > extent_type = PyLong_AsUnsignedLong (py_type); > if (check_python_failure ("PyLong") == -1) { > + Py_DECREF (iter); > Py_DECREF (r); > return -1; > } > if (nbdkit_add_extent (extents, > extent_offset, extent_length, extent_type) == -1) {Do we want to check that the user is making progress? nbdkit_add_extent() ignores 0-length extents, but an iterable that could generate an infinite number of them would never progress. Similarly, we could track extent_offset, and if it exceeds offset+length, we could quit iterating early, rather than consuming ever more CPUs on an interable that is generating code at random (nbdkit_add_extent already ignores any further extents beyond a maximum reply length, but that does not stop the caller from the wasted CPU cycles). But those seem like optimizations, and what you have seems correct even if we don't try to optimize.> + Py_DECREF (iter); > Py_DECREF (r); > return -1; > } > } > > + if (size < 1) { > + nbdkit_error ("extents method cannot return an empty list");s/list/iterable/ ?> + Py_DECREF (iter); > + Py_DECREF (r); > + return -1; > + } > + > + Py_DECREF (iter); > Py_DECREF (r); > } > else { >-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2020-Aug-10 18:34 UTC
Re: [Libguestfs] [PATCH nbdkit] python: Allow extents to return any iterable (which includes lists).
I pushed this one because it works for me and passes the tests, and I really need to get this into RHEL 8.3 ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/