[Libguestfs] [PATCH nbdkit] python: Implement can_extents + extents.

[Eric Blake]

On 8/10/20 5:50 AM, Richard W.M. Jones wrote:
> ---
>   plugins/python/nbdkit-python-plugin.pod | 19 ++++++-
>   plugins/python/python.c                 | 75 +++++++++++++++++++++++++
>   tests/test-python-plugin.py             |  8 ++-
>   tests/test_python.py                    | 73 +++++++++++++++++++++++-
>   4 files changed, 169 insertions(+), 6 deletions(-)
> 
> diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod
> index b20f51f7..d7b6033f 100644
> --- a/plugins/python/nbdkit-python-plugin.pod
> +++ b/plugins/python/nbdkit-python-plugin.pod
> @@ -271,6 +271,13 @@ contents will be garbage collected.
>      # return nbdkit.CACHE_NONE or nbdkit.CACHE_EMULATE
>      # or nbdkit.CACHE_NATIVE

We expose various constants through 'import nbdkit', so...

> +++ b/plugins/python/python.c
> @@ -1020,6 +1020,79 @@ py_can_cache (void *handle)
>       return NBDKIT_CACHE_NONE;
>   }
>   
> +static int
> +py_can_extents (void *handle)
> +{
> +  ACQUIRE_PYTHON_GIL_FOR_CURRENT_SCOPE;
> +  return boolean_callback (handle, "can_extents", "extents");
> +}
> +
> +static int
> +py_extents (void *handle, uint32_t count, uint64_t offset,
> +            uint32_t flags, struct nbdkit_extents *extents)
> +{
> +  ACQUIRE_PYTHON_GIL_FOR_CURRENT_SCOPE;
> +  struct handle *h = handle;
> +  PyObject *fn;
> +  PyObject *r;
> +  Py_ssize_t i, size;
> +
> +  if (callback_defined ("extents", &fn)) {
> +    PyErr_Clear ();
> +
> +    r = PyObject_CallFunction (fn, "OiLI", h->py_h, count, offset, flags);
> +    Py_DECREF (fn);
> +    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).
> +     */
> +    if (!PyList_Check (r)) {
> +      nbdkit_error ("extents method did not return a list");
> +      Py_DECREF (r);
> +      return -1;
> +    }
> +
> +    size = PyList_Size (r);
> +    for (i = 0; i < size; ++i) {
> +      PyObject *t, *py_offset, *py_length, *py_type;
> +      uint64_t extent_offset, extent_length;
> +      uint32_t extent_type;
> +
> +      t = PyList_GetItem (r, i);
> +      if (!PyTuple_Check (t) || PyTuple_Size (t) != 3) {
> +        nbdkit_error ("extents method did not return a list of 3-tuples");
> +        Py_DECREF (r);
> +        return -1;
> +      }
> +      py_offset = PyTuple_GetItem (t, 0);
> +      py_length = PyTuple_GetItem (t, 1);
> +      py_type = PyTuple_GetItem (t, 2);
> +      extent_offset = PyLong_AsUnsignedLongLong (py_offset);
> +      extent_length = PyLong_AsUnsignedLongLong (py_length);
> +      extent_type = PyLong_AsUnsignedLong (py_type);
> +      if (check_python_failure ("PyLong") == -1) {

Is it really right to be doing error checking only once, but after three 
calls into PyLong_* functions?  I'm thinking of someone returning 
('a',1,1).  But as long as the later successful calls don't wipe out an 
earlier failure, this looks like it works.

> +        Py_DECREF (r);
> +        return -1;
> +      }
> +      if (nbdkit_add_extent (extents,
> +                             extent_offset, extent_length, extent_type) == -1) {
> +        Py_DECREF (r);
> +        return -1;
> +      }
> +    }
> +
> +    Py_DECREF (r);
> +  }
> +  else {
> +    nbdkit_error ("%s not implemented", "extents");
> +    return -1;

Here, we may be better off doing the same fallback as the C code, and 
just reporting the entire image as a single data extent, rather than 
always failing.

> +++ b/tests/test_python.py

> @@ -220,3 +232,60 @@ class Test (unittest.TestCase):
>           """Test cache."""
>           self.connect ({"size": 512, "can_cache": "native"})
>           self.h.cache (512, 0)
> +
> +    # We don't have access to the magic constants defined in the
> +    # nbdkit module, so redefine them here.
> +    EXTENT_HOLE = 1
> +    EXTENT_ZERO = 2

...these constants should have been available through 'import nbdkit'.

Otherwise this looks reasonable.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org