Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 27 additions & 1 deletion ocaml/idl/datamodel_host.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,28 @@ let host_query_ha = call ~flags:[`Session]
~allowed_roles:_R_VM_OP
()

let set_iscsi_iqn = call
~name:"set_iscsi_iqn"
~lifecycle:[Published, rel_kolkata, ""]
~doc:"Sets the initiator IQN for the host"
~params:[
Ref _host, "host", "The host";
String, "value", "The value to which the IQN should be set"
]
~allowed_roles:_R_POOL_OP
()

let set_multipathing = call
~name:"set_multipathing"
~lifecycle:[Published, rel_kolkata, ""]
~doc:"Specifies whether multipathing is enabled"
~params:[
Ref _host, "host", "The host";
Bool, "value", "Whether multipathing should be enabled"
]
~allowed_roles:_R_POOL_OP
()

(** Hosts *)
let t =
create_obj ~in_db:true ~in_product_since:rel_rio ~in_oss_since:oss_since_303 ~internal_deprecated_since:None ~persist:PersistEverything ~gen_constructor_destructor:false ~name:_host ~descr:"A physical host" ~gen_events:true
Expand Down Expand Up @@ -1346,6 +1368,8 @@ let host_query_ha = call ~flags:[`Session]
apply_guest_agent_config;
mxgpu_vf_setup;
allocate_resources_for_vm;
set_iscsi_iqn;
set_multipathing;
]
~contents:
([ uid _host;
Expand Down Expand Up @@ -1400,6 +1424,8 @@ let host_query_ha = call ~flags:[`Session]
field ~qualifier:DynamicRO ~in_product_since:rel_cream ~default_value:(Some (VSet [VInt 0L])) ~ty:(Set (Int)) "virtual_hardware_platform_versions" "The set of versions of the virtual hardware platform that the host can offer to its guests";
field ~qualifier:DynamicRO ~default_value:(Some (VRef null_ref)) ~in_product_since:rel_ely ~ty:(Ref _vm) "control_domain" "The control domain (domain 0)";
field ~qualifier:DynamicRO ~lifecycle:[Published, rel_ely, ""] ~ty:(Set (Ref _pool_update)) ~ignore_foreign_key:true "updates_requiring_reboot" "List of updates which require reboot";
field ~qualifier:DynamicRO ~lifecycle:[Published, rel_falcon, ""] ~ty:(Set (Ref _feature)) "features" "List of features available on this host"
field ~qualifier:DynamicRO ~lifecycle:[Published, rel_falcon, ""] ~ty:(Set (Ref _feature)) "features" "List of features available on this host";
field ~qualifier:StaticRO ~lifecycle:[Published, rel_kolkata, ""] ~default_value:(Some (VString "")) ~ty:String "iscsi_iqn" "The initiator IQN for the host";
field ~qualifier:StaticRO ~lifecycle:[Published, rel_kolkata, ""] ~default_value:(Some (VBool false)) ~ty:Bool "multipathing" "Specifies whether multipathing is enabled";
])
()
1 change: 1 addition & 0 deletions ocaml/tests/suite.ml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ let base_suite =
Test_network_event_loop.test;
Test_network.test;
Test_pusb.test;
Test_host_helpers.test;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From now on, when we introduce new tests we should probably use Alcotest from the start. No change needed, this can be ported later with the other tests

]

let () =
Expand Down
4 changes: 3 additions & 1 deletion ocaml/tests/test_common.ml
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ let make_host2 ~__context ?(ref=Ref.make ()) ?(uuid=make_uuid ()) ?(name_label="
~display:`enabled
~virtual_hardware_platform_versions:[]
~control_domain:Ref.null
~updates_requiring_reboot:[];
~updates_requiring_reboot:[]
~iscsi_iqn:""
~multipathing:false;
ref

let make_pif ~__context ~network ~host ?(device="eth0") ?(mAC="C0:FF:EE:C0:FF:EE") ?(mTU=1500L)
Expand Down
126 changes: 126 additions & 0 deletions ocaml/tests/test_host_helpers.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
(*
* Copyright (C) Citrix Systems Inc.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published
* by the Free Software Foundation; version 2.1 only. with the special
* exception on linking described in file LICENSE.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*)

open OUnit

let setup_test_oc_watcher () =
let __context, _ = Test_event_common.event_setup_common () in
let calls = ref [] in
let test_rpc call =
match call.Rpc.name, call.Rpc.params with
| "host.set_iscsi_iqn", [_session_id_rpc;host_rpc;value_rpc] ->
let host = API.ref_host_of_rpc host_rpc in
let v = API.string_of_rpc value_rpc in
Db.Host.set_iscsi_iqn ~__context ~self:host ~value:v;
calls := (host,`set_iscsi_iqn v) :: !calls;
Rpc.{success=true; contents=Rpc.String ""}
| "host.set_multipathing", [_session_id_rpc;host_rpc;value_rpc] ->
let host = API.ref_host_of_rpc host_rpc in
let v = API.bool_of_rpc value_rpc in
Db.Host.set_multipathing ~__context ~self:host ~value:v;
calls := (host,`set_multipathing v) :: !calls;
Rpc.{success=true; contents=Rpc.String ""}
| _ -> Mock_rpc.rpc __context call
in
Context.set_test_rpc __context test_rpc;
let host1 = !Xapi_globs.localhost_ref in
let host2 = Test_common.make_host ~__context () in
let watcher = Xapi_host_helpers.Configuration.watch_other_configs ~__context 0.0 in
let token = watcher "" in
(__context, calls, host1, host2, watcher, token)

let test_host1 () =
(* Test1: update other_config:iscsi_iqn,multipathing on host1, check they appear in the calls list *)
let (__context, calls, host1, host2, watcher, token) = setup_test_oc_watcher () in
Db.Host.set_multipathing ~__context ~self:host1 ~value:false;

Db.Host.add_to_other_config ~__context ~self:host1 ~key:"iscsi_iqn" ~value:"test1";
let token = watcher token in
assert_equal !calls [host1, `set_iscsi_iqn "test1"];

calls := [];
Db.Host.add_to_other_config ~__context ~self:host1 ~key:"multipathing" ~value:"true";
let _token = watcher token in
assert_equal !calls [host1, `set_multipathing true]

let test_host2 () =
(* Test2: update other_config:iscsi_iqn,multipathing on host2, check they appear in the calls list *)
let (__context, calls, host1, host2, watcher, token) = setup_test_oc_watcher () in
Db.Host.set_multipathing ~__context ~self:host2 ~value:true;

Db.Host.add_to_other_config ~__context ~self:host2 ~key:"iscsi_iqn" ~value:"test2";
let token = watcher token in
assert_equal !calls [host2, `set_iscsi_iqn "test2"];

calls := [];
Db.Host.add_to_other_config ~__context ~self:host2 ~key:"multipathing" ~value:"false";
let _token = watcher token in
assert_equal !calls [host2, `set_multipathing false]

let test_different_keys () =
(* Test3: verify that setting other other-config keys causes no set *)
let (__context, calls, host1, host2, watcher, token) = setup_test_oc_watcher () in
Db.Host.add_to_other_config ~__context ~self:host1 ~key:"other_key" ~value:"test1";
let _token = watcher token in
assert_equal !calls []

let test_host_set_iscsi_iqn () =
(* Test3: verify that sequence of DB calls in Host.set_iscsi_iqn don't cause the
watcher to invoke further calls *)
let (__context, calls, host1, host2, watcher, token) = setup_test_oc_watcher () in
Db.Host.add_to_other_config ~__context ~self:host1 ~key:"iscsi_iqn" ~value:"test1";
let token = watcher token in
assert_equal !calls [host1, `set_iscsi_iqn "test1"];
calls := [];
Db.Host.remove_from_other_config ~__context ~self:host1 ~key:"iscsi_iqn";
let token = watcher token in
assert_equal !calls [];
Db.Host.set_iscsi_iqn ~__context ~self:host1 ~value:"test2";
let token = watcher token in
assert_equal !calls [];
Db.Host.add_to_other_config ~__context ~self:host1 ~key:"iscsi_iqn" ~value:"test2";
let _token = watcher token in
assert_equal !calls []

let test_host_set_multipathing () =
(* Test3: verify that sequence of DB calls in Host.set_multipathing don't cause the
watcher to invoke further calls *)
let (__context, calls, host1, host2, watcher, token) = setup_test_oc_watcher () in
Db.Host.set_multipathing ~__context ~self:host2 ~value:false;

Db.Host.add_to_other_config ~__context ~self:host1 ~key:"multipathing" ~value:"true";
let token = watcher token in
assert_equal !calls [host1, `set_multipathing true];
calls := [];

Db.Host.remove_from_other_config ~__context ~self:host1 ~key:"multipathing";
let token = watcher token in
assert_equal !calls [];
Db.Host.set_multipathing ~__context ~self:host1 ~value:false;
let token = watcher token in
assert_equal !calls [];
Db.Host.add_to_other_config ~__context ~self:host1 ~key:"multipathing" ~value:"false";
let _token = watcher token in
assert_equal !calls []

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are testing "success" cases, do we have understood failure cases that also would make sense to check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that setting multipathing should fail (unless the DB itself fails)


let test =
"other_config_watcher" >:::
[
"test_host1" >:: test_host1;
"test_host2" >:: test_host2;
"test_different_keys" >:: test_different_keys;
"test_host_set_iscsi_iqn" >:: test_host_set_iscsi_iqn;
"test_host_set_multipathing" >:: test_host_set_multipathing;
]
10 changes: 10 additions & 0 deletions ocaml/tests/test_sm_features.ml
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,16 @@ let test_sequences =
features = ["VDI_RESIZE", 1L];
};
};
(* Test SMAPIV3 SR_MULTIPATH feature *)
{
raw = ["SR_MULTIPATH"];
smapiv1_features = [Sr_multipath, 1L];
smapiv2_features = ["SR_MULTIPATH/1"];
sm = {
capabilities = ["SR_MULTIPATH"];
features = ["SR_MULTIPATH", 1L];
};
}
]

module ParseSMAPIv1Features = Generic.Make(struct
Expand Down
14 changes: 14 additions & 0 deletions ocaml/xapi/message_forwarding.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2654,6 +2654,20 @@ module Forward = functor(Local: Custom_actions.CUSTOM_ACTIONS) -> struct
(host_uuid ~__context self) (vm_uuid ~__context vm);
let snapshot = Db.VM.get_record ~__context ~self:vm in
VM.allocate_vm_to_host ~__context ~vm ~host:self ~snapshot ()

let set_iscsi_iqn ~__context ~host ~value =
info "Host.set_iscsi_iqn: host='%s' iqn='%s'" (host_uuid ~__context host) value;
let local_fn = Local.Host.set_iscsi_iqn ~host ~value in
do_op_on ~local_fn ~__context ~host
(fun session_id rpc ->
Client.Host.set_iscsi_iqn rpc session_id host value)

let set_multipathing ~__context ~host ~value =
info "Host.set_multipathing: host='%s' value='%s'" (host_uuid ~__context host) (string_of_bool value);
let local_fn = Local.Host.set_multipathing ~host ~value in
do_op_on ~local_fn ~__context ~host
(fun session_id rpc ->
Client.Host.set_multipathing rpc session_id host value)
end

module Host_crashdump = struct
Expand Down
4 changes: 4 additions & 0 deletions ocaml/xapi/records.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1249,6 +1249,10 @@ let host_record rpc session_id host =
make_field ~name:"features"
~get:(fun () -> String.concat "; " (List.map get_uuid_from_ref (x ()).API.host_features))
~get_set:(fun () -> List.map get_uuid_from_ref (x ()).API.host_features) ();
make_field ~name:"iscsi_iqn"
~get:(fun () -> (x ()).API.host_iscsi_iqn) ~set:(fun s -> Client.Host.set_iscsi_iqn rpc session_id host s) ();
make_field ~name:"multipathing"
~get:(fun () -> string_of_bool (x ()).API.host_multipathing) ~set:(fun s -> Client.Host.set_multipathing rpc session_id host (safe_bool_of_string "multipathing" s)) ();
]}

let vdi_record rpc session_id vdi =
Expand Down
2 changes: 2 additions & 0 deletions ocaml/xapi/smint.ml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type capability =
| Sr_stats
| Sr_metadata
| Sr_trim
| Sr_multipath
| Vdi_create | Vdi_delete | Vdi_attach | Vdi_detach | Vdi_mirror
| Vdi_clone | Vdi_snapshot | Vdi_resize | Vdi_activate | Vdi_deactivate
| Vdi_update | Vdi_introduce
Expand All @@ -47,6 +48,7 @@ let string_to_capability_table = [
"SR_SUPPORTS_LOCAL_CACHING", Sr_supports_local_caching;
"SR_METADATA", Sr_metadata;
"SR_TRIM", Sr_trim;
"SR_MULTIPATH", Sr_multipath;
"VDI_CREATE", Vdi_create;
"VDI_DELETE", Vdi_delete;
"VDI_ATTACH", Vdi_attach;
Expand Down
2 changes: 2 additions & 0 deletions ocaml/xapi/xapi.ml
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,8 @@ let server_init() =
"hi-level database upgrade", [ Startup.OnlyMaster ], Xapi_db_upgrade.hi_level_db_upgrade_rules ~__context;
"bringing up management interface", [], bring_up_management_if ~__context;
"Starting periodic scheduler", [Startup.OnThread], Xapi_periodic_scheduler.loop;
"Synchronising host configuration files", [], (fun () -> Xapi_host_helpers.Configuration.sync_config_files ~__context);
"Starting Host other-config watcher", [Startup.OnlyMaster], (fun () -> Xapi_host_helpers.Configuration.start_watcher_thread ~__context);
"Remote requests", [Startup.OnThread], Remote_requests.handle_requests;
];
begin match Pool_role.get_role () with
Expand Down
4 changes: 4 additions & 0 deletions ocaml/xapi/xapi_globs.ml
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,9 @@ let udhcpd_skel = ref (Filename.concat "/etc/xensource" "udhcpd.skel")
let udhcpd_leases_db = ref "/var/lib/xcp/dhcp-leases.db"
let udhcpd_pidfile = ref "/var/run/udhcpd.pid"

