CP-53554: Split plug xenopsd atomic into attach/activate

This consists of two parts, splitting the attach_and_activate into
functionally equivalent attach and activate functions, and splitting the
VBD_plug atomic itself's behaviour into two new atomics, VBD_attach and
VBD_activate.

If the new xenopsd_vbd_plug_unplug_legacy flag is true, the only
difference will be that VBD_plug calls attach and activate sequentially
instead of attach_and_activate.

If xenopsd_vbd_plug_unplug_legacy is false, the VBD_attach and
VBD_activate atomics will be used in place of VBD_plug in all places
that it is used. This should still be functionally equivalent.

The purpose of this change is to allow optimisations in this area as
there are some situations where they do not need to be called at the
same time. For example VBD_attach could be called outside of VM migrate
downtime to reduce the overall downtime.

Signed-off-by: Steven Woods <steven.woods@cloud.com>

Author: snwoods

ocaml/xenopsd/lib/storage.ml

@@ -62,20 +62,20 @@ let vm_of_domid vmdomid =
         "Invalid domid, could not be converted to int, passing empty string." ;
       Storage_interface.Vm.of_string ""
 
-let attach_and_activate ~task ~_vm ~vmdomid ~dp ~sr ~vdi ~read_write =
+let attach ~task ~_vm ~vmdomid ~dp ~sr ~vdi ~read_write =
+  let dbg = get_dbg task in
+  Xenops_task.with_subtask task
+    (Printf.sprintf "VDI.attach3 %s" dp)
+    (transform_exception (fun () ->
+         Client.VDI.attach3 dbg dp sr vdi vmdomid read_write
+     )
+    )
+
+let activate ~task ~_vm ~vmdomid ~dp ~sr ~vdi =
   let dbg = get_dbg task in
-  let result =
-    Xenops_task.with_subtask task
-      (Printf.sprintf "VDI.attach3 %s" dp)
-      (transform_exception (fun () ->
-           Client.VDI.attach3 dbg dp sr vdi vmdomid read_write
-       )
-      )
-  in
   Xenops_task.with_subtask task
     (Printf.sprintf "VDI.activate3 %s" dp)
-    (transform_exception (fun () -> Client.VDI.activate3 dbg dp sr vdi vmdomid)) ;
-  result
+    (transform_exception (fun () -> Client.VDI.activate3 dbg dp sr vdi vmdomid))
 
 let deactivate task dp sr vdi vmdomid =
   debug "Deactivating disk %s %s" (Sr.string_of sr) (Vdi.string_of vdi) ;

ocaml/xenopsd/lib/xenops_server.ml

@@ -37,6 +37,8 @@ let finally = Xapi_stdext_pervasives.Pervasiveext.finally
 
 let domain_shutdown_ack_timeout = ref 60.
 
+let xenopsd_vbd_plug_unplug_legacy = ref true
+
 type context = {
     transferred_fd: Unix.file_descr option
         (** some API calls take a file descriptor argument *)
@@ -122,6 +124,8 @@ type atomic =
   | VM_hook_script_stable of (Vm.id * Xenops_hooks.script * string * Vm.id)
   | VM_hook_script of (Vm.id * Xenops_hooks.script * string)
   | VBD_plug of Vbd.id
+  | VBD_attach of Vbd.id
+  | VBD_activate of Vbd.id
   | VBD_epoch_begin of (Vbd.id * disk * bool)
   | VBD_epoch_end of (Vbd.id * disk)
   | VBD_set_qos of Vbd.id
@@ -195,6 +199,10 @@ let rec name_of_atomic = function
       "VM_hook_script"
   | VBD_plug _ ->
       "VBD_plug"
+  | VBD_attach _ ->
+      "VBD_attach"
+  | VBD_activate _ ->
+      "VBD_activate"
   | VBD_epoch_begin _ ->
       "VBD_epoch_begin"
   | VBD_epoch_end _ ->
@@ -1580,6 +1588,18 @@ let parallel_map name ~id lst f = parallel name ~id (List.concat_map f lst)
 
 let map_or_empty f x = Option.value ~default:[] (Option.map f x)
 
+(* Creates a Serial of 2 or more Atomics. If the number of Atomics could be
+   less than this, use serial or serial_concat *)
+let serial_of name ~id at1 at2 ats =
+  Serial (id, Printf.sprintf "%s VM=%s" name id, at1 :: at2 :: ats)
+
+let vbd_plug vbd_id =
+  if !xenopsd_vbd_plug_unplug_legacy then
+    VBD_plug vbd_id
+  else
+    serial_of "VBD.attach_and_activate" ~id:(VBD_DB.vm_of vbd_id)
+      (VBD_attach vbd_id) (VBD_activate vbd_id) []
+
 let rec atomics_of_operation = function
   | VM_start (id, force) ->
       let vbds_rw, vbds_ro = VBD_DB.vbds id |> vbd_plug_sets in
@@ -1604,7 +1624,7 @@ let rec atomics_of_operation = function
                     [VBD_epoch_begin (vbd.Vbd.id, x, vbd.Vbd.persistent)]
                   )
                   vbd.Vbd.backend
