Skip to content

Commit beede2a

Browse files
authored
CP-51693: extend XMLRPC feature flag to other functions making RPC calls in XAPI (#6979)
There were a few more places that used XMLRPC that were missed the first time. In particular intra-pool communication and XAPI<->xenopsd communication. The feature flag will eventually be enabled (on `feature/perf-edvint` first), but the feature flagged code can be merged to master directly.
2 parents 23101a6 + ee056a7 commit beede2a

5 files changed

Lines changed: 47 additions & 45 deletions

File tree

ocaml/xapi/helpers.ml

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -427,20 +427,22 @@ module TraceHelper = struct
427427
Tracing_propagator.Propagator.Http.inject_into trace_context
428428
end
429429

430+
let choose_rpc () =
431+
let open Xmlrpc_client in
432+
if !Xapi_globs.use_xmlrpc then
433+
(XMLRPC_protocol.rpc, "/")
434+
else
435+
(JSONRPC_protocol.rpc, "/jsonrpc")
436+
430437
(* Note that both this and `make_timeboxed_rpc` are almost always
431438
* partially applied, returning a function of type 'Rpc.request -> Rpc.response'.
432439
* The body is therefore not evaluated until the RPC call is actually being
433440
* made. *)
434-
let make_rpc ~__context rpc : Rpc.response =
435-
let subtask_of = Ref.string_of (Context.get_task_id __context) in
441+
let make_rpc' ~subtask_of ?task_id ~__context rpc : Rpc.response =
442+
let subtask_of = Ref.string_of subtask_of in
436443
let open Xmlrpc_client in
437444
let tracing = Context.set_client_span __context in
438-
let dorpc, path =
439-
if !Xapi_globs.use_xmlrpc then
440-
(XMLRPC_protocol.rpc, "/")
441-
else
442-
(JSONRPC_protocol.rpc, "/jsonrpc")
443-
in
445+
let dorpc, path = choose_rpc () in
444446
let http = xmlrpc ~subtask_of ~version:"1.1" path in
445447
let http = TraceHelper.inject_span_into_req tracing http in
446448
let transport =
@@ -449,25 +451,27 @@ let make_rpc ~__context rpc : Rpc.response =
449451
else
450452
SSL
451453
( SSL.make ~use_stunnel_cache:true ~verify_cert:(Stunnel_client.pool ())
454+
?task_id:(Option.map Ref.string_of task_id)
452455
()
453456
, Pool_role.get_master_address ()
454457
, !Constants.https_port
455458
)
456459
in
457460
dorpc ~srcstr:"xapi" ~dststr:"xapi" ~transport ~http rpc
458461

462+
(* erase optional labeled arguments for partial applications to work *)
463+
let make_rpc ~__context rpc =
464+
make_rpc' ~subtask_of:(Context.get_task_id __context) ~__context rpc
465+
459466
let make_timeboxed_rpc ~__context timeout rpc : Rpc.response =
460-
let subtask_of = Ref.string_of (Context.get_task_id __context) in
467+
let subtask_of = Context.get_task_id __context in
461468
Server_helpers.exec_with_new_task "timeboxed_rpc"
462469
~subtask_of:(Context.get_task_id __context) (fun __context ->
463470
(* Note we need a new task here because the 'resources' (including stunnel pid) are
464471
* associated with the task. To avoid conflating the stunnel with any real resources
465472
* the task has acquired we make a new one specifically for the stunnel pid *)
466-
let open Xmlrpc_client in
467-
let tracing = Context.set_client_span __context in
468-
let http = xmlrpc ~subtask_of ~version:"1.1" "/" in
469-
let http = TraceHelper.inject_span_into_req tracing http in
470473
let task_id = Context.get_task_id __context in
474+
471475
let cancel () =
472476
let resources =
473477
Locking_helpers.Thread_state.get_acquired_resources_by_task task_id
@@ -477,20 +481,9 @@ let make_timeboxed_rpc ~__context timeout rpc : Rpc.response =
477481
let module Scheduler = Xapi_stdext_threads_scheduler.Scheduler in
478482
Scheduler.add_to_queue (Ref.string_of task_id) Scheduler.OneShot timeout
479483
cancel ;
480-
let transport =
481-
if Pool_role.is_master () then
482-
Unix Xapi_globs.unix_domain_socket
483-
else
484-
SSL
485-
( SSL.make ~verify_cert:(Stunnel_client.pool ())
486-
~use_stunnel_cache:true ~task_id:(Ref.string_of task_id) ()
487-
, Pool_role.get_master_address ()
488-
, !Constants.https_port
489-
)
490-
in
491-
let result =
492-
XMLRPC_protocol.rpc ~srcstr:"xapi" ~dststr:"xapi" ~transport ~http rpc
493-
in
484+
485+
let result = make_rpc' ~subtask_of ~task_id ~__context rpc in
486+
494487
Scheduler.remove_from_queue (Ref.string_of task_id) ;
495488
result
496489
)
@@ -525,6 +518,10 @@ let make_remote_rpc_of_url ~verify_cert ~srcstr ~dststr (url, pool_secret) call
525518
http
526519
in
527520
let transport = transport_of_url ~verify_cert url in
521+
(* we should determine the protocol based on Content-type, not the URL,
522+
but since we currently only use the URL to determine the protocol:
523+
keep this as XMLRPC for now, because we don't have JSONRPC duplicates for
524+
all handlers *)
528525
XMLRPC_protocol.rpc ~transport ~srcstr ~dststr ~http call
529526

530527
(* This one uses rpc-light *)
@@ -535,9 +532,10 @@ let make_remote_rpc ?(verify_cert = Stunnel_client.pool ()) ~__context
535532
SSL (SSL.make ~verify_cert (), remote_address, !Constants.https_port)
536533
in
537534
let tracing = Context.tracing_of __context in
538-
let http = xmlrpc ~version:"1.0" "/" in
535+
let dorpc, path = choose_rpc () in
536+
let http = xmlrpc ~version:"1.0" path in
539537
let http = TraceHelper.inject_span_into_req tracing http in
540-
XMLRPC_protocol.rpc ~srcstr:"xapi" ~dststr:"remote_xapi" ~transport ~http xml
538+
dorpc ~srcstr:"xapi" ~dststr:"remote_xapi" ~transport ~http xml
541539

542540
(* Helper type for an object which may or may not be in the local database. *)
543541
type 'a api_object =
@@ -611,10 +609,9 @@ let call_emergency_mode_functions hostname f =
611609
, !Constants.https_port
612610
)
613611
in
614-
let http = xmlrpc ~version:"1.0" "/" in
615-
let rpc =
616-
XMLRPC_protocol.rpc ~srcstr:"xapi" ~dststr:"xapi" ~transport ~http
617-
in
612+
let dorpc, path = choose_rpc () in
613+
let http = xmlrpc ~version:"1.0" path in
614+
let rpc = dorpc ~srcstr:"xapi" ~dststr:"xapi" ~transport ~http in
618615
let session_id =
619616
Client.Client.Session.slave_local_login ~rpc
620617
~psecret:(Xapi_globs.pool_secret ())

