Re: [PATCH v2 0/3] tools/ocaml: Stabilize domain_getinfo for Oxenstored

[Christian Lindig <christian.lindig@xxxxxxxxx>]

There was some confusion about my ack for this series - my apologies. I 
intended to ack’ it entirely.

Acked-by: Christian Lindig <christian.lindig@xxxxxxxxx>


> On 3 Sep 2024, at 12:44, Andrii Sultanov <andrii.sultanov@xxxxxxxxx> wrote:
> 
> This is a V2 of the "Stabilize Oxenstored's interface with" patch
> series, see V1's cover letter for the motivation of these changes.
> 
> Two patches from V1 ("tools/ocaml: Fix OCaml libs rules" and
> "Remove '-cc $(CC)' from OCAMLOPTFLAGS") were commited upstream, so
> they've been dropped from here.
> 
> V2 addresses the following review comments and suggestions:
> 
> * Split the plugin implementation commit into two - build
>  infrastructure-related and implementation itself.
> * Package xenstored_glue interface, since it's needed for an
>  out-of-tree oxenstored to build. Additionally package xenstored_glue_dev
>  with .ml and .mli files.
> * Clean up unnecessary #define, #include, and fix function parameter
>  types as suggested.
> * Improve error handling in xsglue_failwith_xc, additionally version
>  xsg.error to avoid potential future conflicts.
> * Drop the GC lock around xc_domain_getinfo_single, and move Int_val
>  outside of the blocking_section.
> * Use existing logging facilities in oxenstored
> * Plugin now uses logging function provided by the plugin interface
>  (ignoring input by default)
> * domain_getinfolist gets all 32k domains at once instead of listing them
>  in chunks, storing information into an array. OCaml interface was simplified
>  appropriately. (Cxenstored does not do this at the moment - as far as I
>  understand, it also uses the single-domain version of the function).
> 
> I've decided against introducing an enum for the shutdown code, as it adds
> additional complexity (and potential reasons for creating a new plugin 
> version)
> without any benefit - oxenstored does not care about its value at the moment.
> 
> Patch series on Gitlab for ease of review:
> https://gitlab.com/xen-project/people/asultanov/xen/-/compare/staging...plugin-v3
> 
> V2 passed the Gitlab CI: 
> https://gitlab.com/xen-project/people/asultanov/xen/-/pipelines/1437391764
> It was also tested on some hosts.
> 
> Andrii Sultanov (3):
>  tools/ocaml: Build infrastructure for OCaml dynamic libraries
>  ocaml/libs: Implement a dynamically-loaded plugin for
>    Xenctrl.domain_getinfo
>  tools/oxenstored: Use the plugin for Xenctrl.domain_getinfo
> 
> Config.mk                                     |   2 +-
> configure                                     |   7 +
> m4/paths.m4                                   |   4 +
> tools/configure                               |   7 +
> tools/ocaml/Makefile                          |   1 +
> tools/ocaml/Makefile.rules                    |  17 +-
> tools/ocaml/libs/Makefile                     |   2 +-
> tools/ocaml/libs/xenstoredglue/META.in        |   4 +
> tools/ocaml/libs/xenstoredglue/Makefile       |  46 +++++
> .../domain_getinfo_plugin_v1/META.in          |   5 +
> .../domain_getinfo_plugin_v1/Makefile         |  38 ++++
> .../domain_getinfo_stubs_v1.c                 | 172 ++++++++++++++++++
> .../domain_getinfo_v1.ml                      |  36 ++++
> .../domain_getinfo_v1.mli                     |   0
> .../libs/xenstoredglue/plugin_interface_v1.ml |  28 +++
> .../xenstoredglue/plugin_interface_v1.mli     |  36 ++++
> tools/ocaml/xenstored/Makefile                |   5 +-
> tools/ocaml/xenstored/domains.ml              |  63 +++++--
> tools/ocaml/xenstored/paths.ml.in             |   1 +
> 19 files changed, 451 insertions(+), 23 deletions(-)
> create mode 100644 tools/ocaml/libs/xenstoredglue/META.in
> create mode 100644 tools/ocaml/libs/xenstoredglue/Makefile
> create mode 100644 
> tools/ocaml/libs/xenstoredglue/domain_getinfo_plugin_v1/META.in
> create mode 100644 
> tools/ocaml/libs/xenstoredglue/domain_getinfo_plugin_v1/Makefile
> create mode 100644 
> tools/ocaml/libs/xenstoredglue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c
> create mode 100644 
> tools/ocaml/libs/xenstoredglue/domain_getinfo_plugin_v1/domain_getinfo_v1.ml
> create mode 100644 
> tools/ocaml/libs/xenstoredglue/domain_getinfo_plugin_v1/domain_getinfo_v1.mli
> create mode 100644 tools/ocaml/libs/xenstoredglue/plugin_interface_v1.ml
> create mode 100644 tools/ocaml/libs/xenstoredglue/plugin_interface_v1.mli
> 
> -- 
> 2.39.2
>