Skip to content

Commit 4447647

Browse files
committed
storage_access: avoid the addition of multiple SM with the same type
When manipulating SMs, it's important to avoid more than one plugin per type of SM. Use sets for handling collections of types of SMs to avoid duplicated registrations at the end of the processing done for SMs, which happens on xapi statup. Also make sure that, when converting from SM type to SMs, all SMs for each type are handled, both for updating and registering. Previously some set-like operations where being used, but no explicit deduplication was being done. Signed-off-by: Pau Ruiz Safont <pau.safont@vates.tech>
1 parent 36825ec commit 4447647

File tree

1 file changed

+44
-25
lines changed

1 file changed

+44
-25
lines changed

ocaml/xapi/storage_access.ml

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,18 @@ let log_and_unregister ~__context ~reason __FUN (self, rc) =
154154
rc.API.sM_uuid reason ;
155155
try Db.SM.destroy ~__context ~self with _ -> ()
156156

157+
module StringSet = Set.Make (String)
158+
159+
let list_assoc_all a =
160+
List.filter_map (fun (k, v) ->
161+
if String.equal k a then
162+
Some v
163+
else
164+
None
165+
)
166+
167+
let ( let@ ) f x = f x
168+
157169
(** Synchronise the SM table with the SMAPIv1 plugins on the disk and the SMAPIv2
158170
plugins mentioned in the configuration file whitelist. *)
159171
let on_xapi_start ~__context =
@@ -168,63 +180,69 @@ let on_xapi_start ~__context =
168180
|> List.map (fun (rf, rc) -> (rc.API.sM_type, (rf, rc)))
169181
in
170182
let explicitly_configured_drivers =
171-
List.filter_map
172-
(function `Sm x -> Some x | _ -> None)
173-
!Xapi_globs.sm_plugins
183+
!Xapi_globs.sm_plugins
184+
|> List.filter_map (function `Sm x -> Some x | _ -> None)
185+
|> StringSet.of_list
186+
in
187+
let smapiv1_drivers = Sm.supported_drivers () |> StringSet.of_list in
188+
let configured_drivers =
189+
StringSet.union explicitly_configured_drivers smapiv1_drivers
174190
in
175-
let smapiv1_drivers = Sm.supported_drivers () in
176-
let configured_drivers = explicitly_configured_drivers @ smapiv1_drivers in
177191
let in_use_drivers =
178192
List.map (fun (_, rc) -> rc.API.sR_type) (Db.SR.get_all_records ~__context)
193+
|> StringSet.of_list
179194
in
180-
let to_keep = configured_drivers @ in_use_drivers in
195+
let to_keep = StringSet.union configured_drivers in_use_drivers in
181196
(* Query the message switch to detect running SMAPIv2 plugins. *)
182197
let running_smapiv2_drivers =
183198
if !Xcp_client.use_switch then
184-
get_smapiv2_drivers_from_switch ()
199+
get_smapiv2_drivers_from_switch () |> StringSet.of_list
185200
else (* The SMAPIv2 drivers we know about *)
186-
Listext.List.set_difference to_keep smapiv1_drivers
201+
StringSet.diff to_keep smapiv1_drivers
187202
in
188203
(* Add all the running SMAPIv2 drivers *)
189-
let to_keep = to_keep @ running_smapiv2_drivers in
190-
let unused = Listext.List.set_difference (List.map fst existing) to_keep in
204+
let to_keep = StringSet.union to_keep running_smapiv2_drivers in
205+
let existing_types = List.map fst existing |> StringSet.of_list in
206+
let unused = StringSet.diff existing_types to_keep in
191207
let unavailable =
192208
List.filter (fun (_, (_, rc)) -> not (is_available rc)) existing
193209
in
194210
(* Delete all records which aren't configured or in-use *)
195211
let unregister_unused ty =
196-
let sm = List.assoc ty existing in
212+
let sms = list_assoc_all ty existing in
197213
let reason = "it's not in the allowed list and not in-use" in
198-
log_and_unregister ~__context ~reason __FUNCTION__ sm
214+
List.iter (log_and_unregister ~__context ~reason __FUNCTION__) sms
199215
in
200216
let unregister_unavailable (_, sm) =
201217
let reason = "it's unavailable" in
202218
log_and_unregister ~__context ~reason __FUNCTION__ sm
203219
in
204-
List.iter unregister_unused unused ;
220+
StringSet.iter unregister_unused unused ;
205221
List.iter unregister_unavailable unavailable ;
206222

207223
(* Synchronize SMAPIv1 plugins *)
208224

209225
(* Create all missing SMAPIv1 plugins *)
210-
List.iter
226+
StringSet.iter
211227
(fun ty ->
212228
let query_result =
213229
Sm.info_of_driver ty |> Smint.query_result_of_sr_driver_info
214230
in
215231
Xapi_sm.create_from_query_result ~__context query_result
216232
)
217-
(Listext.List.set_difference smapiv1_drivers (List.map fst existing)) ;
233+
(StringSet.diff smapiv1_drivers existing_types) ;
218234
(* Update all existing SMAPIv1 plugins *)
219-
List.iter
235+
StringSet.iter
220236
(fun ty ->
221237
let query_result =
222238
Sm.info_of_driver ty |> Smint.query_result_of_sr_driver_info
223239
in
224-
Xapi_sm.update_from_query_result ~__context (List.assoc ty existing)
225-
query_result
240+
list_assoc_all ty existing
241+
|> List.iter (fun sm ->
242+
Xapi_sm.update_from_query_result ~__context sm query_result
243+
)
226244
)
227-
(Listext.List.intersect smapiv1_drivers (List.map fst existing)) ;
245+
(StringSet.inter smapiv1_drivers existing_types) ;
228246

229247
(* Synchronize SMAPIv2 plugins *)
230248

@@ -245,18 +263,19 @@ let on_xapi_start ~__context =
245263
f query_result
246264
)
247265
in
248-
List.iter
266+
StringSet.iter
249267
(fun ty ->
250268
with_query_result ty (Xapi_sm.create_from_query_result ~__context)
251269
)
252-
(Listext.List.set_difference running_smapiv2_drivers (List.map fst existing)) ;
270+
(StringSet.diff running_smapiv2_drivers existing_types) ;
253271
(* Update all existing SMAPIv2 plugins *)
254-
List.iter
272+
StringSet.iter
255273
(fun ty ->
256-
with_query_result ty
257-
(Xapi_sm.update_from_query_result ~__context (List.assoc ty existing))
274+
let@ qr = with_query_result ty in
275+
list_assoc_all ty existing
276+
|> List.iter (fun sm -> Xapi_sm.update_from_query_result ~__context sm qr)
258277
)
259-
(Listext.List.intersect running_smapiv2_drivers (List.map fst existing))
278+
(StringSet.inter running_smapiv2_drivers existing_types)
260279

261280
let bind ~__context ~pbd =
262281
let dbg = Context.string_of_task __context in

0 commit comments

Comments
 (0)