Skip to content

Commit 1186195

Browse files
committed
CA-94283: Block VDI-Destroy while HA Enabling/Disabling is in progress
1) Adding new fields to pool-record: current_operations and allowed_operations. 2) Block HA `metadata` and `statefile` VDIs deletion while HA Enabling/Disabling is in progress. Signed-off-by: Sharad Yadav <[email protected]>
1 parent 0175b2c commit 1186195

File tree

10 files changed

+154
-6
lines changed

10 files changed

+154
-6
lines changed

ocaml/client_records/record_util.ml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ let string_to_vm_operation x =
8282
then (raise (Api_errors.Server_error(Api_errors.invalid_value, [ "blocked_operation"; x ])))
8383
else List.assoc x table
8484

85+
let pool_operation_to_string = function
86+
| `ha_enable -> "ha_enable"
87+
| `ha_disable -> "ha_disable"
88+
8589
let host_operation_to_string = function
8690
| `provision -> "provision"
8791
| `evacuate -> "evacuate"

ocaml/client_records/records.ml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,12 @@ let pool_record rpc session_id pool =
480480
~add_to_map:(fun k v -> Client.Pool.add_to_other_config rpc session_id pool k v)
481481
~remove_from_map:(fun k -> Client.Pool.remove_from_other_config rpc session_id pool k)
482482
~get_map:(fun () -> (x ()).API.pool_other_config) ();
483+
make_field ~name:"allowed-operations"
484+
~get:(fun () -> String.concat "; " (List.map Record_util.pool_operation_to_string (x ()).API.pool_allowed_operations))
485+
~get_set:(fun () -> List.map Record_util.pool_operation_to_string (x ()).API.pool_allowed_operations) ();
486+
make_field ~name:"current-operations"
487+
~get:(fun () -> String.concat "; " (List.map (fun (a,b) -> Record_util.pool_operation_to_string b) (x ()).API.pool_current_operations))
488+
~get_set:(fun () -> List.map (fun (a,b) -> Record_util.pool_operation_to_string b) (x ()).API.pool_current_operations) ();
483489
make_field ~name:"ha-enabled" ~get:(fun () -> string_of_bool (x ()).API.pool_ha_enabled) ();
484490
make_field ~name:"ha-configuration" ~get:(fun () -> Record_util.s2sm_to_string "; " (x ()).API.pool_ha_configuration) ();
485491
make_field ~name:"ha-statefiles" ~get:(fun () -> String.concat "; " (List.map (fun x -> get_uuid_from_ref (Ref.of_string x)) (x ()).API.pool_ha_statefiles)) ();

ocaml/idl/api_errors.ml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,8 @@ let ballooning_disabled = "BALLOONING_DISABLED"
374374
let ha_host_is_armed = "HA_HOST_IS_ARMED"
375375
let ha_is_enabled = "HA_IS_ENABLED"
376376
let ha_not_enabled = "HA_NOT_ENABLED"
377+
let ha_enable_in_progress = "HA_ENABLE_IN_PROGRESS"
378+
let ha_disable_in_progress = "HA_DISABLE_IN_PROGRESS"
377379
let ha_not_installed = "HA_NOT_INSTALLED"
378380
let ha_host_cannot_see_peers = "HA_HOST_CANNOT_SEE_PEERS"
379381
let ha_too_few_hosts = "HA_TOO_FEW_HOSTS"

ocaml/idl/datamodel.ml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,10 @@ let _ =
11371137
~doc:"The operation could not be performed because HA is enabled on the Pool" ();
11381138
error Api_errors.ha_not_enabled [ ]
11391139
~doc:"The operation could not be performed because HA is not enabled on the Pool" ();
1140+
error Api_errors.ha_enable_in_progress [ ]
1141+
~doc:"The operation could not be performed because HA enable is in progress" ();
1142+
error Api_errors.ha_disable_in_progress [ ]
1143+
~doc:"The operation could not be performed because HA disable is in progress" ();
11401144
error Api_errors.ha_not_installed [ "host" ]
11411145
~doc:"The operation could not be performed because the HA software is not installed on this host." ();
11421146
error Api_errors.ha_host_cannot_see_peers [ "host"; "all"; "subset" ]
@@ -5978,6 +5982,12 @@ let crashdump =
59785982
])
59795983
()
59805984

