From bd3024bd6cf3d806d485203446bddcea894c8768 Mon Sep 17 00:00:00 2001 From: Vincent Liu Date: Tue, 28 Jan 2025 15:23:09 +0000 Subject: [PATCH 1/2] Revert "CP-45016: Clean up the source VM earlier" This reverts commit 889dfa67525da1aa39514b4254d1e980be815c4c. As there is no need to clean up the source VM earlier at all, VM_save already does the job of deactivating the source VM datapath. Signed-off-by: Vincent Liu --- ocaml/xenopsd/lib/xenops_server.ml | 42 ++++++++++-------------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/ocaml/xenopsd/lib/xenops_server.ml b/ocaml/xenopsd/lib/xenops_server.ml index bdab2c0e913..e3f0a77f5e8 100644 --- a/ocaml/xenopsd/lib/xenops_server.ml +++ b/ocaml/xenopsd/lib/xenops_server.ml @@ -2732,23 +2732,6 @@ and perform_exn ?result (op : operation) (t : Xenops_task.task_handle) : unit = ) ; debug "VM.migrate: Synchronisation point 1" in - let pause_src_vm () = - debug - "VM.migrate: pause src vm before allowing destination to proceed" ; - (* cleanup tmp src VM *) - let atomics = - [ - VM_hook_script_stable - ( id - , Xenops_hooks.VM_pre_destroy - , Xenops_hooks.reason__suspend - , new_src_id - ) - ] - @ atomics_of_operation (VM_shutdown (new_src_id, None)) - in - perform_atomics atomics t - in let final_handshake () = Handshake.send vm_fd Handshake.Success ; debug "VM.migrate: Synchronisation point 3" ; @@ -2789,10 +2772,7 @@ and perform_exn ?result (op : operation) (t : Xenops_task.task_handle) : unit = the main VM migration sequence. *) match VGPU_DB.ids id with | [] -> - first_handshake () ; - save () ; - pause_src_vm () ; - final_handshake () + first_handshake () ; save () ; final_handshake () | (_vm_id, dev_id) :: _ -> let url = make_url "/migrate/vgpu/" @@ -2809,12 +2789,20 @@ and perform_exn ?result (op : operation) (t : Xenops_task.task_handle) : unit = first_handshake () ; save ~vgpu_fd:(FD vgpu_fd) () ) ; - pause_src_vm () ; final_handshake () ) ; - let cleanup_src_vm () = - let atomics = - [ + (* cleanup tmp src VM *) + let atomics = + [ + VM_hook_script_stable + ( id + , Xenops_hooks.VM_pre_destroy + , Xenops_hooks.reason__suspend + , new_src_id + ) + ] + @ atomics_of_operation (VM_shutdown (new_src_id, None)) + @ [ VM_hook_script_stable ( id , Xenops_hooks.VM_post_destroy @@ -2823,10 +2811,8 @@ and perform_exn ?result (op : operation) (t : Xenops_task.task_handle) : unit = ) ; VM_remove new_src_id ] - in - perform_atomics atomics t in - cleanup_src_vm () + perform_atomics atomics t | VM_receive_memory { vmr_id= id From 764ec0df40daf80f30a3bb78026eb7b0f2b273d1 Mon Sep 17 00:00:00 2001 From: Vincent Liu Date: Tue, 28 Jan 2025 17:30:13 +0000 Subject: [PATCH 2/2] CA-405502: Change post_detach to post_deactivate Previously the `post_detach_hook` was run after the VDI is detached on the source VM, which is at the very end of the SXM, where the source VM is shutdown. However, the job of `post_detach_hook` is to call `Remote.receive_finalize` which will destroy the mirroring datapath. This should have been called as soon as we deactivate the datapath on the source VM, at which point the source VM will stop writing using that datapath. This commit changes `post_detach_hook` to `post_deactivate_hook` and moves its calling locations accordingly. Signed-off-by: Vincent Liu --- ocaml/xapi/storage_migrate.ml | 2 +- ocaml/xapi/storage_smapiv1_wrapper.ml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ocaml/xapi/storage_migrate.ml b/ocaml/xapi/storage_migrate.ml index 2686208d733..6f153515ed7 100644 --- a/ocaml/xapi/storage_migrate.ml +++ b/ocaml/xapi/storage_migrate.ml @@ -1320,7 +1320,7 @@ let pre_deactivate_hook ~dbg:_ ~dp:_ ~sr ~vdi = s.failed <- true ) -let post_detach_hook ~sr ~vdi ~dp:_ = +let post_deactivate_hook ~sr ~vdi ~dp:_ = let open State.Send_state in let id = State.mirror_id_of (sr, vdi) in State.find_active_local_mirror id diff --git a/ocaml/xapi/storage_smapiv1_wrapper.ml b/ocaml/xapi/storage_smapiv1_wrapper.ml index fbb30a2177f..f71e2b21d99 100644 --- a/ocaml/xapi/storage_smapiv1_wrapper.ml +++ b/ocaml/xapi/storage_smapiv1_wrapper.ml @@ -422,10 +422,10 @@ functor | Vdi_automaton.Deactivate -> Storage_migrate.pre_deactivate_hook ~dbg ~dp ~sr ~vdi ; Impl.VDI.deactivate context ~dbg ~dp ~sr ~vdi ~vm ; + Storage_migrate.post_deactivate_hook ~sr ~vdi ~dp ; vdi_t | Vdi_automaton.Detach -> Impl.VDI.detach context ~dbg ~dp ~sr ~vdi ~vm ; - Storage_migrate.post_detach_hook ~sr ~vdi ~dp ; vdi_t in Sr.add_or_replace vdi new_vdi_t sr_t ;