similar to: [PATCH nbdkit] server: Initialize ‘buf’ to avoid GCC 9.0.1 warning.

If the plugin .pread method did not fill the buffer with data then the contents of the heap could be leaked back to the client. To avoid this create a thread-local data buffer which is initialized to zero and expanded (with zeroes) as required. This buffer is shared between pread and pwrite which makes the code a little bit simpler. Also this may improve locality by reusing the same memory for

On 4/23/19 10:09 AM, Richard W.M. Jones wrote: > If the plugin .pread method did not fill the buffer with data then the > contents of the heap could be leaked back to the client. To avoid > this create a thread-local data buffer which is initialized to zero > and expanded (with zeroes) as required. > > This buffer is shared between pread and pwrite which makes the code a >

Version 1 was here: https://www.redhat.com/archives/libguestfs/2019-April/msg00144.html Version 2 makes a couple of much larger changes: The OCaml patch changes the API of the pread method so it matches what other language bindings are already doing, ie. get the language plugin to return a newly allocated buffer, check it is long enough, copy out the data. The server patch implements a

This commit implements the core NBD protocol for the "base:allocation" Block Status replies. --- server/internal.h | 7 ++ server/protocol.h | 15 +++ server/protocol-handshake-newstyle.c | 81 ++++++++++++++- server/protocol.c | 141 ++++++++++++++++++++++++--- 4 files changed, 229 insertions(+), 15 deletions(-) diff --git

This bug was found by Eric Blake. In the .pread method we allocate a buffer in the server and pass it to the plugin. The plugin is supposed to fill it with data. The buffer was uninitialized so initially contained random heap data, but that's OK provided the plugin fully overwrote it with data. All correctly written plugins ought to do this, however there is the possibility of an

This commit implements the core NBD protocol for the "base:allocation" Block Status replies. --- server/internal.h | 7 + server/protocol.h | 17 +- server/protocol-handshake-newstyle.c | 79 ++++++++- server/protocol.c | 248 +++++++++++++++++++++++++-- 4 files changed, 335 insertions(+), 16 deletions(-) diff --git

I'm not sure exactly which version we're up to, but let's say it's version 4. I'm a lot happier with this version: - all filters have been reviewed and changed where I think that's necessary - can_extents is properly defined and implemented now - NBD protocol is followed - I believe it addresses all previous review points where possible The "only" thing

Not sure what version we're up to, but this reimplements extents using the new simpler structure described in this thread: https://www.redhat.com/archives/libguestfs/2019-March/msg00077.html I also fixed most of the things that Eric pointed out in the previous review, although I need to go back over his replies and check I've got everything. This needs a bit more testing. However the

Now that cleanup.h is in common code, we can use it in our filters. The first round focuses just on places that called nbdkit_extents_free(), as all three callers had multiple exit paths that definitely benefit from the macro. Signed-off-by: Eric Blake <eblake@redhat.com> --- filters/offset/offset.c | 13 +++++-------- filters/partition/partition.c | 12 ++++--------

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

I want to post this but mainly for discussion and early review. It's not safe for these patches to all go upstream yet (because not all filters have been checked/adjusted), but if any patches were to go upstream then probably 1 & 2 only are safe. File, VDDK, memory and data plugins all work, although I have only done minimal testing on them. The current tests, such as they are, all

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.

This has already been pushed upstream. I am simply posting these here so we have a reference in the mailing list in case we find bugs later (as I'm sure we will - it's a complex patch series). Great thanks to Eric Blake for tireless review on this one. It also seems to have identified a few minor bugs in qemu along the way. Rich.

Since commit 86fdb48c6a5362d66865493d9d2172166f99722e we have stored the connection object in thread-local storage. In this very large, but mostly mechanical change we stop passing the connection pointer around everywhere, and instead use the value stored in thread-local storage. This assumes a 1-1 mapping between the connection and the current thread which is true in *most* places.

The upstream protocol recently promoted NBD_CMD_WRITE_ZEROES from experimental to a documented extension. Exposing support for this allows plugin writers to create sparse files when driven by a client that knows how to use the extension; meanwhile, even if a plugin does not support this extension, the server benefits from less network traffic from the client. Eric Blake (5): protocol: Support

When querying the extents of the underlying plugin, we should only translate the starting offset, and let the plugin report for at least as many bytes as our range permits. Otherwise, short-changing the range causes bad behavior such as returning 0 extents, or even failing the creation of an extents tracker: $ cat script case "$1" in get_size) echo 1m;; can_extents) ;;

Tested via: term1$ qemu-nbd -k $PWD/sock -t -f raw -x foo junk --trace=nbd_\* term2$ ./nbdkit -f -v -e bar nbd socket=$PWD/sock export=foo term3$ qemu-io -t none -f raw nbd://localhost:10809/bar --trace=nbd_\* and checking the traces to see that 'w 0 1' vs. 'w -f 0 1' was able to influence whether the FUA flag showed up at the server in term1. Still to go: figure out how to

We have several filters that would benefit from reporting extents to the client that are always aligned to boundaries chosen by the filter, regardless of whether the plugin reports extents at a finer granularity. Add a new helper function to make the work easier, without having to duplicate code in each filter. Any alignment block that straddles more than one plugin extent is reported as a

It's easy to use the sh script to demonstrate that nbdkit is inefficiently calling into .get_size, .can_fua, and friends more than necessary. We've also commented on the list in the past that it would be nice to ensure that when filters call into next_ops, they are not violating constraints (as we've have to fix several bugs in the past where we did not have such checking to protect