# HG changeset patch # User David Scott # Date 1270636851 -3600 # Node ID b08de85a8e1e818051486b6e1bd88bd828093862 # Parent 61f47a47e5419b10177b17554de28c4b336c1754 CA-39386: Perform a best-effort check that we can access a suspended snapshot's suspend_VDI before starting a revert. We check that the SR's PBD is plugged and that the Host has live set to true. Signed-off-by: David Scott diff -r 61f47a47e541 -r b08de85a8e1e ocaml/xapi/helpers.ml --- a/ocaml/xapi/helpers.ml Wed Mar 31 10:30:27 2010 +0100 +++ b/ocaml/xapi/helpers.ml Wed Apr 07 11:40:51 2010 +0100 @@ -408,10 +408,11 @@ (* Return the PBD for specified SR on a specific host *) (* Just say an SR is shared if it has more than one PBD *) +let is_sr_shared ~__context ~self = List.length (Db.SR.get_PBDs ~__context ~self) > 1 (* This fn is only executed by master *) let get_shared_srs ~__context = let srs = Db.SR.get_all ~__context in - List.filter (fun sr->(List.length (Db.SR.get_PBDs ~__context ~self:sr))>1) srs + List.filter (fun self -> is_sr_shared ~__context ~self) srs let get_pool ~__context = List.hd (Db.Pool.get_all ~__context) let get_main_ip_address ~__context = diff -r 61f47a47e541 -r b08de85a8e1e ocaml/xapi/message_forwarding.ml --- a/ocaml/xapi/message_forwarding.ml Wed Mar 31 10:30:27 2010 +0100 +++ b/ocaml/xapi/message_forwarding.ml Wed Apr 07 11:40:51 2010 +0100 @@ -304,6 +304,26 @@ with _ -> false in List.filter filterfn hosts + +(* Given an SR, return a PBD to use for some storage operation. *) +(* In the case of SR.destroy we need to be able to forward the SR operation when all + PBDs are unplugged - this is the reason for the consider_unplugged_pbds optional + argument below. All other SR ops only consider plugged PBDs... *) +let choose_pbd_for_sr ?(consider_unplugged_pbds=false) ~__context ~self () = + let all_pbds = Db.SR.get_PBDs ~__context ~self in + let plugged_pbds = List.filter (fun pbd->Db.PBD.get_currently_attached ~__context ~self:pbd) all_pbds in + let pbds_to_consider = if consider_unplugged_pbds then all_pbds else plugged_pbds in + if Helpers.is_sr_shared ~__context ~self then + let master = Db.Pool.get_master ~__context ~self:(Helpers.get_pool ~__context) in + let master_pbds = Db.Host.get_PBDs ~__context ~self:master in + (* shared SR operations must happen on the master *) + match Listext.List.intersect pbds_to_consider master_pbds with + | pbd :: _ -> pbd (* ok, master plugged *) + | [] -> raise (Api_errors.Server_error(Api_errors.sr_no_pbds, [ Ref.string_of self ])) (* can't do op, master pbd not plugged *) + else + match pbds_to_consider with + | [] -> raise (Api_errors.Server_error(Api_errors.sr_no_pbds, [ Ref.string_of self ])) + | pbd :: _ -> pbd let loadbalance_host_operation ~__context ~hosts ~doc ~op (f: API.ref_host -> unit) = @@ -1244,6 +1264,22 @@ let local_fn = Local.VM.revert ~snapshot in let forward_fn session_id rpc = Local.VM.revert ~__context ~snapshot in + (* We need to do a best-effort check that any suspend_VDI referenced by + the snapshot (not the current VM) is currently accessible. This is because + the revert code first clears space by deleting current VDIs before cloning + the suspend VDI: we want to minimise the probability that the operation fails + part-way through. *) + if Db.VM.get_power_state ~__context ~self:snapshot = `Suspended then begin + let suspend_VDI = Db.VM.get_suspend_VDI ~__context ~self:snapshot in + let sr = Db.VDI.get_SR ~__context ~self:suspend_VDI in + let pbd = choose_pbd_for_sr ~__context ~self:sr () in + let host = Db.PBD.get_host ~__context ~self:pbd in + let metrics = Db.Host.get_metrics ~__context ~self:host in + let live = Db.is_valid_ref metrics && (Db.Host_metrics.get_live ~__context ~self:metrics) in + if not live + then raise (Api_errors.Server_error(Api_errors.host_not_live, [ Ref.string_of host ])) + end; + with_vm_operation ~__context ~self:snapshot ~doc:"VM.revert" ~op:`revert (fun () -> with_vm_operation ~__context ~self:vm ~doc:"VM.reverting" ~op:`reverting (fun () -> @@ -2437,28 +2473,11 @@ (* -------- Forwarding helper functions: ------------------------------------ *) - (* Forward SR operation to host that has a suitable plugged PBD *) - (* In the case of SR.destroy we need to be able to forward the SR operation when all PBDs are unplugged - this - is the reason for the consider_unplugged_pbds optional argument below. All other SR ops only consider plugged PBDs... *) - let forward_sr_op ?(consider_unplugged_pbds=false) ~local_fn ~__context ~self op = - let shared_srs = Helpers.get_shared_srs ~__context in - let all_pbds_for_this_sr = Db.SR.get_PBDs ~__context ~self in - let plugged_pbds_for_this_sr = List.filter (fun pbd->Db.PBD.get_currently_attached ~__context ~self:pbd) all_pbds_for_this_sr in - let pbds_to_consider = if consider_unplugged_pbds then all_pbds_for_this_sr else plugged_pbds_for_this_sr in - let i_am_shared = List.mem self shared_srs in - if i_am_shared then - let master = Db.Pool.get_master ~__context ~self:(Helpers.get_pool ~__context) in - (* we have to do shared SR op on the master; we throw an error if master PBD not plugged *) - match List.filter (fun pbd->(Db.PBD.get_host ~__context ~self:pbd)=master) pbds_to_consider with - _::_ -> (* ok, master plugged *) - local_fn ~__context (* go ahead and do op on master *) - | [] -> raise (Api_errors.Server_error(Api_errors.sr_no_pbds, [ Ref.string_of self ])) (* can't do op, master pbd not plugged *) - else - match pbds_to_consider with - [] -> raise (Api_errors.Server_error(Api_errors.sr_no_pbds, [ Ref.string_of self ])) - | (pbd::_) -> - let host = Db.PBD.get_host ~__context ~self:pbd in - do_op_on ~local_fn ~__context ~host op + (* Forward SR operation to host that has a suitable plugged (or unplugged) PBD *) + let forward_sr_op ?consider_unplugged_pbds ~local_fn ~__context ~self op = + let pbd = choose_pbd_for_sr ?consider_unplugged_pbds ~__context ~self () in + let host = Db.PBD.get_host ~__context ~self:pbd in + do_op_on ~local_fn ~__context ~host op (* do op on a host that can view multiple SRs *) let forward_sr_multiple_op ~local_fn ~__context ~srs op =