CA-408339: Respect xenopsd's NUMA-placement-policy default #6368
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
psafont
approved these changes
Mar 18, 2025
robhoes
commented
Mar 18, 2025
Xenopsd has an experimental feature that aims to optimise NUMA placement. This used to be configured by adding `numa-placement=true` to the file /etc/xenopsd.conf, which tells xenopsd to enable the feature. Later, an actual API was added to configure this: `host.set_numa_affinity_policy`. The expectation was that, while this new API should be preferred, the old xenopsd.conf option would still work for backwards compatibility reasons. This is particularly important for hosts that are upgraded to the new version. Unfortunately, while code exists in xenopsd to read and use the config option when it starts up, when xapi starts up immediately after xenopsd, it always overrides the NUMA config based its own DB field. The field type actually has a "default" option, but this gets translated to "any" (= no NUMA). By default, this means means that the experimental feature is disabled, no matter what the config file says, and can only be enabled through the API. The fix is for xapi to not assign a default value itself, but let xenopsd decide on the default policy. And xenopsd uses its config file to do so, as before. Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
changlei-li
approved these changes
Mar 19, 2025
| if !Xenopsd.numa_placement_compat then Best_effort else Any ; | ||
| info "Default NUMA affinity policy is '%s'" | ||
| Xenops_server.(string_of_numa_affinity_policy !default_numa_affinity_policy) ; | ||
| Xenops_server.numa_placement := !Xenops_server.default_numa_affinity_policy ; |
There was a problem hiding this comment.
I would suggest wrap these lines in a function like Xenops_server.init_numa_affinity_policy !Xenopsd.numa_placement_compat. Then the references in Xenops_server will not need to be exposed.
There was a problem hiding this comment.
Unfortunately, Xenops_server.numa_placement is actually used outside Xenops_server, so we can hide it.
edwintorok
approved these changes
Mar 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Xenopsd has an experimental feature that aims to optimise NUMA
placement. This used to be configured by adding
numa-placement=truetothe file /etc/xenopsd.conf, which tells xenopsd to enable the feature.
Later, an actual API was added to configure this:
host.set_numa_affinity_policy. The expectation was that, while thisnew API should be preferred, the old xenopsd.conf option would still
work for backwards compatibility reasons. This is particularly important
for hosts that are upgraded to the new version.
Unfortunately, while code exists in xenopsd to read and use the config
option when it starts up, when xapi starts up immediately after xenopsd,
it always overrides the NUMA config based its own DB field. The field
type actually has a "default" option, but this gets translated to "any"
(= no NUMA). By default, this means means that the experimental feature
is disabled, no matter what the config file says, and can only be
enabled through the API.
The fix is for xapi to not assign a default value itself, but let
xenopsd decide on the default policy. And xenopsd uses its config file
to do so, as before.