ocaml/xapi/message_forwarding.ml

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,12 @@ let remote_rpc_no_retry _context hostname (task_opt : API.ref_task option) xml =
5959
)
6060
in
6161
let tracing = Context.set_client_span _context in
62+
let dorpc, path = Helpers.choose_rpc () in
6263
let http =
63-
xmlrpc ?task_id:(Option.map Ref.string_of task_opt) ~version:"1.0" "/"
64+
xmlrpc ?task_id:(Option.map Ref.string_of task_opt) ~version:"1.0" path
6465
|> Helpers.TraceHelper.inject_span_into_req tracing
6566
in
66-
XMLRPC_protocol.rpc ~srcstr:"xapi" ~dststr:"dst_xapi" ~transport ~http xml
67+
dorpc ~srcstr:"xapi" ~dststr:"dst_xapi" ~transport ~http xml
6768

6869
(* Use HTTP 1.1, use the stunnel cache and pre-verify the connection *)
6970
let remote_rpc_retry _context hostname (task_opt : API.ref_task option) xml =
@@ -78,11 +79,12 @@ let remote_rpc_retry _context hostname (task_opt : API.ref_task option) xml =
7879
)
7980
in
8081
let tracing = Context.set_client_span _context in
82+
let dorpc, path = Helpers.choose_rpc () in
8183
let http =
82-
xmlrpc ?task_id:(Option.map Ref.string_of task_opt) ~version:"1.1" "/"
84+
xmlrpc ?task_id:(Option.map Ref.string_of task_opt) ~version:"1.1" path
8385
|> Helpers.TraceHelper.inject_span_into_req tracing
8486
in
85-
XMLRPC_protocol.rpc ~srcstr:"xapi" ~dststr:"dst_xapi" ~transport ~http xml
87+
dorpc ~srcstr:"xapi" ~dststr:"dst_xapi" ~transport ~http xml
8688

