* [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field
@ 2024-02-07 22:04 Petr Beneš
2024-02-08 9:13 ` Christian Lindig
0 siblings, 1 reply; 7+ messages in thread
From: Petr Beneš @ 2024-02-07 22:04 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, Petr Beneš,
Christian Lindig, David Scott, Wei Liu, Anthony PERARD
From: Petr Beneš <w1benny@gmail.com>
Add the missing `vmtrace_buf_kb` field to the OCaml bindings to match the
vm.cfg configuration, correcting an oversight from its initial introduction.
Signed-off-by: Petr Beneš <w1benny@gmail.com>
---
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 13 +++++++++++--
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index d6c6eb73db..55923857ec 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -85,6 +85,7 @@ type domctl_create_config =
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+ vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
}
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 3bfc16edba..9b4b45db3a 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -77,6 +77,7 @@ type domctl_create_config = {
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+ vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
}
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 3703f48c74..2b6d3c09df 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -210,10 +210,17 @@ CAMLprim value stub_xc_domain_create(value xch_val, value wanted_domid, value co
#define VAL_MAX_GRANT_FRAMES Field(config, 6)
#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
#define VAL_MAX_GRANT_VERSION Field(config, 8)
-#define VAL_CPUPOOL_ID Field(config, 9)
-#define VAL_ARCH Field(config, 10)
+#define VAL_VMTRACE_BUF_KB Field(config, 9)
+#define VAL_CPUPOOL_ID Field(config, 10)
+#define VAL_ARCH Field(config, 11)
uint32_t domid = Int_val(wanted_domid);
+ uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
+
+ vmtrace_size = ROUNDUP(vmtrace_size << 10, XC_PAGE_SHIFT);
+ if ( vmtrace_size != (uint32_t)vmtrace_size )
+ caml_invalid_argument("vmtrace_buf_kb");
+
int result;
struct xen_domctl_createdomain cfg = {
.ssidref = Int32_val(VAL_SSIDREF),
@@ -223,6 +230,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value wanted_domid, value co
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
+ .vmtrace_size = vmtrace_size,
.cpupool_id = Int32_val(VAL_CPUPOOL_ID),
};
@@ -279,6 +287,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value wanted_domid, value co
#undef VAL_ARCH
#undef VAL_CPUPOOL_ID
+#undef VAL_VMTRACE_BUF_KB
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
#undef VAL_MAX_GRANT_FRAMES
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field
2024-02-07 22:04 [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field Petr Beneš
@ 2024-02-08 9:13 ` Christian Lindig
2024-02-08 21:35 ` Petr Beneš
2024-02-14 7:11 ` Jan Beulich
0 siblings, 2 replies; 7+ messages in thread
From: Christian Lindig @ 2024-02-08 9:13 UTC (permalink / raw)
To: Petr Beneš
Cc: Xen-devel, Andrew Cooper, David Scott, Wei Liu, Anthony PERARD
> On 7 Feb 2024, at 22:04, Petr Beneš <w1benny@gmail.com> wrote:
>
>
> Add the missing `vmtrace_buf_kb` field to the OCaml bindings to match the
> vm.cfg configuration, correcting an oversight from its initial introduction.
>
> Signed-off-by: Petr Beneš <w1benny@gmail.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
This looks correct from an OCaml perspective. Why was the new field added in the middle of the record type domctl_create_config and thus forcing changes to the index of fields coming later in the record versus just appending the new field to the record type?
The critical bit is using the correct type in "Int32_val(VAL_VMTRACE_BUF_KB)” that matches the type "vmtrace_buf_kb: int32;” - which it does.
—- C
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field
2024-02-08 9:13 ` Christian Lindig
@ 2024-02-08 21:35 ` Petr Beneš
2024-02-14 7:11 ` Jan Beulich
1 sibling, 0 replies; 7+ messages in thread
From: Petr Beneš @ 2024-02-08 21:35 UTC (permalink / raw)
To: Christian Lindig
Cc: Xen-devel, Andrew Cooper, David Scott, Wei Liu, Anthony PERARD
> On Thu, Feb 8, 2024 at 10:14 AM Christian Lindig <christian.lindig@cloud.com> wrote:
>
> > On 7 Feb 2024, at 22:04, Petr Beneš <w1benny@gmail.com> wrote:
> >
> >
> > Add the missing `vmtrace_buf_kb` field to the OCaml bindings to match the
> > vm.cfg configuration, correcting an oversight from its initial introduction.
> >
> > Signed-off-by: Petr Beneš <w1benny@gmail.com>
>
> Acked-by: Christian Lindig <christian.lindig@cloud.com>
>
> This looks correct from an OCaml perspective. Why was the new field added in the middle of the record type domctl_create_config and thus forcing changes to the index of fields coming later in the record versus just appending the new field to the record type?
>
To match the position of the field inside of xen_domctl_createdomain.
P.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field
2024-02-08 9:13 ` Christian Lindig
2024-02-08 21:35 ` Petr Beneš
@ 2024-02-14 7:11 ` Jan Beulich
2024-02-14 11:30 ` Petr Beneš
2024-02-14 11:45 ` Andrew Cooper
1 sibling, 2 replies; 7+ messages in thread
From: Jan Beulich @ 2024-02-14 7:11 UTC (permalink / raw)
To: Christian Lindig, Petr Beneš
Cc: Xen-devel, Andrew Cooper, David Scott, Wei Liu, Anthony PERARD
On 08.02.2024 10:13, Christian Lindig wrote:
>> On 7 Feb 2024, at 22:04, Petr Beneš <w1benny@gmail.com> wrote:
>> Add the missing `vmtrace_buf_kb` field to the OCaml bindings to match the
>> vm.cfg configuration, correcting an oversight from its initial introduction.
>>
>> Signed-off-by: Petr Beneš <w1benny@gmail.com>
>
> Acked-by: Christian Lindig <christian.lindig@cloud.com>
>
> This looks correct from an OCaml perspective. Why was the new field added in the middle of the record type domctl_create_config and thus forcing changes to the index of fields coming later in the record versus just appending the new field to the record type?
>
> The critical bit is using the correct type in "Int32_val(VAL_VMTRACE_BUF_KB)” that matches the type "vmtrace_buf_kb: int32;” - which it does.
Is this then perhaps also lacking a
Fixes: 45ba9a7d7688 ("tools/[lib]xl: Add vmtrace_buf_size parameter")
and hence wanting backporting?
Jan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field
2024-02-14 7:11 ` Jan Beulich
@ 2024-02-14 11:30 ` Petr Beneš
2024-02-14 11:45 ` Andrew Cooper
1 sibling, 0 replies; 7+ messages in thread
From: Petr Beneš @ 2024-02-14 11:30 UTC (permalink / raw)
To: Jan Beulich
Cc: Christian Lindig, Xen-devel, Andrew Cooper, David Scott, Wei Liu,
Anthony PERARD
On Wed, Feb 14, 2024 at 8:12 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.02.2024 10:13, Christian Lindig wrote:
> >> On 7 Feb 2024, at 22:04, Petr Beneš <w1benny@gmail.com> wrote:
> >> Add the missing `vmtrace_buf_kb` field to the OCaml bindings to match the
> >> vm.cfg configuration, correcting an oversight from its initial introduction.
> >>
> >> Signed-off-by: Petr Beneš <w1benny@gmail.com>
> >
> > Acked-by: Christian Lindig <christian.lindig@cloud.com>
> >
> > This looks correct from an OCaml perspective. Why was the new field added in the middle of the record type domctl_create_config and thus forcing changes to the index of fields coming later in the record versus just appending the new field to the record type?
> >
> > The critical bit is using the correct type in "Int32_val(VAL_VMTRACE_BUF_KB)” that matches the type "vmtrace_buf_kb: int32;” - which it does.
>
> Is this then perhaps also lacking a
>
> Fixes: 45ba9a7d7688 ("tools/[lib]xl: Add vmtrace_buf_size parameter")
>
> and hence wanting backporting?
>
> Jan
In my opinion, yes.
P.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field
2024-02-14 7:11 ` Jan Beulich
2024-02-14 11:30 ` Petr Beneš
@ 2024-02-14 11:45 ` Andrew Cooper
2024-02-14 13:30 ` Christian Lindig
1 sibling, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2024-02-14 11:45 UTC (permalink / raw)
To: Jan Beulich, Christian Lindig, Petr Beneš
Cc: Xen-devel, David Scott, Wei Liu, Anthony PERARD
On 14/02/2024 7:11 am, Jan Beulich wrote:
> On 08.02.2024 10:13, Christian Lindig wrote:
>>> On 7 Feb 2024, at 22:04, Petr Beneš <w1benny@gmail.com> wrote:
>>> Add the missing `vmtrace_buf_kb` field to the OCaml bindings to match the
>>> vm.cfg configuration, correcting an oversight from its initial introduction.
>>>
>>> Signed-off-by: Petr Beneš <w1benny@gmail.com>
>> Acked-by: Christian Lindig <christian.lindig@cloud.com>
>>
>> This looks correct from an OCaml perspective. Why was the new field added in the middle of the record type domctl_create_config and thus forcing changes to the index of fields coming later in the record versus just appending the new field to the record type?
>>
>> The critical bit is using the correct type in "Int32_val(VAL_VMTRACE_BUF_KB)” that matches the type "vmtrace_buf_kb: int32;” - which it does.
> Is this then perhaps also lacking a
>
> Fixes: 45ba9a7d7688 ("tools/[lib]xl: Add vmtrace_buf_size parameter")
>
> and hence wanting backporting?
It ought to have been part of that at the time, but I suggest not
backporting.
API changes in Ocaml are all breaking (module compatibility is checked
by hash).
Xapi is the only consumer of this interface. I've fixed up the build
against staging, but we're not going to be running KFX under Xapi any
time soon.
Ultimately it's Christian's call.
~Andrew
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field
2024-02-14 11:45 ` Andrew Cooper
@ 2024-02-14 13:30 ` Christian Lindig
0 siblings, 0 replies; 7+ messages in thread
From: Christian Lindig @ 2024-02-14 13:30 UTC (permalink / raw)
To: Andrew Cooper
Cc: Jan Beulich, Petr Beneš,
Xen-devel, David Scott, Wei Liu, Anthony PERARD
> On 14 Feb 2024, at 11:45, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> Xapi is the only consumer of this interface. I've fixed up the build
> against staging, but we're not going to be running KFX under Xapi any
> time soon.
>
> Ultimately it's Christian's call.
After a discussion with Andrew, we will not back port for now as we expect Xapi to use a new version that has it when it becomes relevant.
— C
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-02-14 13:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-07 22:04 [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field Petr Beneš
2024-02-08 9:13 ` Christian Lindig
2024-02-08 21:35 ` Petr Beneš
2024-02-14 7:11 ` Jan Beulich
2024-02-14 11:30 ` Petr Beneš
2024-02-14 11:45 ` Andrew Cooper
2024-02-14 13:30 ` Christian Lindig
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.