[Libguestfs] [nbdkit PATCH] ocaml: Add support for dynamic .thread_model

[Eric Blake]

On 8/15/19 4:52 PM, Eric Blake wrote:
> We do not promise API stability for non-C languages; this is an API
> break as follows: instead of calling 'NBDKit.register_plugin model
> plugin' with a static model, you can now add .thread_model :(unit ->
> thread_model) to plugin or default to PARALLEL.
> 
> Since all existing OCaml plugins will have already thought about
> thread models, they can convert their existing model into the new
> plugin field (and thus, I don't feel too bad making PARALLEL the
> default, even if it is not always the safest).
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> 
> I'm still looking at two followups:
> 1) ./nbdkit doesn't set LD_LIBRARY_PATH=plugins/ocaml/.libs:$LD_LIBRARY_PATH
>  (making ./nbdkit --dump-plugin tests/test-ocaml-plugin.so fail to load
>   when the system nbdkit is too old)
> 2) although --dump-plugin shows thread model, ./nbdkit -v log does not;
>  I need to add a debug() statement for that in server/locks.c

Doing this proved I had a copy-paste bug:

> +static int
> +thread_model_wrapper (void)
> +{
> +  CAMLparam0 ();
> +  CAMLlocal1 (rv);
> +
> +  caml_leave_blocking_section ();
> +
> +  rv = caml_callback_exn (config_complete_fn, Val_unit);

This needs to call thread_model_fn, not config_complete_fn; otherwise
the thread model is wrongly chosen between 0 or 1, and in the
--dump-plugin case I get an assertion in tests/test_ocaml_plugin.ml when
not providing the expected config parameters.

I've now pushed fixes for the two noted improvements, as well as this
patch as fixed.

-- 
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: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190816/c7ff9d23/attachment.sig>