CA-408552: 3/3 Improve bootstrom performance by save db ops

Move ops unrelated db operation outside of returned function

Signed-off-by: Lin Liu <[email protected]>

Author: liulinC

ocaml/xapi/xapi_vdi.ml

@@ -68,6 +68,84 @@ let check_operation_error ~__context ?sr_records:_ ?(pbd_records = [])
   let _ref = Ref.string_of _ref' in
   let current_ops = record.Db_actions.vDI_current_operations in
   let reset_on_boot = record.Db_actions.vDI_on_boot = `reset in
+  let rolling_upgrade_in_progress =
+    Helpers.rolling_upgrade_in_progress ~__context
+  in
+  (* check to see whether it's a local cd drive *)
+  let sr = record.Db_actions.vDI_SR in
+  let sr_type = Db.SR.get_type ~__context ~self:sr in
+  let is_tools_sr = Db.SR.get_is_tools_sr ~__context ~self:sr in
+  (* Check to see if any PBDs are attached *)
+  let open Xapi_database.Db_filter_types in
+  let pbds_attached =
+    match pbd_records with
+    | [] ->
+        Db.PBD.get_records_where ~__context
+          ~expr:
+            (And
+               ( Eq (Field "SR", Literal (Ref.string_of sr))
+               , Eq (Field "currently_attached", Literal "true")
+               )
+            )
+    | _ ->
+        List.filter
+          (fun (_, pbd_record) ->
+            pbd_record.API.pBD_SR = sr && pbd_record.API.pBD_currently_attached
+          )
+          pbd_records
+  in
+
+  (* Only a 'live' operation can be performed if there are active (even RO) devices *)
+  let my_active_vbd_records =
+    match vbd_records with
+    | None ->
+        List.map snd
+          (Db.VBD.get_internal_records_where ~__context
+             ~expr:
+               (And
+                  ( Eq (Field "VDI", Literal _ref)
+                  , Or
+                      ( Eq (Field "currently_attached", Literal "true")
+                      , Eq (Field "reserved", Literal "true")
+                      )
+                  )
+               )
+          )
+    | Some records ->
+        List.filter
+          (fun vbd_record ->
+            vbd_record.Db_actions.vBD_VDI = _ref'
+            && (vbd_record.Db_actions.vBD_currently_attached
+               || vbd_record.Db_actions.vBD_reserved
+               )
+          )
+          records
+  in
+  let my_active_rw_vbd_records =
+    List.filter (fun vbd -> vbd.Db_actions.vBD_mode = `RW) my_active_vbd_records
+  in
+  (* VBD operations (plug/unplug) (which should be transient) cause us to serialise *)
+  let my_has_current_operation_vbd_records =
+    match vbd_records with
+    | None ->
+        List.map snd
+          (Db.VBD.get_internal_records_where ~__context
+             ~expr:
+               (And
+                  ( Eq (Field "VDI", Literal _ref)
+                  , Not (Eq (Field "current_operations", Literal "()"))
+                  )
+               )
+          )
+    | Some records ->
+        List.filter
+          (fun vbd_record ->
+            vbd_record.Db_actions.vBD_VDI = _ref'
+            && vbd_record.Db_actions.vBD_current_operations <> []
+          )
+          records
+  in
+
   (* Policy:
      	   1. any current_operation besides copy implies exclusivity; fail everything
      	      else; except vdi mirroring is in current operations and destroy is performed
@@ -83,11 +161,15 @@ let check_operation_error ~__context ?sr_records:_ ?(pbd_records = [])
      	   5. HA prevents you from deleting statefiles or metadata volumes
      	   6. During rolling pool upgrade, only operations known by older releases are allowed
   *)
+  let sm_features =
+    Xapi_sr_operations.features_of_sr_internal ~__context ~_type:sr_type
+  in
+  let vdi_is_ha_state_or_redolog =
+    List.mem record.Db_actions.vDI_type [`ha_statefile; `redo_log]
+  in
+
   fun op ->
     let* () =
