Change how receive_start2 is called

Vincent-lau · Vincent-lau · commit 8da184b9105f · 2025-04-24T11:12:05.000+01:00
Previously `MIRROR.receive_start2` is called as a remote function, i.e.
`Remote.DATA.MIRROR.receive_start2` and it will be forwarded to the
destination host and multiplexes based on the destination SR type.

This is inconvenient as what `receive_start2` should do is more
dependent on what the source SR type is. For example, if the source SR
is using SMAPIv1, then `receive_start2` needs to tell the destination
host to create snapshots VDIs, while this is not necessary if the source
SR type is of SMAPIv3. Hence instead of calling `Remote.receive_start2`,
call each individual functions inside `receive_start2` remotely, such as
`Remote.VDI.create`, and these SMAPIv2 functions will still be
multiplexed on the destiantion side, based on the destination SR type.
And this is indeed what we want, imagine a case where we are migrating
v1 -&gt; v3, what we want is still create a snapshot VDI, but on the v3 SR.

This does mean that the state tracking, such as `State.add`, which was
previously tracked by the destination host, now needs to be tracked by
the source host. And this will affect a number of other `receive_`
functions such as `receive_finalize2` and `receive_cancel2`, which are
updated accordingly.

For backwards compatability reasons, we still need to preserve
`receive_start` which might still be called from an older host trying to
do a v1 -&gt; v1 migration. And this is done by making sure that the
SMAPIv2 done inside `receive_start` are all local, while the
`receive_start` call itself is remote.

Signed-off-by: Vincent Liu &lt;shuntian.liu2@cloud.com&gt;
diff --git a/ocaml/xapi-idl/storage/storage_interface.ml b/ocaml/xapi-idl/storage/storage_interface.ml
@@ -1136,6 +1136,8 @@ module StorageAPI (R : RPC) = struct
           @-> id_p
           @-> similar_p
           @-> vm_p
+          @-> url_p
+          @-> verify_dest_p
           @-> returning result err
           )
 
@@ -1154,13 +1156,29 @@ module StorageAPI (R : RPC) = struct
       should be used in conjunction with [receive_start2] *)
       let receive_finalize2 =
         declare "DATA.MIRROR.receive_finalize2" []
-          (dbg_p @-> id_p @-> returning unit_p err)
+          (dbg_p
+          @-> id_p
+          @-> sr_p
+          @-> url_p
+          @-> verify_dest_p
+          @-> returning unit_p err
+          )
 
       (** [receive_cancel dbg id] is called in the case of migration failure to
-      do the clean up.*)
+      do the clean up.
+        @deprecated This function is deprecated, and is only here to keep backward 
+        compatibility with old xapis that call Remote.DATA.MIRROR.receive_cancel
+        during SXM.  Use the receive_cancel2 function instead. 
+      *)
       let receive_cancel =
         declare "DATA.MIRROR.receive_cancel" []
           (dbg_p @-> id_p @-> returning unit_p err)
+
+      (** [receive_cancel2 dbg mirror_id url verify_dest] cleans up the side effects
+      done by [receive_start2] on the destination host when the migration fails. *)
+      let receive_cancel2 =
+        declare "DATA.MIRROR.receive_cancel2" []
+          (dbg_p @-> id_p @-> url_p @-> verify_dest_p @-> returning unit_p err)
     end
   end
 
@@ -1240,16 +1258,33 @@ module type MIRROR = sig
     -> dbg:debug_info
     -> sr:sr
     -> vdi_info:vdi_info
-    -> id:Mirror.id
+    -> mirror_id:Mirror.id
     -> similar:Mirror.similars
     -> vm:vm
+    -> url:string
+    -> verify_dest:bool
     -> Mirror.mirror_receive_result
 
   val receive_finalize : context -> dbg:debug_info -> id:Mirror.id -> unit
 
-  val receive_finalize2 : context -> dbg:debug_info -> id:Mirror.id -> unit
+  val receive_finalize2 :
+       context
+    -> dbg:debug_info
+    -> mirror_id:Mirror.id
+    -> sr:sr
+    -> url:string
+    -> verify_dest:bool
+    -> unit
 
   val receive_cancel : context -> dbg:debug_info -> id:Mirror.id -> unit
+
+  val receive_cancel2 :
+       context
+    -> dbg:debug_info
+    -> mirror_id:Mirror.id
+    -> url:string
+    -> verify_dest:bool
+    -> unit
 end
 
 module type Server_impl = sig
@@ -1706,17 +1741,23 @@ module Server (Impl : Server_impl) () = struct
     S.DATA.MIRROR.receive_start (fun dbg sr vdi_info id similar ->
         Impl.DATA.MIRROR.receive_start () ~dbg ~sr ~vdi_info ~id ~similar
     ) ;
-    S.DATA.MIRROR.receive_start2 (fun dbg sr vdi_info id similar vm ->
-        Impl.DATA.MIRROR.receive_start2 () ~dbg ~sr ~vdi_info ~id ~similar ~vm
+    S.DATA.MIRROR.receive_start2
+      (fun dbg sr vdi_info mirror_id similar vm url verify_dest ->
+        Impl.DATA.MIRROR.receive_start2 () ~dbg ~sr ~vdi_info ~mirror_id
+          ~similar ~vm ~url ~verify_dest
     ) ;
     S.DATA.MIRROR.receive_cancel (fun dbg id ->
         Impl.DATA.MIRROR.receive_cancel () ~dbg ~id
     ) ;
+    S.DATA.MIRROR.receive_cancel2 (fun dbg mirror_id url verify_dest ->
+        Impl.DATA.MIRROR.receive_cancel2 () ~dbg ~mirror_id ~url ~verify_dest
+    ) ;
     S.DATA.MIRROR.receive_finalize (fun dbg id ->
         Impl.DATA.MIRROR.receive_finalize () ~dbg ~id
     ) ;
-    S.DATA.MIRROR.receive_finalize2 (fun dbg id ->
-        Impl.DATA.MIRROR.receive_finalize2 () ~dbg ~id
+    S.DATA.MIRROR.receive_finalize2 (fun dbg mirror_id sr url verify_dest ->
+        Impl.DATA.MIRROR.receive_finalize2 () ~dbg ~mirror_id ~sr ~url
+          ~verify_dest
     ) ;
     S.DATA.import_activate (fun dbg dp sr vdi vm ->
         Impl.DATA.import_activate () ~dbg ~dp ~sr ~vdi ~vm
diff --git a/ocaml/xapi-idl/storage/storage_skeleton.ml b/ocaml/xapi-idl/storage/storage_skeleton.ml
@@ -169,14 +169,19 @@ module DATA = struct
     let receive_start ctx ~dbg ~sr ~vdi_info ~id ~similar =
       u "DATA.MIRROR.receive_start"
 
-    let receive_start2 ctx ~dbg ~sr ~vdi_info ~id ~similar ~vm =
+    let receive_start2 ctx ~dbg ~sr ~vdi_info ~mirror_id ~similar ~vm ~url
+        ~verify_dest =
       u "DATA.MIRROR.receive_start2"
 
     let receive_finalize ctx ~dbg ~id = u "DATA.MIRROR.receive_finalize"
 
-    let receive_finalize2 ctx ~dbg ~id = u "DATA.MIRROR.receive_finalize2"
+    let receive_finalize2 ctx ~dbg ~mirror_id ~sr ~url ~verify_dest =
+      u "DATA.MIRROR.receive_finalize2"
 
     let receive_cancel ctx ~dbg ~id = u "DATA.MIRROR.receive_cancel"
+
+    let receive_cancel2 ctx ~dbg ~mirror_id ~url ~verify_dest =
+      u "DATA.MIRROR.receive_cancel2"
   end
 end
 
diff --git a/ocaml/xapi-storage-script/main.ml b/ocaml/xapi-storage-script/main.ml
@@ -1926,6 +1926,7 @@ let bind ~volume_script_dir =
   S.DATA.MIRROR.receive_finalize (u "DATA.MIRROR.receive_finalize") ;
   S.DATA.MIRROR.receive_finalize2 (u "DATA.MIRROR.receive_finalize2") ;
   S.DATA.MIRROR.receive_cancel (u "DATA.MIRROR.receive_cancel") ;
+  S.DATA.MIRROR.receive_cancel2 (u "DATA.MIRROR.receive_cancel2") ;
   S.DP.create (u "DP.create") ;
   S.TASK.cancel (u "TASK.cancel") ;
   S.TASK.list (u "TASK.list") ;
diff --git a/ocaml/xapi/storage_migrate.ml b/ocaml/xapi/storage_migrate.ml
@@ -40,44 +40,29 @@ let choose_backend dbg sr =
 (** module [MigrateRemote] is similar to [MigrateLocal], but most of these functions
 tend to be executed on the receiver side. *)
 module MigrateRemote = struct
-  let receive_finalize ~dbg ~id =
-    let recv_state = State.find_active_receive_mirror id in
-    let open State.Receive_state in
-    Option.iter (fun r -> Local.DP.destroy dbg r.leaf_dp false) recv_state ;
-    State.remove_receive_mirror id
-
-  let receive_finalize2 ~dbg ~id =
-    let recv_state = State.find_active_receive_mirror id in
-    let open State.Receive_state in
-    Option.iter
-      (fun r ->
-        SXM.info
-          "%s Mirror done. Compose on the dest sr %s parent %s and leaf %s"
-          __FUNCTION__ (Sr.string_of r.sr)
-          (Vdi.string_of r.parent_vdi)
-          (Vdi.string_of r.leaf_vdi) ;
-        Local.DP.destroy2 dbg r.leaf_dp r.sr r.leaf_vdi r.mirror_vm false ;
-        Local.VDI.compose dbg r.sr r.parent_vdi r.leaf_vdi ;
-        (* On SMAPIv3, compose would have removed the now invalid dummy vdi, so
-           there is no need to destroy it anymore, while this is necessary on SMAPIv1 SRs. *)
-        log_and_ignore_exn (fun () -> Local.VDI.destroy dbg r.sr r.dummy_vdi) ;
-        Local.VDI.remove_from_sm_config dbg r.sr r.leaf_vdi "base_mirror"
-      )
-      recv_state ;
-    State.remove_receive_mirror id
+  (** [receive_finalize2 dbg mirror_id sr url verify_dest] takes an [sr] parameter
+  which is the source sr and multiplexes based on the type of that *)
+  let receive_finalize2 ~dbg ~mirror_id ~sr ~url ~verify_dest =
+    let (module Migrate_Backend) = choose_backend dbg sr in
+    Migrate_Backend.receive_finalize2 () ~dbg ~mirror_id ~sr ~url ~verify_dest
 
-  let receive_cancel ~dbg ~id =
-    let receive_state = State.find_active_receive_mirror id in
+  let receive_cancel2 ~dbg ~mirror_id ~url ~verify_dest =
+    let (module Remote) =
+      Storage_migrate_helper.get_remote_backend url verify_dest
+    in
+    let receive_state = State.find_active_receive_mirror mirror_id in
     let open State.Receive_state in
     Option.iter
       (fun r ->
-        log_and_ignore_exn (fun () -> Local.DP.destroy dbg r.leaf_dp false) ;
+        D.log_and_ignore_exn (fun () -> Remote.DP.destroy dbg r.leaf_dp false) ;
         List.iter
-          (fun v -> log_and_ignore_exn (fun () -> Local.VDI.destroy dbg r.sr v))
+          (fun v ->
+            D.log_and_ignore_exn (fun () -> Remote.VDI.destroy dbg r.sr v)
+          )
           [r.dummy_vdi; r.leaf_vdi; r.parent_vdi]
       )
       receive_state ;
-    State.remove_receive_mirror id
+    State.remove_receive_mirror mirror_id
 end
 
 (** This module [MigrateLocal] consists of the concrete implementations of the 
@@ -121,11 +106,10 @@ module MigrateLocal = struct
             | None ->
                 debug "Snapshot VDI already cleaned up"
             ) ;
-
-            let (module Remote) =
-              get_remote_backend remote_info.url remote_info.verify_dest
-            in
-            try Remote.DATA.MIRROR.receive_cancel dbg id with _ -> ()
+            try
+              MigrateRemote.receive_cancel2 ~dbg ~mirror_id:id
+                ~url:remote_info.url ~verify_dest:remote_info.verify_dest
+            with _ -> ()
           )
         | None ->
             ()
@@ -145,11 +129,10 @@ module MigrateLocal = struct
   let prepare ~dbg ~sr ~vdi ~dest ~local_vdi ~mirror_id ~mirror_vm ~url
       ~verify_dest =
     try
-      let (module Remote) = get_remote_backend url verify_dest in
+      let (module Migrate_Backend) = choose_backend dbg sr in
       let similars = similar_vdis ~dbg ~sr ~vdi in
-
-      Remote.DATA.MIRROR.receive_start2 dbg dest local_vdi mirror_id similars
-        mirror_vm
+      Migrate_Backend.receive_start2 () ~dbg ~sr:dest ~vdi_info:local_vdi
+        ~mirror_id ~similar:similars ~vm:mirror_vm ~url ~verify_dest
     with e ->
       error "%s Caught error %s while preparing for SXM" __FUNCTION__
         (Printexc.to_string e) ;
@@ -215,7 +198,7 @@ module MigrateLocal = struct
       | Storage_error (Migration_mirror_snapshot_failure reason) ) as e ->
         error "%s: Caught %s: during storage migration preparation" __FUNCTION__
           reason ;
-        MigrateRemote.receive_cancel ~dbg ~id:mirror_id ;
+        MigrateRemote.receive_cancel2 ~dbg ~mirror_id ~url ~verify_dest ;
         raise e
     | Storage_error (Migration_mirror_copy_failure reason) as e ->
         error "%s: Caught %s: during storage migration copy" __FUNCTION__ reason ;
@@ -331,9 +314,12 @@ module MigrateLocal = struct
       )
       copy_ops ;
     List.iter
-      (fun (id, _recv_state) ->
-        debug "Receive in progress: %s" id ;
-        log_and_ignore_exn (fun () -> Local.DATA.MIRROR.receive_cancel dbg id)
+      (fun (mirror_id, (recv_state : State.Receive_state.t)) ->
+        debug "Receive in progress: %s" mirror_id ;
+        log_and_ignore_exn (fun () ->
+            MigrateRemote.receive_cancel2 ~dbg ~mirror_id ~url:recv_state.url
+              ~verify_dest:recv_state.verify_dest
+        )
       )
       recv_ops ;
     State.clear ()
@@ -405,7 +391,8 @@ let post_deactivate_hook ~sr ~vdi ~dp:_ =
          let (module Remote) = get_remote_backend r.url verify_dest in
          debug "Calling receive_finalize2" ;
          log_and_ignore_exn (fun () ->
-             Remote.DATA.MIRROR.receive_finalize2 "Mirror-cleanup" id
+             MigrateRemote.receive_finalize2 ~dbg:"Mirror-cleanup" ~mirror_id:id
+               ~sr ~url:r.url ~verify_dest
          ) ;
          debug "Finished calling receive_finalize2" ;
          State.remove_local_mirror id ;
@@ -525,12 +512,6 @@ let killall = MigrateLocal.killall
 
 let stat = MigrateLocal.stat
 
-let receive_finalize = MigrateRemote.receive_finalize
-
-let receive_finalize2 = MigrateRemote.receive_finalize2
-
-let receive_cancel = MigrateRemote.receive_cancel
-
 (* The remote end of this call, SR.update_snapshot_info_dest, is implemented in
  * the SMAPIv1 section of storage_migrate.ml. It needs to access the setters
  * for snapshot_of, snapshot_time and is_a_snapshot, which we don't want to add
diff --git a/ocaml/xapi/storage_mux.ml b/ocaml/xapi/storage_mux.ml
@@ -838,23 +838,25 @@ module Mux = struct
           ~similar
 
       (** see storage_smapiv{1,3}_migrate.receive_start2 *)
-      let receive_start2 () ~dbg:_ ~sr:_ ~vdi_info:_ ~id:_ ~similar:_ ~vm:_ =
+      let receive_start2 () ~dbg:_ ~sr:_ ~vdi_info:_ ~mirror_id:_ ~similar:_
+          ~vm:_ =
         u __FUNCTION__
 
       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
+        Storage_smapiv1_migrate.MIRROR.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_finalize2 () ~dbg:_ ~mirror_id:_ ~sr:_ ~url:_ ~verify_dest:_ =
+        u __FUNCTION__
 
       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
+        Storage_smapiv1_migrate.MIRROR.receive_cancel () ~dbg:di.log ~id
+
+      let receive_cancel2 () ~dbg:_ ~mirror_id:_ ~url:_ ~verify_dest:_ =
+        u __FUNCTION__
     end
   end
 
diff --git a/ocaml/xapi/storage_smapiv1.ml b/ocaml/xapi/storage_smapiv1.ml
@@ -1143,15 +1143,20 @@ module SMAPIv1 : Server_impl = struct
       let receive_start _context ~dbg:_ ~sr:_ ~vdi_info:_ ~id:_ ~similar:_ =
         assert false
 
-      let receive_start2 _context ~dbg:_ ~sr:_ ~vdi_info:_ ~id:_ ~similar:_
-          ~vm:_ =
+      let receive_start2 _context ~dbg:_ ~sr:_ ~vdi_info:_ ~mirror_id:_
+          ~similar:_ ~vm:_ ~url:_ ~verify_dest:_ =
         assert false
 
       let receive_finalize _context ~dbg:_ ~id:_ = assert false
 
-      let receive_finalize2 _context ~dbg:_ ~id:_ = assert false
+      let receive_finalize2 _context ~dbg:_ ~mirror_id:_ ~sr:_ ~url:_
+          ~verify_dest:_ =
+        assert false
 
       let receive_cancel _context ~dbg:_ ~id:_ = assert false
+
+      let receive_cancel2 _context ~dbg:_ ~mirror_id:_ ~url:_ ~verify_dest:_ =
+        assert false
     end
   end
 
diff --git a/ocaml/xapi/storage_smapiv1_migrate.ml b/ocaml/xapi/storage_smapiv1_migrate.ml
diff --git a/ocaml/xapi/storage_smapiv1_wrapper.ml b/ocaml/xapi/storage_smapiv1_wrapper.ml
diff --git a/ocaml/xapi/storage_smapiv3_migrate.ml b/ocaml/xapi/storage_smapiv3_migrate.ml
