Skip to content

Commit

Permalink
[block]: remove the old blktap1 disk pausing infrastructure
Browse files Browse the repository at this point in the history 
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 <dave.scott@eu.citrix.com>
  • Loading branch information
David Scott committed Nov 30, 2011
1 parent 0bb53c6 commit be796c8
Show file tree
Hide file tree
Showing 12 changed files with 76 additions and 469 deletions.
70 changes: 0 additions & 70 deletions ocaml/xapi/quicktest.ml
View file
Expand Up @@ -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 =
Expand Down Expand Up @@ -629,7 +560,6 @@ let vm_powercycle_test s vm =
()
| _ -> ()
end;
vbd_pause_unpause_test s vm;
powercycle_test s vm;
success test

Expand Down
79 changes: 0 additions & 79 deletions ocaml/xapi/sm.ml
View file
Expand Up @@ -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
)
)


9 changes: 0 additions & 9 deletions ocaml/xapi/vmops.ml
View file
Expand Up @@ -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).
Expand Down
150 changes: 7 additions & 143 deletions ocaml/xapi/xapi_vbd.ml
View file
Expand Up @@ -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
7 changes: 2 additions & 5 deletions ocaml/xapi/xapi_vdi.ml
View file
Expand Up @@ -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

Expand Down

0 comments on commit be796c8

Please sign in to comment.