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

Andrii Sultanov posted 3 patches 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/cover.1725363427.git.andrii.sultanov@cloud.com
There is a newer version of this series
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
[PATCH v2 0/3] tools/ocaml: Stabilize domain_getinfo for Oxenstored
Posted by Andrii Sultanov 3 months ago
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
Re: [PATCH v2 0/3] tools/ocaml: Stabilize domain_getinfo for Oxenstored
Posted by Christian Lindig 2 months, 4 weeks ago
There was some confusion about my ack for this series - my apologies. I intended to ack’ it entirely.

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


> On 3 Sep 2024, at 12:44, Andrii Sultanov <andrii.sultanov@cloud.com> 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
>