| Message ID | cover.1725363427.git.andrii.sultanov@cloud.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | tools/ocaml: Stabilize domain_getinfo for Oxenstored | expand |
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 >
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