From afe0ff62c4d0b2e59ffcb32df3122cfa8ce4d7a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Thu, 12 Oct 2023 12:10:16 +0100 Subject: [PATCH 1/9] [maintenance]: commit lifecycle changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Edwin Török --- ocaml/idl/datamodel_lifecycle.ml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ocaml/idl/datamodel_lifecycle.ml b/ocaml/idl/datamodel_lifecycle.ml index 2f8a9a86ec6..b8c5319563e 100644 --- a/ocaml/idl/datamodel_lifecycle.ml +++ b/ocaml/idl/datamodel_lifecycle.ml @@ -91,6 +91,8 @@ let prototyped_of_message = function Some "22.26.0" | "VTPM", "create" -> Some "22.26.0" + | "host", "apply_recommended_guidances" -> + Some "23.18.0" | "host", "set_https_only" -> Some "22.27.0" | "pool", "set_update_sync_enabled" -> From 3b52b722b77d53d37bc208530662fa6bc85d8edb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Wed, 11 Oct 2023 17:14:50 +0100 Subject: [PATCH 2/9] CP-43755: Pam: avoid sleep(1) call when multithreaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This enables PAM to be used in multithreaded mode (currently XAPI has a global lock around auth). Using an off-cpu flamegraph I identified that concurrent PAM calls are slow due to a call to `sleep(1)`. `pam_authenticate` calls `crypt_r` which calls `NSSLOW_Init` which on first use will try to initialize the just `dlopen`-ed library. If it encounters a race condition it does a `sleep(1)`. This race condition can be quite reliably reproduced when performing a lot of PAM authentications from multiple threads in parallel. GDB can also be used to confirm this by putting a breakpoint on `sleep`: ``` #0 __sleep (seconds=seconds@entry=1) at ../sysdeps/unix/sysv/linux/sleep.c:42 #1 0x00007ffff1548e22 in freebl_RunLoaderOnce () at lowhash_vector.c:122 #2 0x00007ffff1548f31 in freebl_InitVector () at lowhash_vector.c:131 #3 NSSLOW_Init () at lowhash_vector.c:148 #4 0x00007ffff1b8f09a in __sha512_crypt_r (key=key@entry=0x7fffd8005a60 "pamtest-edvint", salt=0x7ffff31e17b8 "dIJbsXKc0", #5 0x00007ffff1b8d070 in __crypt_r (key=key@entry=0x7fffd8005a60 "pamtest-edvint", salt=, #6 0x00007ffff1dc9abc in verify_pwd_hash (p=p@entry=0x7fffd8005a60 "pamtest-edvint", hash=, nullok=nullok@entry=0) at passverify.c:111 #7 0x00007ffff1dc9139 in _unix_verify_password (pamh=pamh@entry=0x7fffd8002910, name=0x7fffd8002ab0 "pamtest-edvint", p=0x7fffd8005a60 "pamtest-edvint", ctrl=ctrl@entry=8389156) at support.c:777 #8 0x00007ffff1dc6556 in pam_sm_authenticate (pamh=0x7fffd8002910, flags=, argc=, argv=) at pam_unix_auth.c:178 #9 0x00007ffff7bcef1a in _pam_dispatch_aux (use_cached_chain=, resumed=, h=, flags=1, pamh=0x7fffd8002910) at pam_dispatch.c:110 #10 _pam_dispatch (pamh=pamh@entry=0x7fffd8002910, flags=1, choice=choice@entry=1) at pam_dispatch.c:426 #11 0x00007ffff7bce7e0 in pam_authenticate (pamh=0x7fffd8002910, flags=flags@entry=1) at pam_auth.c:34 #12 0x00000000005ae567 in XA_mh_authorize (username=username@entry=0x7fffd80028d0 "pamtest-edvint", password=password@entry=0x7fffd80028f0 "pamtest-edvint", error=error@entry=0x7ffff31e1be8) at xa_auth.c:83 #13 0x00000000005adf20 in stub_XA_mh_authorize (username=, password=) at xa_auth_stubs.c:42 ``` `pam_start` and `pam_end` doesn't help here, because on `pam_end` the library is `dlclose`-ed, so on next `pam_authenticate` it will have to go through the initialization code again. (This initialization code would've belonged into `pam_start`, not `pam_authenticate`, but there are several layers here including a call to `crypt_r`). Upstream has fixed this problem >5 years ago by switching to libxcrypt instead. Signed-off-by: Edwin Török --- ocaml/auth/dune | 2 +- ocaml/auth/xa_auth_stubs.c | 29 +++++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/ocaml/auth/dune b/ocaml/auth/dune index 8d6774ab817..f963fbb591b 100644 --- a/ocaml/auth/dune +++ b/ocaml/auth/dune @@ -4,7 +4,7 @@ (names xa_auth xa_auth_stubs) ) (name pam) - (c_library_flags -lpam) + (c_library_flags -lpam -lcrypt) (wrapped false) ) diff --git a/ocaml/auth/xa_auth_stubs.c b/ocaml/auth/xa_auth_stubs.c index 78dfb131eaf..6c6c7a5b915 100644 --- a/ocaml/auth/xa_auth_stubs.c +++ b/ocaml/auth/xa_auth_stubs.c @@ -11,8 +11,9 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Lesser General Public License for more details. */ -/* - */ + +/* must be at the beginning, it affects defines in other headers that cannot be reenabled later */ +#define _GNU_SOURCE #include #include @@ -70,6 +71,30 @@ CAMLprim value stub_XA_mh_chpasswd(value username, value new_password){ CAMLreturn(ret); } +#include +/* 'constructor' attribute will ensure this function gets call early during program startup. */ +void __attribute__((constructor)) stub_XA_workaround(void) +{ + struct crypt_data data; + memset(&data, 0, sizeof(data)); + + /* Initialize and load crypt library used for password hashing. + This library is loaded and initialized at [pam_authenticate] time and not at [pam_start]. + If it detects a race condition (multiple threads with their own PAM contexts trying to call 'crypt_r'), + then it does [sleep 1] as can be seen in this call trace: + [pam_authenticate -> crypt_r -> __sha512_crypt_r -> freebl_InitVector -> freebl_RunLoaderOnce -> sleep]. + + As a workaround link with 'libcrypt' and call 'crypt_r' with a setting that will make it take tha sha512 route + to ensure that the library gets initialized while we're still single-threaded and stays loaded and initialized. + + '$6$' is the setting for sha512 according to crypt(5). + + Upstream has switched to using libxcrypt instead which doesn't have these problems, when we switch then + this workaround can be dropped. + */ + crypt_r("", "$6$", &data); +} + /* * Local variables: * mode: C From 0dc99391245ce7fd990490f96081af10f46b14af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Wed, 11 Oct 2023 17:29:43 +0100 Subject: [PATCH 3/9] CP-43755: Split internal and external auth locks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The codepaths are completely independent. Rename `serialize` to `throttle`. This prepares the code for parallel authentication, with independently tunable thresholds. No functional change. Signed-off-by: Edwin Török --- ocaml/xapi/xapi_session.ml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ocaml/xapi/xapi_session.ml b/ocaml/xapi/xapi_session.ml index 42d590bb54d..c6021dedd0a 100644 --- a/ocaml/xapi/xapi_session.ml +++ b/ocaml/xapi/xapi_session.ml @@ -255,7 +255,11 @@ let local_superuser = "root" let xapi_internal_originator = "xapi" -let serialize_auth = Mutex.create () +let throttle_auth_internal = Mutex.create () + +let throttle_auth_external = Mutex.create () + +let with_throttle = with_lock let wipe_string_contents str = for i = 0 to Bytes.length str - 1 do @@ -272,13 +276,13 @@ let wipe_params_after_fn params fn = with e -> wipe params ; raise e let do_external_auth uname pwd = - with_lock serialize_auth (fun () -> + with_lock throttle_auth_external (fun () -> (Ext_auth.d ()).authenticate_username_password uname (Bytes.unsafe_to_string pwd) ) let do_local_auth uname pwd = - with_lock serialize_auth (fun () -> + with_throttle throttle_auth_internal (fun () -> try Pam.authenticate uname (Bytes.unsafe_to_string pwd) with Failure msg -> raise @@ -288,7 +292,7 @@ let do_local_auth uname pwd = ) let do_local_change_password uname newpwd = - with_lock serialize_auth (fun () -> + with_throttle throttle_auth_internal (fun () -> Pam.change_password uname (Bytes.unsafe_to_string newpwd) ) From 8aa69bba142fde6cfffe4b9dbd63dd0e2051a1ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Wed, 11 Oct 2023 17:45:17 +0100 Subject: [PATCH 4/9] CP-43755: Locking_helpers: introduce Semaphore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Small wrapper around OCaml's Semaphore module. It doesn't yet use Thread_state to avoid potential performance issues. The semaphore is initialized with a count of 1, and its maximum can be changed at runtime using [set_max] in a safe way that takes into account threads that already use the semaphore: we store the current max in the record and call [acquire] or [release] the appropriate number of times to make it match the new desired max. This always works, even if there are threads currently using the semaphore: if we decrease it below max then 'set_max' will block until sufficient number of threads have released the semaphore. [set_max] will be useful for implementing the semaphore maximum value as a datamodel field, and won't require restarting XAPI to change it. Even reading this value from a config file would be difficult otherwise because the semaphore needs to be initialized with a count at the time it is created, which is before any configuration files are read. No functional change. Signed-off-by: Edwin Török --- ocaml/xapi/locking_helpers.ml | 52 ++++++++++++++++++++++++++++++++++ ocaml/xapi/locking_helpers.mli | 24 ++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/ocaml/xapi/locking_helpers.ml b/ocaml/xapi/locking_helpers.ml index 5ec18803f63..2327f310e21 100644 --- a/ocaml/xapi/locking_helpers.ml +++ b/ocaml/xapi/locking_helpers.ml @@ -223,3 +223,55 @@ module Named_mutex = struct finally f (fun () -> Thread_state.released r) ) end + +module Semaphore = struct + type t = { + name: string + ; sem: Semaphore.Counting.t + ; max_lock: Mutex.t + ; mutable max: int + } + + let create name = + let max = 1 in + {name; sem= Semaphore.Counting.make max; max_lock= Mutex.create (); max} + + let execute (x : t) f = + Semaphore.Counting.acquire x.sem ; + let finally () = Semaphore.Counting.release x.sem in + Fun.protect ~finally f + + let set_max t n = + if n < 1 then + Fmt.invalid_arg + "The semaphore '%s' must have at least 1 resource available, \ + requested: %d" + t.name n ; + (* ensure only 1 thread attempts to modify the maximum at a time, this is a slow path *) + with_lock t.max_lock @@ fun () -> + D.debug + "Setting semaphore '%s' to have at most %d resource (current max: %d)" + t.name n t.max ; + + (* requested to decrease maximum, this might block *) + while t.max > n do + if not (Semaphore.Counting.try_acquire t.sem) then ( + D.debug + "Semaphore '%s' has >%d resources in use, waiting until some of them \ + are released" + t.name n ; + (* may block *) + Semaphore.Counting.acquire t.sem + ) ; + t.max <- t.max - 1 + done ; + + (* requested to increase maximum, this doesn't block *) + while t.max < n do + (* doesn't block, semaphores can also be acquired and released from any thread *) + Semaphore.Counting.release t.sem ; + t.max <- t.max + 1 + done ; + D.debug "Semaphore '%s' updated to have %d resources (%d in use)" t.name n + (Semaphore.Counting.get_value t.sem) +end diff --git a/ocaml/xapi/locking_helpers.mli b/ocaml/xapi/locking_helpers.mli index b61a42bed57..3c58a8186ea 100644 --- a/ocaml/xapi/locking_helpers.mli +++ b/ocaml/xapi/locking_helpers.mli @@ -48,3 +48,27 @@ module Named_mutex : sig val execute : t -> (unit -> 'a) -> 'a end + +module Semaphore : sig + (** a semaphore that allows at most N operations to proceed at a time *) + type t + + val create : string -> t + (** [create name] creates a semaphore with an initial count of 1. + @see {!set_max} *) + + val execute : t -> (unit -> 'a) -> 'a + (** [execute sem f] executes [f] after acquiring the [sem]aphore. + Releases the semaphore on all codepaths. *) + + val set_max : t -> int -> unit + (** [set_max sem n] sets the semaphore's maximum count to [n]. + Once all threads that are inside {!execute} release the semaphore then the semaphore will accept + at most [n] {!execute} calls in parallel until it blocks. + + Increasing a semaphore's count is done immediately, + whereas decreasing the count may block until sufficient number of threads release the semaphore. + + It is safe to call this function in parallel. + *) +end From 4903400f8f921d10a1203ea42c0c62f6bb56003a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Wed, 11 Oct 2023 17:49:27 +0100 Subject: [PATCH 5/9] CP-43755: xapi_session: switch to using Semaphore instead of Mutex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default count is still 1, so the behaviour is equivalent to Mutex. Internally Semaphore uses a Mutex + Condition and its performance is very similar to Mutex. No functional change. Signed-off-by: Edwin Török --- ocaml/xapi/xapi_session.ml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ocaml/xapi/xapi_session.ml b/ocaml/xapi/xapi_session.ml index c6021dedd0a..3d06f84732b 100644 --- a/ocaml/xapi/xapi_session.ml +++ b/ocaml/xapi/xapi_session.ml @@ -255,11 +255,11 @@ let local_superuser = "root" let xapi_internal_originator = "xapi" -let throttle_auth_internal = Mutex.create () +let throttle_auth_internal = Locking_helpers.Semaphore.create "Internal auth" -let throttle_auth_external = Mutex.create () +let throttle_auth_external = Locking_helpers.Semaphore.create "External auth" -let with_throttle = with_lock +let with_throttle = Locking_helpers.Semaphore.execute let wipe_string_contents str = for i = 0 to Bytes.length str - 1 do @@ -276,7 +276,7 @@ let wipe_params_after_fn params fn = with e -> wipe params ; raise e let do_external_auth uname pwd = - with_lock throttle_auth_external (fun () -> + with_throttle throttle_auth_external (fun () -> (Ext_auth.d ()).authenticate_username_password uname (Bytes.unsafe_to_string pwd) ) From 8d4ff98688fb1e383c8f752fd917ced83201e447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Wed, 11 Oct 2023 18:06:38 +0100 Subject: [PATCH 6/9] CP-43755: Datamodel_pool: introduce local_auth_max_threads and ext_auth_max_threads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These can be changed at runtime. Although changing the AD max threads is still experimental as we haven't checked whether it is thread safe. PAM itself is thread safe in the way we currently use it (each thread gets its own PAM context). Signed-off-by: Edwin Török --- ocaml/idl/datamodel_common.ml | 2 +- ocaml/idl/datamodel_pool.ml | 18 ++++++++++++++++++ ocaml/idl/schematest.ml | 2 +- ocaml/tests/common/test_common.ml | 3 ++- ocaml/xapi/dbsync_master.ml | 3 ++- ocaml/xapi/message_forwarding.ml | 12 ++++++++++++ ocaml/xapi/xapi.ml | 13 +++++++++++++ ocaml/xapi/xapi_pool.ml | 6 ++++++ ocaml/xapi/xapi_pool.mli | 6 ++++++ ocaml/xapi/xapi_session.ml | 6 ++++++ ocaml/xapi/xapi_session.mli | 4 ++++ 11 files changed, 71 insertions(+), 4 deletions(-) diff --git a/ocaml/idl/datamodel_common.ml b/ocaml/idl/datamodel_common.ml index fd2c0dca28a..6d7521f736d 100644 --- a/ocaml/idl/datamodel_common.ml +++ b/ocaml/idl/datamodel_common.ml @@ -10,7 +10,7 @@ open Datamodel_roles to leave a gap for potential hotfixes needing to increment the schema version.*) let schema_major_vsn = 5 -let schema_minor_vsn = 766 +let schema_minor_vsn = 767 (* Historical schema versions just in case this is useful later *) let rio_schema_major_vsn = 5 diff --git a/ocaml/idl/datamodel_pool.ml b/ocaml/idl/datamodel_pool.ml index f0dab069e73..d9e69ccdb33 100644 --- a/ocaml/idl/datamodel_pool.ml +++ b/ocaml/idl/datamodel_pool.ml @@ -1103,6 +1103,16 @@ let set_update_sync_enabled = ] ~allowed_roles:_R_POOL_OP () +let set_local_auth_max_threads = + call ~flags:[`Session] ~name:"set_local_auth_max_threads" ~lifecycle:[] + ~params:[(Ref _pool, "self", "The pool"); (Int, "value", "The new maximum")] + ~allowed_roles:_R_POOL_OP () + +let set_ext_auth_max_threads = + call ~flags:[`Session] ~name:"set_ext_auth_max_threads" ~lifecycle:[] + ~params:[(Ref _pool, "self", "The pool"); (Int, "value", "The new maximum")] + ~allowed_roles:_R_POOL_OP () + (** A pool class *) let t = create_obj ~in_db:true ~in_product_since:rel_rio ~in_oss_since:None @@ -1189,6 +1199,8 @@ let t = ; reset_telemetry_uuid ; configure_update_sync ; set_update_sync_enabled + ; set_local_auth_max_threads + ; set_ext_auth_max_threads ] ~contents: ([uid ~in_oss_since:None _pool] @@ -1419,6 +1431,12 @@ let t = "coordinator_bias" "true if bias against pool master when scheduling vms is enabled, \ false otherwise" + ; field ~qualifier:StaticRO ~ty:Int ~default_value:(Some (VInt 1L)) + ~lifecycle:[] "local_auth_max_threads" + "Maximum number of threads to use for PAM authentication" + ; field ~qualifier:StaticRO ~ty:Int ~default_value:(Some (VInt 1L)) + ~lifecycle:[] "ext_auth_max_threads" + "Maximum number of threads to use for external (AD) authentication" ; field ~lifecycle:[] ~qualifier:DynamicRO ~ty:(Ref _secret) ~default_value:(Some (VRef null_ref)) "telemetry_uuid" "The UUID of the pool for identification of telemetry data" diff --git a/ocaml/idl/schematest.ml b/ocaml/idl/schematest.ml index 9ed36bb2957..85dd08e07d5 100644 --- a/ocaml/idl/schematest.ml +++ b/ocaml/idl/schematest.ml @@ -2,7 +2,7 @@ let hash x = Digest.string x |> Digest.to_hex (* BEWARE: if this changes, check that schema has been bumped accordingly in ocaml/idl/datamodel_common.ml, usually schema_minor_vsn *) -let last_known_schema_hash = "f4d0ee4f27d7fc0377add334197f6cd8" +let last_known_schema_hash = "95077aed35b715c362c7cf98902de578" let current_schema_hash : string = let open Datamodel_types in diff --git a/ocaml/tests/common/test_common.ml b/ocaml/tests/common/test_common.ml index 938985e67ec..a17dc8f6c54 100644 --- a/ocaml/tests/common/test_common.ml +++ b/ocaml/tests/common/test_common.ml @@ -312,7 +312,8 @@ let make_pool ~__context ~master ?(name_label = "") ?(name_description = "") ~repository_proxy_url ~repository_proxy_username ~repository_proxy_password ~migration_compression ~coordinator_bias ~telemetry_uuid ~telemetry_frequency ~telemetry_next_collection ~last_update_sync - ~update_sync_frequency ~update_sync_day ~update_sync_enabled ; + ~local_auth_max_threads:8L ~ext_auth_max_threads:8L ~update_sync_frequency + ~update_sync_day ~update_sync_enabled ; pool_ref let default_sm_features = diff --git a/ocaml/xapi/dbsync_master.ml b/ocaml/xapi/dbsync_master.ml index 3a4c33231ee..6cbec514c37 100644 --- a/ocaml/xapi/dbsync_master.ml +++ b/ocaml/xapi/dbsync_master.ml @@ -52,7 +52,8 @@ let create_pool_record ~__context = ~telemetry_next_collection:Xapi_stdext_date.Date.epoch ~last_update_sync:Xapi_stdext_date.Date.epoch ~update_sync_frequency:`weekly ~update_sync_day:0L - ~update_sync_enabled:false + ~update_sync_enabled:false ~local_auth_max_threads:1L + ~ext_auth_max_threads:1L let set_master_ip ~__context = let ip = diff --git a/ocaml/xapi/message_forwarding.ml b/ocaml/xapi/message_forwarding.ml index 292345828d6..2cd2f8c6fdc 100644 --- a/ocaml/xapi/message_forwarding.ml +++ b/ocaml/xapi/message_forwarding.ml @@ -1130,6 +1130,18 @@ functor (pool_uuid ~__context self) value ; Local.Pool.set_update_sync_enabled ~__context ~self ~value + + let set_local_auth_max_threads ~__context ~self ~value = + info "%s: pool='%s' value='%Ld'" __FUNCTION__ + (pool_uuid ~__context self) + value ; + Local.Pool.set_local_auth_max_threads ~__context ~self ~value + + let set_ext_auth_max_threads ~__context ~self ~value = + info "%s: pool='%s' value='%Ld'" __FUNCTION__ + (pool_uuid ~__context self) + value ; + Local.Pool.set_ext_auth_max_threads ~__context ~self ~value end module VM = struct diff --git a/ocaml/xapi/xapi.ml b/ocaml/xapi/xapi.ml index ad23fe97975..dff9913ee00 100644 --- a/ocaml/xapi/xapi.ml +++ b/ocaml/xapi/xapi.ml @@ -886,7 +886,16 @@ let server_init () = has just started up we want to wait forever for the master to appear. (See CA-25481) *) let initial_connection_timeout = !Master_connection.connection_timeout in Master_connection.connection_timeout := -1. ; + (* never timeout *) + let initialize_auth_semaphores ~__context = + let pool = Helpers.get_pool ~__context in + Xapi_session.set_local_auth_max_threads + (Db.Pool.get_local_auth_max_threads ~__context ~self:pool) ; + Xapi_session.set_ext_auth_max_threads + (Db.Pool.get_ext_auth_max_threads ~__context ~self:pool) + in + let call_extauth_hook_script_after_xapi_initialize ~__context = (* CP-709 *) (* in each initialization of xapi, extauth_hook script must be called in case this host was *) @@ -1390,6 +1399,10 @@ let server_init () = , [] , fun () -> Storage_migrate.killall ~dbg:"xapi init" ) + ; ( "Initialize threaded authentication" + , [Startup.NoExnRaising] + , fun () -> initialize_auth_semaphores ~__context + ) ; (* Start the external authentification plugin *) ( "Calling extauth_hook_script_before_xapi_initialize" , [Startup.NoExnRaising] diff --git a/ocaml/xapi/xapi_pool.ml b/ocaml/xapi/xapi_pool.ml index d1e4945bf72..1cbaf207e40 100644 --- a/ocaml/xapi/xapi_pool.ml +++ b/ocaml/xapi/xapi_pool.ml @@ -3669,3 +3669,9 @@ let configure_update_sync ~__context ~self ~update_sync_frequency let set_update_sync_enabled ~__context ~self ~value = Pool_periodic_update_sync.set_enabled ~__context ~value ; Db.Pool.set_update_sync_enabled ~__context ~self ~value + +let set_local_auth_max_threads ~__context:_ ~self:_ ~value = + Xapi_session.set_local_auth_max_threads value + +let set_ext_auth_max_threads ~__context:_ ~self:_ ~value = + Xapi_session.set_ext_auth_max_threads value diff --git a/ocaml/xapi/xapi_pool.mli b/ocaml/xapi/xapi_pool.mli index f2cd79c6346..941b3e4a87a 100644 --- a/ocaml/xapi/xapi_pool.mli +++ b/ocaml/xapi/xapi_pool.mli @@ -408,3 +408,9 @@ val configure_update_sync : val set_update_sync_enabled : __context:Context.t -> self:API.ref_pool -> value:bool -> unit + +val set_local_auth_max_threads : + __context:Context.t -> self:API.ref_pool -> value:int64 -> unit + +val set_ext_auth_max_threads : + __context:Context.t -> self:API.ref_pool -> value:int64 -> unit diff --git a/ocaml/xapi/xapi_session.ml b/ocaml/xapi/xapi_session.ml index 3d06f84732b..2c1b33bb675 100644 --- a/ocaml/xapi/xapi_session.ml +++ b/ocaml/xapi/xapi_session.ml @@ -261,6 +261,12 @@ let throttle_auth_external = Locking_helpers.Semaphore.create "External auth" let with_throttle = Locking_helpers.Semaphore.execute +let set_local_auth_max_threads n = + Locking_helpers.Semaphore.set_max throttle_auth_internal @@ Int64.to_int n + +let set_ext_auth_max_threads n = + Locking_helpers.Semaphore.set_max throttle_auth_external @@ Int64.to_int n + let wipe_string_contents str = for i = 0 to Bytes.length str - 1 do Bytes.set str i '\000' diff --git a/ocaml/xapi/xapi_session.mli b/ocaml/xapi/xapi_session.mli index 2dd95cd0d2f..422afd46cc3 100644 --- a/ocaml/xapi/xapi_session.mli +++ b/ocaml/xapi/xapi_session.mli @@ -106,3 +106,7 @@ val get_failed_login_stats : unit -> string option val get_total_sessions : unit -> Int64.t (** Retrieves the amount of sessions opened since the last time xapi was started *) + +val set_local_auth_max_threads : int64 -> unit + +val set_ext_auth_max_threads : int64 -> unit From d90369debdf3b28f09832fe00c7ef00488479a12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Wed, 11 Oct 2023 18:29:38 +0100 Subject: [PATCH 7/9] CP-43755: Increase default max threads for PAM from 1 to 8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have 16 vCPUs at most in Dom0 and we don't want to take them all up with authentication threads, so limit this at 8. Signed-off-by: Edwin Török --- ocaml/idl/datamodel_pool.ml | 2 +- ocaml/xapi/dbsync_master.ml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ocaml/idl/datamodel_pool.ml b/ocaml/idl/datamodel_pool.ml index d9e69ccdb33..0e4ac9b907a 100644 --- a/ocaml/idl/datamodel_pool.ml +++ b/ocaml/idl/datamodel_pool.ml @@ -1431,7 +1431,7 @@ let t = "coordinator_bias" "true if bias against pool master when scheduling vms is enabled, \ false otherwise" - ; field ~qualifier:StaticRO ~ty:Int ~default_value:(Some (VInt 1L)) + ; field ~qualifier:StaticRO ~ty:Int ~default_value:(Some (VInt 8L)) ~lifecycle:[] "local_auth_max_threads" "Maximum number of threads to use for PAM authentication" ; field ~qualifier:StaticRO ~ty:Int ~default_value:(Some (VInt 1L)) diff --git a/ocaml/xapi/dbsync_master.ml b/ocaml/xapi/dbsync_master.ml index 6cbec514c37..6afe4178933 100644 --- a/ocaml/xapi/dbsync_master.ml +++ b/ocaml/xapi/dbsync_master.ml @@ -52,7 +52,7 @@ let create_pool_record ~__context = ~telemetry_next_collection:Xapi_stdext_date.Date.epoch ~last_update_sync:Xapi_stdext_date.Date.epoch ~update_sync_frequency:`weekly ~update_sync_day:0L - ~update_sync_enabled:false ~local_auth_max_threads:1L + ~update_sync_enabled:false ~local_auth_max_threads:8L ~ext_auth_max_threads:1L let set_master_ip ~__context = From 6fe2f506982e7c34796e3b75fe5da7d7726e270e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Thu, 12 Oct 2023 12:44:12 +0100 Subject: [PATCH 8/9] fix(dune): gen_lifecycle depends on git describe output, which is outside of the normal source and build dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this when using the dune cache 'update-dm-lifecycle' will print the wrong version numbers. Signed-off-by: Edwin Török --- ocaml/idl/dune | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ocaml/idl/dune b/ocaml/idl/dune index 4e550eff2ad..3dfa75af8c4 100644 --- a/ocaml/idl/dune +++ b/ocaml/idl/dune @@ -67,7 +67,7 @@ ; use the binary promoted file from the source dir (not the build dir) that has ; the correct version number embedded (rule - (deps gen_lifecycle.exe) + (deps gen_lifecycle.exe (universe)) (action (with-stdout-to datamodel_lifecycle.ml.generated (system %{project_root}/../../ocaml/idl/gen_lifecycle.exe)))) ; 'diff' handles promotion too, see https://dune.readthedocs.io/en/stable/concepts.html?highlight=diffing#diffing-and-promotion From c5b097015e828504592a56165059e88d2b1bc6d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Thu, 12 Oct 2023 12:44:48 +0100 Subject: [PATCH 9/9] CP-43755: commit lifecycle changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Edwin Török --- ocaml/idl/datamodel_lifecycle.ml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ocaml/idl/datamodel_lifecycle.ml b/ocaml/idl/datamodel_lifecycle.ml index b8c5319563e..dc8296bd58e 100644 --- a/ocaml/idl/datamodel_lifecycle.ml +++ b/ocaml/idl/datamodel_lifecycle.ml @@ -55,6 +55,10 @@ let prototyped_of_field = function Some "23.9.0" | "pool", "telemetry_uuid" -> Some "23.9.0" + | "pool", "ext_auth_max_threads" -> + Some "23.26.0-next" + | "pool", "local_auth_max_threads" -> + Some "23.26.0-next" | "pool", "coordinator_bias" -> Some "22.37.0" | "pool", "migration_compression" -> @@ -95,6 +99,10 @@ let prototyped_of_message = function Some "23.18.0" | "host", "set_https_only" -> Some "22.27.0" + | "pool", "set_ext_auth_max_threads" -> + Some "23.26.0-next" + | "pool", "set_local_auth_max_threads" -> + Some "23.26.0-next" | "pool", "set_update_sync_enabled" -> Some "23.18.0" | "pool", "configure_update_sync" ->