8789
let call_slave_with_session remote_rpc_fn __context host
8890
(task_opt : API.ref_task option) f =

ocaml/xapi/rpc_retry.ml

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,13 @@ functor
3434
struct
3535
let transport = Unix Meta.server_path
3636

37-
let simple_rpc =
38-
XMLRPC_protocol.rpc ~srcstr:Meta.client_name ~dststr:Meta.server_name
39-
~transport
40-
~http:(xmlrpc ~version:"1.0" "/")
37+
let simple_rpc () =
38+
let dorpc, path = Helpers.choose_rpc () in
39+
dorpc ~srcstr:Meta.client_name ~dststr:Meta.server_name ~transport
40+
~http:(xmlrpc ~version:"1.0" path)
4141

4242
let rpc call =
43+
let simple_rpc = simple_rpc () in
4344
let rec aux ~retrying =
4445
let response' =
4546
try

ocaml/xapi/xapi_http.ml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ let validate_session __context session_id realm =
2929
(* Talk to the master over the network. NB we deliberately use the network rather than
3030
the unix domain socket because we don't want to accidentally bypass the authentication *)
3131
let inet_rpc xml =
32-
let version = "1.1" and path = "/" in
32+
let version = "1.1" in
3333
let http = 80 and https = !Constants.https_port in
3434
(* Bypass SSL for localhost, this works even if the management interface
3535
is disabled. *)
@@ -44,8 +44,9 @@ let inet_rpc xml =
4444
, https
4545
)
4646
in
47+
let dorpc, path = Helpers.choose_rpc () in
4748
let http = xmlrpc ~version path in
48-
XMLRPC_protocol.rpc ~srcstr:"xapi" ~dststr:"xapi" ~transport ~http xml
49+
dorpc ~srcstr:"xapi" ~dststr:"xapi" ~transport ~http xml
4950

5051
open Client
5152
open Http

ocaml/xapi/xapi_pool.ml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3591,11 +3591,12 @@ let perform ~local_fn ~__context ~host ~remote_fn =
35913591
in
35923592
let task_id = Option.map Ref.string_of task_opt in
35933593
let tracing = Context.set_client_span __context in
3594-
let http = xmlrpc ?task_id ~version:"1.0" "/" in
3594+
let dorpc, path = Helpers.choose_rpc () in
3595+
let http = xmlrpc ?task_id ~version:"1.0" path in
35953596
let http = Helpers.TraceHelper.inject_span_into_req tracing http in
35963597
let port = !Constants.https_port in
35973598
let transport = SSL (SSL.make ~verify_cert ?task_id (), hostname, port) in
3598-
XMLRPC_protocol.rpc ~srcstr:"xapi" ~dststr:"dst_xapi" ~transport ~http xml
3599+
dorpc ~srcstr:"xapi" ~dststr:"dst_xapi" ~transport ~http xml
35993600
in
36003601
let open Message_forwarding in
36013602
do_op_on_common ~local_fn ~__context ~host ~remote_fn

0 commit comments

Comments
 (0)