[block]: remove the old blktap1 disk pausing infrastructure

When the upgrade to blktap2 happened, we never had a chance to remove the unused code.

This change:
* simplifies the udev block handling, removing dependencies on blktap2.py
* simplifies the migration code, which nolonger needs to worry about blktap1
  backends being paused
* removes complicated synchronisation logic needed by the old pause/unpause API
* removes a very slow test from quicktest (the VBD pause/unpause one)
* unifies PV and HVM codepaths in checkpoint, for increased reliability
  (HVM guests and PV guests both use the slow resume)

Note in the blktap2 universe, the SM backend operations (eg vdi_snapshot)
manipulate the tapdisks directly (via tap-ctl invocations). This complexity
was only needed because blktap1 lacked such a direct interface.

Signed-off-by: David Scott <[email protected]>

Author: David Scott

ocaml/xapi/quicktest.ml

@@ -265,75 +265,6 @@ let compare_snapshots session_id test one two =
 		compare_vms session_id test x y in
 	List.iter2 compare_all one_s two_s
 
-(* CA-24232 serialising VBD.pause VBD.unpause *)
-let vbd_pause_unpause_test session_id vm =
-	let vm = Client.VM.clone !rpc session_id vm "vbd-pause-unpause-test" in
-	let test = make_test "VBD.pause and VBD.unpause" 1 in
-	start test;
-	finally
-		(fun () ->
-			let vbds = Client.VM.get_VBDs !rpc session_id vm in
-			(* CA-24275 *)
-			debug test "VBD.pause should fail for offline VMs";
-			let vbd = List.hd vbds in
-			begin
-				try
-					ignore(Client.VBD.pause !rpc session_id vbd);
-					failed test "VBD.pause should not have succeeded";
-				with 
-					| (Api_errors.Server_error(code, params)) when code = Api_errors.vm_bad_power_state -> ()
-					| e ->
-						failed test (Printf.sprintf "Unexpected exception from VBD.pause: %s" (Printexc.to_string e))
-			end;
-			debug test "VBD.unpause should fail for offline VMs";
-			begin
-				try
-					ignore(Client.VBD.unpause !rpc session_id vbd "");
-					failed test "VBD.unpause should not have succeeded";
-				with 
-					| (Api_errors.Server_error(code, params)) when code = Api_errors.vm_bad_power_state -> ()
-					| e ->
-						failed test (Printf.sprintf "Unexpected exception from VBD.pause: %s" (Printexc.to_string e))
-			end;
-			debug test "Starting VM";
-			Client.VM.start !rpc session_id vm false false;
-			debug test "A solitary unpause should succeed";
-			Client.VBD.unpause !rpc session_id vbd "";
-			debug test "100 pause/unpause cycles should succeed";
-			for i = 0 to 100 do
-				let token = Client.VBD.pause !rpc session_id vbd in
-				Client.VBD.unpause !rpc session_id vbd token
-			done;
-			debug test "force-shutdown should still work even if a device is paused";
-			debug test "pausing device";
-			let token = Client.VBD.pause !rpc session_id vbd in
-			debug test "performing hard shutdown";
-			let (_: unit) = Client.VM.hard_shutdown !rpc session_id vm in
-			begin
-				try
-					ignore(Client.VBD.unpause !rpc session_id vbd "");
-					failed test "VBD.unpause should not have succeeded";
-				with 
-					| (Api_errors.Server_error(code, params)) when code = Api_errors.vm_bad_power_state -> ()
-					| e ->
-						failed test (Printf.sprintf "Unexpected exception from VBD.pause: %s" (Printexc.to_string e))
-			end;
-			debug test "starting VM again";
-			try
-				Client.VM.start !rpc session_id vm false false;
-				Client.VBD.unpause !rpc session_id vbd token;
-			with 
-				| Api_errors.Server_error(code, params) as e -> 
-					debug test (Printf.sprintf "Api_error %s [ %s ]" code (String.concat "; " params));
-					raise e
-				| e ->
-					debug test (Printf.sprintf "Exception: %s" (Printexc.to_string e));
-					raise e
-		) (fun () -> 
-			Client.VM.hard_shutdown !rpc session_id vm;
-			vm_uninstall test session_id vm);
-	success test
-
 let read_sys path = Stringext.String.strip Stringext.String.isspace (Unixext.string_of_file path)
 
 let verify_network_connectivity session_id test vm =
@@ -629,7 +560,6 @@ let vm_powercycle_test s vm =
 		()
   | _ -> ()
   end;
-  vbd_pause_unpause_test s vm;
   powercycle_test s vm;
   success test
 

ocaml/xapi/sm.ml

