Fairly straightforward, as the code in plugins.c already takes care of necessary .pwrite fallbacks. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- plugins/python/nbdkit-python-plugin.pod | 8 +++++++- plugins/python/python.c | 7 +++++++ tests/test.py | 4 ++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index f504cf7a..ec126f5a 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -208,6 +208,13 @@ contents will be garbage collected. def can_trim(h): # return a boolean +=item C<can_zero> + +(Optional) + + def can_zero(h): + # return a boolean + =item C<pread> (Required) @@ -317,7 +324,6 @@ C<config_help>, C<magic_config_key>, C<can_fua>, C<can_cache>, -C<can_zero>, C<can_fast_zero>, C<can_extents>, C<can_multi_conn>, diff --git a/plugins/python/python.c b/plugins/python/python.c index d18bcb5e..aa09dce7 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -718,6 +718,12 @@ py_can_trim (void *handle) return boolean_callback (handle, "can_trim", "trim"); } +static int +py_can_zero (void *handle) +{ + return boolean_callback (handle, "can_zero", "zero"); +} + #define py_config_help \ "script=<FILENAME> (required) The Python plugin to run.\n" \ "[other arguments may be used by the plugin that you load]" @@ -744,6 +750,7 @@ static struct nbdkit_plugin plugin = { .can_write = py_can_write, .can_flush = py_can_flush, .can_trim = py_can_trim, + .can_zero = py_can_zero, .pread = py_pread, .pwrite = py_pwrite, diff --git a/tests/test.py b/tests/test.py index a83f1181..1b7d3e0e 100644 --- a/tests/test.py +++ b/tests/test.py @@ -32,6 +32,10 @@ def can_trim(h): return True +def can_zero(h): + return True + + def pread(h, count, offset): global disk return disk[offset:offset+count] -- 2.21.0

