similar to: [nbdkit PATCH] blocksize: Fix .extents when plugin changes type within minblock

--- filters/blocksize/nbdkit-blocksize-filter.pod | 4 ++-- filters/blocksize/blocksize.c | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/filters/blocksize/nbdkit-blocksize-filter.pod b/filters/blocksize/nbdkit-blocksize-filter.pod index b6cf4dd..0abed2f 100644 --- a/filters/blocksize/nbdkit-blocksize-filter.pod +++

The upstream NBD protocol recently clarified that servers can advertise block size limitations to clients that ask with NBD_OPT_GO (although we're still a ways off from implementing that in nbdkit); and that in the absence of that, then clients should agree on limits using out-of-band information or stick to sane defaults (everything 512-byte-aligned, no reads or writes larger than 32M). But

The upstream NBD protocol recently clarified that servers can advertise block size limitations to clients that ask with NBD_OPT_GO (although we're still a ways off from implementing that in nbdkit); and that in the absence of that, then clients should agree on limits using out-of-band information or stick to sane defaults (everything 512-byte-aligned, no reads or writes larger than 32M). But

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(+) diff --git a/filters/offset/offset.c b/filters/offset/offset.c index 058571d..35791bf 100644 --- a/filters/offset/offset.c +++ b/filters/offset/offset.c @@ -37,6 +37,7 @@ #include <stdlib.h> #include

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(+) diff --git a/filters/offset/offset.c b/filters/offset/offset.c index 058571d..f949f6d 100644 --- a/filters/offset/offset.c +++ b/filters/offset/offset.c @@ -37,6 +37,7 @@ #include <stdlib.h> #include

--- filters/partition/partition.c | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/filters/partition/partition.c b/filters/partition/partition.c index c1fe7a1..ace9f4b 100644 --- a/filters/partition/partition.c +++ b/filters/partition/partition.c @@ -38,6 +38,7 @@ #include <stdint.h> #include <string.h> #include <inttypes.h> +#include

--- filters/partition/partition.c | 37 +++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/filters/partition/partition.c b/filters/partition/partition.c index c1fe7a1..bc60480 100644 --- a/filters/partition/partition.c +++ b/filters/partition/partition.c @@ -38,6 +38,7 @@ #include <stdint.h> #include <string.h> #include <inttypes.h> +#include

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 Thu, Mar 28, 2019 at 04:18:38PM +0000, Richard W.M. Jones wrote: >--- > filters/partition/partition.c | 37 +++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > >diff --git a/filters/partition/partition.c b/filters/partition/partition.c >index c1fe7a1..bc60480 100644 >--- a/filters/partition/partition.c >+++ b/filters/partition/partition.c >@@ -38,6

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 =

I examined each filter to see which ones implement a cache and do not properly consider consistency across clients for flush requests. For these filters we should force .can_multi_conn to return false. I believe only one filter (blocksize) needs to be updated and all the other ones are safe. --- filters/blocksize/blocksize.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git

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 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, > +

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

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:

The truncate filter tried to be careful to lock access to setting or reading the real_size variable learned from calling next_ops->get_size, in anticipation of not behaving incorrectly if the NBD protocol makes dynamic resize supported, and where the global variable could serve as a cache rather than having to always call next_ops->get_size to recompute things. However, nbdkit-plugin.pod

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 ++++--------

On 1/5/19 8:50 AM, Richard W.M. Jones wrote: > I examined each filter to see which ones implement a cache and do not > properly consider consistency across clients for flush requests. For > these filters we should force .can_multi_conn to return false. > > I believe only one filter (blocksize) needs to be updated and all the > other ones are safe. > --- >

On 3/20/19 10:31 AM, Richard W.M. Jones wrote: > I think the extents map is just too complicated and is unnecessarily > so. How about instead we define the plugin interface to be: > > int can_extents (void *handle); // as before > int extents (void *handle, uint32_t count, uint64_t offset, uint32_t flags, > struct nbdkit_extents_list *list); > > and

Copies somewhat from the file plugin, with the difference that we always provide an .extents even if one or more of the split files does not support SEEK_HOLE. Testing is possible on a file system that supports sparse files when using nbdsh. Signed-off-by: Eric Blake <eblake@redhat.com> --- I'm going ahead and committing this, but am also posting this for review in case we later find