@@ -255,82 +255,3 @@ let sr_content_type ~__context ~sr =
 	 Hashtbl.replace sr_content_type_cache sr ty;
 	 ty)
 
-(*****************************************************************************)
-(* Storage-related utility functions                                         *)
-
-open Client
-
-(** Call a function 'f' after pausing all VBDs which are currently_attached to a particular VDI.
-    Take care to unpause everything on the way out of the function. 
-    Notes on how the locking is supposed to work:
-    1. The whole disk should be locked against concurrent hotplug attempts
-    2. A disk might be spontaneously unplugged between the point where we example 'currently_attached'
-    and the point where we call VBD.pause -- it is safe to skip over these.
-    3. In Orlando concurrent VM.snapshot calls are allowed (as part of the snapshot-with-quiesce work)
-    we must pause VBDs in order of device number to avoid a deadlock where 'VM.get_VBDs' returns the
-    devices in a different order in a parallel call
-*)
-let with_all_vbds_paused ~__context ~vdis f = 
-  (* We need to keep track of the VBDs we've paused so we can go back and unpause them *)
-  let paused_so_far = ref [] in
-  let vbds = List.concat (List.map (fun vdi -> Db.VDI.get_VBDs ~__context ~self:vdi) vdis) in  
-  let vbds = List.filter 
-    (fun self -> Db.VBD.get_currently_attached ~__context ~self 
-                 && Db.VM.get_power_state ~__context ~self:(Db.VBD.get_VM ~__context ~self) <> `Suspended) 
-    vbds in
-  (* CA-24232: prevent concurrent snapshots of the same VM leading to deadlock since the database
-     doesn't guarantee to return records in the same order. *)
-  let vbds = List.sort (fun a b ->
-			  let a_device = Db.VBD.get_device ~__context ~self:a 
-			  and b_device = Db.VBD.get_device ~__context ~self:b in
-			  compare a_device b_device) vbds in
-  Helpers.call_api_functions ~__context
-    (fun rpc session_id ->
-       finally
-	 (fun () ->
-	    (* Attempt to pause all the VBDs *)
-	    List.iter
-	      (fun vbd ->
-		 (* NB it is possible for a disk to spontaneously unplug itself: 
-		    we are safe to skip these VBDs. *)
-		 (* NB it is possible for a VM to suddenly migrate; if so we retry *)
-		 let finished = ref false in
-		 while not !finished do
-		   try
-			   let token = Client.VBD.pause rpc session_id vbd in
-			   paused_so_far := (vbd, token) :: !paused_so_far;
-			   finished := true
-		   with 
-		   | Api_errors.Server_error(code, _) when code = Api_errors.device_not_attached ->
-		       warn "Not pausing VBD %s because it appears to have spontaneously unplugged" (Ref.string_of vbd);
-		       finished := true
-		   | Api_errors.Server_error(code, _) when code = Api_errors.vm_bad_power_state ->
-		       warn "Not pausing VBD %s because the VM has shutdown" (Ref.string_of vbd);
-		       finished := true
-		   | Api_errors.Server_error(code, [ cls; reference ]) when code = Api_errors.handle_invalid && reference = Ref.string_of vbd ->
-		       warn "Not pausing VBD %s because it has been deleted" reference;
-		       finished := true
-		   | Api_errors.Server_error(code, _) when code = Api_errors.vm_not_resident_here ->
-		       warn "Pausing VBD %s temporarily failed because VM has just migrated; retrying" (Ref.string_of vbd);
-		       (* !finished = false *)
-		   | e ->
-		       error "Error pausing VBD %s: %s" (Ref.string_of vbd) (ExnHelper.string_of_exn e);
-		       raise e (* let this propagate *)
-		 done
-	      ) vbds;
-	    (* Now all the VBDs are paused we can call the main function *)
-	    f ()
-	 )
-	 (fun () ->
-	    (* I would have used Helpers.log_exn_continue but I wanted to log the errors as warnings
-	       and not as only debug messages *)
-	    List.iter
-	      (fun (vbd, token) ->
-		 try Client.VBD.unpause rpc session_id vbd token
-		 with e ->
-		   warn "Failed to unpause VBD %s: %s" (Ref.string_of vbd) (ExnHelper.string_of_exn e))
-	      !paused_so_far
-	 )
-    )
-	 
-

ocaml/xapi/vmops.ml

@@ -471,15 +471,6 @@ let destroy_domain ?(preserve_xs_vm=false) ?(unplug_frontends=true) ?(clear_curr
 				(fun () ->
 					Db.VBD.set_currently_attached ~__context ~self:vbd ~value:false
 				) ()
-		) all_vbds;
-
-	(* We unpause every VBD which allows any pending VBD.pause thread to unblock, acquire the VM lock in turn and check the state *)
-	List.iter
-		(fun vbd ->
-			Helpers.log_exn_continue (Printf.sprintf "Vmops.destroy_domain: pre-emptively unpausing VBD: %s" (Ref.string_of vbd))
-				(fun () ->
-					Xapi_vbd.clean_up_on_domain_destroy vbd (* effect is to unblock threads, not actually unpause *)
-				) ();
 		) all_vbds
 
 (* Destroy a VM's domain and all runtime state (domid etc).

ocaml/xapi/xapi_vbd.ml

@@ -293,147 +293,11 @@ let refresh ~__context ~vbd ~vdi =
 
 open Threadext
 open Pervasiveext
+open Fun
 
-(* VBD.pause and VBD.unpause are used for two purposes:
-   1. to stop IO and give blkback/blktap a kick during disk online resize/ disk snapshot
-   2. to stop IO while the background coalescing daemon moves metadata around
-   As explained in CA-24232, there is an issue where both purposes of call can happen simulataneously
-   and so we need to perform some serialisation otherwise the first 'unpause' will prematurely reactivate
-   the device.
-
-   We assume that all client threads issue 'pause' then 'unpause' calls in sequence, never in parallel.
-   We assume that clients *rarely* fail between 'pause' and 'unpause' but in this extreme case, the 
-   VM can always be force-shutdown (ie the VM is 'unlocked' while a disk is paused)
-
-   We let one pause request through at a time and set vbd_pause_state.in_progress to true. Other
-   pause threads wait until in_progress is reset to false by device_is_unpaused.
-   We count the number of threads running VBD.pause in order to remove the vbd_pause_state from
-   the hashtable when the unpause happens.   
-*)
-
-(* One of these is stored per actively-paused VBD *)
-type vbd_pause_state = {
-  m: Mutex.t;
-  c: Condition.t;
-  mutable in_progress: bool; (* true if a VBD.pause thread is performing the operation *) (* protected by 'm' *)
-
-  mutable blocked_threads: int; (* counts the number of threads blocked in pause *) (* protected by 'paused_vbds_m' *)
-}
-
-(* Table of vbd_pause_state records *)
-let paused_vbds = Hashtbl.create 10
-let paused_vbds_m = Mutex.create ()
-
-(* Either the pause just failed or an unpause has succeeded: wake up any blocked threads or cleanup if none are waiting *)
-let device_is_unpaused self state =
-  (* If no threads are waiting in the queue to perform fresh pauses then we can delete the vbd_pause_state record.
-     If some thread(s) are waiting then we signal them. *)
-  Mutex.execute paused_vbds_m
-    (fun () -> 
-       debug "device_is_unpaused blocked_threads=%d" state.blocked_threads;
-       if state.blocked_threads = 0 
-       then (Hashtbl.remove paused_vbds self; debug "Removed hashtbl entry for %s" (Ref.string_of self))
-       else (Mutex.execute state.m (fun () -> state.in_progress <- false); Condition.signal state.c))
-
-let pause_common ~__context ~vm ~self =
-
-	(* XXX: these checks may be redundant since the message forwarding layer checks this stuff *)
-	let localhost = Helpers.get_localhost ~__context in
-	
-	(* Since we blocked not holding the per-VM mutex, we need to double-check that the VM is still present *)
-	if not(Helpers.is_running ~__context ~self:vm)
-	then raise (Api_errors.Server_error(Api_errors.vm_bad_power_state, 
-										[ Ref.string_of vm; 
-										  Record_util.power_to_string `Running; 
-										  Record_util.power_to_string (Db.VM.get_power_state ~__context ~self:vm) ]));
-	
-	if not(Db.VBD.get_currently_attached ~__context ~self)
-	then raise (Api_errors.Server_error(Api_errors.device_not_attached, [ Ref.string_of self ]));
-	
-	(* Make sure VM has not migrated *)
-	if Db.VM.get_resident_on ~__context ~self:vm <> localhost
-	then raise (Api_errors.Server_error(Api_errors.vm_not_resident_here, [ Ref.string_of vm; Ref.string_of localhost ]));
-		
-	let device = Xen_helpers.device_of_vbd ~__context ~self in
-	try
-		with_xs (fun xs -> Device.Vbd.pause ~xs device)
-	with Device.Device_shutdown | Device.Device_not_found ->
-		raise (Api_errors.Server_error(Api_errors.device_not_attached, [ Ref.string_of self ]))
-
-let pause ~__context ~self : string = 
-  let vm = Db.VBD.get_VM ~__context ~self in
-
-  (* 1. Find the current vbd pause state record or make a new one *)
-  let state = Mutex.execute paused_vbds_m 
-    (fun () -> 
-       let state = 
-	 if Hashtbl.mem paused_vbds self 
-	 then Hashtbl.find paused_vbds self
-	 else 
-	   let state = { m = Mutex.create (); c = Condition.create (); in_progress = false; blocked_threads = 0 } in
-	   Hashtbl.replace paused_vbds self state;
-	   state in
-       state.blocked_threads <- state.blocked_threads + 1;
-       state
-    ) in
-  (* Multiple VBD.pause threads may end up here. Note the vbd_pause_state record will linger until
-     the final unpause *)
-  try
-    finally
-      (fun () ->
-	 (* Only one VBD.pause thread is allowed to acquire the 'in_progress' flag/lock *)
-	 Mutex.execute state.m
-	   (fun () -> 
-	      while state.in_progress do Condition.wait state.c state.m done;
-	      state.in_progress <- true);
-	 (* One lucky VBD.pause thread will get here at a time *)
-	 pause_common ~__context ~vm ~self
-      )
-      (* Decrement the 'blocked_threads' refcount *)
-      (fun () -> Mutex.execute paused_vbds_m (fun () -> state.blocked_threads <- state.blocked_threads - 1));
-      (* This thread returns leaving the 'in_progress' flag true -- this is the lock. *)
-
-  with e ->
-    (* Something bad happened when we were trying to pause so unwind and cleanup *)
-    error "Unexpected error during VBD.pause: %s" (ExnHelper.string_of_exn e);
-    device_is_unpaused self state;
-    raise e
-
-let state_of_vbd self = 
-  Mutex.execute paused_vbds_m
-    (fun () -> 
-       if Hashtbl.mem paused_vbds self 
-       then Some (Hashtbl.find paused_vbds self)
-       else None) 
-
-let unpause ~__context ~self ~token = 
-  (* Find the vbd_pause_state record which must exist if the client has called VBD.pause beforehand 
-     UNLESS someone restarted xapi *)
-  let state = state_of_vbd self in
-  
-  (* Note we don't check that the client has waited for the VBD.pause to return: we trust the client not
-     to be stupid anyway by the nature of this API *)
-  let device = Xen_helpers.device_of_vbd ~__context ~self in
-  try
-    with_xs (fun xs -> Device.Vbd.unpause ~xs device token);
-    Opt.iter (device_is_unpaused self) state
-  with 
-  | Device.Device_not_paused ->
-      debug "Ignoring Device_not_paused exception";
-      Opt.iter (device_is_unpaused self) state
-  | Device.Device_not_found ->
-      debug "Ignoring Device_not_found exception";
-      Opt.iter (device_is_unpaused self) state
-  | Device.Pause_token_mismatch ->
-      warn "Unpause left device paused because supplied token did not match"
-
-let flush ~__context self =
-  debug "Flushing vbd %s" (Ref.string_of self);
-  let token = pause ~__context ~self in
-  unpause ~__context ~self ~token 
-
-(** Called on domain destroy to signal any blocked pause threads to re-evaluate the state of the world 
-    and give up *)
-let clean_up_on_domain_destroy self = 
-  let state = state_of_vbd self in
-  Opt.iter (device_is_unpaused self) state
+let pause ~__context ~self =
+	let vdi = Db.VBD.get_VDI ~__context ~self in
+	let sr = Db.VDI.get_SR ~__context ~self:vdi |> Ref.string_of in
+	raise (Api_errors.Server_error(Api_errors.sr_operation_not_supported, [ sr ]))
+
+let unpause = pause

ocaml/xapi/xapi_vdi.ml

@@ -418,12 +418,9 @@ let resize_online ~__context ~vdi ~size =
   Sm.assert_pbd_is_plugged ~__context ~sr:(Db.VDI.get_SR ~__context ~self:vdi);
   Xapi_vdi_helpers.assert_managed ~__context ~vdi;
 
-  (* Need to carefully pause and unpause all active VBDs *)
-  let vdi_info = Sm.with_all_vbds_paused ~__context ~vdis:[vdi]
-    (fun () ->
-       Sm.call_sm_vdi_functions ~__context ~vdi
+  let vdi_info = Sm.call_sm_vdi_functions ~__context ~vdi
 	 (fun srconf srtype sr ->
-	    Sm.vdi_resize_online srconf srtype sr vdi size)
+	    Sm.vdi_resize_online srconf srtype sr vdi size
     ) in
   after_resize ~__context ~vdi ~size vdi_info