From 588448a30ac8d2688c0cbd3b17f726f9af28ceb4 Mon Sep 17 00:00:00 2001 From: Changlei Li Date: Tue, 10 Dec 2024 14:18:03 +0800 Subject: [PATCH] CA-402901: Update leaked dp to Sr When add leaked datapath: 1. add leaked datapath to Sr.vdis 2. write to db file 3. log enhance In XSI-1752/1753, there are storage exceptions rasied when destroying datapath. Then the procedure fails and the state of VDI becomes incorrect, which leads to various abnormal results in subsequent operations. We can find logs as "Leaked datapath: dp: xxxx". The leaked datapath is designed to redestory the datapath and refresh the state before next storage operation via function remove_datapaths_andthen_nolock. But we don't find logs about redestorying like "Attempting to destroy datapath". It seems leaked datapath mechanism doesn't take effect. This commit is to fix this bug. We should add leaked datapath to Sr.vdis to make the leaked datapath realy work. And write to db file to avoid losing the leaked datapath when xapi restart. Signed-off-by: Changlei Li --- ocaml/xapi/storage_smapiv1_wrapper.ml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/ocaml/xapi/storage_smapiv1_wrapper.ml b/ocaml/xapi/storage_smapiv1_wrapper.ml index ae1f21f72f..1ca477b44f 100644 --- a/ocaml/xapi/storage_smapiv1_wrapper.ml +++ b/ocaml/xapi/storage_smapiv1_wrapper.ml @@ -453,6 +453,9 @@ functor List.fold_left perform_one vdi_t ops let perform_nolock context ~dbg ~dp ~sr ~vdi ~vm this_op = + debug "perform_nolock dp=%s, sr=%s, vdi=%s, vm=%s, op=%s" dp + (s_of_sr sr) (s_of_vdi vdi) (s_of_vm vm) + (Vdi_automaton.string_of_op this_op) ; match Host.find sr !Host.host with | None -> raise (Storage_error (Sr_not_attached (s_of_sr sr))) @@ -473,6 +476,15 @@ functor superstate to superstate'. These may fail: if so we revert the datapath+VDI state to the most appropriate value. *) let ops = Vdi_automaton.( - ) superstate superstate' in + debug "perform_nolock %s -> %s: %s" + (Vdi_automaton.string_of_state superstate) + (Vdi_automaton.string_of_state superstate') + (String.concat ", " + (List.map + (fun (op, _) -> Vdi_automaton.string_of_op op) + ops + ) + ) ; side_effects context dbg dp sr sr_t vdi vdi_t vm ops with e -> let e = @@ -529,7 +541,8 @@ functor ) with e -> if not allow_leak then ( - ignore (Vdi.add_leaked dp vdi_t) ; + Sr.add_or_replace vdi (Vdi.add_leaked dp vdi_t) sr_t ; + Everything.to_file !host_state_path (Everything.make ()) ; raise e ) else ( (* allow_leak means we can forget this dp *)