[PATCH v1 3/4] tools/oxenstored: Use the plugin for Xenctrl.domain_getinfo

Andrii Sultanov posted 4 patches 3 months ago
[PATCH v1 3/4] tools/oxenstored: Use the plugin for Xenctrl.domain_getinfo
Posted by Andrii Sultanov 3 months ago
Oxenstored dynamically loads the plugin provided in
ocaml/libs/xenstoredglue.
The plugin is verified to be providing the specified plugin_interface
during its loading.

If a V2 of the plugin is produced, V1 will still be present, and a new
version should only be loaded if it's verified to exist
(New oxenstored can run in an environment with only V1 of the plugin).

Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
---
 Config.mk                         |  2 +-
 configure                         |  7 ++++
 m4/paths.m4                       |  4 ++
 tools/configure                   |  7 ++++
 tools/ocaml/xenstored/Makefile    |  5 ++-
 tools/ocaml/xenstored/domains.ml  | 63 +++++++++++++++++++++----------
 tools/ocaml/xenstored/paths.ml.in |  1 +
 7 files changed, 68 insertions(+), 21 deletions(-)

diff --git a/Config.mk b/Config.mk
index 1a3938b6c4..4be7d8a573 100644
--- a/Config.mk
+++ b/Config.mk
@@ -160,7 +160,7 @@ endef
 BUILD_MAKE_VARS := sbindir bindir LIBEXEC LIBEXEC_BIN libdir SHAREDIR \
                    XENFIRMWAREDIR XEN_CONFIG_DIR XEN_SCRIPT_DIR XEN_LOCK_DIR \
                    XEN_RUN_DIR XEN_PAGING_DIR XEN_DUMP_DIR XEN_LOG_DIR \
-                   XEN_LIB_DIR XEN_RUN_STORED
+                   XEN_LIB_DIR XEN_RUN_STORED XEN_CTRL_DOMAININFO_PLUGIN
 
 buildmakevars2file = $(eval $(call buildmakevars2file-closure,$(1)))
 define buildmakevars2file-closure
diff --git a/configure b/configure
index 2d7b20aa50..1a473ad2dd 100755
--- a/configure
+++ b/configure
@@ -631,6 +631,7 @@ XEN_PAGING_DIR
 XEN_LOCK_DIR
 INITD_DIR
 SHAREDIR
+XEN_CTRL_DOMAININFO_PLUGIN
 XEN_LIB_DIR
 XEN_RUN_STORED
 XEN_LOG_DIR
@@ -2199,6 +2200,12 @@ XEN_LIB_DIR=$localstatedir/lib/xen
 printf "%s\n" "#define XEN_LIB_DIR \"$XEN_LIB_DIR\"" >>confdefs.h
 
 
+XEN_CTRL_DOMAININFO_PLUGIN=$LIBEXEC_BIN/xenctrl_plugin
+
+
+printf "%s\n" "#define XEN_CTRL_DOMAININFO_PLUGIN \"$XEN_CTRL_DOMAININFO_PLUGIN\"" >>confdefs.h
+
+
 SHAREDIR=$prefix/share
 
 
