| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-02-08 | |||
| 18:25:24 | bauzas | to check the core status and online if a governor helper is called ? | |
| 18:25:30 | sean-k-mooney | when its set to govoner the libvirt chack that checks the cores should be online should catch this | |
| 18:25:32 | gibi | bauzas: yeah | |
| 18:25:47 | bauzas | ok, I can augment power_down and up | |
| 18:26:06 | bauzas | gibi: sean-k-mooney: fwiw, I'll be traveling tomorrow back-and-forth to Paris | |
| 18:26:13 | bauzas | so most of my day will be trains | |
| 18:26:19 | bauzas | but I'll be online | |
| 18:26:24 | bauzas | and on work status | |
| 18:26:39 | gibi | maybe the other direction is trickier. so you have a governor startegy and you set the cpu to low. Then you reconfigure to cpu_state. And you boot a VM that need the cpu so you online it, but it still has the low governor set | |
| 18:26:50 | sean-k-mooney | bauzas: you chould change https://review.opendev.org/c/openstack/nova/+/821228/5/nova/virt/libvirt/host.py#755 | |
| 18:27:08 | sean-k-mooney | we shoudl check if cpu power management is enabled and its set to cpu_state | |
| 18:27:30 | sean-k-mooney | if its set to govoner then we can detect that cores are offlien and online them | |
| 18:27:42 | sean-k-mooney | or raise an error and stop the compute | |
| 18:27:58 | bauzas | damn operators who like to play with config options | |
| 18:28:01 | sean-k-mooney | this is not really something you should change when there are instance on the host | |
| 18:28:42 | sean-k-mooney | gibi: the other direction wee should not really do anything (state->govoner) | |
| 18:28:45 | bauzas | sean-k-mooney: true, I wonder whether it would be rather preferable to detect it at startup and fail | |
| 18:29:04 | sean-k-mooney | sorry (govoner->state) | |
| 18:29:07 | gibi | I'm OK to reject the reconfiguration with instance on the host, BUT we power down CPUs at init_host without an instance | |
| 18:29:19 | sean-k-mooney | because you are allowed to manage the govoner today outside of nova | |
| 18:29:25 | gibi | so you don't need an instance to get a discrepancy | |
| 18:29:26 | sean-k-mooney | so you may have change the state | |
| 18:29:40 | bauzas | I think we're entering danger zone | |
| 18:29:53 | sean-k-mooney | with that we could say if you have ONF.libvirt.cpu_power_management=true | |
| 18:30:00 | bauzas | here, we're asking operators to think about what they gonna do | |
| 18:30:08 | sean-k-mooney | then all govenros in the cpu_dedicated_set must be the same | |
| 18:30:24 | sean-k-mooney | we could detect that in that case and catch the (govoner->state) change | |
| 18:30:40 | bauzas | I mean, if I'm an operator that already sets governor levels, then I should NOT let nova play with my cores | |
| 18:31:13 | sean-k-mooney | so (govoner->state) we can detect that there is a govoner differnce on the cpus, (govoner<-state) we can detect the offline cores | |
| 18:31:28 | bauzas | sean-k-mooney: I don't want to add logic while I can add documentation about supported features | |
| 18:31:56 | sean-k-mooney | i agree on documenting that you cant cahnge this with instances on the host | |
| 18:32:05 | sean-k-mooney | and shoudl reboot if you do change it or something like that | |
| 18:32:18 | sean-k-mooney | but we can detect and error or fix it fi we want too | |
| 18:32:45 | sean-k-mooney | if we have CONF.libvirt.cpu_power_management=true i think that shoudl mean you cannot manage the cpus outside of nova | |
| 18:32:49 | gibi | again, becase we apply the config to the cpus at init_host regardless of any instance on the host you don't need an instance on the host to cause problems | |
| 18:33:11 | sean-k-mooney | gibi: we dont your corect | |
| 18:33:46 | sean-k-mooney | but we can detect it and document that you should not do this without a host reboot | |
| 18:33:53 | bauzas | I'm confused | |
| 18:34:02 | gibi | sean-k-mooney: I agree to document | |
| 18:34:07 | gibi | sean-k-mooney: and detect if possible | |
| 18:34:35 | sean-k-mooney | detect is possibel but i think bauzas would prefer to not require that for the feature to merge and maybe to it later | |
| 18:35:06 | sean-k-mooney | (govoner->state) the concer is we leave some cpus in low perfomacne mode | |
| 18:35:13 | sean-k-mooney | (state-) | |
| 18:35:26 | sean-k-mooney | (state->govoner) the concern is we leave some cores offline | |
| 18:35:44 | sean-k-mooney | we can detect boot and make it a hard error from init_host | |
| 18:36:03 | sean-k-mooney | in either case a host reboot will resolve it or they operator can fix it | |
| 18:37:35 | gibi | yepp | |
| 18:38:47 | sean-k-mooney | bauzas: does ^ make sense | |
| 18:39:06 | sean-k-mooney | the offline core check is triival its just https://review.opendev.org/c/openstack/nova/+/821228/5/nova/virt/libvirt/host.py#755 | |
| 18:39:27 | sean-k-mooney | the inconsitent govoner state is also trivial | |
| 18:39:48 | sean-k-mooney | i dont think we shoudl check if its explictly powersave or anything like that | |
| 18:39:48 | opendevreview | Merged openstack/placement master: Modify the placement API policies defaults and scope_type https://review.opendev.org/c/openstack/placement/+/865618 | |
| 18:40:22 | sean-k-mooney | just in cpu_sate mofe assert the govoner is the same for all cores in cpu_dedicated_set | |
| 18:41:29 | sean-k-mooney | s/mofe/mode/ | |
| 18:41:40 | bauzas | I just want to clarify | |
| 18:42:28 | bauzas | are we OK with doing this in init_host() ? | |
| 18:42:47 | gibi | yes | |
| 18:42:48 | sean-k-mooney | yep | |
| 18:42:58 | bauzas | OK | |
| 18:43:36 | bauzas | and in init_host(), do we agree on detecting that if power strategy is set to governor, all cores should be up ? | |
| 18:43:47 | sean-k-mooney | i would prefer if you put these in a new fucntions that you invoked form there for simpler testing | |
| 18:44:00 | sean-k-mooney | bauzas: yes | |
| 18:44:22 | sean-k-mooney | if its set to govoner the old requiremnt that all core must be up should still apply | |
| 18:44:33 | bauzas | https://review.opendev.org/c/openstack/nova/+/868237/9/nova/virt/libvirt/driver.py#826 | |
| 18:44:47 | bauzas | I already call power_down_all_cores() | |
| 18:45:03 | bauzas | in power_down_all_cores() I can do two checks | |
| 18:45:08 | bauzas | besides the existing one | |
| 18:45:31 | sean-k-mooney | you should not over complicate that function | |
| 18:45:36 | bauzas | 1/ if strategy is set to governor, all cpus need to be online | |
| 18:46:10 | bauzas | 3/ if strategy is set to cpu_state, then all governors should be identical | |
| 18:46:16 | bauzas | s/3/2 | |
| 18:46:32 | sean-k-mooney | i would prefer a new validate_cores() or similr function and only call libvirt_cpu.power_down_all_dedicated_cpus() if the mode is state | |
| 18:47:00 | sean-k-mooney | bauzas: yes those are the two checks that i think you shoudl do | |
| 18:47:08 | sean-k-mooney | but not in libvirt_cpu.power_down_all_dedicated_cpus() | |
| 18:47:36 | sean-k-mooney | do it ins libvirt_cpu.validate_all_cpus() | |
| 18:47:39 | bauzas | I can manage that request | |
| 18:47:40 | gibi | for 2/ if all cores has governor low as nova set it to that before the reconfiguration then we have a problem still | |
| 18:47:49 | sean-k-mooney | gibi: no | |
| 18:47:53 | sean-k-mooney | we cant check for that | |
| 18:48:04 | sean-k-mooney | because the admin might have set it to low intentionally | |
| 18:48:12 | sean-k-mooney | or to any other value | |
| 18:48:36 | bauzas | this is why I think docs is important | |
| 18:48:53 | bauzas | (which is missing, but we can write it before RC1) | |
| 18:48:55 | sean-k-mooney | thats why i stated the requiremnt that they should all be the same if the power state is manged by nova | |
| 18:49:06 | bauzas | I'm OK with this | |
| 18:49:12 | bauzas | nova will try to detect | |
| 18:49:39 | bauzas | if the operator explicitely manages all the governors and wants to set nova to turn off cores, that's his choice | |
| 18:49:46 | gibi | I undrestand that we cannot catch that if the operator changed a governor directly. And I agree that we should not be able to catch that. But if an empty compute was configured with governor strategy, then reconfigured to cpu_state strategy then that compute will have all dediceted cpus in low performance mode for ever | |
| 18:49:55 | bauzas | he'll probably end up with problems, but meh, unsupported | |
| 18:50:11 | sean-k-mooney | gibi: that is fixed by the host reboot | |
| 18:50:28 | sean-k-mooney | that why i think we need to document that if you want to change this you should reboot the host | |
| 18:50:35 | sean-k-mooney | so we start form a clean state | |
| 18:50:35 | gibi | sean-k-mooney: so we will say in the doc that the startegy can only be change via host reboot? | |
| 18:50:52 | sean-k-mooney | that is what im suggestign yes | |
| 18:50:54 | gibi | OK | |
| 18:50:58 | gibi | that will solve it yes | |
| 18:50:59 | bauzas | gibi: the problem is that we can't assume that governor_high is the default value *before* | |
| 18:51:06 | gibi | bauzas: I know :) | |
| 18:51:13 | gibi | host reboot, it is | |
| 18:51:14 | bauzas | ok, so docs | |