similar to: [nbdkit PATCH 0/4] Start using cleanup macros in filters/plugins

I do have a question about whether patch 2 is right, or whether I've exposed a bigger problem in the truncate (and possibly other) filter, but the rest seem fairly straightforward. Eric Blake (4): server: Check for pthread lock failures truncate: Factor out reading real_size under mutex plugins: Check for mutex failures filters: Check for mutex failures filters/cache/cache.c

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

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

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.

While working on adding assertions to pthread_mutex_lock calls, I noticed that the truncate filter's use of mutex didn't really protect us, and isn't really necessary. Cleaning that up also spotted a couple of other potential cleanups. Eric Blake (4): filters: Drop useless .open callbacks truncate: Fix corruption when plugin changes per-connection size truncate: Test for safe

Although we haven't always checked that pthread_mutex_[un]lock succeeded, it never hurts to avoid blatant programming bugs such as when EINVAL can detect use of an uninitialized mutex. Signed-off-by: Eric Blake <eblake@redhat.com> --- Applies on top of my series to move cleanup.c to common/ Should I also go through and add checking to other bare pthread_mutex_[un]lock() calls?

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) ;;

Ultimately, both the blocksize and swab filters want to return aligned extents to the client. I'm posting this as a snapshot of my work in progress on how I plan to get there (it's not quite working yet, but I'm done for today and wanted to at least document my ideas). I might also add a convenience function for nbdkit_extents_offset, since we have a number of filters that repeat the

On 3/28/19 11:18 AM, Richard W.M. Jones wrote: > --- > filters/truncate/truncate.c | 55 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > + extents2 = nbdkit_extents_new (0, real_size_copy); > + if (offset + count <= real_size_copy) > + n = count; > + else > + n = real_size_copy - offset; > + if (next_ops->extents

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

On 4/23/19 2:06 PM, Eric Blake wrote: > 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

For review only, this needs some clean up and more tests. My eyes are going cross-eyed looking at the calculate_offset_of_entry function, so time to take a break ... Rich.

A suggested readahead filter. I've only lightly tested this, but it seems to work fine with qemu-img convert. The commit needs proper tests. Rich.

Includes a rework of the previously posted patch for --run improvements (mostly with improved comments and commit message; I decided that waiting for the captive nbdkit to exit was overkill), and four new patches. The tests are intentionally separate, to allow rearranging the order of the series to see the failures being fixed. Eric Blake (6): server: Propagate unexpected nbdkit failure with

Simpler, and including tests. Rich.

This fix isn't exhaustive but it fixes some obvious problems in the filters. Rich.

For existing commits, this is almost identical to v1, except that I updated some commit messages and reordered the commits in a somewhat more logical sequence. The main changes are the extra commits: [06/11] plugins: Return NBD_FLAG_CAN_MULTI_CONN from some readonly plugins. - Readonly plugins that can set the flag unconditionally. [09/11] partitioning: Return NBD_FLAG_CAN_MULTI_CONN. [10/11]

I have dropped the map filter from this series for now while I try to get it working. However I think the truncate filter is in a good shape. This incorporates all feedback from Eric's review. Also there are three small fixes to the filter code, all revealed when I was testing using multiple filters which we'd not done much of before. Rich.

Based on Rich's review of my v1 that touched only cache.c, I have now tried to bring all three filters with alignment rounding in line with one another. There is definitely room for future improvements once we teach nbdkit to let filters and plugins advertise block sizes, but I'm hoping to get NBD_CMD_CACHE implemented first. Eric Blake (2): blocksize: Process requests in linear order

This patch series enhances the cache filter in a few ways, primarily adding a "cache-on-read" feature (similar to qemu's copyonread); and adding the ability to limit the cache size and the antecedent of that which is having a method to reclaim cache blocks. As the cache is stored as a sparse temporary file, reclaiming cache blocks simply means punching holes in the temporary file.