similar to: New extents structure proposal

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

Second version based on nbdkit_extent* calls, as discussed here: https://www.redhat.com/archives/libguestfs/2019-March/msg00033.html Note in particular there is some subtlety about how filters would work in this implementation. See docs/nbdkit-filter.pod for the details. Rich.

I'm not sure which version we're up to now. Anyway I believe this addresses all the points that Eric raised in: https://www.redhat.com/archives/libguestfs/2019-March/msg00038.html https://www.redhat.com/archives/libguestfs/2019-March/msg00040.html In particular: - default state of extents_map is all allocated disk - support hole + non-zero - you can now iterate with bounds -

On 3/12/19 12: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. > --- > +++ b/docs/nbdkit-filter.pod > +The C<extents_map> parameter passed to this function is empty. True only if any earlier filter also passed in an empty map. Maybe it's worth explicitly mentioning that

On Wed, Mar 20, 2019 at 11:20:15AM -0500, Eric Blake wrote: > 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

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

On 3/19/19 11:34 AM, Richard W.M. Jones wrote: > This pair of calls allows plugins to describe which extents in the > virtual disk are allocated, holes or zeroes. > --- > + void nbdkit_extents_free (struct nbdkit_extents_map *); > + > +Frees an existing extents map. Is this like free() where it is safe to call on NULL? > + > +=head3 Iterating over nbdkit_extents_map

Allows you to safely use nbdkit-offset-filter on top of a plugin supporting extents. --- filters/offset/offset.c | 43 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/filters/offset/offset.c b/filters/offset/offset.c index 058571d..4e3f74d 100644 --- a/filters/offset/offset.c +++ b/filters/offset/offset.c @@ -37,6 +37,7 @@ #include <stdlib.h>

This pair of calls allows plugins to describe which extents in the virtual disk are allocated, holes or zeroes. --- docs/nbdkit-filter.pod | 64 +++++++++++++++++++++ docs/nbdkit-plugin.pod | 123 ++++++++++++++++++++++++++++++++++++++++ include/nbdkit-common.h | 20 ++++++- include/nbdkit-filter.h | 15 ++++- include/nbdkit-plugin.h | 6 +- server/internal.h | 7 ++-

On Tue, Mar 12, 2019 at 01:58:52PM -0500, Eric Blake wrote: > On 3/12/19 12: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. > > --- > > > +++ b/docs/nbdkit-filter.pod > > > +The C<extents_map> parameter passed to this function is empty. > >

This pair of calls allows plugins to describe which extents in the virtual disk are allocated, holes or zeroes. --- docs/nbdkit-filter.pod | 45 +++++++++++++++++ docs/nbdkit-plugin.pod | 106 ++++++++++++++++++++++++++++++++++++++++ include/nbdkit-common.h | 18 ++++++- include/nbdkit-filter.h | 12 ++++- include/nbdkit-plugin.h | 6 ++- server/internal.h | 7 ++- server/extents.c

This pair of calls allows plugins to describe which extents in the virtual disk are allocated, holes or zeroes. --- docs/nbdkit-filter.pod | 107 ++++++++ docs/nbdkit-plugin.pod | 103 +++++++ include/nbdkit-common.h | 10 +- include/nbdkit-filter.h | 25 +- include/nbdkit-plugin.h | 6 +- server/internal.h | 5 + server/extents.c | 574

On Tue, Mar 12, 2019 at 07:51:57AM -0500, Eric Blake wrote: > On 3/12/19 6:20 AM, Richard W.M. Jones wrote: > > This tentative commit implements extents/can_extents, roughly as > > discussed in the previous thread here: > > > > https://www.redhat.com/archives/libguestfs/2019-March/msg00017.html > > > > I can't say that I'm a big fan of having 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

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

This tentative commit implements extents/can_extents, roughly as discussed in the previous thread here: https://www.redhat.com/archives/libguestfs/2019-March/msg00017.html I can't say that I'm a big fan of having the plugin allocate an extents array. There are no other plugin callbacks currently where we require the plugin to allocate complex data structures (or indeed do any allocation

On Wed, Mar 20, 2019 at 12:35:33AM -0500, Eric Blake wrote: > On 3/19/19 11:34 AM, Richard W.M. Jones wrote: > > This pair of calls allows plugins to describe which extents in the > > virtual disk are allocated, holes or zeroes. > > --- > > > > + void nbdkit_extents_free (struct nbdkit_extents_map *); > > + > > +Frees an existing extents map. >

This fix isn't exhaustive but it fixes some obvious problems in the filters. 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.