diff --git a/m4/paths.m4 b/m4/paths.m4
index 3f94c62efb..e538445810 100644
--- a/m4/paths.m4
+++ b/m4/paths.m4
@@ -144,6 +144,10 @@ XEN_LIB_DIR=$localstatedir/lib/xen
 AC_SUBST(XEN_LIB_DIR)
 AC_DEFINE_UNQUOTED([XEN_LIB_DIR], ["$XEN_LIB_DIR"], [Xen's lib dir])
 
+XEN_CTRL_DOMAININFO_PLUGIN=$LIBEXEC_BIN/xenctrl_plugin
+AC_SUBST(XEN_CTRL_DOMAININFO_PLUGIN)
+AC_DEFINE_UNQUOTED([XEN_CTRL_DOMAININFO_PLUGIN], ["$XEN_CTRL_DOMAININFO_PLUGIN"], [Xenctrl's plugin for Oxenstored])
+
 SHAREDIR=$prefix/share
 AC_SUBST(SHAREDIR)
 
diff --git a/tools/configure b/tools/configure
index 7f98303fdd..24b12a1f5d 100755
--- a/tools/configure
+++ b/tools/configure
@@ -743,6 +743,7 @@ XEN_PAGING_DIR
 XEN_LOCK_DIR
 INITD_DIR
 SHAREDIR
+XEN_CTRL_DOMAININFO_PLUGIN
 XEN_LIB_DIR
 XEN_RUN_STORED
 XEN_LOG_DIR
@@ -4530,6 +4531,12 @@ XEN_LIB_DIR=$localstatedir/lib/xen
 printf "%s\n" "#define XEN_LIB_DIR \"$XEN_LIB_DIR\"" >>confdefs.h
 
 
+XEN_CTRL_DOMAININFO_PLUGIN=$LIBEXEC_BIN/xenctrl_plugin
+
+
+printf "%s\n" "#define XEN_CTRL_DOMAININFO_PLUGIN \"$XEN_CTRL_DOMAININFO_PLUGIN\"" >>confdefs.h
+
+
 SHAREDIR=$prefix/share
 
 
diff --git a/tools/ocaml/xenstored/Makefile b/tools/ocaml/xenstored/Makefile
index fa45305d8c..09896732ed 100644
--- a/tools/ocaml/xenstored/Makefile
+++ b/tools/ocaml/xenstored/Makefile
@@ -15,7 +15,8 @@ OCAMLINCLUDE += \
 	-I $(OCAML_TOPLEVEL)/libs/xb \
 	-I $(OCAML_TOPLEVEL)/libs/mmap \
 	-I $(OCAML_TOPLEVEL)/libs/xc \
-	-I $(OCAML_TOPLEVEL)/libs/eventchn
+	-I $(OCAML_TOPLEVEL)/libs/eventchn \
+	-I $(OCAML_TOPLEVEL)/libs/xenstoredglue
 
 LIBS = syslog.cma syslog.cmxa poll.cma poll.cmxa
 syslog_OBJS = syslog
@@ -59,6 +60,7 @@ INTF = symbol.cmi trie.cmi syslog.cmi systemd.cmi poll.cmi
 
 XENSTOREDLIBS = \
 	unix.cmxa \
+	dynlink.cmxa \
 	-ccopt -L -ccopt . syslog.cmxa \
 	-ccopt -L -ccopt . systemd.cmxa \
 	-ccopt -L -ccopt . poll.cmxa \
@@ -66,6 +68,7 @@ XENSTOREDLIBS = \
 	-ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/eventchn $(OCAML_TOPLEVEL)/libs/eventchn/xeneventchn.cmxa \
 	-ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/xc $(OCAML_TOPLEVEL)/libs/xc/xenctrl.cmxa \
 	-ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/xb $(OCAML_TOPLEVEL)/libs/xb/xenbus.cmxa \
+	-ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/xenstoredglue $(OCAML_TOPLEVEL)/libs/xenstoredglue/plugin_interface_v1.cmxa \
 	-ccopt -L -ccopt $(XEN_ROOT)/tools/libs/ctrl
 
 PROGRAMS = oxenstored
diff --git a/tools/ocaml/xenstored/domains.ml b/tools/ocaml/xenstored/domains.ml
index 7a3056c364..e3edee6de6 100644
--- a/tools/ocaml/xenstored/domains.ml
+++ b/tools/ocaml/xenstored/domains.ml
@@ -20,10 +20,36 @@ let warn fmt  = Logging.warn  "domains" fmt
 
 let xc = Xenctrl.interface_open ()
 
-type domains = {
-  eventchn: Event.t;
-  table: (Xenctrl.domid, Domain.t) Hashtbl.t;
+let load_plug fname =
+  let fail_with fmt = Printf.ksprintf failwith fmt in
+  let fname = Dynlink.adapt_filename fname in
+  if Sys.file_exists fname then
+    try Dynlink.loadfile fname with
+    | Dynlink.Error err as e ->
+        Printf.eprintf "ERROR loading plugin '%s': %s\n%!" fname
+          (Dynlink.error_message err);
+        raise e
+    | _ -> fail_with "Unknown error while loading plugin"
+  else fail_with "Plugin file '%s' does not exist" fname
+
+let () =
+  let filepath = Paths.xen_ctrl_plugin ^ "/domain_getinfo_v1.cmxs" in
+  Printf.printf "Trying to load plugin '%s'\n%!" filepath;
+  let list_files = Sys.readdir Paths.xen_ctrl_plugin in
+  Printf.printf "Directory listing of '%s'\n%!" Paths.xen_ctrl_plugin;
+  Array.iter (fun x -> Printf.printf "\t%s\n%!" x) list_files;
+  Dynlink.allow_only [ "Plugin_interface_v1" ];
+  load_plug filepath
+
+module Plugin =
+  (val Plugin_interface_v1.get_plugin_v1 ()
+      : Plugin_interface_v1.Domain_getinfo_V1)
+
+let handle = Plugin.interface_open ()
 
+type domains = {
+  eventchn : Event.t;
+  table : (Plugin.domid, Domain.t) Hashtbl.t;
   (* N.B. the Queue module is not thread-safe but oxenstored is single-threaded. *)
   (* Domains queue up to regain conflict-credit; we have a queue for
      	   domains that are carrying some penalty and so are below the
@@ -93,22 +119,21 @@ let cleanup doms =
   let notify = ref false in
   let dead_dom = ref [] in
 
-  Hashtbl.iter (fun id _ -> if id <> 0 then
-                   try
-                     let info = Xenctrl.domain_getinfo xc id in
-                     if info.Xenctrl.shutdown || info.Xenctrl.dying then (
-                       debug "Domain %u died (dying=%b, shutdown %b -- code %d)"
-                         id info.Xenctrl.dying info.Xenctrl.shutdown info.Xenctrl.shutdown_code;
-                       if info.Xenctrl.dying then
-                         dead_dom := id :: !dead_dom
-                       else
-                         notify := true;
-                     )
-                   with Xenctrl.Error _ ->
-                     debug "Domain %u died -- no domain info" id;
-                     dead_dom := id :: !dead_dom;
-               ) doms.table;
-  List.iter (fun id ->
+  Hashtbl.iter
+    (fun id _ ->
+      if id <> 0 then (
+        try
+          let info = Plugin.domain_getinfo handle id in
+          if info.Plugin.shutdown || info.Plugin.dying then (
+            debug "Domain %u died (dying=%b, shutdown %b -- code %d)" id
+              info.Plugin.dying info.Plugin.shutdown info.Plugin.shutdown_code;
+            if info.Plugin.dying then dead_dom := id :: !dead_dom else notify := true)
+        with Plugin.Error _ ->
+          debug "Domain %u died -- no domain info" id;
+          dead_dom := id :: !dead_dom))
+    doms.table;
+  List.iter
+    (fun id ->
       let dom = Hashtbl.find doms.table id in
       Domain.close dom;
       Hashtbl.remove doms.table id;
diff --git a/tools/ocaml/xenstored/paths.ml.in b/tools/ocaml/xenstored/paths.ml.in
index 37949dc8f3..67276dda94 100644
--- a/tools/ocaml/xenstored/paths.ml.in
+++ b/tools/ocaml/xenstored/paths.ml.in
@@ -2,3 +2,4 @@ let xen_log_dir = "@XEN_LOG_DIR@"
 let xen_config_dir = "@XEN_CONFIG_DIR@"
 let xen_run_dir = "@XEN_RUN_DIR@"
 let xen_run_stored = "@XEN_RUN_STORED@"
+let xen_ctrl_plugin = "@XEN_CTRL_DOMAININFO_PLUGIN@"
-- 
2.39.2
Re: [PATCH v1 3/4] tools/oxenstored: Use the plugin for Xenctrl.domain_getinfo
Posted by Andrew Cooper 3 months ago
On 22/08/2024 10:06 am, Andrii Sultanov wrote:
> diff --git a/tools/ocaml/xenstored/domains.ml b/tools/ocaml/xenstored/domains.ml
> index 7a3056c364..e3edee6de6 100644
> --- a/tools/ocaml/xenstored/domains.ml
> +++ b/tools/ocaml/xenstored/domains.ml
> @@ -20,10 +20,36 @@ let warn fmt  = Logging.warn  "domains" fmt
>  
>  let xc = Xenctrl.interface_open ()
>  
> -type domains = {
> -  eventchn: Event.t;
> -  table: (Xenctrl.domid, Domain.t) Hashtbl.t;
> +let load_plug fname =
> +  let fail_with fmt = Printf.ksprintf failwith fmt in
> +  let fname = Dynlink.adapt_filename fname in
> +  if Sys.file_exists fname then
> +    try Dynlink.loadfile fname with
> +    | Dynlink.Error err as e ->
> +        Printf.eprintf "ERROR loading plugin '%s': %s\n%!" fname
> +          (Dynlink.error_message err);
> +        raise e
> +    | _ -> fail_with "Unknown error while loading plugin"
> +  else fail_with "Plugin file '%s' does not exist" fname
> +
> +let () =
> +  let filepath = Paths.xen_ctrl_plugin ^ "/domain_getinfo_v1.cmxs" in
> +  Printf.printf "Trying to load plugin '%s'\n%!" filepath;
> +  let list_files = Sys.readdir Paths.xen_ctrl_plugin in
> +  Printf.printf "Directory listing of '%s'\n%!" Paths.xen_ctrl_plugin;
> +  Array.iter (fun x -> Printf.printf "\t%s\n%!" x) list_files;
> +  Dynlink.allow_only [ "Plugin_interface_v1" ];
> +  load_plug filepath
> +
> +module Plugin =
> +  (val Plugin_interface_v1.get_plugin_v1 ()
> +      : Plugin_interface_v1.Domain_getinfo_V1)
> +
> +let handle = Plugin.interface_open ()
>  
> +type domains = {
> +  eventchn : Event.t;
> +  table : (Plugin.domid, Domain.t) Hashtbl.t;

Looking at just this hunk in isolation, I think you want to split this
patch into two; one to load the plugin, and a second to switch away from
using Xenctrl.domain_getinfo.

As this is Oxenstored, rather than a library, assumptions about stdout
are less bad, but it would still be nice if the logging could be plumbed
into the main logging functionality.  Or, does this all run so early on
boot that we haven't parsed oxenstored.conf yet?

~Andrew