Re: [Xen-devel] [PATCH 22/28] libxl: ocaml: event management

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-03-25 at 14:45 +0000, Rob Hoes wrote:
> This patch add the facilities needed to interact with the event system
> in libxl. This is useful, for instance, for getting a callback when a
> domains dies, as well as to use the asyncronous versions of some of libxl's
> calls.
> 
> The functions dealing with timeouts are still TBD.
> 
> Signed-off-by: Rob Hoes <rob.hoes@xxxxxxxxxx>
> ---
>  tools/ocaml/libs/xl/xenlight.ml.in   |   42 +++++++++
>  tools/ocaml/libs/xl/xenlight.mli.in  |   21 +++++
>  tools/ocaml/libs/xl/xenlight_stubs.c |  161 
> ++++++++++++++++++++++++++++++++++
>  3 files changed, 224 insertions(+)
> 
> diff --git a/tools/ocaml/libs/xl/xenlight.ml.in 
> b/tools/ocaml/libs/xl/xenlight.ml.in
> index 63b8bf8..96d6a38 100644
> --- a/tools/ocaml/libs/xl/xenlight.ml.in
> +++ b/tools/ocaml/libs/xl/xenlight.ml.in
> @@ -73,6 +73,17 @@ external test_raise_exception: unit -> unit = 
> "stub_raise_exception"
>  type domid = int
>  type devid = int
>  
> +(* type for event callbacks *)
> +type for_libxl
> +
> +type event =
> +     | POLLIN (* There is data to read *)
> +     | POLLPRI (* There is urgent data to read *)
> +     | POLLOUT (* Writing now will not block *)
> +     | POLLERR (* Error condition (revents only) *)
> +     | POLLHUP (* Device has been disconnected (revents only) *)
> +     | POLLNVAL (* Invalid request: fd not open (revents only). *)

Can you reuse these from your poll library in the previous patch?


> +int timeout_register(void *user, void **for_app_registration_out,
> +                          struct timeval abs, void *for_libxl)
> +{
> +     return 0;
> +}
> +
> +int timeout_modify(void *user, void **for_app_registration_update,
> +                         struct timeval abs)
> +{
> +     return 0;
> +}
> +
> +void timeout_deregister(void *user, void *for_app_registration)
> +{
> +     return;
> +}

Worth failing noisily until these are implemented?

> +
> +value stub_xl_osevent_register_hooks(value ctx, value user)
> +{
> +     CAMLparam2(ctx, user);
> +     libxl_osevent_hooks *hooks;
> +     hooks = malloc(sizeof(*hooks));
> +
> +     hooks->fd_register = fd_register;
> +     hooks->fd_modify = fd_modify;
> +     hooks->fd_deregister = fd_deregister;
> +     hooks->timeout_register = timeout_register;
> +     hooks->timeout_modify = timeout_modify;
> +     hooks->timeout_deregister = timeout_deregister;
> +
> +     libxl_osevent_register_hooks(CTX, hooks, (void *) user);

This user thing will be retained by libxl -- is that safe from an ocaml
gc point of view?

> +     CAMLreturn((value) hooks);

Another instance of the problematic heap allocation pattern Dave pointed
out?

> +void event_occurs(void *user, const libxl_event *event)
> +{
> +     CAMLparam0();
> +     CAMLlocalN(args, 2);
> +     value *func = caml_named_value("xl_event_occurs_callback");
> +
> +     args[0] = (value) user;
> +     args[1] = Val_event((libxl_event *) event);
> +     //libxl_event_free(CTX, event); // no ctx here!

Is it leaked or do you free it somewhere else? I suppose "func" must do
it? (which makes sense actually)

[...]
> +value stub_xl_event_register_callbacks(value ctx, value user)
> +{
> +     CAMLparam2(ctx, user);
> +     libxl_event_hooks *hooks;
> +     
> +     hooks = malloc(sizeof(*hooks));

Another heap alloc?

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel