Skip to content

Commit

Permalink
CA-390025: do not override SR's client-set metadata on update (xapi-p…
Browse files Browse the repository at this point in the history
…roject#6165)

Some plugins may not store the client-set metadata, and return a static
value
when replying to the update. This would override the values that a
client
used when the SR was created, or set afterwards, which is unexpected.

Now name_label and name_description fields returned by the plugins are
ignored
on update.

Current set_name_label and set_name_description rely on the update
mechanism to
work. Instead, add database call at the end of the methods to ensure
both xapi
and the SR backend are synchronized, even when the latter fails to
update the
values.

Tested on GFS2 tests (JR 4175192), as well as ring3 bvt + bst (209177),
and storage validation tests (SR 209180)
  • Loading branch information
psafont authored Dec 10, 2024
2 parents 2c9c9e7 + 098546a commit d8baca7
Showing 1 changed file with 2 additions and 22 deletions.
24 changes: 2 additions & 22 deletions ocaml/xapi/xapi_sr.ml
Original file line number Diff line number Diff line change
Expand Up @@ -360,23 +360,6 @@ let create ~__context ~host ~device_config ~(physical_size : int64) ~name_label
Helpers.assert_rolling_upgrade_not_in_progress ~__context ;
debug "SR.create name_label=%s sm_config=[ %s ]" name_label
(String.concat "; " (List.map (fun (k, v) -> k ^ " = " ^ v) sm_config)) ;
(* This breaks the udev SR which doesn't support sr_probe *)
(*
let probe_result = probe ~__context ~host ~device_config ~_type ~sm_config in
begin
match Xml.parse_string probe_result with
| Xml.Element("SRlist", _, children) -> ()
| _ ->
(* Figure out what was missing, then throw the appropriate error *)
match String.lowercase_ascii _type with
| "lvmoiscsi" ->
if not (List.exists (fun (s,_) -> "targetiqn" = String.lowercase_ascii s) device_config)
then raise (Api_errors.Server_error ("SR_BACKEND_FAILURE_96",["";"";probe_result]))
else if not (List.exists (fun (s,_) -> "scsiid" = String.lowercase_ascii s) device_config)
then raise (Api_errors.Server_error ("SR_BACKEND_FAILURE_107",["";"";probe_result]))
| _ -> ()
end;
*)
let sr_uuid = Uuidx.make () in
let sr_uuid_str = Uuidx.to_string sr_uuid in
(* Create the SR in the database before creating on disk, so the backends can read the sm_config field. If an error happens here
Expand Down Expand Up @@ -592,9 +575,6 @@ let update ~__context ~sr =
Db.SR.get_uuid ~__context ~self:sr |> Storage_interface.Sr.of_string
in
let sr_info = C.SR.stat (Ref.string_of task) sr' in
Db.SR.set_name_label ~__context ~self:sr ~value:sr_info.name_label ;
Db.SR.set_name_description ~__context ~self:sr
~value:sr_info.name_description ;
Db.SR.set_physical_size ~__context ~self:sr ~value:sr_info.total_space ;
Db.SR.set_physical_utilisation ~__context ~self:sr
~value:(Int64.sub sr_info.total_space sr_info.free_space) ;
Expand Down Expand Up @@ -863,7 +843,7 @@ let set_name_label ~__context ~sr ~value =
(Storage_interface.Sr.of_string sr')
value
) ;
update ~__context ~sr
Db.SR.set_name_label ~__context ~self:sr ~value

let set_name_description ~__context ~sr ~value =
let open Storage_access in
Expand All @@ -877,7 +857,7 @@ let set_name_description ~__context ~sr ~value =
(Storage_interface.Sr.of_string sr')
value
) ;
update ~__context ~sr
Db.SR.set_name_description ~__context ~self:sr ~value

let set_virtual_allocation ~__context ~self ~value =
Db.SR.set_virtual_allocation ~__context ~self ~value
Expand Down

0 comments on commit d8baca7

Please sign in to comment.