-      let rolling_upgrade_in_progress =
-        Helpers.rolling_upgrade_in_progress ~__context
-      in
       if
         rolling_upgrade_in_progress
         && not
@@ -113,30 +195,6 @@ let check_operation_error ~__context ?sr_records:_ ?(pbd_records = [])
       else
         Ok ()
     in
-    (* check to see whether it's a local cd drive *)
-    let sr = record.Db_actions.vDI_SR in
-    let sr_type = Db.SR.get_type ~__context ~self:sr in
-    let is_tools_sr = Db.SR.get_is_tools_sr ~__context ~self:sr in
-    (* Check to see if any PBDs are attached *)
-    let open Xapi_database.Db_filter_types in
-    let pbds_attached =
-      match pbd_records with
-      | [] ->
-          Db.PBD.get_records_where ~__context
-            ~expr:
-              (And
-                 ( Eq (Field "SR", Literal (Ref.string_of sr))
-                 , Eq (Field "currently_attached", Literal "true")
-                 )
-              )
-      | _ ->
-          List.filter
-            (fun (_, pbd_record) ->
-              pbd_record.API.pBD_SR = sr
-              && pbd_record.API.pBD_currently_attached
-            )
-            pbd_records
-    in
     let* () =
       if pbds_attached = [] && op = `resize then
         Error (Api_errors.sr_no_pbds, [Ref.string_of sr])
@@ -146,58 +204,6 @@ let check_operation_error ~__context ?sr_records:_ ?(pbd_records = [])
 
     (* check to see whether VBDs exist which are using this VDI *)
 
-    (* Only a 'live' operation can be performed if there are active (even RO) devices *)
-    let my_active_vbd_records =
-      match vbd_records with
-      | None ->
-          List.map snd
-            (Db.VBD.get_internal_records_where ~__context
-               ~expr:
-                 (And
-                    ( Eq (Field "VDI", Literal _ref)
-                    , Or
-                        ( Eq (Field "currently_attached", Literal "true")
-                        , Eq (Field "reserved", Literal "true")
-                        )
-                    )
-                 )
-            )
-      | Some records ->
-          List.filter
-            (fun vbd_record ->
-              vbd_record.Db_actions.vBD_VDI = _ref'
-              && (vbd_record.Db_actions.vBD_currently_attached
-                 || vbd_record.Db_actions.vBD_reserved
-                 )
-            )
-            records
-    in
-    let my_active_rw_vbd_records =
-      List.filter
-        (fun vbd -> vbd.Db_actions.vBD_mode = `RW)
-        my_active_vbd_records
-    in
-    (* VBD operations (plug/unplug) (which should be transient) cause us to serialise *)
-    let my_has_current_operation_vbd_records =
-      match vbd_records with
-      | None ->
-          List.map snd
-            (Db.VBD.get_internal_records_where ~__context
-               ~expr:
-                 (And
-                    ( Eq (Field "VDI", Literal _ref)
-                    , Not (Eq (Field "current_operations", Literal "()"))
-                    )
-                 )
-            )
-      | Some records ->
-          List.filter
-            (fun vbd_record ->
-              vbd_record.Db_actions.vBD_VDI = _ref'
-              && vbd_record.Db_actions.vBD_current_operations <> []
-            )
-            records
-    in
     (* If the VBD is currently_attached then some operations can still be
        performed ie: VDI.clone (if the VM is suspended we have to have the
         'allow_clone_suspended_vm' flag); VDI.snapshot; VDI.resize_online;
@@ -275,9 +281,6 @@ let check_operation_error ~__context ?sr_records:_ ?(pbd_records = [])
       else
         Ok ()
     in
-    let sm_features =
-      Xapi_sr_operations.features_of_sr_internal ~__context ~_type:sr_type
-    in
     let* () = check_sm_feature_error op sm_features sr in
     let allowed_for_cbt_metadata_vdi =
       match op with
@@ -341,16 +344,7 @@ let check_operation_error ~__context ?sr_records:_ ?(pbd_records = [])
       else
         Ok ()
     in
-    let vdi_is_ha_state_or_redolog =
-      List.mem record.Db_actions.vDI_type [`ha_statefile; `redo_log]
-    in
     let check_destroy () =
-      let ha_enable_in_progress =
-        Xapi_pool_helpers.ha_enable_in_progress ~__context
-      in
-      let ha_disable_in_progress =
-        Xapi_pool_helpers.ha_disable_in_progress ~__context
-      in
       if sr_type = "udev" then
         Error (Api_errors.vdi_is_a_physical_device, [_ref])
       else if is_tools_sr then

ocaml/xapi/xapi_vm_lifecycle.ml

@@ -393,8 +393,7 @@ let nested_virt ~__context vm metrics =
     let key = "nested-virt" in
     Vm_platform.is_true ~key ~platformdata ~default:false
 
-let is_mobile ~__context vm strict =
-  let metrics = Db.VM.get_metrics ~__context ~self:vm in
+let is_mobile ~__context vm strict metrics =
   (not @@ nomigrate ~__context vm metrics)
   && (not @@ nested_virt ~__context vm metrics)
   || not strict
@@ -447,6 +446,42 @@ let check_operation_error ~__context ~ref =
       vmr.Db_actions.vM_VBDs
     |> List.filter (Db.is_valid_ref __context)
   in
+  let current_ops = vmr.Db_actions.vM_current_operations in
+  let metrics = Db.VM.get_metrics ~__context ~self:ref in
+  let is_nested_virt = nested_virt ~__context ref metrics in
+  let is_domain_zero =
+    Db.VM.get_by_uuid ~__context ~uuid:vmr.Db_actions.vM_uuid
+    |> Helpers.is_domain_zero ~__context
+  in
+  let vdis_reset_and_caching =
+    List.filter_map
+      (fun vdi ->
+        try
+          let sm_config = Db.VDI.get_sm_config ~__context ~self:vdi in
+          Some
+            ( List.assoc_opt "on_boot" sm_config = Some "reset"
+            , bool_of_assoc "caching" sm_config
+            )
+        with _ -> None
+      )
+      vdis
+  in
+  let sriov_pcis = nvidia_sriov_pcis ~__context vmr.Db_actions.vM_VGPUs in
+  let is_not_sriov pci = not @@ List.mem pci sriov_pcis in
+  let pcis = vmr.Db_actions.vM_attached_PCIs in
+  let is_appliance_valid =
+    Db.is_valid_ref __context vmr.Db_actions.vM_appliance
+  in
+  let is_protection_policy_valid =
+    Db.is_valid_ref __context vmr.Db_actions.vM_protection_policy
+  in
+  let rolling_upgrade_in_progress =
+    Helpers.rolling_upgrade_in_progress ~__context
+  in
+  let is_snapshort_schedule_valid =
+    Db.is_valid_ref __context vmr.Db_actions.vM_snapshot_schedule
+  in
+
   fun ~op ~strict ->
     let current_error = None in
     let check c f = match c with Some e -> Some e | None -> f () in
@@ -470,7 +505,6 @@ let check_operation_error ~__context ~ref =
     (* if other operations are in progress, check that the new operation is allowed concurrently with them. *)
     let current_error =
       check current_error (fun () ->
-          let current_ops = vmr.Db_actions.vM_current_operations in
           if
             List.length current_ops <> 0
             && not (is_allowed_concurrently ~op ~current_ops)
@@ -520,15 +554,13 @@ let check_operation_error ~__context ~ref =
       check current_error (fun () ->
           match op with
           | (`suspend | `checkpoint | `pool_migrate | `migrate_send)
-            when not (is_mobile ~__context ref strict) ->
+            when not (is_mobile ~__context ref strict metrics) ->
               Some (Api_errors.vm_is_immobile, [ref_str])
           | _ ->
               None
       )
     in
     let current_error =
-      let metrics = Db.VM.get_metrics ~__context ~self:ref in
-      let is_nested_virt = nested_virt ~__context ref metrics in
       check current_error (fun () ->
           match op with
           | `changing_dynamic_range when is_nested_virt && strict ->
@@ -542,10 +574,6 @@ let check_operation_error ~__context ~ref =
     (* make use of the Helpers.ballooning_enabled_for_vm function.   *)
     let current_error =
       check current_error (fun () ->
-          let is_domain_zero =
-            Db.VM.get_by_uuid ~__context ~uuid:vmr.Db_actions.vM_uuid
-            |> Helpers.is_domain_zero ~__context
-          in
           if (op = `changing_VCPUs || op = `destroy) && is_domain_zero then
             Some
               ( Api_errors.operation_not_allowed
@@ -592,19 +620,6 @@ let check_operation_error ~__context ~ref =
     (* Check for an error due to VDI caching/reset behaviour *)
     let current_error =
       check current_error (fun () ->
-          let vdis_reset_and_caching =
-            List.filter_map
-              (fun vdi ->
-                try
-                  let sm_config = Db.VDI.get_sm_config ~__context ~self:vdi in
-                  Some
-                    ( List.assoc_opt "on_boot" sm_config = Some "reset"
-                    , bool_of_assoc "caching" sm_config
-                    )
-                with _ -> None
-              )
-              vdis
-          in
           if
             op = `checkpoint
             || op = `snapshot
@@ -633,9 +648,6 @@ let check_operation_error ~__context ~ref =
     (* If a PCI device is passed-through, check if the operation is allowed *)
     let current_error =
       check current_error @@ fun () ->
-      let sriov_pcis = nvidia_sriov_pcis ~__context vmr.Db_actions.vM_VGPUs in
-      let is_not_sriov pci = not @@ List.mem pci sriov_pcis in
-      let pcis = vmr.Db_actions.vM_attached_PCIs in
       match op with
       | (`suspend | `checkpoint | `pool_migrate | `migrate_send)
         when List.exists is_not_sriov pcis ->
@@ -666,9 +678,6 @@ let check_operation_error ~__context ~ref =
     in
     (* Check for errors caused by VM being in an appliance. *)
     let current_error =
-      let is_appliance_valid =
-        Db.is_valid_ref __context vmr.Db_actions.vM_appliance
-      in
       check current_error (fun () ->
           if is_appliance_valid then
             check_appliance ~vmr ~op ~ref_str
@@ -678,9 +687,6 @@ let check_operation_error ~__context ~ref =
     in
     (* Check for errors caused by VM being assigned to a protection policy. *)
     let current_error =
-      let is_protection_policy_valid =
-        Db.is_valid_ref __context vmr.Db_actions.vM_protection_policy
-      in
       check current_error (fun () ->
           if is_protection_policy_valid then
             check_protection_policy ~vmr ~op ~ref_str
@@ -690,9 +696,6 @@ let check_operation_error ~__context ~ref =
     in
     (* Check for errors caused by VM being assigned to a snapshot schedule. *)
     let current_error =
-      let is_snapshort_schedule_valid =
-        Db.is_valid_ref __context vmr.Db_actions.vM_snapshot_schedule
-      in
       check current_error (fun () ->
           if is_snapshort_schedule_valid then
             check_snapshot_schedule ~vmr ~ref_str op
@@ -716,9 +719,6 @@ let check_operation_error ~__context ~ref =
       )
     in
     let current_error =
-      let rolling_upgrade_in_progress =
-        Helpers.rolling_upgrade_in_progress ~__context
-      in
       check current_error (fun () ->
           if
             rolling_upgrade_in_progress