-              ; [VBD_plug vbd.Vbd.id]
+              ; [vbd_plug vbd.Vbd.id]
               ]
         )
       in
@@ -1692,7 +1712,7 @@ let rec atomics_of_operation = function
         let name_one = pf "VBD.activate_and_plug %s" typ in
         parallel_map name_multi ~id vbds (fun vbd ->
             serial name_one ~id
-              [VBD_set_active (vbd.Vbd.id, true); VBD_plug vbd.Vbd.id]
+              [VBD_set_active (vbd.Vbd.id, true); vbd_plug vbd.Vbd.id]
         )
       in
       [
@@ -1825,7 +1845,7 @@ let rec atomics_of_operation = function
       ]
       |> List.concat
   | VBD_hotplug id ->
-      [VBD_set_active (id, true); VBD_plug id]
+      [VBD_set_active (id, true); vbd_plug id]
   | VBD_hotunplug (id, force) ->
       [VBD_unplug (id, force); VBD_set_active (id, false)]
   | VIF_hotplug id ->
@@ -2017,7 +2037,15 @@ let rec perform_atomic ~progress_callback ?result (op : atomic)
       Xenops_hooks.vm ~script ~reason ~id ~extra_args
   | VBD_plug id ->
       debug "VBD.plug %s" (VBD_DB.string_of_id id) ;
-      B.VBD.plug t (VBD_DB.vm_of id) (VBD_DB.read_exn id) ;
+      B.VBD.attach t (VBD_DB.vm_of id) (VBD_DB.read_exn id) ;
+      B.VBD.activate t (VBD_DB.vm_of id) (VBD_DB.read_exn id) ;
+      VBD_DB.signal id
+  | VBD_attach id ->
+      debug "VBD.attach %s" (VBD_DB.string_of_id id) ;
+      B.VBD.attach t (VBD_DB.vm_of id) (VBD_DB.read_exn id)
+  | VBD_activate id ->
+      debug "VBD.activate %s" (VBD_DB.string_of_id id) ;
+      B.VBD.activate t (VBD_DB.vm_of id) (VBD_DB.read_exn id) ;
       VBD_DB.signal id
   | VBD_set_active (id, b) ->
       debug "VBD.set_active %s %b" (VBD_DB.string_of_id id) b ;
@@ -2445,6 +2473,8 @@ and trigger_cleanup_after_failure_atom op t =
   match op with
   | VBD_eject id
   | VBD_plug id
+  | VBD_attach id
+  | VBD_activate id
   | VBD_set_active (id, _)
   | VBD_epoch_begin (id, _, _)
   | VBD_epoch_end (id, _)

ocaml/xenopsd/lib/xenops_server_plugin.ml

@@ -207,7 +207,9 @@ module type S = sig
 
     val epoch_end : Xenops_task.task_handle -> Vm.id -> disk -> unit
 
-    val plug : Xenops_task.task_handle -> Vm.id -> Vbd.t -> unit
+    val attach : Xenops_task.task_handle -> Vm.id -> Vbd.t -> unit
+
+    val activate : Xenops_task.task_handle -> Vm.id -> Vbd.t -> unit
 
     val unplug : Xenops_task.task_handle -> Vm.id -> Vbd.t -> bool -> unit
 

ocaml/xenopsd/lib/xenops_server_simulator.ml

@@ -673,7 +673,9 @@ module VBD = struct
 
   let epoch_end _ (_vm : Vm.id) (_disk : disk) = ()
 
-  let plug _ (vm : Vm.id) (vbd : Vbd.t) = with_lock m (add_vbd vm vbd)
+  let attach _ (vm : Vm.id) (vbd : Vbd.t) = with_lock m (add_vbd vm vbd)
+
+  let activate _ (_vm : Vm.id) (_vbd : Vbd.t) = ()
 
   let unplug _ vm vbd _ = with_lock m (remove_vbd vm vbd)
 

ocaml/xenopsd/lib/xenops_server_skeleton.ml

@@ -145,7 +145,9 @@ module VBD = struct
 
   let epoch_end _ _ _ = ()
 
-  let plug _ _ _ = unimplemented "VBD.plug"
+  let attach _ _ _ = unimplemented "VBD.attach"
+
+  let activate _ _ _ = unimplemented "VBD.activate"
 
   let unplug _ _ _ _ = unimplemented "VBD.unplug"
 

ocaml/xenopsd/lib/xenopsd.ml

@@ -283,6 +283,11 @@ let options =
     , (fun () -> string_of_int !test_open)
     , "TESTING only: open N file descriptors"
     )
+  ; ( "xenopsd-vbd-plug-unplug-legacy"
+    , Arg.Bool (fun x -> Xenops_server.xenopsd_vbd_plug_unplug_legacy := x)
+    , (fun () -> string_of_bool !Xenops_server.xenopsd_vbd_plug_unplug_legacy)
+    , "False if we want to split the plug atomic into attach/activate"
+    )
   ]
 
 let path () = Filename.concat !sockets_path "xenopsd"