5985+
let pool_operations =
5986+
Enum ("pool_allowed_operations",
5987+
[ "ha_enable", "Indicates this pool is in the process of enabling HA";
5988+
"ha_disable", "Indicates this pool is in the process of disabling HA";
5989+
])
5990+
59815991
let pool_enable_ha = call
59825992
~in_product_since:rel_miami
59835993
~name:"enable_ha"
@@ -6604,8 +6614,8 @@ let pool =
66046614
; pool_disable_ssl_legacy
66056615
]
66066616
~contents:
6607-
[uid ~in_oss_since:None _pool
6608-
; field ~in_oss_since:None ~qualifier:RW ~ty:String "name_label" "Short name"
6617+
([uid ~in_oss_since:None _pool] @
6618+
[ field ~in_oss_since:None ~qualifier:RW ~ty:String "name_label" "Short name"
66096619
; field ~in_oss_since:None ~qualifier:RW ~ty:String "name_description" "Description"
66106620
; field ~in_oss_since:None ~qualifier:DynamicRO ~ty:(Ref _host) "master" "The host that is pool master"
66116621
; field ~in_oss_since:None ~qualifier:RW ~ty:(Ref _sr) "default_SR" "Default SR for VDIs"
@@ -6635,7 +6645,7 @@ let pool =
66356645
; field ~in_oss_since:None ~in_product_since:rel_boston ~qualifier:DynamicRO ~ty:(Set (Ref _vdi)) "metadata_VDIs" "The set of currently known metadata VDIs for this pool"
66366646
; field ~in_oss_since:None ~in_product_since:rel_dundee ~qualifier:DynamicRO ~default_value:(Some (VString "xhad")) ~ty:String "ha_cluster_stack" "The ha cluster manager stack"
66376647

6638-
]
6648+
] @ (allowed_and_current_operations pool_operations) )
66396649
()
66406650

66416651
(** Auth class *)

ocaml/test/test_common.ml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ let make_pool ~__context ~master ?(name_label="") ?(name_description="")
125125
?(gui_config=[]) ?(health_check_config=[]) ?(wlb_url="") ?(wlb_username="") ?(wlb_password=Ref.null)
126126
?(wlb_enabled=false) ?(wlb_verify_cert=false) ?(redo_log_enabled=false)
127127
?(redo_log_vdi=Ref.null) ?(vswitch_controller="") ?(restrictions=[])
128-
?(other_config=[Xapi_globs.memory_ratio_hvm; Xapi_globs.memory_ratio_pv])
128+
?(current_operations=[]) ?(allowed_operations=[])
129+
?(other_config=[Xapi_globs.memory_ratio_hvm; Xapi_globs.memory_ratio_pv])
129130
?(ha_cluster_stack="xhad") () =
130131
let pool_ref = Ref.make () in
131132
Db.Pool.create ~__context ~ref:pool_ref
@@ -135,6 +136,7 @@ let make_pool ~__context ~master ?(name_label="") ?(name_description="")
135136
~ha_plan_exists_for ~ha_allow_overcommit ~ha_overcommitted ~blobs ~tags
136137
~gui_config ~health_check_config ~wlb_url ~wlb_username ~wlb_password ~wlb_enabled
137138
~wlb_verify_cert ~redo_log_enabled ~redo_log_vdi ~vswitch_controller
139+
~current_operations ~allowed_operations
138140
~restrictions ~other_config ~ha_cluster_stack;
139141
pool_ref
140142

ocaml/xapi/OMakefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ XAPI_MODULES = $(COMMON) \
239239
xapi_logs_download \
240240
xapi_vncsnapshot \
241241
xapi_pool \
242+
xapi_pool_helpers \
242243
xapi_upgrade \
243244
xapi_blob \
244245
xapi_message \

