Improve doc and logging

Improve the documentation of functions such as `VDI.compose` and `DATA.MIRROR.receive_cancel`. Also use more consistent logging style. Signed-off-by: Vincent Liu <[email protected]>
xapi-project · Dec 5, 2024 · a72bf25 · a72bf25
1 parent 2e71188
commit a72bf25
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 36 deletions.
diff --git a/ocaml/xapi-idl/storage/storage_interface.ml b/ocaml/xapi-idl/storage/storage_interface.ml
@@ -913,8 +913,8 @@ module StorageAPI (R : RPC) = struct
       declare "VDI.set_content_id" []
         (dbg_p @-> sr_p @-> vdi_p @-> content_id_p @-> returning unit_p err)
 
-    (** [compose task sr vdi1 vdi2] layers the updates from [vdi2] onto [vdi1],
-        modifying [vdi2] *)
+    (** [compose task sr parent child] layers the updates from [child] onto [parent],
+        modifying [child] *)
     let compose =
       let vdi1_p = Param.mk ~name:"vdi1" Vdi.t in
       let vdi2_p = Param.mk ~name:"vdi2" Vdi.t in
@@ -1040,7 +1040,8 @@ module StorageAPI (R : RPC) = struct
           @-> returning result err
           )
 
-      (** Called on the receiving end to prepare for receipt of the storage.*)
+      (** Called on the receiving end to prepare for receipt of the storage. This
+      function should be used in conjunction with [receive_finalize2]*)
       let receive_start2 =
         let similar_p = Param.mk ~name:"similar" Mirror.similars in
         let result = Param.mk ~name:"result" Mirror.mirror_receive_result in
@@ -1071,6 +1072,8 @@ module StorageAPI (R : RPC) = struct
         declare "DATA.MIRROR.receive_finalize2" []
           (dbg_p @-> id_p @-> returning unit_p err)
 
+      (** [receive_cancel dbg id] is called in the case of migration failure to
+      do the clean up.*)
       let receive_cancel =
         declare "DATA.MIRROR.receive_cancel" []
           (dbg_p @-> id_p @-> returning unit_p err)

diff --git a/ocaml/xapi/storage_mux.ml b/ocaml/xapi/storage_mux.ml
@@ -835,41 +835,67 @@ module Mux = struct
       with_dbg ~name:"DATA.copy" ~dbg @@ fun dbg -> Storage_migrate.copy ~dbg
 
     module MIRROR = struct
-      let start () ~dbg =
-        with_dbg ~name:"DATA.MIRROR.start" ~dbg @@ fun dbg ->
-        Storage_migrate.start ~dbg
-
-      let stop () ~dbg =
-        with_dbg ~name:"DATA.MIRROR.stop" ~dbg @@ fun {log= dbg; _} ->
-        Storage_migrate.stop ~dbg
+      let start () ~dbg ~sr ~vdi ~dp ~mirror_vm ~copy_vm ~url ~dest ~verify_dest
+          =
+        with_dbg ~name:"DATA.MIRROR.start" ~dbg @@ fun di ->
+        info
+          "%s dbg:%s sr: %s vdi: %s dp:%s mirror_vm: %s copy_vm: %s url: %s \
+           dest sr: %s verify_dest: %B"
+          __FUNCTION__ dbg (s_of_sr sr) (s_of_vdi vdi) dp (s_of_vm mirror_vm)
+          (s_of_vm copy_vm) url (s_of_sr dest) verify_dest ;
+        Storage_migrate.start ~dbg:di ~sr ~vdi ~dp ~mirror_vm ~copy_vm ~url
+          ~dest ~verify_dest
+
+      let stop () ~dbg ~id =
+        with_dbg ~name:"DATA.MIRROR.stop" ~dbg @@ fun di ->
+        info "%s dbg:%s mirror_id: %s" __FUNCTION__ dbg id ;
+        Storage_migrate.stop ~dbg:di.log ~id
 
       let list () ~dbg =
