similar to: [nbdkit PATCH] extents: Do not shorten overlaps by 0

On 3/28/19 11:18 AM, Richard W.M. Jones wrote: > This new pair of callbacks allows plugins to describe which extents in > the virtual disk are allocated, holes or zeroes. > --- > +++ b/server/extents.c > @@ -0,0 +1,210 @@ > +/* nbdkit > + * Copyright (C) 2019 Red Hat Inc. > + * All rights reserved. We started work on this phrase but it got waylaid by extents work; maybe

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

A devious plugin can cause an 8-fold increase in the reply size in relation to the request size, by alternating status every byte of the request. In the worst case, this can cause the client to reject our response and kill the connection because the response is too large. What's more, it consumes a lot of memory to track that many extents. Let's put an upper bound on the maximum number of

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.

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

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

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:

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/26/19 4:17 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. > --- > +=head2 C<.extents> > + > + int extents (void *handle, uint32_t count, uint64_t offset, > + uint32_t flags, struct nbdkit_extents *extents); > + > +During the data serving phase, this

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/28/19 11:18 AM, Richard W.M. Jones wrote: > These plugins are both based on the same sparse array structure which > supports a simple implementation of extents. > --- > common/sparse/sparse.h | 7 +- > common/sparse/sparse.c | 37 ++++++++++- > plugins/data/data.c | 16 ++++- > plugins/memory/memory.c | 16 ++++- > README |

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`

On 3/20/19 5:11 PM, Richard W.M. Jones wrote: > These plugins are both based on the same sparse array structure which > supports a simple implementation of extents. > --- > +int > +sparse_array_extents (struct sparse_array *sa, > + uint32_t count, uint64_t offset, > + struct nbdkit_extents *extents) > +{ > + uint32_t n, type;

On Tue, Jul 07, 2020 at 05:22:46PM -0500, Eric Blake wrote: [...] > +/* Compute aligned extents on behalf of a filter. */ > +int > +nbdkit_extents_aligned (struct nbdkit_next_ops *next_ops, > + nbdkit_backend *nxdata, > + uint32_t count, uint64_t offset, > + uint32_t flags, uint32_t align, > +

--- server/extents.c | 49 ++++++++++++++++++------------------------------ 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/server/extents.c b/server/extents.c index 2d609652..4ab5946c 100644 --- a/server/extents.c +++ b/server/extents.c @@ -42,6 +42,7 @@ #include <assert.h> #include "minmax.h" +#include "vector.h" #include "internal.h"

On 3/24/20 3:12 PM, Eric Blake wrote: >> (For non-mingw platforms) this breaks the source API promises rather >> seriously, so if I understand your proposal correctly I don't think >> this is a good idea.  It's possibly something we can consider for >> internal plugins, or for the V3 API. > > How does it break API to request that someone link against a

On 3/20/19 5:11 PM, Richard W.M. Jones wrote: > This uses a new API VixDiskLib_QueryAllocatedBlocks provided in > VDDK >= 6.7. > > Thanks: Martin Kletzander. > --- > plugins/vddk/vddk-structs.h | 15 +++- > plugins/vddk/vddk.c | 138 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 152 insertions(+), 1 deletion(-) > > +static int >

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

This was a rather longer trip around the houses than I anticipated! The basic purpose of the patch series is to set $nbdkit_stdio_safe to "0" or "1" in sh and eval plugin scripts. To do that, I ended up adding a nicer way to manipulate environ lists, and to do that, I ended up adding a whole generic vector implementation which is applicable in a lot of different places.

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.