MVD CP-52334 multi-version driver API/CLI - small changes

Address review comments

Signed-off-by: Christian Lindig <[email protected]>

doc/content/toolstack/features/MVD/index.md

@@ -1,4 +1,4 @@
-'++
++++
 title = "Multi-version drivers"
 +++
 

ocaml/idl/datamodel_driver_variant.ml

@@ -16,8 +16,6 @@ open Datamodel_types
 open Datamodel_common
 open Datamodel_roles
 
-(** This is pure data with no methods *)
-
 let select =
   call ~name:"select" ~in_oss_since:None ~lifecycle:[]
     ~doc:
@@ -45,7 +43,7 @@ let t =
       ; field ~lifecycle:[] ~qualifier:DynamicRO ~ty:(Ref _host_driver) "driver"
           "Driver this variant is a part of"
       ; field ~lifecycle:[] ~qualifier:DynamicRO ~ty:String "version"
-          "Unique versions of this driver variant"
+          "Unique version of this driver variant"
       ; field ~lifecycle:[] ~qualifier:DynamicRO ~ty:Bool "hardware_present"
           "True if the hardware for this variant is present on the host"
       ; field ~lifecycle:[] ~qualifier:DynamicRO ~ty:Float "priority"

ocaml/xapi-cli-server/cli_operations.ml

@@ -29,7 +29,7 @@ open Records
 
 let failwith str = raise (Cli_util.Cli_failure str)
 
-let failwith' fmt = Printf.ksprintf failwith fmt
+let failwithfmt fmt = Printf.ksprintf failwith fmt
 
 exception ExitWithError of int
 
@@ -8012,7 +8012,7 @@ module Host_driver = struct
 
     match List.find_opt by_name variants with
     | None ->
-        failwith' "%s does not identify a variant of this driver" name
+        failwithfmt "%s does not identify a variant of this driver" name
     | Some (variant, _) ->
         Client.Host_driver.select ~rpc ~session_id ~self:driver ~variant
 

ocaml/xapi/message_forwarding.ml

@@ -6610,7 +6610,8 @@ functor
     module Host_driver = struct
       (** select needs to be executed on the host of the driver *)
       let select ~__context ~self ~variant =
