-
Notifications
You must be signed in to change notification settings - Fork 929
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.
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
Add limits.cpu.pin_strategy
setting to disable VM CPU auto pinning by default
#14171
Add limits.cpu.pin_strategy
setting to disable VM CPU auto pinning by default
#14171
Conversation
460df45
to
ffe7a87
Compare
Heads up @mionaalex - the "Documentation" label was applied to this issue. |
ffe7a87
to
bc4de3b
Compare
ab52340
to
96fdbdf
Compare
limits.cpu.pin_strategy
setting to disable VM CPU auto pinning by defaultlimits.cpu.pin_strategy
setting to disable VM CPU auto pinning by default
96fdbdf
to
a8e75ba
Compare
If we modify the current tests in Currently, the checks in if journalctl --quiet --no-hostname --no-pager --boot=0 --unit=snap.lxd.daemon.service | grep "Scheduler: virtual-machine"; then
... However, due to my implementation of checking the config setting in if c.Type() == instancetype.VM {
if conf["limits.cpu.pin_strategy"] == "none" {
continue
} else if conf["limits.cpu.pin_strategy"] == "auto" {
logger.Debugf("CPU auto pinning enabled for %q", c.Name())
}
} Therefore, I think the only change required (for now) to ensure we don't introduce a regression to For reference: canonical/lxd-ci@972ef98. Please let me know if you have any suggestions 😄 |
a8e75ba
to
d83c50c
Compare
You will need to introduce a new API extension for the new config key and can test for support of that when testing to enable the functionality. |
d83c50c
to
7d85357
Compare
@kadinsayani I couldn't find any reference to back that but I think it's best for the commit adding the API extension to be the first in the PR. This way, when @tomponline does the backporting (if any, dunno for this one) he can quickly identify that a collection of commits need to be considered at once for the backport. |
Sounds good, thanks for letting me know! |
7d85357
to
0ca7deb
Compare
@tomponline will we be back porting the commit containing the new API extension to 6.1? Currently, the tests in https://github.com/mihalicyn/lxd-ci/blob/972ef98d660c3c2efba3d8aa7408126a1b555ff9/tests/cpu-vm#L12 |
yes we can because it defaults to disabled |
@kadinsayani please can you ensure that the change to disable auto pin for containers is separate so i can choose to backport it or not. |
0ca7deb
to
3c071ad
Compare
limits.cpu.pin_strategy
setting to disable VM CPU auto pinning by defaultlimits.cpu.pin_strategy
setting to disable CPU auto pinning by default
limits.cpu.pin_strategy
setting to disable CPU auto pinning by defaultlimits.cpu.pin_strategy
setting to disable CPU auto pinning by default
93789b2
to
4620ebf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can you add tests for the limits.cpu.pin_strategy being acceptable in a profile but not on container local config? Suggest in test_config_profiles
needs a rebase |
4620ebf
to
0b371bd
Compare
Signed-off-by: Kadin Sayani <[email protected]>
…chines Signed-off-by: Kadin Sayani <[email protected]>
Signed-off-by: Kadin Sayani <[email protected]>
Signed-off-by: Kadin Sayani <[email protected]>
Unless `limits.cpu.pin_strategy` is set to auto, VM CPU auto pinning is disabled Signed-off-by: Kadin Sayani <[email protected]>
Signed-off-by: Kadin Sayani <[email protected]>
0b371bd
to
5097f38
Compare
Tests added and rebased 👍 |
…e casting to `int64` Signed-off-by: Kadin Sayani <[email protected]>
… use `instancetype.Type` instead of `string` Signed-off-by: Kadin Sayani <[email protected]>
… `ParseUint32Range` Signed-off-by: Kadin Sayani <[email protected]>
…g and profile settings Signed-off-by: Kadin Sayani <[email protected]>
5097f38
to
f6dc890
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
This PR includes updates to `tests/cpu-vm`. Changes: - Check if feature is present by checking API extension `limits_cpu_pin_strategy` (landing with canonical/lxd#14171); - Launch ephemeral VM with `limits.cpu.pin_strategy=auto` to check for CPU pinning, and `limits.cpu.pin_strategy=none` to ensure CPU's are not automatically pinned; and - Add check to ensure `limits.cpu.pin_strategy` can't be set while VM is running.
Fixes #14133.
This pull request introduces a configuration option for virtual machines,
limits.cpu.pin_strategy
. This option allows users to specify the strategy for CPU auto pinning. The possible values arenone
(disables CPU auto pinning - also the new default) andauto
(enables CPU auto pinning).For reference when back-porting: 56bd496