similar to: [PATCH nbdkit 2/2] filters: Be careful to set *err if nbdkit_add_extent or nbdkit_extents_new fail.

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.

There's more that can be done (in particular, use of CLEANUP_FREE), but this is enough to at least see if I'm on the right track. I couldn't figure out an obvious difference between common/include and common/utils, but it looks like the former is for things that are inlineable via .h only, while the latter is when you need to link in a convenience library, so this landed in the

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

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

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

Thanks: Eric Blake for reporting the bug. --- filters/offset/offset.c | 4 +++- filters/partition/partition.c | 4 +++- filters/truncate/truncate.c | 8 +++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/filters/offset/offset.c b/filters/offset/offset.c index 24ccb4c..633a1c7 100644 --- a/filters/offset/offset.c +++ b/filters/offset/offset.c @@ -156,8 +156,10 @@

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

On 3/28/19 11:18 AM, Richard W.M. Jones wrote: > Allows you to safely use nbdkit-offset-filter on top of a plugin > supporting extents. > --- > filters/offset/offset.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > + > + extents2 = nbdkit_extents_new (offs + offset, real_size - offset); > + if (extents2 == NULL) { > + *err =

On 4/24/19 6:04 AM, Richard W.M. Jones wrote: > Thanks: Eric Blake for reporting the bug. > --- > filters/offset/offset.c | 4 +++- > filters/partition/partition.c | 4 +++- > filters/truncate/truncate.c | 8 +++++++- > 3 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/filters/offset/offset.c b/filters/offset/offset.c > index 24ccb4c..633a1c7

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

This filter caches the last result of the extents() call and offers a nice speed-up for clients that only support req_on=1 in combination with plugins like vddk, which has no overhead for returning information for multiple extents in one call, but that call is very time-consuming. Quick test showed that on a fast connection and a sparsely allocated 16G disk with a OS installed `qemu-img map` runs

On 3/20/19 5:11 PM, Richard W.M. Jones wrote: > This pair of calls allows plugins to describe which extents in the > virtual disk are allocated, holes or zeroes. > --- I see you've already made a couple of tweaks to your block-status branch since posting this (skipping 0-legnth add_extent, and coalescing consecutive add_extents with the same type), but here's some more review:

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

This filter caches the last result of the extents() call and offers a nice speed-up for clients that only support req_one=1 in combination with plugins like vddk, which has no overhead for returning information for multiple extents in one call, but that call is very time-consuming. Quick test showed that on a fast connection and a sparsely allocated 16G disk with a OS installed `qemu-img map`

I pushed a few of the msys2 patches upstream. I changed the way that $(SHARED_LDFLAGS) works so it's more to my liking, and the others were pushed unchanged. Three patches remain which I'm posting on the mailing list for proper review. Rich.

On 3/20/19 5:11 PM, Richard W.M. Jones wrote: > Allows you to safely use nbdkit-offset-filter on top of a plugin > supporting extents. > --- > filters/offset/offset.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) If we don't change our minds on the interface in patch 1, then this looks correct. > +/* Extents. */ > +static int >

On 3/28/19 11:18 AM, Richard W.M. Jones wrote: > --- > filters/truncate/truncate.c | 55 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > + > + /* If the entire request is beyond the end of the underlying plugin > + * then this is the easy case: return a hole. > + */ > + if (offset >= real_size_copy) > + return

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

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.