search for: extents_map

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.

...PNOTSUPP> (while plugins have automatic fallback to C<.pwrite>, filters do not). +=head2 C<.extents> + + int (*extents) (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offset, uint32_t flags, + struct nbdkit_extents_map *extents_map, + int *err); + +This intercepts the plugin C<.extents> method and can be used to +modify extent requests. + +This function will not be called if C<.can_extents> returned false; in +turn, the filter should not call C<next_ops-E<gt>extents> if +C&...

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 - removed the nbdkit_extents_clear function; the name is confusing and it doesn't seem like it would be useful - lots of doc changes and clarifications Rich.

...PNOTSUPP> (while plugins have automatic fallback to C<.pwrite>, filters do not). +=head2 C<.extents> + + int (*extents) (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offset, uint32_t flags, + struct nbdkit_extents_map *extents_map, + int *err); + +This intercepts the plugin C<.extents> method and can be used to +modify extent requests. + +This function will not be called if C<.can_extents> returned false; in +turn, the filter should not call C<next_ops-E<gt>extents> if +C&...

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 the filter must in turn pass an empty map if it calls next_ops? > If the > +filter does not need to adjust extents from the underlyin...

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

...: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. > > True only if any earlier filter also passed in an empty map. Maybe it's > worth explicitly mentioning that the filter must in turn pass an empty > map if it calls next_ops? This is true and the text should be clarified. However...

...@@ -132,6 +133,47 @@ offset_zero (struct nbdkit_next_ops *next_ops, void *nxdata, return next_ops->zero (nxdata, count, offs + offset, flags, err); } +/* Extents. */ +static int +subtract_offset (uint64_t offs, uint64_t length, uint32_t type, + void *vp) +{ + struct nbdkit_extents_map *extents_map = vp; + + assert (offs >= offset); + offs -= offset; + return nbdkit_extent_add (extents_map, offs, length, type); +} + +static int +offset_extents (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, +...

...#39; is either the data to be written or the - * data to be returned, and points to a buffer of size 'count' bytes. + * check them again. + * + * 'buf' is either the data to be written or the data to be returned, + * and points to a buffer of size 'count' bytes. + * + * 'extents_map' is an empty extents map used for block status + * requests only. * * In all cases, the return value is the system errno value that will * later be converted to the nbd error to send back to the client (0 @@ -173,7 +205,7 @@ validate_request (struct connection *conn, static uint32_t han...

...PNOTSUPP> (while plugins have automatic fallback to C<.pwrite>, filters do not). +=head2 C<.extents> + + int (*extents) (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offset, uint32_t flags, + struct nbdkit_extents_map *extents_map, + int *err); + +This intercepts the plugin C<.extents> method and can be used to +modify extent requests. + +This function will not be called if C<.can_extents> returned false; in +turn, the filter should not call C<next_ops-E<gt>extents> if +C&...

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 have the extents_list be a simple list. The first extent you add must start at offset.

...he full flexibility of random-access extent probing rather than linear is over-engineered). I like the idea. I'm trying to figure out if filters would still need a foreach visitor. But my initial guess is no. Consider - the earlier proposal required the offset filter to do: + struct nbdkit_extents_map *map2; + + map2 = nbdkit_extents_new (); + if (map2 == NULL) + return -1; + if (next_ops->extents (nxdata, count, offs + offset, + flags, map2, err) == -1) { + nbdkit_extents_free (map2); + return -1; + } + + /* Transform offsets in map2, return result in ex...

...gt; probing rather than linear is over-engineered). > > I like the idea. > > I'm trying to figure out if filters would still need a foreach visitor. > But my initial guess is no. Consider - the earlier proposal required > the offset filter to do: > > + struct nbdkit_extents_map *map2; > + > + map2 = nbdkit_extents_new (); > + if (map2 == NULL) > + return -1; > + if (next_ops->extents (nxdata, count, offs + offset, > + flags, map2, err) == -1) { > + nbdkit_extents_free (map2); > + return -1; > + } > + &g...

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 > + > +One function is provided to filters only to iterate over the extents > +map: > + > + typedef int (*nbdkit_extents_callb...

...t 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. > > Is this like free() where it is safe to call on NULL? Yes - otherwise CLEANUP_EXTENTS_FREE wouldn't work. Do we need to document this? > > +=item C<NBDKIT_EXTENTS_FOREACH_FLAG_RANGE> > > + > > +If th...