From 49ef7e7d7c3602cc8e53d2052fce9d3a12840ea2 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Thu, 21 Nov 2019 15:44:39 +0000 Subject: [PATCH 05/19] python: Implement nbdkit API version 2. To avoid breaking existing plugins, Python plugins wishing to use version 2 of the API must opt in by declaring: API_VERSION = 2 (Plugins which do not do this are assumed to want API version 1). For v2 API, we also avoid a copy by passing a buffer into pread. It's more efficient if we pass the C buffer directly to Python code. In some cases the Python code will be able to write directly into the C buffer using functions like file.readinto and socket.recv_into. This avoids an extra copy. Thanks: Nir Soffer https://www.redhat.com/archives/libguestfs/2019-November/thread.html#00220 (cherry picked from commit a9b2637cf4f00fb8a25ffaf31ee83be5fe019ae2) --- plugins/python/example.py | 20 +++- plugins/python/nbdkit-python-plugin.pod | 69 +++++++----- plugins/python/python.c | 139 +++++++++++++++++++----- tests/python-exception.py | 4 +- tests/shebang.py | 5 +- tests/test.py | 28 +++-- 6 files changed, 190 insertions(+), 75 deletions(-) diff --git a/plugins/python/example.py b/plugins/python/example.py index 60f9d7f0..c04b7e29 100644 --- a/plugins/python/example.py +++ b/plugins/python/example.py @@ -34,6 +34,12 @@ import errno disk = bytearray(1024 * 1024) +# There are several variants of the API. nbdkit will call this +# function first to determine which one you want to use. This is the +# latest version at the time this example was written. +API_VERSION = 2 + + # This just prints the extra command line parameters, but real plugins # should parse them and reject any unknown parameters. def config(key, value): @@ -54,20 +60,22 @@ def get_size(h): return len(disk) -def pread(h, count, offset): +def pread(h, buf, offset, flags): global disk - return disk[offset:offset+count] + end = offset + len(buf) + buf[:] = disk[offset:end] + # or if reading from a file you can use: + #f.readinto(buf) - -def pwrite(h, buf, offset): +def pwrite(h, buf, offset, flags): global disk end = offset + len(buf) disk[offset:end] = buf -def zero(h, count, offset, may_trim): +def zero(h, count, offset, flags): global disk - if may_trim: + if flags & nbdkit.FLAG_MAY_TRIM: disk[offset:offset+count] = bytearray(count) else: nbdkit.set_error(errno.EOPNOTSUPP) diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index 3680fd65..4923d9da 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -33,11 +33,12 @@ To write a Python nbdkit plugin, you create a Python file which contains at least the following required functions (in the top level C<__main__> module): + API_VERSION = 2 def open(readonly): # see below def get_size(h): # see below - def pread(h, count, offset): + def pread(h, buf, offset, flags): # see below Note that the subroutines must have those literal names (like C), @@ -82,6 +83,18 @@ I<--dump-plugin> option, eg: python_version=3.7.0 python_pep_384_abi_version=3 +=head2 API versions + +The nbdkit API has evolved and new versions are released periodically. +To ensure backwards compatibility plugins have to opt in to the new +version. From Python you do this by declaring a constant in your +module: + + API_VERSION = 2 + +(where 2 is the latest version at the time this documentation was +written). All newly written Python modules must have this constant. + =head2 Executable script If you want you can make the script executable and include a "shebang" @@ -199,16 +212,12 @@ contents will be garbage collected. (Required) - def pread(h, count, offset): - # construct a buffer of length count bytes and return it + def pread(h, buf, offset, flags): + # read into the buffer -The body of your C function should construct a buffer of length -(at least) C bytes. You should read C bytes from the -disk starting at C. - -The returned buffer can be any type compatible with the Python 3 -buffer protocol, such as bytearray, bytes or memoryview -(L) +The body of your C function should read exactly C +bytes of data starting at disk C and write it into the buffer +C. C is always 0. NBD only supports whole reads, so your function should try to read the whole region (perhaps requiring a loop). If the read fails or @@ -219,13 +228,13 @@ C first. (Optional) - def pwrite(h, buf, offset): + def pwrite(h, buf, offset, flags): length = len (buf) # no return value The body of your C function should write the buffer C to the disk. You should write C bytes to the disk starting at -C. +C. C may contain C. NBD only supports whole writes, so your function should try to write the whole region (perhaps requiring a loop). If the write @@ -236,11 +245,12 @@ fails or is partial, your function should throw an exception, (Optional) - def flush(h): + def flush(h, flags): # no return value The body of your C function should do a L or L or equivalent on the backing store. +C is always 0. If the flush fails, your function should throw an exception, optionally using C first. @@ -249,32 +259,35 @@ using C first. (Optional) - def trim(h, count, offset): + def trim(h, count, offset, flags): # no return value -The body of your C function should "punch a hole" in the -backing store. If the trim fails, your function should throw an -exception, optionally using C first. +The body of your C function should "punch a hole" in the backing +store. C may contain C. If the trim fails, +your function should throw an exception, optionally using +C first. =item C (Optional) - def zero(h, count, offset, may_trim): + def zero(h, count, offset, flags): # no return value -The body of your C function should ensure that C bytes -of the disk, starting at C, will read back as zero. If -C is true, the operation may be optimized as a trim as long -as subsequent reads see zeroes. +The body of your C function should ensure that C bytes of +the disk, starting at C, will read back as zero. C is +a bitmask which may include C, +C, C. NBD only supports whole writes, so your function should try to -write the whole region (perhaps requiring a loop). If the write -fails or is partial, your function should throw an exception, -optionally using C first. In particular, if -you would like to automatically fall back to C (perhaps -because there is nothing to optimize if C is false), -use C. +write the whole region (perhaps requiring a loop). + +If the write fails or is partial, your function should throw an +exception, optionally using C first. In particular, +if you would like to automatically fall back to C (perhaps +because there is nothing to optimize if +S> is false), use +S>. =back diff --git a/plugins/python/python.c b/plugins/python/python.c index 47da0838..0f28595f 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -46,6 +46,8 @@ #define PY_SSIZE_T_CLEAN 1 #include +#define NBDKIT_API_VERSION 2 + #include #include "cleanup.h" @@ -60,6 +62,7 @@ */ static const char *script; static PyObject *module; +static int py_api_version = 1; static int last_error; @@ -285,9 +288,14 @@ py_dump_plugin (void) PyObject *fn; PyObject *r; + /* Python version and ABI. */ printf ("python_version=%s\n", PY_VERSION); printf ("python_pep_384_abi_version=%d\n", PYTHON_ABI_VERSION); + /* Maximum nbdkit API version supported. */ + printf ("nbdkit_python_maximum_api_version=%d\n", NBDKIT_API_VERSION); + + /* If the script has a dump_plugin function, call it. */ if (script && callback_defined ("dump_plugin", &fn)) { PyErr_Clear (); @@ -297,6 +305,30 @@ py_dump_plugin (void) } } +static int +get_py_api_version (void) +{ + PyObject *obj; + long value; + + obj = PyObject_GetAttrString (module, "API_VERSION"); + if (obj == NULL) + return 1; /* Default to API version 1. */ + + value = PyLong_AsLong (obj); + Py_DECREF (obj); + + if (value < 1 || value > NBDKIT_API_VERSION) { + nbdkit_error ("%s: API_VERSION requested unknown version: %ld. " + "This plugin supports API versions between 1 and %d.", + script, value, NBDKIT_API_VERSION); + return -1; + } + + nbdkit_debug ("module requested API_VERSION %ld", value); + return (int) value; +} + static int py_config (const char *key, const char *value) { @@ -359,6 +391,11 @@ py_config (const char *key, const char *value) "nbdkit requires these callbacks.", script); return -1; } + + /* Get the API version. */ + py_api_version = get_py_api_version (); + if (py_api_version == -1) + return -1; } else if (callback_defined ("config", &fn)) { /* Other parameters are passed to the Python .config callback. */ @@ -469,8 +506,8 @@ py_get_size (void *handle) } static int -py_pread (void *handle, void *buf, - uint32_t count, uint64_t offset) +py_pread (void *handle, void *buf, uint32_t count, uint64_t offset, + uint32_t flags) { PyObject *obj = handle; PyObject *fn; @@ -485,24 +522,40 @@ py_pread (void *handle, void *buf, PyErr_Clear (); - r = PyObject_CallFunction (fn, "OiL", obj, count, offset); + switch (py_api_version) { + case 1: + r = PyObject_CallFunction (fn, "OiL", obj, count, offset); + break; + case 2: + r = PyObject_CallFunction (fn, "ONLI", obj, + PyMemoryView_FromMemory ((char *)buf, count, PyBUF_WRITE), + offset, flags); + break; + default: abort (); + } Py_DECREF (fn); if (check_python_failure ("pread") == -1) return ret; - if (PyObject_GetBuffer (r, &view, PyBUF_SIMPLE) == -1) { - nbdkit_error ("%s: value returned from pread does not support the " - "buffer protocol", - script); - goto out; - } + if (py_api_version == 1) { + /* In API v1 the Python pread function had to return a buffer + * protocol compatible function. In API v2+ it writes directly to + * the C buffer so this code is not used. + */ + if (PyObject_GetBuffer (r, &view, PyBUF_SIMPLE) == -1) { + nbdkit_error ("%s: value returned from pread does not support the " + "buffer protocol", + script); + goto out; + } - if (view.len < count) { - nbdkit_error ("%s: buffer returned from pread is too small", script); - goto out; - } + if (view.len < count) { + nbdkit_error ("%s: buffer returned from pread is too small", script); + goto out; + } - memcpy (buf, view.buf, count); + memcpy (buf, view.buf, count); + } ret = 0; out: @@ -515,8 +568,8 @@ out: } static int -py_pwrite (void *handle, const void *buf, - uint32_t count, uint64_t offset) +py_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, + uint32_t flags) { PyObject *obj = handle; PyObject *fn; @@ -525,9 +578,19 @@ py_pwrite (void *handle, const void *buf, if (callback_defined ("pwrite", &fn)) { PyErr_Clear (); - r = PyObject_CallFunction (fn, "ONL", obj, + switch (py_api_version) { + case 1: + r = PyObject_CallFunction (fn, "ONL", obj, PyMemoryView_FromMemory ((char *)buf, count, PyBUF_READ), offset); + break; + case 2: + r = PyObject_CallFunction (fn, "ONLI", obj, + PyMemoryView_FromMemory ((char *)buf, count, PyBUF_READ), + offset, flags); + break; + default: abort (); + } Py_DECREF (fn); if (check_python_failure ("pwrite") == -1) return -1; @@ -542,7 +605,7 @@ py_pwrite (void *handle, const void *buf, } static int -py_flush (void *handle) +py_flush (void *handle, uint32_t flags) { PyObject *obj = handle; PyObject *fn; @@ -551,7 +614,15 @@ py_flush (void *handle) if (callback_defined ("flush", &fn)) { PyErr_Clear (); - r = PyObject_CallFunctionObjArgs (fn, obj, NULL); + switch (py_api_version) { + case 1: + r = PyObject_CallFunctionObjArgs (fn, obj, NULL); + break; + case 2: + r = PyObject_CallFunction (fn, "OI", obj, flags); + break; + default: abort (); + } Py_DECREF (fn); if (check_python_failure ("flush") == -1) return -1; @@ -566,7 +637,7 @@ py_flush (void *handle) } static int -py_trim (void *handle, uint32_t count, uint64_t offset) +py_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags) { PyObject *obj = handle; PyObject *fn; @@ -575,7 +646,15 @@ py_trim (void *handle, uint32_t count, uint64_t offset) if (callback_defined ("trim", &fn)) { PyErr_Clear (); - r = PyObject_CallFunction (fn, "OiL", obj, count, offset); + switch (py_api_version) { + case 1: + r = PyObject_CallFunction (fn, "OiL", obj, count, offset); + break; + case 2: + r = PyObject_CallFunction (fn, "OiLI", obj, count, offset, flags); + break; + default: abort (); + } Py_DECREF (fn); if (check_python_failure ("trim") == -1) return -1; @@ -590,7 +669,7 @@ py_trim (void *handle, uint32_t count, uint64_t offset) } static int -py_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) +py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) { PyObject *obj = handle; PyObject *fn; @@ -600,9 +679,19 @@ py_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) PyErr_Clear (); last_error = 0; - r = PyObject_CallFunction (fn, "OiLO", - obj, count, offset, - may_trim ? Py_True : Py_False); + switch (py_api_version) { + case 1: { + int may_trim = flags & NBDKIT_FLAG_MAY_TRIM; + r = PyObject_CallFunction (fn, "OiLO", + obj, count, offset, + may_trim ? Py_True : Py_False); + break; + } + case 2: + r = PyObject_CallFunction (fn, "OiLI", obj, count, offset, flags); + break; + default: abort (); + } Py_DECREF (fn); if (last_error == EOPNOTSUPP || last_error == ENOTSUP) { /* When user requests this particular error, we want to diff --git a/tests/python-exception.py b/tests/python-exception.py index d0c79bb0..ee4a3f3a 100644 --- a/tests/python-exception.py +++ b/tests/python-exception.py @@ -62,5 +62,5 @@ def get_size(h): return 0 -def pread(h, count, offset): - return "" +def pread(h, buf, offset): + buf[:] = bytearray(len(buf)) diff --git a/tests/shebang.py b/tests/shebang.py index 6f336230..0634589a 100755 --- a/tests/shebang.py +++ b/tests/shebang.py @@ -13,6 +13,7 @@ def get_size(h): return len(disk) -def pread(h, count, offset): +def pread(h, buf, offset): global disk - return disk[offset:offset+count] + end = offset + len(buf) + buf[:] = disk[offset:end] diff --git a/tests/test.py b/tests/test.py index 9a2e947d..4db56623 100644 --- a/tests/test.py +++ b/tests/test.py @@ -3,6 +3,9 @@ import nbdkit disk = bytearray(1024*1024) +API_VERSION = 2 + + def config_complete(): print ("set_error = %r" % nbdkit.set_error) @@ -32,25 +35,26 @@ def can_trim(h): return True -def pread(h, count, offset): +def pread(h, buf, offset, flags): global disk - return disk[offset:offset+count] + end = offset + len(buf) + buf[:] = disk[offset:end] -def pwrite(h, buf, offset): +def pwrite(h, buf, offset, flags): global disk end = offset + len(buf) disk[offset:end] = buf -def zero(h, count, offset, may_trim=False): +def flush(h, flags): + pass + + +def trim(h, count, offset, flags): + pass + + +def zero(h, count, offset, flags): global disk disk[offset:offset+count] = bytearray(count) - - -def flush(h): - pass - - -def trim(h, count, offset): - pass -- 2.18.2