All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xen-devel] [PATCH] tools/ocaml: Build fix following libxl API changes
@ 2019-09-20 16:19 Anthony PERARD
  2019-09-20 17:00 ` Anthony PERARD
  2019-09-20 17:50 ` Andrew Cooper
  0 siblings, 2 replies; 6+ messages in thread
From: Anthony PERARD @ 2019-09-20 16:19 UTC (permalink / raw)
  To: xen-devel
  Cc: Anthony PERARD, Ian Jackson, Christian Lindig, Wei Liu, David Scott

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(-)

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");
 
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] tools/ocaml: Build fix following libxl API changes
  2019-09-20 16:19 [Xen-devel] [PATCH] tools/ocaml: Build fix following libxl API changes Anthony PERARD
@ 2019-09-20 17:00 ` Anthony PERARD
  2019-09-20 17:15   ` Ian Jackson
  2019-09-20 17:50 ` Andrew Cooper
  1 sibling, 1 reply; 6+ messages in thread
From: Anthony PERARD @ 2019-09-20 17:00 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Christian Lindig, Wei Liu, David Scott

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] tools/ocaml: Build fix following libxl API changes
  2019-09-20 17:00 ` Anthony PERARD
@ 2019-09-20 17:15   ` Ian Jackson
  2019-09-23  7:45     ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Ian Jackson @ 2019-09-20 17:15 UTC (permalink / raw)
  To: Anthony PERARD; +Cc: xen-devel, Christian Lindig, Wei Liu, David Scott

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] tools/ocaml: Build fix following libxl API changes
  2019-09-20 16:19 [Xen-devel] [PATCH] tools/ocaml: Build fix following libxl API changes Anthony PERARD
  2019-09-20 17:00 ` Anthony PERARD
@ 2019-09-20 17:50 ` Andrew Cooper
  1 sibling, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2019-09-20 17:50 UTC (permalink / raw)
  To: Anthony PERARD, xen-devel
  Cc: Ian Jackson, Christian Lindig, Wei Liu, David Scott

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] tools/ocaml: Build fix following libxl API changes
  2019-09-20 17:15   ` Ian Jackson
@ 2019-09-23  7:45     ` Jan Beulich
  2019-09-23 10:14       ` Ian Jackson
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2019-09-23  7:45 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Anthony PERARD, xen-devel, Christian Lindig, Wei Liu, DavidScott

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] tools/ocaml: Build fix following libxl API changes
  2019-09-23  7:45     ` Jan Beulich
@ 2019-09-23 10:14       ` Ian Jackson
  0 siblings, 0 replies; 6+ messages in thread
From: Ian Jackson @ 2019-09-23 10:14 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Anthony Perard, xen-devel, Christian Lindig, Wei Liu, DavidScott

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-23 10:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-20 16:19 [Xen-devel] [PATCH] tools/ocaml: Build fix following libxl API changes Anthony PERARD
2019-09-20 17:00 ` Anthony PERARD
2019-09-20 17:15   ` Ian Jackson
2019-09-23  7:45     ` Jan Beulich
2019-09-23 10:14       ` Ian Jackson
2019-09-20 17:50 ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.