search for: pread_structured

On Wed, Jul 24, 2019 at 01:43:23PM -0500, Eric Blake wrote: > On 7/24/19 11:54 AM, Richard W.M. Jones wrote: > > This is a bitmask so using an unsigned type is slightly safer. This > > is not an ABI change since the types are compatible. > > > > Thanks: Eric Blake. > > --- > > examples/strict-structured-reads.c | 2 +- > > generator/generator

...lement - the API supports the DF flag for compilation, but the server must also support it (nbd_can_df) before you can use it. So any further restrictions we decide to encode in the generator rather (or in addition) to restrictions in lib/rw.c can be a later patch. > +++ b/ocaml/tests/test_405_pread_structured.ml > @@ -54,11 +54,13 @@ let () = > NBD.pread_structured nbd buf 0_L (f 42); > assert (buf = expected); > > - NBD.pread_structured nbd buf 0_L (f 42) ~flags:[NBD.cmd_flag_df]; > + let flags = let open NBD.CMD_FLAG in [DF] in > + > + NBD.pread_structured nbd buf 0_...

On 7/24/19 11:54 AM, Richard W.M. Jones wrote: > This is a bitmask so using an unsigned type is slightly safer. This > is not an ABI change since the types are compatible. > > Thanks: Eric Blake. > --- > examples/strict-structured-reads.c | 2 +- > generator/generator | 67 ++++++++++++++++-------------- > interop/structured-read.c | 2 +- >

...of Exception that contains the last error information). --- Applies on top of my fixes to Rich's proposal for a Mutable(Int) callback parameter in the generator. I validated that: except Exception as e: print(e) in 405-pread-structured.py is sufficient to produce output resembling: nbd_pread_structured: read: command failed: Protocol error Traceback (most recent call last): File "./t/405-pread-structured.py", line 48, in <module> buf = h.pread_structured (512, 0, 43, f, nbd.CMD_FLAG_DF) File "/home/eblake/libnbd/python/nbd.py", line 517, in pread_structured r...

Now that we have a way to pass Mutable(Int "error") to callbacks, that's a much nicer way for language bindings to use than relying on the value of errno after returning -1. Update the semantics for block status to match what pread_structured does, getting errno out of the equation, and changing things to call the callback for a second context even after an earlier callback failure sets the error. This is an API/ABI break to existing clients, but that's okay because we have not yet declared stable API. (If you need to compile agai...

...L && (cb).free == NULL) Semantic change. In generator, you used CALLBACK_IS_NULL() for both Closure and OClosure. For OClosure, the new semantics are still correct. But for Closure, we now no longer return EFAULT when the callback itself is missing but a .free was provided. This changes pread_structured and block_status to accept NULL for the callback; which is probably not a good idea. In short, there are some points in the code that care only whether .callback is NULL, and others that care whether both pointers are NULL. > #define CALLBACK_IS_NOT_NULL(cb) (! CALLBACK_IS_NULL ((cb))) >...

As the subject says, how close are we to being able to declare a stable API for libnbd? I believe these are the main topics: * Do we need to have an extra thread for writing? I'm unclear about whether b92392b717 (which allows the state machine to break during reply processing) means we definitely don't need threads. I imagine that two threads doing simultaneous send(2) and

...offset"; - Int "status"; + UInt "status"; Mutable (Int "error");]}; Flags "flags" ]; ret = RInt64; @@ -1772,7 +1772,7 @@ documented in C<nbd_pread_structured>."; Closure { cbname="chunk"; cbargs=[BytesIn ("subbuf", "count"); UInt64 "offset"; - Int "status"; + UInt &quo...

...That may still be worthwhile to pursue. > > Is there anything else? Do we like the signature of all the callbacks? Right now, there is a slight inconsistency in that the 'int *error' parameter is last for block_status and notify callbacks, but comes before 'int status' for pread_structured. It would be a simple API switch to pread_structured to put it last there as well, but something we can't do after declaring stability. > > We could also consider doing a "soft stable API" release where we bump > the version up to 0.9.x, announce that we're going to mak...

...ally, but the latter by not compressing holes over the wire), it is worth exposing to clients, if only for testing purposes. I chose to specifically reject the flag for plain nbd_[aio_]pread (about all it can do is cause a read to fail that would otherwise succeed) and only accept it for nbd_[aio_]pread_structured, but it would be easy enough to change the one to forward to the other if we wanted. --- generator/generator | 22 +++++++++++++++++++--- interop/structured-read.c | 31 ++++++++++++++++++++++++------- lib/flags.c | 11 +++++++++++ lib/nbd-protocol.h...

...44 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -783,9 +783,18 @@ nbdplug_flush (void *handle, uint32_t flags) return nbdplug_reply (h, s); } +/* XXX: Hack this signature to compile to 0.1.3 (minimum configure + * version) as well as 0.1.5 (API change, with witness of the addition + * of pread_structured at the same release). Once configure requires a + * higher minimum version, the #if hack can be dropped. + */ static int nbdplug_extent (void *opaque, const char *metacontext, uint64_t offset, - uint32_t *entries, size_t nr_entries) + uint32_t *entries, size_t nr_ent...

