To avoid breaking existing plugins, Python plugins wishing to use version 2 of the API must opt in by declaring: def api_version(): return 2 (Plugins which do not do this are assumed to want API version 1). --- plugins/python/example.py | 15 +++- plugins/python/nbdkit-python-plugin.pod | 61 ++++++++++----- plugins/python/python.c | 100 ++++++++++++++++++++---- tests/test.py | 20 +++-- 4 files changed, 148 insertions(+), 48 deletions(-) diff --git a/plugins/python/example.py b/plugins/python/example.py index 60f9d7f..25a0049 100644 --- a/plugins/python/example.py +++ b/plugins/python/example.py @@ -34,6 +34,13 @@ 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. +def api_version(): + return 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 +61,20 @@ def get_size(h): return len(disk) -def pread(h, count, offset): +def pread(h, count, offset, flags): global disk return disk[offset:offset+count] -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 6453474..882c0d8 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -82,6 +82,19 @@ 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 function in your +module: + + def api_version(): + return 2 + +(where 2 is the latest version at the time this documentation was +written). All newly written Python modules must have this function. + =head2 Executable script If you want you can make the script executable and include a "shebang" @@ -120,6 +133,10 @@ nbdkit-plugin(3). =over 4 +=item C<api_version> + +There are no arguments. It must return C<2>. + =item C<dump_plugin> (Optional) @@ -199,12 +216,12 @@ contents will be garbage collected. (Required) - def pread(h, count, offset): + def pread(h, count, offset, flags): # construct a bytearray of length count bytes and return it The body of your C<pread> function should construct a buffer of length (at least) C<count> bytes. You should read C<count> bytes from the -disk starting at C<offset>. +disk starting at C<offset>. C<flags> 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 @@ -215,13 +232,13 @@ C<nbdkit.set_error> first. (Optional) - def pwrite(h, buf, offset): + def pwrite(h, buf, offset, flags): length = len (buf) # no return value The body of your C<pwrite> function should write the C<buf> string to the disk. You should write C<count> bytes to the disk starting at -C<offset>. +C<offset>. C<flags> may contain C<nbdkit.FLAG_FUA>. NBD only supports whole writes, so your function should try to write the whole region (perhaps requiring a loop). If the write @@ -232,11 +249,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<flush> function should do a L<sync(2)> or L<fdatasync(2)> or equivalent on the backing store. +C<flags> is always 0. If the flush fails, your function should throw an exception, optionally using C<nbdkit.set_error> first. @@ -245,32 +263,35 @@ using C<nbdkit.set_error> first. (Optional) - def trim(h, count, offset): + def trim(h, count, offset, flags): # no return value -The body of your C<trim> function should "punch a hole" in the -backing store. If the trim fails, your function should throw an -exception, optionally using C<nbdkit.set_error> first. +The body of your C<trim> function should "punch a hole" in the backing +store. C<flags> may contain C<nbdkit.FLAG_FUA>. If the trim fails, +your function should throw an exception, optionally using +C<nbdkit.set_error> first. =item C<zero> (Optional) - def zero(h, count, offset, may_trim): + def zero(h, count, offset, flags): # no return value -The body of your C<zero> function should ensure that C<count> bytes -of the disk, starting at C<offset>, will read back as zero. If -C<may_trim> is true, the operation may be optimized as a trim as long -as subsequent reads see zeroes. +The body of your C<zero> function should ensure that C<count> bytes of +the disk, starting at C<offset>, will read back as zero. C<flags> is +a bitmask which may include C<nbdkit.FLAG_MAY_TRIM>, +C<nbdkit.FLAG_FUA>, C<nbdkit.FLAG_FAST_ZERO>. 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<nbdkit.set_error> first. In particular, if -you would like to automatically fall back to C<pwrite> (perhaps -because there is nothing to optimize if C<may_trim> is false), -use C<nbdkit.set_error(errno.EOPNOTSUPP)>. +write the whole region (perhaps requiring a loop). + +If the write fails or is partial, your function should throw an +exception, optionally using C<nbdkit.set_error> first. In particular, +if you would like to automatically fall back to C<pwrite> (perhaps +because there is nothing to optimize if +S<C<flags & nbdkit.FLAG_MAY_TRIM>> is false), use +S<C<nbdkit.set_error (errno.EOPNOTSUPP)>>. =back diff --git a/plugins/python/python.c b/plugins/python/python.c index 69cf4e9..20e05e0 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -46,6 +46,8 @@ #define PY_SSIZE_T_CLEAN 1 #include <Python.h> +#define NBDKIT_API_VERSION 2 + #include <nbdkit-plugin.h> #include "cleanup.h" @@ -60,6 +62,7 @@ */ static const char *script; static PyObject *module; +static int py_api_version = 1; static int last_error; @@ -356,6 +359,26 @@ py_config (const char *key, const char *value) "nbdkit requires these callbacks.", script); return -1; } + + /* Get the API version. */ + if (callback_defined ("api_version", &fn)) { + PyErr_Clear (); + + r = PyObject_CallObject (fn, NULL); + Py_DECREF (fn); + if (check_python_failure ("api_version") == -1) + return -1; + py_api_version = (int) PyLong_AsLong (r); + Py_DECREF (r); + if (check_python_failure ("PyLong_AsLong") == -1) + return -1; + if (py_api_version < 1 || py_api_version > NBDKIT_API_VERSION) { + nbdkit_error ("%s: api_version() requested unknown version: %d. " + "This plugin supports API versions between 1 and %d.", + script, py_api_version, NBDKIT_API_VERSION); + return -1; + } + } } else if (callback_defined ("config", &fn)) { /* Other parameters are passed to the Python .config callback. */ @@ -466,8 +489,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; @@ -480,7 +503,15 @@ py_pread (void *handle, void *buf, PyErr_Clear (); - r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL); + switch (py_api_version) { + case 1: + r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL); + break; + case 2: + r = PyObject_CallFunction (fn, "OiLI", obj, count, offset, flags, NULL); + break; + default: abort (); + } Py_DECREF (fn); if (check_python_failure ("pread") == -1) return -1; @@ -505,8 +536,8 @@ py_pread (void *handle, void *buf, } 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; @@ -515,9 +546,19 @@ py_pwrite (void *handle, const void *buf, if (callback_defined ("pwrite", &fn)) { PyErr_Clear (); - r = PyObject_CallFunction (fn, "ONL", obj, - PyByteArray_FromStringAndSize (buf, count), - offset, NULL); + switch (py_api_version) { + case 1: + r = PyObject_CallFunction (fn, "ONL", obj, + PyByteArray_FromStringAndSize (buf, count), + offset, NULL); + break; + case 2: + r = PyObject_CallFunction (fn, "ONLI", obj, + PyByteArray_FromStringAndSize (buf, count), + offset, flags, NULL); + break; + default: abort (); + } Py_DECREF (fn); if (check_python_failure ("pwrite") == -1) return -1; @@ -532,7 +573,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; @@ -541,7 +582,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, NULL); + break; + default: abort (); + } Py_DECREF (fn); if (check_python_failure ("flush") == -1) return -1; @@ -556,7 +605,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; @@ -565,7 +614,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, NULL); + switch (py_api_version) { + case 1: + r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL); + break; + case 2: + r = PyObject_CallFunction (fn, "OiLI", obj, count, offset, flags, NULL); + break; + default: abort (); + } Py_DECREF (fn); if (check_python_failure ("trim") == -1) return -1; @@ -580,7 +637,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; @@ -590,9 +647,20 @@ 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, NULL); + 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, NULL); + break; + } + case 2: + r = PyObject_CallFunction (fn, "OiLI", + obj, count, offset, flags, NULL); + 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/test.py b/tests/test.py index 9a2e947..ac80d96 100644 --- a/tests/test.py +++ b/tests/test.py @@ -3,6 +3,10 @@ import nbdkit disk = bytearray(1024*1024) +def api_version(): + return 2 + + def config_complete(): print ("set_error = %r" % nbdkit.set_error) @@ -32,25 +36,25 @@ def can_trim(h): return True -def pread(h, count, offset): +def pread(h, count, offset, flags): global disk return disk[offset:offset+count] -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): - global disk - disk[offset:offset+count] = bytearray(count) +def flush(h, flags): + pass -def flush(h): +def trim(h, count, offset, flags): pass -def trim(h, count, offset): - pass +def zero(h, count, offset, flags): + global disk + disk[offset:offset+count] = bytearray(count) -- 2.23.0

