search for: myfilter_extents

...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 underlying plugin it > +can simply pass this through to the layer below: > + > + myfilter_extents (...) > + { > + return next_ops->extents (nxdata, count, offset, flags, extents_map, err); > + } > + > +It is also possible for filters to transform the extents map received > +back from the layer below. Usually this must be done by allocating a > +new map which is passed...

...and the text should be clarified. However I'm having a hard time thinking of a case where this would be useful. > > If the > > +filter does not need to adjust extents from the underlying plugin it > > +can simply pass this through to the layer below: > > + > > + myfilter_extents (...) > > + { > > + return next_ops->extents (nxdata, count, offset, flags, extents_map, err); > > + } > > + > > +It is also possible for filters to transform the extents map received > > +back from the layer below. Usually this must be done by allocating a...

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.

...xt_ops-E<gt>extents> if +C<next_ops-E<gt>can_extents> did not return true. + +The C<extents_map> parameter passed to this function is empty. If the +filter does not need to adjust extents from the underlying plugin it +can simply pass this through to the layer below: + + myfilter_extents (...) + { + return next_ops->extents (nxdata, count, offset, flags, extents_map, err); + } + +It is also possible for filters to transform the extents map received +back from the layer below. Usually this must be done by allocating a +new map which is passed to the layer below. Without error...

On Sat, Mar 23, 2019 at 11:25:06AM -0500, Eric Blake wrote: > On 3/20/19 5:11 PM, Richard W.M. Jones wrote: > > + myfilter_extents (...) > > + { > > + size_t i; > > + struct nbdkit_extents *extents2; > > + struct nbdkit_extent e; > > + > > + extents2 = nbdkit_extents_new (offset); > > + next_ops->extents (nxdata, count, offset, flags, extents2, err); > > + for (i =...

...next_ops-E<gt>can_extents> did not return true. + +The C<extents_map> parameter passed to this function is empty +(representing a fully allocated disk). If the filter does not need to +adjust extents from the underlying plugin it can simply pass this +through to the layer below: + + myfilter_extents (...) + { + return next_ops->extents (nxdata, count, offset, flags, + extents_map, err); + } + +It is also possible for filters to transform the extents map received +back from the layer below. Usually this must be done by allocating a +new, empty map which is pass...

...ts> returned false; in +turn, the filter should not call C<next_ops-E<gt>extents> if +C<next_ops-E<gt>can_extents> did not return true. + +The C<extents> parameter passed to this function is empty, and +should usually be passed straight to the underlying plugin: + + myfilter_extents (...) + { + return next_ops->extents (nxdata, count, offset, flags, + extents, err); + } + +It is also possible for filters to transform the extents list received +back from the layer below. Without error checking it would look like +this: + + myfilter_extents (......

...ts> returned false; in +turn, the filter should not call C<next_ops-E<gt>extents> if +C<next_ops-E<gt>can_extents> did not return true. + +The C<extents> parameter passed to this function is empty, and +should usually be passed straight to the underlying plugin: + + myfilter_extents (...) + { + return next_ops->extents (nxdata, count, offset, flags, + extents, err); + } + +It is also possible for filters to transform the extents list received +back from the layer below. Without error checking it would look like +this: + + myfilter_extents (......

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 -

...escing consecutive add_extents with the same type), but here's some more review: > +++ b/docs/nbdkit-filter.pod > +It is also possible for filters to transform the extents list received > +back from the layer below. Without error checking it would look like > +this: > + > + myfilter_extents (...) > + { > + size_t i; > + struct nbdkit_extents *extents2; > + struct nbdkit_extent e; > + > + extents2 = nbdkit_extents_new (offset); > + next_ops->extents (nxdata, count, offset, flags, extents2, err); > + for (i = 0; i < nbdkit_extents_size (extents2...

...next_ops-E<gt>can_extents> did not return true. + +The C<extents_map> parameter passed to this function is empty +(representing a fully allocated disk). If the filter does not need to +adjust extents from the underlying plugin it can simply pass this +through to the layer below: + + myfilter_extents (...) + { + return next_ops->extents (nxdata, count, offset, flags, + extents_map, err); + } + +It is also possible for filters to transform the extents map received +back from the layer below. Usually this must be done by allocating a +new, empty map which is pass...

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

...ed false; in +turn, the filter should not call C<next_ops-E<gt>extents> if +C<next_ops-E<gt>can_extents> did not return true. + +It is possible for filters to transform the extents list received back +from the layer below. Without error checking it would look like this: + + myfilter_extents (..., uint32_t count, uint64_t offset, ...) + { + size_t i; + struct nbdkit_extents *extents2; + struct nbdkit_extent e; + int64_t size; + + size = next_ops->get_size (nxdata); + extents2 = nbdkit_extents_new (offset + shift, size - shift); + next_ops->extents (nxdata, count, of...

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

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

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.