Expose support for the FUA flag to pwrite, zero, and trim, as well as a can_fua callback, for use in python plugins. There are some slight semantic differences: the C plugin had to switch to a new API with a single uint32_t flags argument (so we don't have to keep adding new ABI when new flags are added), but in so doing, there is no way to probe whether a C plugin supports FUA flags, so the C .can_fua has to return a tri-state. But for python plugins, thanks to the fact that python functions have named and optional parameters, it's a lot cleaner to provide new flags as a bool option apiece, instead of requiring the python program to parse out bits from an int, and easy to introspect if FUA support is present, so can_fua only has to return a bool on whether to advertise FUA support to the client. One other thing that makes for some interesting C glue code: if at least one of the three callbacks supports FUA, then the C can_fua function claims native support, but in so doing, the python plugin must gracefully handle the fallback to flush for the remaining callbacks that did not support FUA, since nbdkit plugins.c won't do it. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- plugins/python/nbdkit-python-plugin.pod | 46 +++++++- plugins/python/python.c | 148 +++++++++++++++++++++--- plugins/python/example.py | 6 +- 3 files changed, 179 insertions(+), 21 deletions(-) diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index ec126f5a..6940f4d9 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -215,6 +215,22 @@ contents will be garbage collected. def can_zero(h): # return a boolean +=item C<can_fua> + +(Optional) + + def can_fua(h): + # return a boolean + +Unlike the C counterpart, the Python callback does not need a +tri-state return value, because Python introspection is sufficient to +learn whether callbacks support FUA. Thus, this function only returns +a bool, which merely controls whether Forced Unit Access (FUA) support +is advertised to the client on a per-connection basis. If omitted or +this returns true, FUA support is advertised as native if any of the +C<pwrite>, C<zero>, or C<trim> callbacks support an optional parameter +C<fua>; or as emulated if there is a C<flush> callback. + =item C<pread> (Required) @@ -239,7 +255,7 @@ C<nbdkit.set_error> first. (Optional) - def pwrite(h, buf, offset): + def pwrite(h, buf, offset, fua=False): length = len (buf) # no return value @@ -247,6 +263,12 @@ The body of your C<pwrite> function should write the buffer C<buf> to the disk. You should write C<count> bytes to the disk starting at C<offset>. +Your function may support an optional C<fua> parameter; if it is +present and the caller sets it to True, then your callback must not +return success until the write has landed in persistent storage. If +it is absent but C<can_fua> returned True, then nbdkit emulates a +client request for FUA by calling C<flush>. + 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, @@ -262,6 +284,11 @@ fails or is partial, your function should throw an exception, The body of your C<flush> function should do a L<sync(2)> or L<fdatasync(2)> or equivalent on the backing store. +This function will not be called directly by the client if +C<can_flush> returned false; however, it may still be called by nbdkit +if C<can_fua> returned True but C<pwrite>, C<zero>, or C<trim> lacked +a C<fua> parameter. + If the flush fails, your function should throw an exception, optionally using C<nbdkit.set_error> first. @@ -269,18 +296,24 @@ using C<nbdkit.set_error> first. (Optional) - def trim(h, count, offset): + def trim(h, count, offset, fua=False): # 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. +Your function may support an optional C<fua> parameter; if it is +present and the caller sets it to True, then your callback must not +return success until the trim has landed in persistent storage. If it +is absent but C<can_fua> returned True, then nbdkit emulates a client +request for FUA by calling C<flush>. + =item C<zero> (Optional) - def zero(h, count, offset, may_trim=False): + def zero(h, count, offset, may_trim=False, fua=False): # no return value The body of your C<zero> function should ensure that C<count> bytes @@ -292,6 +325,12 @@ optimize the operation by using a trim, as long as subsequent reads see zeroes. If you omit the optional parameter, or if the caller sets it to False, writing zeroes should not punch holes. +Your function may support an optional C<fua> parameter; if it is +present and the caller sets it to True, then your callback must not +return success until the write has landed in persistent storage. If +it is absent but C<can_fua> returned True, then nbdkit emulates a +client request for FUA by calling C<flush>. + 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, @@ -322,7 +361,6 @@ C<longname>, C<description>, C<config_help>, C<magic_config_key>, -C<can_fua>, C<can_cache>, C<can_fast_zero>, C<can_extents>, diff --git a/plugins/python/python.c b/plugins/python/python.c index 1954881c..677420bd 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -64,6 +64,9 @@ static const char *script; static PyObject *module; static int last_error; +static int pwrite_has_fua; +static int zero_has_fua; +static int trim_has_fua; static PyObject * set_error (PyObject *self, PyObject *args) @@ -368,6 +371,32 @@ py_config (const char *key, const char *value) "nbdkit requires these callbacks.", script); return -1; } + + /* One-time setup to learn which callbacks have a fua parameter */ + if (callback_defined ("pwrite", &fn)) { + pwrite_has_fua = callback_has_parameter (fn, "fua"); + Py_DECREF (fn); + if (pwrite_has_fua < 0) { + check_python_failure ("config"); + return -1; + } + } + if (callback_defined ("zero", &fn)) { + zero_has_fua = callback_has_parameter (fn, "fua"); + Py_DECREF (fn); + if (zero_has_fua < 0) { + check_python_failure ("config"); + return -1; + } + } + if (callback_defined ("trim", &fn)) { + trim_has_fua = callback_has_parameter (fn, "fua"); + Py_DECREF (fn); + if (trim_has_fua < 0) { + check_python_failure ("config"); + return -1; + } + } } else if (callback_defined ("config", &fn)) { /* Other parameters are passed to the Python .config callback. */ @@ -524,22 +553,52 @@ out: return ret; } +static int py_flush (void *handle, uint32_t flags); + static int py_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, uint32_t flags) { PyObject *obj = handle; PyObject *fn; + PyObject *args; + PyObject *kwargs; PyObject *r; + int fua = (flags & NBDKIT_FLAG_FUA) != 0; + int need_flush = fua && !pwrite_has_fua; - assert (!flags); + assert (!(flags & ~NBDKIT_FLAG_FUA)); if (callback_defined ("pwrite", &fn)) { PyErr_Clear (); - r = PyObject_CallFunction (fn, "ONL", obj, + args = Py_BuildValue ("ONL", obj, PyMemoryView_FromMemory ((char *)buf, count, PyBUF_READ), offset); + if (!args) { + check_python_failure ("pwrite"); + Py_DECREF (fn); + return -1; + } + kwargs = PyDict_New (); + if (!kwargs) { + check_python_failure ("pwrite"); + Py_DECREF (args); + Py_DECREF (fn); + return -1; + } + if (pwrite_has_fua && + PyDict_SetItemString (kwargs, "fua", + fua ? Py_True : Py_False) == -1) { + check_python_failure ("pwrite"); + Py_DECREF (kwargs); + Py_DECREF (args); + Py_DECREF (fn); + return -1; + } + r = PyObject_Call (fn, args, kwargs); Py_DECREF (fn); + Py_DECREF (args); + Py_DECREF (kwargs); if (check_python_failure ("pwrite") == -1) return -1; Py_DECREF (r); @@ -549,7 +608,7 @@ py_pwrite (void *handle, const void *buf, return -1; } - return 0; + return need_flush ? py_flush (handle, 0) : 0; } static int @@ -582,14 +641,42 @@ py_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags) { PyObject *obj = handle; PyObject *fn; + PyObject *args; + PyObject *kwargs; PyObject *r; + int fua = (flags & NBDKIT_FLAG_FUA) != 0; + int need_flush = fua && !trim_has_fua; - assert (!flags); + assert (!(flags & ~NBDKIT_FLAG_FUA)); if (callback_defined ("trim", &fn)) { PyErr_Clear (); - r = PyObject_CallFunction (fn, "OiL", obj, count, offset); + args = Py_BuildValue ("OiL", obj, count, offset); + if (!args) { + check_python_failure ("trim"); + Py_DECREF (fn); + return -1; + } + kwargs = PyDict_New (); + if (!kwargs) { + check_python_failure ("trim"); + Py_DECREF (args); + Py_DECREF (fn); + return -1; + } + if (trim_has_fua && + PyDict_SetItemString (kwargs, "fua", + fua ? Py_True : Py_False) == -1) { + check_python_failure ("trim"); + Py_DECREF (kwargs); + Py_DECREF (args); + Py_DECREF (fn); + return -1; + } + r = PyObject_Call (fn, args, kwargs); Py_DECREF (fn); + Py_DECREF (args); + Py_DECREF (kwargs); if (check_python_failure ("trim") == -1) return -1; Py_DECREF (r); @@ -599,7 +686,7 @@ py_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags) return -1; } - return 0; + return need_flush ? py_flush (handle, 0) : 0; } static int @@ -611,8 +698,10 @@ py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) PyObject *kwargs; PyObject *r; int may_trim = (flags & NBDKIT_FLAG_MAY_TRIM) != 0; + int fua = (flags & NBDKIT_FLAG_FUA) != 0; + int need_flush = fua && !zero_has_fua; - assert (!(flags & ~NBDKIT_FLAG_MAY_TRIM)); + assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA))); if (callback_defined ("zero", &fn)) { static int zero_may_trim = -1; @@ -648,6 +737,15 @@ py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) Py_DECREF (fn); return -1; } + if (zero_has_fua && + PyDict_SetItemString (kwargs, "fua", + fua ? Py_True : Py_False) == -1) { + check_python_failure ("zero"); + Py_DECREF (kwargs); + Py_DECREF (args); + Py_DECREF (fn); + return -1; + } r = PyObject_Call (fn, args, kwargs); Py_DECREF (fn); Py_DECREF (args); @@ -665,7 +763,7 @@ py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) if (check_python_failure ("zero") == -1) return -1; Py_DECREF (r); - return 0; + return need_flush ? py_flush (handle, 0) : 0; } nbdkit_debug ("zero missing, falling back to pwrite"); @@ -674,7 +772,8 @@ py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) } static int -boolean_callback (void *handle, const char *can_fn, const char *plain_fn) +boolean_callback (void *handle, const char *can_fn, const char *plain_fn, + int missing) { PyObject *obj = handle; PyObject *fn; @@ -699,37 +798,55 @@ boolean_callback (void *handle, const char *can_fn, const char *plain_fn) else if (plain_fn && callback_defined (plain_fn, NULL)) return 1; else - return 0; + return missing; } static int py_is_rotational (void *handle) { - return boolean_callback (handle, "is_rotational", NULL); + return boolean_callback (handle, "is_rotational", NULL, 0); } static int py_can_write (void *handle) { - return boolean_callback (handle, "can_write", "pwrite"); + return boolean_callback (handle, "can_write", "pwrite", 0); } static int py_can_flush (void *handle) { - return boolean_callback (handle, "can_flush", "flush"); + return boolean_callback (handle, "can_flush", "flush", 0); } static int py_can_trim (void *handle) { - return boolean_callback (handle, "can_trim", "trim"); + return boolean_callback (handle, "can_trim", "trim", 0); } static int py_can_zero (void *handle) { - return boolean_callback (handle, "can_zero", "zero"); + return boolean_callback (handle, "can_zero", "zero", 0); +} + +static int +py_can_fua (void *handle) +{ + /* We have to convert a python bool into the nbdkit tristate. */ + switch (boolean_callback (handle, "can_fua", NULL, 1)) { + case -1: + return -1; + case 0: + return NBDKIT_FUA_NONE; + default: + if (pwrite_has_fua || zero_has_fua || trim_has_fua) + return NBDKIT_FUA_NATIVE; + if (callback_defined ("flush", NULL)) + return NBDKIT_FUA_EMULATE; + return NBDKIT_FUA_NONE; + } } #define py_config_help \ @@ -759,6 +876,7 @@ static struct nbdkit_plugin plugin = { .can_flush = py_can_flush, .can_trim = py_can_trim, .can_zero = py_can_zero, + .can_fua = py_can_fua, .pread = py_pread, .pwrite = py_pwrite, diff --git a/plugins/python/example.py b/plugins/python/example.py index 9205f107..ca7c2661 100644 --- a/plugins/python/example.py +++ b/plugins/python/example.py @@ -59,13 +59,15 @@ def pread(h, count, offset): return disk[offset:offset+count] -def pwrite(h, buf, offset): +# Since everything is stored in memory, flush is a no-op; this example +# does not provide a flush(), but honors the fua flag by doing nothing. +def pwrite(h, buf, offset, fua=False): global disk end = offset + len(buf) disk[offset:end] = buf -def zero(h, count, offset, may_trim=False): +def zero(h, count, offset, may_trim=False, fua=False): global disk if may_trim: disk[offset:offset+count] = bytearray(count) -- 2.21.0