Re: [Xen-devel] [PATCH v3-RESEND 20/28] libxl: ocaml: allow device operations to be called asynchronously

[Rob Hoes <Rob.Hoes@xxxxxxxxxx>]

> On Mon, 2013-10-21 at 14:32 +0100, Rob Hoes wrote:
> > Signed-off-by: Rob Hoes <rob.hoes@xxxxxxxxxx>
> > ---
> >  tools/ocaml/libs/xl/genwrap.py       |    6 +++---
> >  tools/ocaml/libs/xl/xenlight_stubs.c |   14 +++++++++++---
> >  2 files changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/ocaml/libs/xl/genwrap.py
> > b/tools/ocaml/libs/xl/genwrap.py index daeffdf..721a336 100644
> > --- a/tools/ocaml/libs/xl/genwrap.py
> > +++ b/tools/ocaml/libs/xl/genwrap.py
> > @@ -22,9 +22,9 @@ builtins = {
> >      "libxl_cpuid_policy_list": ("unit",                "%(c)s = 0",
> "Val_unit"),
> >      }
> >
> > -DEVICE_FUNCTIONS = [ ("add",            ["ctx", "t", "domid", "unit"]),
> > -                     ("remove",         ["ctx", "t", "domid", "unit"]),
> > -                     ("destroy",        ["ctx", "t", "domid", "unit"]),
> > +DEVICE_FUNCTIONS = [ ("add",            ["ctx", "t", "domid", "?async:'a",
> "unit", "unit"]),
> > +                     ("remove",         ["ctx", "t", "domid", "?async:'a", 
> > "unit", "unit"]),
> > +                     ("destroy",        ["ctx", "t", "domid", "?async:'a", 
> > "unit", "unit"]),
> 
> I probably don't speak enough ocaml to make heads or tails of it but can you
> include the resulting ocaml type in the changelog? Is it really
>         ctx-> t -> ?async -> () -> ()
> ?     
> 
> I guess I don't fully grok "-> ()" if it can chain like that ;-)

The type of the function is "ctx-> t -> domid -> ?async -> () -> ()". The last 
() is the return type. The one before that is the type of the last argument of 
the function, which means you always have to call the function with () at the 
end. This used to mark "the end" of the function. It is not allowed in OCaml 
for functions to have optional arguments at the end.

Synchronous example:
    remove ctx my_disk 666 ()

Asynchronous example:
    remove ctx my_disk 666 ~async:my_user_id ()

The manual says: "A function taking some optional arguments must also take at 
least one non-optional argument. The criterion for deciding whether an optional 
argument has been omitted is the non-labeled application of an argument 
appearing after this optional argument in the function type." 
[http://caml.inria.fr/pub/docs/manual-ocaml-4.00/manual006.html]

So another option would be to move the ?async argument further to the left, and 
skip the extra (), e.g. "ctx -> ?async -> t -> domid -> ()". But I wanted to 
keep the async stuff at the end (like in the libxl function), and adding a () 
is quite common in OCaml.
 
Rob

> 
> >                     ]
> >
> >  functions = { # ( name , [type1,type2,....] ) diff --git
> > a/tools/ocaml/libs/xl/xenlight_stubs.c
> > b/tools/ocaml/libs/xl/xenlight_stubs.c
> > index 5b38e7c..7e56db9 100644
> > --- a/tools/ocaml/libs/xl/xenlight_stubs.c
> > +++ b/tools/ocaml/libs/xl/xenlight_stubs.c
> > @@ -396,15 +396,23 @@ void async_callback(libxl_ctx *ctx, int rc, void
> > *for_callback)  #define STRINGIFY(x) _STRINGIFY(x)
> >
> >  #define _DEVICE_ADDREMOVE(type,op)
>       \
> > -value stub_xl_device_##type##_##op(value ctx, value info, value domid)
>       \
> > +value stub_xl_device_##type##_##op(value ctx, value info, value domid,
>       \
> > +   value async, value unit)                                        \
> >  {                                                                  \
> > -   CAMLparam3(ctx, info, domid);
>       \
> > +   CAMLparam5(ctx, info, domid, async, unit);                      \
> 
> Only one unit here?
> 
> >     libxl_device_##type c_info;                                     \
> >     int ret, marker_var;                                            \
> > +   libxl_asyncop_how ao_how;                                       \
> >                                                                     \
> >     device_##type##_val(CTX, &c_info, info);                        \
> >                                                                     \
> > -   ret = libxl_device_##type##_##op(CTX, Int_val(domid), &c_info, 0); \
> > +   if (async != Val_none) {                                        \
> > +           ao_how.callback = async_callback;                       \
> > +           ao_how.u.for_callback = (void *) Some_val(async);       \
> > +   }                                                               \
> > +                                                                   \
> > +   ret = libxl_device_##type##_##op(CTX, Int_val(domid), &c_info,
>       \
> > +           async != Val_none ? &ao_how : NULL);                    \
> >                                                                     \
> >     libxl_device_##type##_dispose(&c_info);
>       \
> >                                                                     \
> 


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