search for: nbdkit_flag_req_one

...d_reply (h, s) : -1; } /* Read extents of the file. */ @@ -1246,13 +1246,13 @@ nbd_extents (void *handle, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents) { struct handle *h = handle; - int c; + struct transaction *s; assert (!(flags & ~NBDKIT_FLAG_REQ_ONE) && h->extents); - c = nbd_request_full (h, flags & NBDKIT_FLAG_REQ_ONE ? NBD_CMD_FLAG_REQ_ONE : 0, + s = nbd_request_full (h, flags & NBDKIT_FLAG_REQ_ONE ? NBD_CMD_FLAG_REQ_ONE : 0, NBD_CMD_BLOCK_STATUS, offset, count, NULL, NULL,...

...t covered by any extent +information are assumed to be holes, so plugins may if they wish only +return information about allocated extents. + +However no two extents in the returned array may overlap. This is +treated as an error by nbdkit. + +The C<flags> parameter may contain the flag C<NBDKIT_FLAG_REQ_ONE> +which means that the client is only requesting information about the +extent overlapping C<offset>. The plugin may ignore this flag, or as +an optimization it may return just a single entry for C<offset>. + +If there is an error, C<.extents> should call C<nbdkit_error>...

...c int +log_extents (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, + struct nbdkit_extents *extents, int *err) +{ + struct handle *h = handle; + uint64_t id = get_id (h); + int r; + + assert (!(flags & ~(NBDKIT_FLAG_REQ_ONE))); + output (h, "Extents", id, + "offset=0x%" PRIx64 " count=0x%x req_one=%d ...", + offs, count, !!(flags & NBDKIT_FLAG_REQ_ONE)); + r = next_ops->extents (nxdata, count, offs, flags, extents, err); + if (r == -1) + output_return (h, &q...

...+ NBDKIT_THREAD_MODEL_PARALLEL); > + > + PyModule_AddIntConstant (m, "FLAG_MAY_TRIM", NBDKIT_FLAG_MAY_TRIM); > + PyModule_AddIntConstant (m, "FLAG_FUA", NBDKIT_FLAG_FUA); > + PyModule_AddIntConstant (m, "FLAG_REQ_ONE", NBDKIT_FLAG_REQ_ONE); > + PyModule_AddIntConstant (m, "FLAG_FAST_ZERO", NBDKIT_FLAG_FAST_ZERO); > + > + PyModule_AddIntConstant (m, "FUA_NONE", NBDKIT_FUA_NONE); > + PyModule_AddIntConstant (m, "FUA_EMULATE", NBDKIT_FUA_EMULATE); > + PyModule_AddIntConstant (m,...

.....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 <assert.h> > Either: a) you forgot `assert (!(flags & ~(NBDKIT_FLAG_REQ_ONE)));` in this and the previous patch, b) this is a leftover from previous version, or c) I missed something. > #include <nbdkit-filter.h> > >@@ -222,6 +223,41 @@ partition_zero (struct nbdkit_next_ops *next_ops, void *nxdata, > return next_ops->zero (nxdata, count, of...

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

...c int +log_extents (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, + struct nbdkit_extents *extents, int *err) +{ + struct handle *h = handle; + uint64_t id = get_id (h); + int r; + + assert (!(flags & ~(NBDKIT_FLAG_REQ_ONE))); + output (h, "Extents", id, + "offset=0x%" PRIx64 " count=0x%x req_one=%d ...", + offs, count, !!(flags & NBDKIT_FLAG_REQ_ONE)); + r = next_ops->extents (nxdata, count, offs, flags, extents, err); + if (r == -1) + output_return (h, &q...

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.

...xtents overlapping +the range C<[offset...offset+count-1]>. The C<extents_map> parameter +points to an opaque object which the callback should fill in by +calling C<nbdkit_extent_add> etc. See L</Extents map> below. + +The C<flags> parameter may contain the flag C<NBDKIT_FLAG_REQ_ONE> +which means that the client is only requesting information about the +extent overlapping C<offset>. The plugin may ignore this flag, or as +an optimization it may return just a single extent for C<offset>. + +If there is an error, C<.extents> should call C<nbdkit_error&gt...

...cent extents to it. Plugins must return at least one extent, but are not required to return more than one extent (regardless of flags). This would be simple enough to implement for both VDDK and file. (For VDDK the plugin needs to synthesize the holes but that's only a slight complication). NBDKIT_FLAG_REQ_ONE is now simple to implement (if generating the extents, stop after generating the first one; if it's the server processing the extents, take the first extent in the list). The ‘end’ that we discussed before is now implicit (it's the byte of the last extent in the list). Also an observation...

...xtents overlapping +the range C<[offset...offset+count-1]>. The C<extents_map> parameter +points to an opaque object which the callback should fill in by +calling C<nbdkit_extent_add> etc. See L</Extents map> below. + +The C<flags> parameter may contain the flag C<NBDKIT_FLAG_REQ_ONE> +which means that the client is only requesting information about the +extent overlapping C<offset>. The plugin may ignore this flag, or as +an optimization it may return just a single extent for C<offset>. + +If there is an error, C<.extents> should call C<nbdkit_error&gt...

...he range. It may also return extent information for less +than the whole range, but it must return at least one extent +overlapping C<offset>. + +The extents B<must> be added in ascending order, and B<must> be +contiguous. + +The C<flags> parameter may contain the flag C<NBDKIT_FLAG_REQ_ONE> +which means that the client is only requesting information about the +extent overlapping C<offset>. The plugin may ignore this flag, or as +an optimization it may return just a single extent for C<offset>. + + int nbdkit_add_extent (struct nbdkit_extents *extents, +...

...extents beyond +the end of the range. It may also return extent information for less +than the whole range, but it must return at least one extent. + +The extents B<must> be added in ascending order, and B<must> be +contiguous. + +The C<flags> parameter may contain the flag C<NBDKIT_FLAG_REQ_ONE> +which means that the client is only requesting information about the +extent overlapping C<offset>. The plugin may ignore this flag, or as +an optimization it may return just a single extent for C<offset>. +(Note that implementing this optimization correctly is difficult and +subt...

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 -

...t information for less +than the whole range, but it must return at least one extent +overlapping C<offset>. + +The extents B<must> be added in ascending order, and B<must> be +contiguous. + +The C<flags> parameter of the C<.extents> callback may contain the +flag C<NBDKIT_FLAG_REQ_ONE>. This means that the client is only +requesting information about the extent overlapping C<offset>. The +plugin may ignore this flag, or as an optimization it may return just +a single extent for C<offset>. + + int nbdkit_add_extent (struct nbdkit_extents *extents, +...

...t information for less > +than the whole range, but it must return at least one extent > +overlapping C<offset>. > + > +The extents B<must> be added in ascending order, and B<must> be > +contiguous. > + > +The C<flags> parameter may contain the flag C<NBDKIT_FLAG_REQ_ONE> > +which means that the client is only requesting information about the > +extent overlapping C<offset>. The plugin may ignore this flag, or as > +an optimization it may return just a single extent for C<offset>. I wonder if this paragraph belongs up at [1]. Looks good t...

...AD_MODEL_PARALLEL); > > > + > > > + PyModule_AddIntConstant (m, "FLAG_MAY_TRIM", NBDKIT_FLAG_MAY_TRIM); > > > + PyModule_AddIntConstant (m, "FLAG_FUA", NBDKIT_FLAG_FUA); > > > + PyModule_AddIntConstant (m, "FLAG_REQ_ONE", NBDKIT_FLAG_REQ_ONE); > > > + PyModule_AddIntConstant (m, "FLAG_FAST_ZERO", NBDKIT_FLAG_FAST_ZERO); > > > + > > > + PyModule_AddIntConstant (m, "FUA_NONE", NBDKIT_FUA_NONE); > > > + PyModule_AddIntConstant (m, "FUA_EMULATE", NBDKIT_FUA_EMULA...

I'm not observing any noticeable performance differences, but I'm liking the diffstat. I can't push this patch until we release a new libnbd version with the _notify API addition, but am posting it now for playing with things. Eric Blake (2): nbd: Move transaction info from heap to stack nbd: Use nbdkit aio_*_notify variants plugins/nbd/nbd.c | 217

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

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