[Libguestfs] [PATCH libnbd] api: Rename nbd_aio_*_callback to nbd_aio_*.

[Eric Blake]

On 8/13/19 10:37 AM, Richard W.M. Jones wrote:
> The original nbd_aio_* (non-callback) functions are removed and
> replaced with the renamed callback variants.
> 
> This is a simple mechanical change to the API:
> 
> (1) Any existing call to nbd_aio_*_callback can simply be renamed to
>     nbd_aio_*
> 
> (2) Any existing call to nbd_aio_* must have two extra NULL parameters
>     added before the final flags parameter.
> 
> In non-C languages, only change (1) is required.

It is still possible to compile for 0.9.6 and this patch simultaneously,
by checking LIBNBD_HAVE_NBD_AIO_PREAD_CALLBACK as a witness of which API
style to use (although it's also just as easy to bump minimum version
requirements to 0.9.7, once we have a release including this and any
other API changes being discussed...).


> +++ b/docs/libnbd.pod
> @@ -276,9 +276,8 @@ command has completed:
>   }
>  
>  For almost all high level synchronous calls (eg. C<nbd_pread>) there
> -are two low level asynchronous equivalents (eg. C<nbd_aio_pread> for
> -starting a command, and C<nbd_aio_pread_callback> for also registering
> -a callback to be invoked right before the command is complete).
> +is a low level asynchronous equivalents (eg. C<nbd_aio_pread> for

equivalent

> +starting a command).
>  
>  =head2 glib2 integration
>  
> @@ -600,8 +599,8 @@ will use your login name):
>  =head1 CALLBACKS
>  
>  Some libnbd calls take function pointers (eg.
> -C<nbd_set_debug_callback>, C<nbd_pread_callback>).  Libnbd can call

Eww - we had a bogus link (that should have been
nbd_aio_pread_callback). Nice that we fix that as a side effect.

> -these functions while processing.
> +C<nbd_set_debug_callback>, C<nbd_aio_pread>).  Libnbd can call these
> +functions while processing.
>  


> -  "aio_pread_callback", {
>      default_call with
>      args = [ BytesPersistOut ("buf", "count"); UInt64 "offset" ];
>      optargs = [ OClosure completion_closure; OFlags ("flags", cmd_flags) ];
>      ret = RInt64;
>      permitted_states = [ Connected ];
> -    shortdesc = "read from the NBD server, with callback on completion";
> +    shortdesc = "read from the NBD server";
>      longdesc = "\
>  Issue a read command to the NBD server.  This returns the
>  unique positive 64 bit cookie for this command, or C<-1> on
>  error.
>  
> -When the command completes, C<callback>
> -will be invoked as described in L<libnbd(3)/Completion callbacks>.
> +To check if the command completed, call C<nbd_aio_command_completed>.
> +Or supply the optional C<completion> callback which will be invoked
> +as described in L<libnbd(3)/Completion callbacks>.
>  
>  Note that you must ensure C<buf> is valid until the command has
>  completed.  Other parameters behave as documented in C<nbd_pread>.";

While we're here, should we also mention that C<completion> and
C<completion_user_data> remain valid until the command completes? Or is
that overkill (the function pointer remains valid unless you dlclose()
something providing it; and it's kind of obvious that you want the user
data to remain valid).  It may matter more for functions that don't take
a buf (such as nbd_aio_trim).

> +++ b/lib/rw.c

Yep, definitely a mechanical conversion.

> +++ b/ocaml/examples/asynch_copy.ml
> @@ -48,7 +48,7 @@ let asynch_copy src dst =
>      if !soff < size && NBD.aio_in_flight src < max_reads_in_flight then (
>        let bs = min bs (size -^ !soff) in
>        let buf = NBD.Buffer.alloc (Int64.to_int bs) in
> -      ignore (NBD.aio_pread_callback src buf !soff
> +      ignore (NBD.aio_pread src buf !soff
>                  ~completion:(read_completed buf !soff));

Pre-existing; but is the indentation off here? (I would have guessed two
fewer spaces, so that ~completion starts just after the ( above)

>        soff := !soff +^ bs
>      );
> @@ -59,7 +59,7 @@ let asynch_copy src dst =
>      List.iter (
>        fun (buf, offset) ->
>          (* Note the size of the write is implicitly stored in buf. *)
> -        ignore (NBD.aio_pwrite_callback dst buf offset
> +        ignore (NBD.aio_pwrite dst buf offset
>                    ~completion:(write_completed buf))

and here


> +++ b/ocaml/tests/test_590_aio_copy.ml
> @@ -71,7 +71,7 @@ let asynch_copy src dst =
>      if !soff < size && NBD.aio_in_flight src < max_reads_in_flight then (
>        let bs = min bs (size -^ !soff) in
>        let buf = NBD.Buffer.alloc (Int64.to_int bs) in
> -      ignore (NBD.aio_pread_callback src buf !soff
> +      ignore (NBD.aio_pread src buf !soff
>                  ~completion:(read_completed buf !soff));

But you are consistent about it, so don't change it if my sense of OCaml
style isn't right :)

ACK with typo fix.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 484 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190813/189a988b/attachment.sig>