-        info "%s" __FUNCTION__ ;
+        info "Host_driver.select %s %s" (Ref.string_of self)
+          (Ref.string_of variant) ;
         let host = Db.Host_driver.get_host ~__context ~self in
         let local_fn = Local.Host_driver.select ~self ~variant in
         do_op_on ~__context ~local_fn ~host (fun session_id rpc ->
@@ -6619,15 +6620,15 @@ functor
 
       (** deselect needs to be executed on the host of the driver *)
       let deselect ~__context ~self =
-        info "%s" __FUNCTION__ ;
+        info "Host_driver.deselect %s" (Ref.string_of self) ;
         let host = Db.Host_driver.get_host ~__context ~self in
         let local_fn = Local.Host_driver.deselect ~self in
         do_op_on ~__context ~local_fn ~host (fun session_id rpc ->
             Client.Host_driver.deselect ~rpc ~session_id ~self
         )
 
       let rescan ~__context ~host =
-        info "%s" __FUNCTION__ ;
+        info "Host_driver.rescan %s" (Ref.string_of host) ;
         let local_fn = Local.Host_driver.rescan ~host in
         do_op_on ~__context ~local_fn ~host (fun session_id rpc ->
             Client.Host_driver.rescan ~rpc ~session_id ~host
@@ -6636,7 +6637,7 @@ functor
 
     module Driver_variant = struct
       let select ~__context ~self =
-        info "%s" __FUNCTION__ ;
+        info "Driver_variant.select %s" (Ref.string_of self) ;
         let drv = Db.Driver_variant.get_driver ~__context ~self in
         let host = Db.Host_driver.get_host ~__context ~self:drv in
         let local_fn = Local.Driver_variant.select ~self in

ocaml/xapi/xapi_host_driver.ml

@@ -16,12 +16,7 @@ module D = Debug.Make (struct let name = __MODULE__ end)
 
 open D
 module Unixext = Xapi_stdext_unix.Unixext
-
-(*
-module DriverMap = Map.Make (String)
-module DriverSet = Set.Make (String)
-*)
-module T = Xapi_host_driver_tool
+module Tool = Xapi_host_driver_tool
 
 let invalid_value field value =
   raise Api_errors.(Server_error (invalid_value, [field; value]))
@@ -34,16 +29,6 @@ let internal_error fmt =
     )
     fmt
 
-let drivertool args =
-  let path = !Xapi_globs.driver_tool in
-  try
-    let stdout, _stderr = Forkhelpers.execute_command_get_output path args in
-    debug "%s: executed %s %s" __FUNCTION__ path (String.concat " " args) ;
-    stdout
-  with e ->
-    internal_error "%s: failed to run %s %s: %s" __FUNCTION__ path
-      (String.concat " " args) (Printexc.to_string e)
-
 module Variant = struct
   let create ~__context ~name ~version ~driver ~hw_present ~priority ~dev_status
       =
@@ -55,7 +40,7 @@ module Variant = struct
     ref
 
   let destroy ~__context ~self =
-    debug "Destroying driver variant %s" (Ref.string_of self) ;
+    debug "%s: destroying driver variant %s" __FUNCTION__ (Ref.string_of self) ;
     Db.Driver_variant.destroy ~__context ~self
 
   (** create' is like create but updates an exisiting entry if it
@@ -92,7 +77,7 @@ module Variant = struct
     let d = Db.Host_driver.get_record ~__context ~self:drv in
     let v = Db.Driver_variant.get_record ~__context ~self in
     let stdout =
-      drivertool ["select"; d.API.host_driver_name; v.API.driver_variant_name]
+      Tool.call ["select"; d.API.host_driver_name; v.API.driver_variant_name]
     in
     info "%s: %s" __FUNCTION__ stdout ;
     Db.Host_driver.set_selected_variant ~__context ~self:drv ~value:self
@@ -156,7 +141,7 @@ let select ~__context ~self ~variant =
       let d = Db.Host_driver.get_record ~__context ~self in
       let v = Db.Driver_variant.get_record ~__context ~self:variant in
       let stdout =
-        drivertool ["select"; d.API.host_driver_name; v.API.driver_variant_name]
+        Tool.call ["select"; d.API.host_driver_name; v.API.driver_variant_name]
       in
       info "%s: %s" __FUNCTION__ stdout ;
       Db.Host_driver.set_selected_variant ~__context ~self ~value:variant
@@ -168,7 +153,7 @@ let select ~__context ~self ~variant =
 let deselect ~__context ~self =
   D.debug "%s  driver %s" __FUNCTION__ (Ref.string_of self) ;
   let d = Db.Host_driver.get_record ~__context ~self in
-  let stdout = drivertool ["deselect"; d.API.host_driver_name] in
+  let stdout = Tool.call ["deselect"; d.API.host_driver_name] in
   info "%s: %s" __FUNCTION__ stdout ;
   Db.Host_driver.set_active_variant ~__context ~self ~value:Ref.null ;
   Db.Host_driver.set_selected_variant ~__context ~self ~value:Ref.null
@@ -186,32 +171,33 @@ let remove ~__context ~host ~except =
 (** Runs on [host]. We update or create an entry for each driver
     reported by drivertool and remove any extra driver that is in xapi. *)
 let scan ~__context ~host =
-  T.Mock.install () ;
+  Tool.Mock.install () ;
   let null = Ref.null in
   let drivers (* on this host *) =
-    drivertool ["list"]
-    |> T.parse
+    Tool.call ["list"]
+    |> Tool.parse
     |> List.map @@ fun (_name, driver) ->
        let driver_ref =
-         create' ~__context ~host ~name:driver.T.name
-           ~friendly_name:driver.T.name ~info:driver.T.info ~active_variant:null
-           ~selected_variant:null ~description:driver.T.descr ~_type:driver.T.ty
+         create' ~__context ~host ~name:driver.Tool.name
+           ~friendly_name:driver.Tool.name ~info:driver.Tool.info
+           ~active_variant:null ~selected_variant:null
+           ~description:driver.Tool.descr ~_type:driver.Tool.ty
        in
-       (driver.T.variants
+       (driver.Tool.variants
        |> List.iter @@ fun (name, v) ->
           let var_ref =
-            Variant.create' ~__context ~name ~version:v.T.version
-              ~driver:driver_ref ~hw_present:v.T.hw_present
-              ~priority:v.T.priority ~dev_status:v.T.dev_status
+            Variant.create' ~__context ~name ~version:v.Tool.version
+              ~driver:driver_ref ~hw_present:v.Tool.hw_present
+              ~priority:v.Tool.priority ~dev_status:v.Tool.dev_status
           in
-          ( match driver.T.selected with
+          ( match driver.Tool.selected with
           | Some v when v = name ->
               Db.Host_driver.set_selected_variant ~__context ~self:driver_ref
                 ~value:var_ref
           | _ ->
               ()
           ) ;
-          match driver.T.active with
+          match driver.Tool.active with
           | Some v when v = name ->
               Db.Host_driver.set_active_variant ~__context ~self:driver_ref
                 ~value:var_ref

ocaml/xapi/xapi_host_driver_tool.ml

@@ -16,9 +16,7 @@
    host-driver-tool that manages multi-version drivers and reports
    the state of them in JSON *)
 
-module D = Debug.Make (struct let name = __MODULE__ end)
-
-open D
+open Debug.Make (struct let name = __MODULE__ end)
 
 let internal_error fmt =
   Printf.ksprintf
@@ -234,6 +232,16 @@ let read path =
   | exception e ->
       raise e
 
+let call args =
+  let path = !Xapi_globs.driver_tool in
+  try
+    let stdout, _stderr = Forkhelpers.execute_command_get_output path args in
+    debug "%s: executed %s %s" __FUNCTION__ path (String.concat " " args) ;
+    stdout
+  with e ->
+    internal_error "%s: failed to run %s %s: %s" __FUNCTION__ path
+      (String.concat " " args) (Printexc.to_string e)
+
 module Mock = struct
   let drivertool_sh =
     {|#!/usr/bin/env bash

ocaml/xapi/xapi_host_driver_tool.mli

@@ -37,6 +37,9 @@ val parse : string -> (string * driver) list
 val read : string -> (string * driver) list
 (** read from a file whose path is provided *)
 
+val call : string list -> string
+(** invoke drivertool with argumtns and return stdout *)
+
 (** install a mock drivertool.sh *)
 module Mock : sig
   val install : unit -> unit