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 --- plugins/python/example.py | 8 ++++--- plugins/python/nbdkit-python-plugin.pod | 16 +++++-------- plugins/python/python.c | 32 +++++++++++++++---------- tests/python-exception.py | 4 ++-- tests/shebang.py | 5 ++-- 5 files changed, 36 insertions(+), 29 deletions(-) diff --git a/plugins/python/example.py b/plugins/python/example.py index c85d2f8..c04b7e2 100644 --- a/plugins/python/example.py +++ b/plugins/python/example.py @@ -60,10 +60,12 @@ def get_size(h): return len(disk) -def pread(h, count, offset, flags): +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, flags): global disk diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index 0b31ebc..4065ec7 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -38,7 +38,7 @@ C<__main__> module): # see below def get_size(h): # see below - def pread(h, count, offset, flags): + def pread(h, buf, offset, flags): # see below Note that the subroutines must have those literal names (like C<open>), @@ -249,16 +249,12 @@ contents will be garbage collected. (Required) - def pread(h, count, offset, flags): - # 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<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>. C<flags> is always 0. - -The returned buffer can be any type compatible with the Python 3 -buffer protocol, such as bytearray, bytes or memoryview -(L<https://docs.python.org/3/c-api/buffer.html>) +The body of your C<pread> function should read exactly C<len(buf)> +bytes of data starting at disk C<offset> and write it into the buffer +C<buf>. 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 diff --git a/plugins/python/python.c b/plugins/python/python.c index 252ca37..79766df 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -527,7 +527,9 @@ py_pread (void *handle, void *buf, uint32_t count, uint64_t offset, r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL); break; case 2: - r = PyObject_CallFunction (fn, "OiLI", obj, count, offset, flags, NULL); + r = PyObject_CallFunction (fn, "ONLI", obj, + PyMemoryView_FromMemory ((char *)buf, count, PyBUF_WRITE), + offset, flags, NULL); break; default: abort (); } @@ -535,19 +537,25 @@ py_pread (void *handle, void *buf, uint32_t count, uint64_t offset, 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: diff --git a/tests/python-exception.py b/tests/python-exception.py index d0c79bb..ee4a3f3 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 6f33623..0634589 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] -- 2.23.0

--- tests/test-python-plugin.py | 12 +++--------- tests/test_python.py | 18 ++---------------- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/tests/test-python-plugin.py b/tests/test-python-plugin.py index d7335bd..8e90bc2 100644 --- a/tests/test-python-plugin.py +++ b/tests/test-python-plugin.py @@ -95,16 +95,10 @@ def can_cache (h): elif cache == "native": return nbdkit.CACHE_NATIVE -def pread (h, count, offset, flags): +def pread (h, buf, offset, flags): assert flags == 0 - pread_result = cfg.get ('pread_result', "bytearray") - b = h['disk'][offset:offset+count] - if pread_result == "bytearray": - return b - elif pread_result == "bytes": - return bytes (b) - elif pread_result == "memoryview": - return memoryview (b) + end = offset + len(buf) + buf[:] = h['disk'][offset:end] def pwrite (h, buf, offset, flags): expect_fua = cfg.get ('pwrite_expect_fua', False) diff --git a/tests/test_python.py b/tests/test_python.py index b2c60f0..6b9f297 100755 --- a/tests/test_python.py +++ b/tests/test_python.py @@ -155,26 +155,12 @@ class Test (unittest.TestCase): # Not yet implemented: can_extents. - def test_pread_bytearray (self): - """Test pread returning bytearray.""" + def test_pread (self): + """Test pread.""" self.connect ({"size": 512}) buf = self.h.pread (512, 0) assert buf == bytearray (512) - def test_pread_bytes (self): - """Test pread returning bytes.""" - self.connect ({"size": 512, - "pread_result": "bytes"}) - buf = self.h.pread (512, 0) - assert buf == bytearray (512) - - def test_pread_memoryview (self): - """Test pread returning memoryview.""" - self.connect ({"size": 512, - "pread_result": "memoryview"}) - buf = self.h.pread (512, 0) - assert buf == bytearray (512) - # Test pwrite + flags. def test_pwrite (self): self.connect ({"size": 512}) -- 2.23.0