-        with_dbg ~name:"DATA.MIRROR.list" ~dbg @@ fun {log= dbg; _} ->
-        Storage_migrate.list ~dbg
-
-      let stat () ~dbg =
-        with_dbg ~name:"DATA.MIRROR.stat" ~dbg @@ fun {log= dbg; _} ->
-        Storage_migrate.stat ~dbg
-
-      let receive_start () ~dbg =
-        with_dbg ~name:"DATA.MIRROR.receive_start" ~dbg @@ fun {log= dbg; _} ->
-        Storage_migrate.receive_start ~dbg
-
-      let receive_start2 () ~dbg =
-        with_dbg ~name:"DATA.MIRROR.receive_start2" ~dbg @@ fun {log= dbg; _} ->
-        Storage_migrate.receive_start2 ~dbg
-
-      let receive_finalize () ~dbg =
-        with_dbg ~name:"DATA.MIRROR.receive_finalize" ~dbg
-        @@ fun {log= dbg; _} -> Storage_migrate.receive_finalize ~dbg
-
-      let receive_finalize2 () ~dbg =
-        with_dbg ~name:"DATA.MIRROR.receive_finalize2" ~dbg
-        @@ fun {log= dbg; _} -> Storage_migrate.receive_finalize2 ~dbg
-
-      let receive_cancel () ~dbg =
-        with_dbg ~name:"DATA.MIRROR.receive_cancel" ~dbg @@ fun {log= dbg; _} ->
-        Storage_migrate.receive_cancel ~dbg
+        with_dbg ~name:"DATA.MIRROR.list" ~dbg @@ fun di ->
+        info "%s dbg: %s" __FUNCTION__ dbg ;
+        Storage_migrate.list ~dbg:di.log
+
+      let stat () ~dbg ~id =
+        with_dbg ~name:"DATA.MIRROR.stat" ~dbg @@ fun di ->
+        info "%s dbg: %s mirror_id: %s" __FUNCTION__ di.log id ;
+        Storage_migrate.stat ~dbg:di.log ~id
+
+      let receive_start () ~dbg ~sr ~vdi_info ~id ~similar =
+        with_dbg ~name:"DATA.MIRROR.receive_start" ~dbg @@ fun di ->
+        info "%s dbg: %s sr: %s vdi_info: %s mirror_id: %s similar: %s"
+          __FUNCTION__ dbg (s_of_sr sr)
+          (string_of_vdi_info vdi_info)
+          id
+          (String.concat ";" similar) ;
+        Storage_migrate.receive_start ~dbg:di.log ~sr ~vdi_info ~id ~similar
+
+      let receive_start2 () ~dbg ~sr ~vdi_info ~id ~similar ~vm =
+        with_dbg ~name:"DATA.MIRROR.receive_start2" ~dbg @@ fun di ->
+        info "%s dbg: %s sr: %s vdi_info: %s mirror_id: %s similar: %s vm: %s"
+          __FUNCTION__ dbg (s_of_sr sr)
+          (string_of_vdi_info vdi_info)
+          id
+          (String.concat ";" similar)
+          (s_of_vm vm) ;
+        info "%s dbg:%s" __FUNCTION__ dbg ;
+        Storage_migrate.receive_start2 ~dbg:di.log ~sr ~vdi_info ~id ~similar
+          ~vm
+
+      let receive_finalize () ~dbg ~id =
+        with_dbg ~name:"DATA.MIRROR.receive_finalize" ~dbg @@ fun di ->
+        info "%s dbg: %s mirror_id: %s" __FUNCTION__ dbg id ;
+        Storage_migrate.receive_finalize ~dbg:di.log ~id
+
+      let receive_finalize2 () ~dbg ~id =
+        with_dbg ~name:"DATA.MIRROR.receive_finalize2" ~dbg @@ fun di ->
+        info "%s dbg: %s mirror_id: %s" __FUNCTION__ dbg id ;
+        Storage_migrate.receive_finalize2 ~dbg:di.log ~id
+
+      let receive_cancel () ~dbg ~id =
+        with_dbg ~name:"DATA.MIRROR.receive_cancel" ~dbg @@ fun di ->
+        info "%s dbg: %s mirror_id: %s" __FUNCTION__ dbg id ;
+        Storage_migrate.receive_cancel ~dbg:di.log ~id
 
       let import_activate () ~dbg ~dp ~sr ~vdi ~vm =
         with_dbg ~name:"DATA.MIRROR.import_activate" ~dbg @@ fun di ->

diff --git a/ocaml/xapi/xapi_vm_migrate.ml b/ocaml/xapi/xapi_vm_migrate.ml
@@ -479,6 +479,7 @@ let remove_stale_pcis ~__context ~vm =
   in
   List.iter remove stale_pcis
 
+(** Called on the destination side *)
 let pool_migrate_complete ~__context ~vm ~host:_ =
   let id = Db.VM.get_uuid ~__context ~self:vm in
   debug "VM.pool_migrate_complete %s" id ;