ocaml/xapi/dbsync_master.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ let create_pool_record ~__context =
3434
~ha_host_failures_to_tolerate:0L ~ha_plan_exists_for:0L ~ha_allow_overcommit:false ~ha_overcommitted:false ~blobs:[] ~tags:[] ~gui_config:[] ~health_check_config:[]
3535
~wlb_url:"" ~wlb_username:"" ~wlb_password:Ref.null ~wlb_enabled:false ~wlb_verify_cert:false
3636
~redo_log_enabled:false ~redo_log_vdi:Ref.null ~vswitch_controller:"" ~restrictions:[]
37+
~current_operations:[] ~allowed_operations:[]
3738
~other_config:[
3839
Xapi_globs.memory_ratio_hvm;
3940
Xapi_globs.memory_ratio_pv;

ocaml/xapi/message_forwarding.ml

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,26 @@ module Forward = functor(Local: Custom_actions.CUSTOM_ACTIONS) -> struct
647647
module Pool = struct
648648
include Local.Pool
649649

650+
(** Add to the Pool's current operations, call a function and then remove from the
651+
current operations. Ensure the allowed_operations are kept up to date. *)
652+
let with_pool_operation ~__context ~self ~doc ~op f =
653+
let task_id = Ref.string_of (Context.get_task_id __context) in
654+
retry_with_global_lock ~__context ~doc
655+
(fun () ->
656+
Xapi_pool_helpers.assert_operation_valid ~__context ~self ~op;
657+
Db.Pool.add_to_current_operations ~__context ~self ~key:task_id ~value:op);
658+
Xapi_pool_helpers.update_allowed_operations ~__context ~self;
659+
(* Then do the action with the lock released *)
660+
finally f
661+
(* Make sure to clean up at the end *)
662+
(fun () ->
663+
try
664+
Db.Pool.remove_from_current_operations ~__context ~self ~key:task_id;
665+
Xapi_pool_helpers.update_allowed_operations ~__context ~self;
666+
Early_wakeup.broadcast (Datamodel._pool, Ref.string_of self);
667+
with
668+
_ -> ())
669+
650670
let eject ~__context ~host =
651671
info "Pool.eject: pool = '%s'; host = '%s'" (current_pool_uuid ~__context) (host_uuid ~__context host);
652672
let local_fn = Local.Pool.eject ~host in
@@ -663,11 +683,19 @@ module Forward = functor(Local: Custom_actions.CUSTOM_ACTIONS) -> struct
663683
(String.concat ", " (List.map Ref.string_of heartbeat_srs))
664684
(String.concat "; " (List.map (fun (k, v) -> k ^ "=" ^ v) configuration))
665685
cluster_stack ;
666-
Local.Pool.enable_ha __context heartbeat_srs configuration cluster_stack
686+
let pool = Helpers.get_pool ~__context in
687+
with_pool_operation ~__context ~doc:"Pool.ha_disable" ~self:pool ~op:`ha_disable
688+
(fun () ->
689+
Local.Pool.enable_ha __context heartbeat_srs configuration cluster_stack
690+
)
667691

668692
let disable_ha ~__context =
669693
info "Pool.disable_ha: pool = '%s'" (current_pool_uuid ~__context);
670-
Local.Pool.disable_ha __context
694+
let pool = Helpers.get_pool ~__context in
695+
with_pool_operation ~__context ~doc:"Pool.ha_disable" ~self:pool ~op:`ha_disable
696+
(fun () ->
697+
Local.Pool.disable_ha __context
698+
)
671699

672700
let ha_prevent_restarts_for ~__context ~seconds =
673701
info "Pool.ha_prevent_restarts_for: pool = '%s'; seconds = %Ld" (current_pool_uuid ~__context) seconds;

ocaml/xapi/xapi_pool_helpers.ml

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
(*
2+
* Copyright (C) 2006-2009 Citrix Systems Inc.
3+
*
4+
* This program is free software; you can redistribute it and/or modify
5+
* it under the terms of the GNU Lesser General Public License as published
6+
* by the Free Software Foundation; version 2.1 only. with the special
7+
* exception on linking described in file LICENSE.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU Lesser General Public License for more details.
13+
*)
14+
15+
module D=Debug.Make(struct let name="xapi" end)
16+
open D
17+
18+
open Db_filter
19+
open Record_util
20+
open Threadext
21+
open Api_errors
22+
23+
let all_operations = [ `ha_enable; `ha_disable ]
24+
25+
(** Returns a table of operations -> API error options (None if the operation would be ok) *)
26+
let valid_operations ~__context record _ref' =
27+
let _ref = Ref.string_of _ref' in
28+
let current_ops = List.map snd record.Db_actions.pool_current_operations in
29+
30+
let table = Hashtbl.create 10 in
31+
List.iter (fun x -> Hashtbl.replace table x None) all_operations;
32+
let set_errors (code: string) (params: string list) (ops: API.pool_allowed_operations_set) =
33+
List.iter (fun op ->
34+
if Hashtbl.find table op = None
35+
then Hashtbl.replace table op (Some(code, params))) ops in
36+
37+
(* HA enable or disable cannot run if HA enable is in progress *)
38+
if List.mem `ha_enable current_ops
39+
then begin
40+
set_errors Api_errors.ha_enable_in_progress [] [ `ha_enable ];
41+
set_errors Api_errors.ha_enable_in_progress [] [ `ha_disable ]
42+
end;
43+
(* HA enable or disable cannot run if HA disable is in progress *)
44+
if List.mem `ha_disable current_ops
45+
then begin
46+
set_errors Api_errors.ha_disable_in_progress [] [ `ha_enable ];
47+
set_errors Api_errors.ha_disable_in_progress [] [ `ha_disable ]
48+
end;
49+
50+
(* HA disable cannot run if HA is already disabled on a pool *)
51+
(* HA enable cannot run if HA is already enabled on a pool *)
52+
let ha_enabled = Db.Pool.get_ha_enabled ~__context ~self:(Helpers.get_pool ~__context) in
53+
if ha_enabled then
54+
set_errors Api_errors.ha_is_enabled [] [ `ha_enable ]
55+
else
56+
set_errors Api_errors.ha_not_enabled [] [ `ha_disable ];
57+
58+
table
59+
60+
let throw_error table op =
61+
if not(Hashtbl.mem table op)
62+
then raise (Api_errors.Server_error(Api_errors.internal_error, [ Printf.sprintf "xapi_pool_helpers.assert_operation_valid unknown operation: %s" (pool_operation_to_string op) ]));
63+
64+
match Hashtbl.find table op with
65+
| Some (code, params) -> raise (Api_errors.Server_error(code, params))
66+
| None -> ()
67+
68+
let assert_operation_valid ~__context ~self ~(op:API.pool_allowed_operations) =
69+
let all = Db.Pool.get_record_internal ~__context ~self in
70+
let table = valid_operations ~__context all self in
71+
throw_error table op
72+
73+
let update_allowed_operations ~__context ~self : unit =
74+
let all = Db.Pool.get_record_internal ~__context ~self in
75+
let valid = valid_operations ~__context all self in
76+
let keys = Hashtbl.fold (fun k v acc -> if v = None then k :: acc else acc) valid [] in
77+
Db.Pool.set_allowed_operations ~__context ~self ~value:keys
78+
79+
(* Checks whether HA enable is in progress *)
80+
let ha_enable_in_progress ~__context =
81+
let pool = Helpers.get_pool ~__context in
82+
let current_ops = Db.Pool.get_current_operations ~__context ~self:pool in
83+
if List.exists (fun (_, x) -> x = `ha_enable) current_ops then true else false
84+
85+
(* Checks whether HA disable is in progress *)
86+
let ha_disable_in_progress ~__context =
87+
let pool = Helpers.get_pool ~__context in
88+
let current_ops = Db.Pool.get_current_operations ~__context ~self:pool in
89+
if List.exists (fun (_, x) -> x = `ha_disable) current_ops then true else false
90+

ocaml/xapi/xapi_vdi.ml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,10 @@ let check_operation_error ~__context ?(sr_records=[]) ?(pbd_records=[]) ?(vbd_re
139139
else
140140
if ha_enabled && List.mem record.Db_actions.vDI_type [ `ha_statefile; `redo_log ]
141141
then Some (Api_errors.ha_is_enabled, [])
142+
else if List.mem record.Db_actions.vDI_type [`ha_statefile; `metadata ] && Xapi_pool_helpers.ha_enable_in_progress ~__context
143+
then Some (Api_errors.ha_enable_in_progress, [])
144+
else if List.mem record.Db_actions.vDI_type [`ha_statefile; `metadata ] && Xapi_pool_helpers.ha_disable_in_progress ~__context
145+
then Some (Api_errors.ha_disable_in_progress, [])
142146
else
143147
if not Smint.(has_capability Vdi_delete sm_features)
144148
then Some (Api_errors.sr_operation_not_supported, [Ref.string_of sr])

0 commit comments

Comments
 (0)