| Message ID | 20190920161902.1353598-1-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/ocaml: Build fix following libxl API changes | expand |
On Fri, Sep 20, 2019 at 05:19:02PM +0100, Anthony PERARD wrote: > The following libxl API became asynchronous and gained an additional > `ao_how' parameter: > libxl_domain_pause() > libxl_domain_unpause() > libxl_send_trigger() > > Adapt the ocaml binding. > > Build tested only. > > Fixes: edaa631ddcee665cdfae1cf6bc7492c791e01ef4 > Fixes: 95627b87c3159928458ee586e8c5c593bdd248d8 > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > > Notes: > Currently, all libxl API that takes an `ao_how` have `?async:'a -> unit` > in the ocaml definition (and an extra unused value unit in the c stub > file), is that `unit` needed ? > > I tried to add it, but then for stub_xl_send_trigger() I had to use > CAMLparam6, and that doesn't exist. I discovered CAMLxparam1 macro, but that's not better: File "xenlight.ml", line 1735, characters 25-84: Error: An external function with more than 5 arguments requires a second stub function for native-code compilation
Anthony PERARD writes ("Re: [PATCH] tools/ocaml: Build fix following libxl API changes"):
> On Fri, Sep 20, 2019 at 05:19:02PM +0100, Anthony PERARD wrote:
> > The following libxl API became asynchronous and gained an additional
> > `ao_how' parameter:
> > libxl_domain_pause()
> > libxl_domain_unpause()
> > libxl_send_trigger()
> >
> > Adapt the ocaml binding.
> >
> > Build tested only.
> >
> > Fixes: edaa631ddcee665cdfae1cf6bc7492c791e01ef4
> > Fixes: 95627b87c3159928458ee586e8c5c593bdd248d8
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > ---
> >
> > Notes:
> > Currently, all libxl API that takes an `ao_how` have `?async:'a -> unit`
> > in the ocaml definition (and an extra unused value unit in the c stub
> > file), is that `unit` needed ?
> >
> > I tried to add it, but then for stub_xl_send_trigger() I had to use
> > CAMLparam6, and that doesn't exist.
>
> I discovered CAMLxparam1 macro, but that's not better:
> File "xenlight.ml", line 1735, characters 25-84:
> Error: An external function with more than 5 arguments requires a second stub function
> for native-code compilation
In order to unbreak the build I have acked and pushed this patch right
away, but IMO a review from an ocaml maintainer is quite important
here.
Thanks,
Ian.
On 20/09/2019 17:19, Anthony PERARD wrote: > The following libxl API became asynchronous and gained an additional > `ao_how' parameter: > libxl_domain_pause() > libxl_domain_unpause() > libxl_send_trigger() > > Adapt the ocaml binding. > > Build tested only. > > Fixes: edaa631ddcee665cdfae1cf6bc7492c791e01ef4 > Fixes: 95627b87c3159928458ee586e8c5c593bdd248d8 > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> This library is entirely unused, full of memory leaks, and has a number of areas needing extremely careful design due to the differing behaviours of the libxl and ocaml runtimes. Another option would be to drop the bindings entirely. > --- > > Notes: > Currently, all libxl API that takes an `ao_how` have `?async:'a -> unit` > in the ocaml definition (and an extra unused value unit in the c stub > file), is that `unit` needed ? > > I tried to add it, but then for stub_xl_send_trigger() I had to use > CAMLparam6, and that doesn't exist. In the Ocaml FFI, with more than 5 parameters, the entire set of parameters is spilled to memory as an array, so the C side of the function ends with a CAMLparam2 (list pointer and number of entries), and has extra marshalling to do. ~Andrew
On 20.09.2019 19:15, Ian Jackson wrote: > Anthony PERARD writes ("Re: [PATCH] tools/ocaml: Build fix following libxl API changes"): >> On Fri, Sep 20, 2019 at 05:19:02PM +0100, Anthony PERARD wrote: >>> The following libxl API became asynchronous and gained an additional >>> `ao_how' parameter: >>> libxl_domain_pause() >>> libxl_domain_unpause() >>> libxl_send_trigger() >>> >>> Adapt the ocaml binding. >>> >>> Build tested only. >>> >>> Fixes: edaa631ddcee665cdfae1cf6bc7492c791e01ef4 >>> Fixes: 95627b87c3159928458ee586e8c5c593bdd248d8 >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >>> --- >>> >>> Notes: >>> Currently, all libxl API that takes an `ao_how` have `?async:'a -> unit` >>> in the ocaml definition (and an extra unused value unit in the c stub >>> file), is that `unit` needed ? >>> >>> I tried to add it, but then for stub_xl_send_trigger() I had to use >>> CAMLparam6, and that doesn't exist. >> >> I discovered CAMLxparam1 macro, but that's not better: >> File "xenlight.ml", line 1735, characters 25-84: >> Error: An external function with more than 5 arguments requires a second stub function >> for native-code compilation > > In order to unbreak the build I have acked and pushed this patch right > away, but IMO a review from an ocaml maintainer is quite important > here. According to osstest results accumulated over the weekend and the state of the tree, did you perhaps commit the change but forgot to actually push it? Jan
Jan Beulich writes ("Re: [Xen-devel] [PATCH] tools/ocaml: Build fix following libxl API changes"):
> According to osstest results accumulated over the weekend and the
> state of the tree, did you perhaps commit the change but forgot
> to actually push it?
Gah, apparently so. Now done.
Ian.
diff --git a/tools/ocaml/libs/xl/xenlight.ml.in b/tools/ocaml/libs/xl/xenlight.ml.in index 80e620a9be66..954e56fc740b 100644 --- a/tools/ocaml/libs/xl/xenlight.ml.in +++ b/tools/ocaml/libs/xl/xenlight.ml.in @@ -41,10 +41,10 @@ module Domain = struct external reboot : ctx -> domid -> unit = "stub_libxl_domain_reboot" external destroy : ctx -> domid -> ?async:'a -> unit -> unit = "stub_libxl_domain_destroy" external suspend : ctx -> domid -> Unix.file_descr -> ?async:'a -> unit -> unit = "stub_libxl_domain_suspend" - external pause : ctx -> domid -> unit = "stub_libxl_domain_pause" - external unpause : ctx -> domid -> unit = "stub_libxl_domain_unpause" + external pause : ctx -> domid -> ?async:'a -> unit = "stub_libxl_domain_pause" + external unpause : ctx -> domid -> ?async:'a -> unit = "stub_libxl_domain_unpause" - external send_trigger : ctx -> domid -> trigger -> int -> unit = "stub_xl_send_trigger" + external send_trigger : ctx -> domid -> trigger -> int -> ?async:'a -> unit = "stub_xl_send_trigger" external send_sysrq : ctx -> domid -> char -> unit = "stub_xl_send_sysrq" end diff --git a/tools/ocaml/libs/xl/xenlight.mli.in b/tools/ocaml/libs/xl/xenlight.mli.in index b2c06b5eed76..c08304ae8b01 100644 --- a/tools/ocaml/libs/xl/xenlight.mli.in +++ b/tools/ocaml/libs/xl/xenlight.mli.in @@ -43,10 +43,10 @@ module Domain : sig external reboot : ctx -> domid -> unit = "stub_libxl_domain_reboot" external destroy : ctx -> domid -> ?async:'a -> unit -> unit = "stub_libxl_domain_destroy" external suspend : ctx -> domid -> Unix.file_descr -> ?async:'a -> unit -> unit = "stub_libxl_domain_suspend" - external pause : ctx -> domid -> unit = "stub_libxl_domain_pause" - external unpause : ctx -> domid -> unit = "stub_libxl_domain_unpause" + external pause : ctx -> domid -> ?async:'a -> unit = "stub_libxl_domain_pause" + external unpause : ctx -> domid -> ?async:'a -> unit = "stub_libxl_domain_unpause" - external send_trigger : ctx -> domid -> trigger -> int -> unit = "stub_xl_send_trigger" + external send_trigger : ctx -> domid -> trigger -> int -> ?async:'a -> unit = "stub_xl_send_trigger" external send_sysrq : ctx -> domid -> char -> unit = "stub_xl_send_sysrq" end diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c index 0140780a342e..37b046df6351 100644 --- a/tools/ocaml/libs/xl/xenlight_stubs.c +++ b/tools/ocaml/libs/xl/xenlight_stubs.c @@ -622,32 +622,38 @@ value stub_libxl_domain_suspend(value ctx, value domid, value fd, value async, v CAMLreturn(Val_unit); } -value stub_libxl_domain_pause(value ctx, value domid) +value stub_libxl_domain_pause(value ctx, value domid, value async) { - CAMLparam2(ctx, domid); + CAMLparam3(ctx, domid, async); int ret; uint32_t c_domid = Int_val(domid); + libxl_asyncop_how *ao_how = aohow_val(async); caml_enter_blocking_section(); - ret = libxl_domain_pause(CTX, c_domid); + ret = libxl_domain_pause(CTX, c_domid, ao_how); caml_leave_blocking_section(); + free(ao_how); + if (ret != 0) failwith_xl(ret, "domain_pause"); CAMLreturn(Val_unit); } -value stub_libxl_domain_unpause(value ctx, value domid) +value stub_libxl_domain_unpause(value ctx, value domid, value async) { - CAMLparam2(ctx, domid); + CAMLparam3(ctx, domid, async); int ret; uint32_t c_domid = Int_val(domid); + libxl_asyncop_how *ao_how = aohow_val(async); caml_enter_blocking_section(); - ret = libxl_domain_unpause(CTX, c_domid); + ret = libxl_domain_unpause(CTX, c_domid, ao_how); caml_leave_blocking_section(); + free(ao_how); + if (ret != 0) failwith_xl(ret, "domain_unpause"); @@ -1031,20 +1037,23 @@ value stub_xl_domain_sched_params_set(value ctx, value domid, value scinfo) CAMLreturn(Val_unit); } -value stub_xl_send_trigger(value ctx, value domid, value trigger, value vcpuid) +value stub_xl_send_trigger(value ctx, value domid, value trigger, value vcpuid, value async) { - CAMLparam4(ctx, domid, trigger, vcpuid); + CAMLparam5(ctx, domid, trigger, vcpuid, async); int ret; uint32_t c_domid = Int_val(domid); libxl_trigger c_trigger = LIBXL_TRIGGER_UNKNOWN; int c_vcpuid = Int_val(vcpuid); + libxl_asyncop_how *ao_how = aohow_val(async); trigger_val(CTX, &c_trigger, trigger); caml_enter_blocking_section(); - ret = libxl_send_trigger(CTX, c_domid, c_trigger, c_vcpuid); + ret = libxl_send_trigger(CTX, c_domid, c_trigger, c_vcpuid, ao_how); caml_leave_blocking_section(); + free(ao_how); + if (ret != 0) failwith_xl(ret, "send_trigger");
The following libxl API became asynchronous and gained an additional `ao_how' parameter: libxl_domain_pause() libxl_domain_unpause() libxl_send_trigger() Adapt the ocaml binding. Build tested only. Fixes: edaa631ddcee665cdfae1cf6bc7492c791e01ef4 Fixes: 95627b87c3159928458ee586e8c5c593bdd248d8 Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: Currently, all libxl API that takes an `ao_how` have `?async:'a -> unit` in the ocaml definition (and an extra unused value unit in the c stub file), is that `unit` needed ? I tried to add it, but then for stub_xl_send_trigger() I had to use CAMLparam6, and that doesn't exist. tools/ocaml/libs/xl/xenlight.ml.in | 6 +++--- tools/ocaml/libs/xl/xenlight.mli.in | 6 +++--- tools/ocaml/libs/xl/xenlight_stubs.c | 27 ++++++++++++++++++--------- 3 files changed, 24 insertions(+), 15 deletions(-)