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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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