search for: nbd_aio_foo_callback

...+When the command completes, C<callback> > > will be invoked as described in L<libnbd(3)/Completion callbacks>. > > + > > Do we need wording anywhere (probably in the .pod, so we only state it > once) that mentions that the callback routine is not invoked if > nbd_aio_FOO_callback returns -1? I guess it's implicit from the fact that returning -1 indicates an error. In fact the relationship ought to be stronger than that - the command should run if and only if nbd_aio_FOO_callback returns >= 0. (That's not actually true at the moment because an error can happen...

As discussed in this thread, the parameter is an invitation to write code with race conditions: https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00309 --- docs/libnbd.pod | 6 +- examples/glib-main-loop.c | 10 ++-- examples/strict-structured-reads.c | 2 +- generator/generator | 57

On Sat, Jun 29, 2019 at 08:28:28AM -0500, Eric Blake wrote: > As mentioned in the previous patch, there are situations where an aio > client wants instant notification when a given command is complete, > rather than having to maintain a separate data structure to track all > in-flight commands and then iterate over that structure to learn which > commands are complete. It's

...LAG_DONE on an OFFSET_DATA/BLOCK_STATUS reply, we could set VALID|FREE; if the server defers NBD_REPLY_FLAG_DONE to an NBD_REPLY_TYPE_NONE packet then the two flags will definitely not be set at the same time), or we could always defer the FREE flag until the overall command is ready to retire; for nbd_aio_FOO_callback, it may indeed be easier to set VALID|FREE on the single use of the callback at the time it is ready to retire. But this idea does make it possible for libnbd to inform the callback about its last expected use. > > The extra ‘valid_flag’ contains one or both of LIBNBD_CALLBACK_VALID > a...

...k> > +error. > + > +When the command completes, C<callback> > will be invoked as described in L<libnbd(3)/Completion callbacks>. > + Do we need wording anywhere (probably in the .pod, so we only state it once) that mentions that the callback routine is not invoked if nbd_aio_FOO_callback returns -1? ACK. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

...ht get confusing. My though there is to rename the latter to 'cookie' or 'id' or any other term you can think of which would fit (which is more of a doc change, and not an actual API change). > > What do you think? Do you want me to go ahead and rename nbd_aio_FOO_notify to nbd_aio_FOO_callback? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

...tweaked things in v2. I've got two threads of thoughts below. > > (Note it is also possible for the library to call the callback with > valid_flag == LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, meaning it's the > last valid call.) [1] In my previous reply, I assumed that the nbd_aio_FOO_callback function would use this paradigm... > +++ b/examples/strict-structured-reads.c > @@ -127,11 +131,14 @@ read_chunk (void *opaque, const void *bufv, size_t count, uint64_t offset, > } > > static int > -read_verify (void *opaque, int64_t cookie, int *error) > +read_verify (...

...> threads of thoughts below. > > > > > (Note it is also possible for the library to call the callback with > > valid_flag == LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, meaning it's the > > last valid call.) > > [1] In my previous reply, I assumed that the nbd_aio_FOO_callback > function would use this paradigm... > > > > +++ b/examples/strict-structured-reads.c > > > @@ -127,11 +131,14 @@ read_chunk (void *opaque, const void *bufv, size_t count, uint64_t offset, > > } > > > > static int > > -read_verify (void *opaqu...

...*data = opaque; > int ret = -1; > > + if (!(valid_flag & LIBNBD_CALLBACK_VALID)) > + return 0; ...but this one is wrong; it calls free(data) which should be deferred to the valid_flag & LIBNBD_CALLBACK_FREE portion. (It looks like later on in the patch, I see that the nbd_aio_FOO_callback callback is always called exactly once with VALID|FREE, so your bug here is masked because of that calling convention - but we should really fix this one to be a better example). > +++ b/generator/generator > @@ -849,10 +849,7 @@ and arg = > written by the f...

...-1; > > > > + if (!(valid_flag & LIBNBD_CALLBACK_VALID)) > > + return 0; > > ...but this one is wrong; it calls free(data) which should be deferred > to the valid_flag & LIBNBD_CALLBACK_FREE portion. (It looks like later > on in the patch, I see that the nbd_aio_FOO_callback callback is always > called exactly once with VALID|FREE, so your bug here is masked because > of that calling convention - but we should really fix this one to be a > better example). Ah, I see. I can fix this in the commit. > > @@ -3196,6 +3180,12 @@ let rec print_arg_list ?(han...

On Mon, Jul 22, 2019 at 10:08:25AM +0100, Richard W.M. Jones wrote: > On Sat, Jul 20, 2019 at 07:38:45AM +0100, Richard W.M. Jones wrote: > > More thoughts on callbacks, etc. following on from: > > https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00184 > > > > Closure lifetimes > > ----------------- Here's a possibly better idea which still

When using the nbd_aio_FOO_callback commands, there is nothing further to be learned about the command by calling nbd_aio_command_completed() compared to what the callback already had access to. There are still scenarios where manually retiring the command after the fact is useful (whether the return was 0 to keep the status unchang...

This implements most of what I wrote here: https://www.redhat.com/archives/libguestfs/2019-July/msg00213.html