Skip to content

CA-285605: Make PIF.set_disallow_unplug idempotent #3503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 12, 2018
Merged

CA-285605: Make PIF.set_disallow_unplug idempotent #3503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6aa46a0
CA-285605: Make PIF.set_disallow_unplug idempotent
Mar 12, 2018
2b5f8b8
CA-285605: Update tests for idempotent PIF.set_disallow_unplug
Mar 12, 2018
b59685c
CA-285605: Do nothing if value is the same
Mar 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions ocaml/tests/test_clustering.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,18 +358,16 @@ let test_disallow_unplug_with_clustering () =
check_disallow_unplug true __context pif
"check_disallow_unplug called by test_disallow_unplug_with_clustering after setting disallow_unplug:true";

(* PIF.disallow_unplug should become RO upon introduce cluster_host object *)
(* PIF.disallow_unplug should become RO upon introduce cluster_host object, should throw exception when changing value *)
let _ = Test_common.make_cluster_and_cluster_host ~__context ~network ~host () in
Alcotest.check_raises
"check_disallow_unplug called by test_disallow_unplug_with_clustering after attaching cluster and cluster_host to network"
(Api_errors.(Server_error(clustering_enabled_on_network, [Ref.string_of network])))
(fun () -> Xapi_pif.set_disallow_unplug ~__context ~self:pif ~value:true);
(fun () -> Xapi_pif.set_disallow_unplug ~__context ~self:pif ~value:false);

(* PIF.set_disallow_unplug should raise same error even when value is the same *)
Alcotest.check_raises
"PIF.set_disallow_unplug:true called by test_disallow_unplug_with_clustering after attaching cluster and cluster_host to network"
(Api_errors.(Server_error(clustering_enabled_on_network, [Ref.string_of network])))
(fun () -> Xapi_pif.set_disallow_unplug ~__context ~self:pif ~value:true)
Xapi_pif.set_disallow_unplug ~__context ~self:pif ~value:true;
check_disallow_unplug true __context pif
"PIF.set_disallow_unplug should be idempotent even with clustering"

let test_disallow_unplug_ro_with_clustering_enabled =
[ "test_disallow_unplug_no_clustering", `Quick, test_disallow_unplug_no_clustering
Expand Down
11 changes: 7 additions & 4 deletions ocaml/xapi/xapi_pif.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,10 +775,13 @@ let pif_has_clustering_enabled ~__context (self : API.ref_PIF) network =
| a::_ -> true

let set_disallow_unplug ~__context ~self ~value =
let network = Db.PIF.get_network ~__context ~self in
if pif_has_clustering_enabled ~__context self network
then raise Api_errors.(Server_error(clustering_enabled_on_network, [Ref.string_of network]))
else Db.PIF.set_disallow_unplug ~__context ~self ~value
if (Db.PIF.get_disallow_unplug ~__context ~self) <> value
then begin
let network = Db.PIF.get_network ~__context ~self in
if pif_has_clustering_enabled ~__context self network
then raise Api_errors.(Server_error(clustering_enabled_on_network, [Ref.string_of network]))
else Db.PIF.set_disallow_unplug ~__context ~self ~value
end

let rec unplug ~__context ~self =
assert_pif_is_managed ~__context ~self;
Expand Down