This change is pure refactoring and should have no effect. --- plugins/python/nbdkit-python-plugin.pod | 12 ++-- plugins/python/python.c | 90 +++++-------------------- 2 files changed, 24 insertions(+), 78 deletions(-) diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index 51e0f57..0fd4dcb 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -184,25 +184,25 @@ contents will be garbage collected. def get_size(h): # return the size of the disk -=item C<can_write> +=item C<is_rotational> (Optional) - def can_write(h): + def is_rotational(h): # return a boolean -=item C<can_flush> +=item C<can_write> (Optional) - def can_flush(h): + def can_write(h): # return a boolean -=item C<is_rotational> +=item C<can_flush> (Optional) - def is_rotational(h): + def can_flush(h): # return a boolean =item C<can_trim> diff --git a/plugins/python/python.c b/plugins/python/python.c index 20e05e0..9445343 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -684,110 +684,56 @@ py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) } static int -py_can_write (void *handle) +boolean_callback (void *handle, const char *can_fn, const char *plain_fn) { PyObject *obj = handle; PyObject *fn; PyObject *r; int ret; - if (callback_defined ("can_write", &fn)) { + if (callback_defined (can_fn, &fn)) { PyErr_Clear (); r = PyObject_CallFunctionObjArgs (fn, obj, NULL); Py_DECREF (fn); - if (check_python_failure ("can_write") == -1) + if (check_python_failure (can_fn) == -1) return -1; ret = r == Py_True; Py_DECREF (r); return ret; } - /* No Python can_write callback, but there's a Python pwrite callback - * defined, so return 1. (In C modules, nbdkit would do this). + /* No Python ‘can_fn’ (eg. ‘can_write’), but if there's a Python + * ‘plain_fn’ (eg. ‘pwrite’) callback defined, return 1. (In C + * modules, nbdkit would do this). */ - else if (callback_defined ("pwrite", NULL)) + else if (plain_fn && callback_defined (plain_fn, NULL)) return 1; else return 0; } static int -py_can_flush (void *handle) +py_is_rotational (void *handle) { - PyObject *obj = handle; - PyObject *fn; - PyObject *r; - int ret; - - if (callback_defined ("can_flush", &fn)) { - PyErr_Clear (); - - r = PyObject_CallFunctionObjArgs (fn, obj, NULL); - Py_DECREF (fn); - if (check_python_failure ("can_flush") == -1) - return -1; - ret = r == Py_True; - Py_DECREF (r); - return ret; - } - /* No Python can_flush callback, but there's a Python flush callback - * defined, so return 1. (In C modules, nbdkit would do this). - */ - else if (callback_defined ("flush", NULL)) - return 1; - else - return 0; + return boolean_callback (handle, "is_rotational", NULL); } static int -py_is_rotational (void *handle) +py_can_write (void *handle) { - PyObject *obj = handle; - PyObject *fn; - PyObject *r; - int ret; - - if (callback_defined ("is_rotational", &fn)) { - PyErr_Clear (); + return boolean_callback (handle, "can_write", "pwrite"); +} - r = PyObject_CallFunctionObjArgs (fn, obj, NULL); - Py_DECREF (fn); - if (check_python_failure ("is_rotational") == -1) - return -1; - ret = r == Py_True; - Py_DECREF (r); - return ret; - } - else - return 0; +static int +py_can_flush (void *handle) +{ + return boolean_callback (handle, "can_flush", "flush"); } static int py_can_trim (void *handle) { - PyObject *obj = handle; - PyObject *fn; - PyObject *r; - int ret; - - if (callback_defined ("can_trim", &fn)) { - PyErr_Clear (); - - r = PyObject_CallFunctionObjArgs (fn, obj, NULL); - Py_DECREF (fn); - if (check_python_failure ("can_trim") == -1) - return -1; - ret = r == Py_True; - Py_DECREF (r); - return ret; - } - /* No Python can_trim callback, but there's a Python trim callback - * defined, so return 1. (In C modules, nbdkit would do this). - */ - else if (callback_defined ("trim", NULL)) - return 1; - else - return 0; + return boolean_callback (handle, "can_trim", "trim"); } #define py_config_help \ @@ -812,9 +758,9 @@ static struct nbdkit_plugin plugin = { .close = py_close, .get_size = py_get_size, + .is_rotational = py_is_rotational, .can_write = py_can_write, .can_flush = py_can_flush, - .is_rotational = py_is_rotational, .can_trim = py_can_trim, .pread = py_pread, -- 2.23.0

On Thu, Nov 21, 2019 at 04:58:12PM +0000, Richard W.M. Jones wrote: This is wrong because can_cache (like can_fua) returns NONE | EMULATE | NATIVE. I'm working on an updated patch series + test which fixes this. 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/

--- plugins/python/nbdkit-python-plugin.pod | 8 +++++++- plugins/python/python.c | 7 +++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index 96a83ef..ca1cc03 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -191,6 +191,13 @@ contents will be garbage collected. def is_rotational(h): # return a boolean +=item C<can_multi_conn> + +(Optional) + + def can_multi_conn(h): + # return a boolean + =item C<can_write> (Optional) @@ -351,7 +358,6 @@ C<config_help>, C<magic_config_key>, C<can_fua>, C<can_extents>, -C<can_multi_conn>, C<extents>. These are not yet supported. diff --git a/plugins/python/python.c b/plugins/python/python.c index 468b1ec..ce9607f 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -751,6 +751,12 @@ py_is_rotational (void *handle) return boolean_callback (handle, "is_rotational", NULL); } +static int +py_can_multi_conn (void *handle) +{ + return boolean_callback (handle, "can_multi_conn", NULL); +} + static int py_can_write (void *handle) { @@ -810,6 +816,7 @@ static struct nbdkit_plugin plugin = { .get_size = py_get_size, .is_rotational = py_is_rotational, + .can_multi_conn = py_can_multi_conn, .can_write = py_can_write, .can_flush = py_can_flush, .can_trim = py_can_trim, -- 2.23.0