MVD CP-52334 multi-version driver API/CLI - incremental update

When scanning a host for drivers, don't remove and re-create them but instead update an entry if it already exists. Signed-off-by: Christian Lindig <[email protected]>
xapi-project · Jan 10, 2025 · 26eb2b0 · 26eb2b0
1 parent faf52c1
commit 26eb2b0
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 84 deletions.
diff --git a/doc/content/toolstack/features/MVD/index.md b/doc/content/toolstack/features/MVD/index.md
@@ -1,4 +1,4 @@
-'++
++++
 title = "Multi-version drivers"
 +++
 

diff --git a/ocaml/idl/datamodel_driver_variant.ml b/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"

diff --git a/ocaml/idl/datamodel_lifecycle.ml b/ocaml/idl/datamodel_lifecycle.ml
@@ -158,7 +158,7 @@ let prototyped_of_message = function
   | "Driver_variant", "select" ->
       Some "24.39.0"
   | "Host_driver", "rescan" ->
-      Some "24.39.0-next"
+      Some "24.40.0"
   | "Host_driver", "deselect" ->
       Some "24.35.0"
   | "Host_driver", "select" ->

diff --git a/ocaml/idl/schematest.ml b/ocaml/idl/schematest.ml
@@ -3,7 +3,7 @@ let hash x = Digest.string x |> Digest.to_hex
 (* BEWARE: if this changes, check that schema has been bumped accordingly in
    ocaml/idl/datamodel_common.ml, usually schema_minor_vsn *)
 
-let last_known_schema_hash = "863c257bad0d20800297cf968c294e4e"
+let last_known_schema_hash = "36cb241822399344b119cce654162d3f"
 
 let current_schema_hash : string =
   let open Datamodel_types in

diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/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
 

diff --git a/ocaml/xapi/message_forwarding.ml b/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

diff --git a/ocaml/xapi/xapi_host_driver.ml b/ocaml/xapi/xapi_host_driver.ml
@@ -16,34 +16,11 @@ 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]))
 
-let internal_error fmt =
-  Printf.ksprintf
-    (fun msg ->
-      error "%s" msg ;
-      raise Api_errors.(Server_error (internal_error, [msg]))
-    )
-    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 +32,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 +69,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
@@ -126,15 +103,16 @@ let create' ~__context ~host ~name ~friendly_name ~_type ~description ~info:inf
   match Db.Host_driver.get_refs_where ~__context ~expr with
   | [] ->
       (* no such entry exists - create it *)
-      create ~__context ~host ~name ~friendly_name ~info:inf
-        ~active_variant:null ~selected_variant:null ~description ~_type
+      create ~__context ~host ~name ~friendly_name ~info:inf ~active_variant
+        ~selected_variant ~description ~_type
   | [self] ->
       (* one existing entry - update it *)
       info "%s: updating host driver %s" __FUNCTION__ name ;
       Db.Host_driver.set_friendly_name ~__context ~self ~value:name ;
       Db.Host_driver.set_info ~__context ~self ~value:inf ;
-      Db.Host_driver.set_active_variant ~__context ~self ~value:null ;
-      Db.Host_driver.set_selected_variant ~__context ~self ~value:null ;
+      Db.Host_driver.set_active_variant ~__context ~self ~value:active_variant ;
+      Db.Host_driver.set_selected_variant ~__context ~self
+        ~value:selected_variant ;
       Db.Host_driver.set_description ~__context ~self ~value:description ;
       Db.Host_driver.set_type ~__context ~self ~value:_type ;
       self
@@ -156,7 +134,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,51 +146,60 @@ 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
 
-(** remove all host driver entries for this host *)
-let reset ~__context ~host =
+(** remove all host driver entries  that are not in [except]. We exepect
+    any list to be short *)
+let remove ~__context ~host ~except =
   D.debug "%s" __FUNCTION__ ;
   let open Xapi_database.Db_filter_types in
   let expr = Eq (Field "host", Literal (Ref.string_of host)) in
-  let drivers = Db.Host_driver.get_refs_where ~__context ~expr in
-  drivers |> List.iter (fun self -> destroy ~__context ~self)
+  Db.Host_driver.get_refs_where ~__context ~expr
+  |> List.filter (fun driver -> not @@ List.mem driver except)
+  |> List.iter (fun self -> destroy ~__context ~self)
 
-(** Runs on [host] *)
+(** 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
-  drivertool ["list"]
-  |> T.parse
-  |> List.iter @@ 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
-     in
-     driver.T.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
-        in
-        ( match driver.T.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
-        | Some v when v = name ->
-            Db.Host_driver.set_active_variant ~__context ~self:driver_ref
-              ~value:var_ref
-        | _ ->
-            ()
+  let drivers (* on this host *) =
+    Tool.call ["list"]
+    |> Tool.parse
+    |> List.map @@ fun (_name, driver) ->
+       let driver_ref =
+         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.Tool.variants
+       |> List.iter @@ fun (name, v) ->
+          let var_ref =
+            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.Tool.selected with
+          | Some v when v = name ->
+              Db.Host_driver.set_selected_variant ~__context ~self:driver_ref
+                ~value:var_ref
+          | _ ->
+              ()
+          ) ;
+          match driver.Tool.active with
+          | Some v when v = name ->
+              Db.Host_driver.set_active_variant ~__context ~self:driver_ref
+                ~value:var_ref
+          | _ ->
+              ()
+       ) ;
+       driver_ref
+  in
+  remove ~__context ~host ~except:drivers
 
 (** Runs on [host] *)
 let rescan ~__context ~host = debug "%s" __FUNCTION__ ; scan ~__context ~host
diff --git a/ocaml/xapi/xapi_host_driver_tool.ml b/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

diff --git a/ocaml/xapi/xapi_host_driver_tool.mli b/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

diff --git a/quality-gate.sh b/quality-gate.sh
@@ -25,9 +25,7 @@ verify-cert () {
 }
 
 mli-files () {
-<<<<<<< HEAD
-  N=496
-  N=499
+  N=498
   X="ocaml/tests"
   X+="|ocaml/quicktest"
   X+="|ocaml/message-switch/core_test"
-Original file line number
+Diff line change
@@ -1,4 +1,4 @@
-    '++
+    +++
     title = "Multi-version drivers"
     +++
@@ Expand Down @@