...erver's replies, or if you want to use C<LIBNBD_CMD_FLAG_DF>. The C<flags> parameter must be C<0> for now (it exists for future NBD -protocol extensions)."; +protocol extensions)." +^ strict_call_description; see_also = [Link "aio_pread"; Link "pread_structured"; - Link "get_block_size"]; + Link "get_block_size"; Link "set_strict_mode"]; example = Some "examples/fetch-first-sector.c"; }; @@ -1646,9 +1670,11 @@ The C<flags> parameter may be C<0> for no flags, or...

...mall change to the OCaml API -- putting related flags into a submodule, basically renaming them. Note we don't provide guarantees for non-C APIs. --- generator/generator | 180 +++++++++++++++++------ ocaml/nbd-c.h | 15 -- ocaml/tests/test_405_pread_structured.ml | 6 +- ocaml/tests/test_410_pwrite.ml | 3 +- ocaml/tests/test_460_block_status.ml | 3 +- ocaml/tests/test_510_aio_pwrite.ml | 3 +- 6 files changed, 142 insertions(+), 68 deletions(-) diff --git a/generator/generator b/generator/generator index 78c6ca6..5823686 10...

...design question of whether to make an OClosure type, > > where C has two functions nbd_aio_pread and nbd_aio_pread_callback for > > convenience, but where other languages have only a single nbd.aio_pread > > where the callback parameter is optional (the Closure type for > > pread_structured chunk and for block_status extent will still be > > mandatory; it is only the completion callback that is currently causing > > us twice the API because we are treating it as pseudo-optional). Or > > maybe we just require C clients of nbd_aio_pread to always provide > > par...

As before, but this one has working Python bindings. OCaml still TBD. Rich.

...to use > C<LIBNBD_CMD_FLAG_DF>. > > The C<flags> parameter must be C<0> for now (it exists for future NBD > -protocol extensions)."; > +protocol extensions)." > +^ strict_call_description; > see_also = [Link "aio_pread"; Link "pread_structured"; > - Link "get_block_size"]; > + Link "get_block_size"; Link "set_strict_mode"]; > example = Some "examples/fetch-first-sector.c"; > }; > > @@ -1646,9 +1670,11 @@ The C<flags> parameter may...

...ion of free callbacks. We then have the design question of whether to make an OClosure type, where C has two functions nbd_aio_pread and nbd_aio_pread_callback for convenience, but where other languages have only a single nbd.aio_pread where the callback parameter is optional (the Closure type for pread_structured chunk and for block_status extent will still be mandatory; it is only the completion callback that is currently causing us twice the API because we are treating it as pseudo-optional). Or maybe we just require C clients of nbd_aio_pread to always provide parameters for callbacks, but document that...

...; String "msg"] }]) ]; One style of spacing (the list starts with '[{ ', and cbargs has no space), > ret = RErr; > shortdesc = "set the debug callback"; > longdesc = "\ > @@ -1345,10 +1350,11 @@ protocol extensions)."; > "pread_structured", { > default_call with > args = [ BytesOut ("buf", "count"); UInt64 "offset"; > - Opaque "data"; > - Callback ("chunk", [ Opaque "data"; BytesIn ("subbuf", "count"); &...

...[{ cbname="debug_fn"; + cbargs=[String "context"; String "msg"] }]) ]; ret = RErr; shortdesc = "set the debug callback"; longdesc = "\ @@ -1345,10 +1350,11 @@ protocol extensions)."; "pread_structured", { default_call with args = [ BytesOut ("buf", "count"); UInt64 "offset"; - Opaque "data"; - Callback ("chunk", [ Opaque "data"; BytesIn ("subbuf", "count"); + Closure...

...ver"; > + longdesc = "\ As I mentioned re the previous commit, I think the implicit errno is problematic, and an explicit int parameter would be preferable. I think I also have bikeshedding concerns about the name of this function. Do you have any better suggestions, such as 'pread_structured', 'structured_pread', 'pread_discontinuous', ...? Given that, the overall idea looks sound to me. > +of C<buf> within the original buffer). The C<status> parameter is > +one of > + > +=over 4 > + > +=item C<LIBNBD_READ_DATA> = 1 > + >...