let iscsi_initiator_config_file = ref "/etc/iscsi/initiatorname.iscsi"
let multipathing_config_file = ref "/var/run/nonpersistent/multipath_enabled"

let busybox = ref "busybox"

let xe_path = ref "xe"
Expand Down Expand Up @@ -1112,6 +1115,7 @@ module Resources = struct
"logconfig", log_config_file, "Configure the logging policy";
"cpu-info-file", cpu_info_file, "Where to cache boot-time CPU info";
"server-cert-path", server_cert_path, "Path to server ssl certificate";
"iscsi_initiatorname", iscsi_initiator_config_file, "Path to the initiatorname.iscsi file"
]
let essential_dirs = [
"sm-dir", sm_dir, "Directory containing SM plugins";
Expand Down
30 changes: 30 additions & 0 deletions ocaml/xapi/xapi_host.ml
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,8 @@ let create ~__context ~uuid ~name_label ~name_description ~hostname ~address ~ex
~virtual_hardware_platform_versions:(if host_is_us then Xapi_globs.host_virtual_hardware_platform_versions else [0L])
~control_domain:Ref.null
~updates_requiring_reboot:[]
~iscsi_iqn:""
~multipathing:false
;
(* If the host we're creating is us, make sure its set to live *)
Db.Host_metrics.set_last_updated ~__context ~self:metrics ~value:(Date.of_float (Unix.gettimeofday ()));
Expand Down Expand Up @@ -1766,3 +1768,31 @@ let mxgpu_vf_setup ~__context ~host =
let allocate_resources_for_vm ~__context ~self ~vm ~live =
(* Implemented entirely in Message_forwarding *)
()

let set_iscsi_iqn ~__context ~host ~value =
if value = "" then raise Api_errors.(Server_error (invalid_value, ["value"; value]));
(* Note, the following sequence is carefully written - see the
other-config watcher thread in xapi_host_helpers.ml *)
Db.Host.remove_from_other_config ~__context ~self:host ~key:"iscsi_iqn";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is out of scope for this PR, and I suggest not to touch it. But, as a remark for the future, we really need a with_other_config_update and replace_in_other_config pair of helpers. I have seen quite a few instances of these patterns...

(* we need to first set the iscsi_iqn field and then the other-config field:
* setting the other-config field triggers and update on the iscsi_iqn
* field if they are different
*
* we want to keep the legacy `other_config:iscsi_iqn` and the new `iscsi_iqn`
* fields in sync:
* when you update the `iscsi_iqn` field we want to update `other_config`,
* but when updating `other_config` we want to update `iscsi_iqn` too.
* we have to be careful not to introduce an infinite loop of updates.
* *)
Db.Host.set_iscsi_iqn ~__context ~self:host ~value;
Db.Host.add_to_other_config ~__context ~self:host ~key:"iscsi_iqn" ~value;
Xapi_host_helpers.Configuration.set_initiator_name value

let set_multipathing ~__context ~host ~value =
(* Note, the following sequence is carefully written - see the
other-config watcher thread in xapi_host_helpers.ml *)
Db.Host.remove_from_other_config ~__context ~self:host ~key:"multipathing";
Db.Host.set_multipathing ~__context ~self:host ~value;
Db.Host.add_to_other_config ~__context ~self:host ~key:"multipathing" ~value:(string_of_bool value);
Xapi_host_helpers.Configuration.set_multipathing value

2 changes: 2 additions & 0 deletions ocaml/xapi/xapi_host.mli
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,5 @@ val apply_guest_agent_config : __context:Context.t -> host:API.ref_host -> unit
val mxgpu_vf_setup : __context:Context.t -> host:API.ref_host -> unit

val allocate_resources_for_vm : __context:Context.t -> self:API.ref_host -> vm:API.ref_VM -> live:bool -> unit
val set_iscsi_iqn : __context:Context.t -> host:API.ref_host -> value:string -> unit
val set_multipathing : __context:Context.t -> host:API.ref_host -> value:bool -> unit
Loading