From 9ad46260077d799bb43d7debfb8d123cb99b7aa4 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 If there are storage exceptions raised when destroying datapath, the procedure fails and the state of VDI becomes incorrect, which leads to various abnormalresults in subsequent operations. To handle this, the leaked datapath is designed to redestroy the datapath and refresh the state before next storage operation via function remove_datapaths_andthen_nolock. But this mechanism doesn't take effect in current code. This commit is to fix this bug. Leaked datapath should be added to Sr.vdis to make the leaked datapath really work. And write to db file to avoid losing the leaked datapath if xapi restarts. 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..55067efd9d 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 "%s dp=%s, sr=%s, vdi=%s, vm=%s, op=%s" __FUNCTION__ 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 "%s %s -> %s: %s" __FUNCTION__ + (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 *)