| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-02-08 | |||
| 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 | |
| 18:51:40 | sean-k-mooney | docs and the 2 checks you wrote above please | |
| 18:51:45 | sean-k-mooney | docs is the most imporant | |
| 18:51:56 | sean-k-mooney | but i would like to see the check if possibel too | |
| 18:52:05 | gibi | works for me | |
| 18:52:15 | gibi | thanks folks for the discussion | |
| 18:52:19 | bauzas | ++ | |
| 18:52:28 | bauzas | will be working on it on the train tomorrow | |
| 18:53:53 | sean-k-mooney | i will be around for reviews tomorrow and then wednesday | |
| 18:54:05 | sean-k-mooney | so feel free to ping me | |
| 18:54:32 | sean-k-mooney | i would normaly take the week of valentines off but since it FF week im going to be here for wednesday-friday | |
| 18:57:39 | bauzas | don't feel obliged | |
| 18:58:02 | bauzas | we have already promised more than what we can offer | |
| 18:58:08 | bauzas | and we're short in time | |
| 18:58:20 | bauzas | the numbers will be terrible, but I can surely explain those | |
| 20:21:34 | opendevreview | Elod Illes proposed openstack/nova stable/train: DNM: CI test https://review.opendev.org/c/openstack/nova/+/873116 | |
| 21:09:56 | gmann | bauzas: with placement change merged (https://review.opendev.org/c/openstack/placement/+/865618) we can mark this BP as completed https://blueprints.launchpad.net/placement/+spec/policy-defaults-improvement | |
| #openstack-nova - 2023-02-09 | |||
| 03:26:09 | opendevreview | Yusuke Okada proposed openstack/nova master: Fix failed count for anti-affinity check https://review.opendev.org/c/openstack/nova/+/873216 | |
| 03:30:13 | opendevreview | Yusuke Okada proposed openstack/nova master: Fix failed count for anti-affinity check https://review.opendev.org/c/openstack/nova/+/873216 | |
| 06:05:34 | opendevreview | Nobuhiro MIKI proposed openstack/nova master: libvirt: Add 'COMPUTE_ADDRESS_SPACE_*' traits support https://review.opendev.org/c/openstack/nova/+/873221 | |
| 09:14:09 | Uggla | bauzas, gibi o/ | |
| 09:16:39 | Uggla | bauzas, gibi looking at the https://review.opendev.org/c/openstack/nova/+/839401/22 are we all agree to remove the ShareMappingLibvirt* although that is not following the spec and might be more difficult if we want to create a dedicated module (os-share) later ? | |
| 09:17:55 | Uggla | bauzas, gibi, of course doing that will simplify the code but I will have to review lot of stuff. | |
| 09:39:51 | bauzas | Uggla: I agree with you | |
| 09:40:58 | gibi | ...loading context | |
| 09:44:42 | gibi | For me the content of ShareMappingLibvirt is OK, it is just placed to a too public place. | |
| 09:45:48 | gibi | i.e. I'm OK to use inheritance to model that a ShareMapping my need to be mounted by a libvirt or by other virt driver | |
| 09:48:18 | Uggla | gibi, hum I think what bauzas and john expect is the removal of convert and inheritance and move that in the driver itself. | |
| 09:48:44 | gibi | sure tha is also an option | |
| 09:49:21 | Uggla | gibi, if not in the object where do you put the act code ? | |
| 09:50:30 | gibi | the libvirt specific classes can be placed to nova.virt.libvirt afaik. But as you said bauzas and johnthetubaguy might want not to have them as ovos even if they are moved under nova.virt.libvirt | |
| 09:51:13 | gibi | so to expedite things I think it would be easier to simply remove the driver specific ovo | |
| 09:51:34 | gibi | and create functions in the driver that takes a generic ShareMapping and do the driver specific bits with it | |
| 09:52:20 | gibi | I guess that what bauzas and johnthetubaguy suggests | |
| 09:54:23 | Uggla | gibi, yes sounds like it is ok for you to go in this way as well ? | |
| 09:54:46 | gibi | yeah, OK with me | |
| 09:56:10 | Uggla | ok so I'll change the code in this way. | |
| 09:57:54 | bauzas | gibi: tbc, I w | |
| 09:58:10 | bauzas | I don't want to have a ovo inheritance for this object | |
| 09:58:18 | bauzas | Uggla: ^ | |
| 09:58:46 | bauzas | since this object is not about providing fields or persisting them | |
| 09:58:53 | bauzas | by RPC calls | |
| 09:59:15 | bauzas | but yeah, we can have a specific python object for it in libvirt if Uggla wants | |
| 09:59:28 | Uggla | bauzas, yep I will keep only the ShareMapping / ShareMappingList objects | |
| 10:00:36 | Uggla | bauzas, not sure it is worth creating an object in the libvirt part, I'll see. | |
| 10:05:37 | gibi | bauzas: we have examples where we use ovo not just to persist data. We even have examples to driver specific data in ovo https://github.com/openstack/nova/blob/master/nova/objects/migrate_data.py | |
| 10:06:19 | gibi | or driver specific ovo | |
| 10:09:27 | bauzas | gibi: sure, but those objects are used for being passing between services | |
| 10:10:07 | bauzas | and those are only for fields | |
| 10:10:31 | Uggla | sounds like https://miro.medium.com/v2/resize:fit:720/format:webp/1*LzKppsHIMabQhnCCdpdQXg.jpeg :) | |
| 10:10:40 | bauzas | gibi: while the previous ShareMappingLibvirt objects were directly calling libvirt | |
| 10:14:54 | gibi | bauzas: I never really got why we are not coupling data and the methods using that data together into the ovos. That is one of the main reason classes exists to couple data and code acting on that data together :) | |
| 10:15:26 | gibi | but I rest my case. It is a theorethical discussion | |
| 10:15:51 | bauzas | gibi: well I remember when dansmith created those objects | |
| 10:16:13 | bauzas | gibi: and I'm pretty sure I can find you some links explaining how this works | |
| 10:16:26 | bauzas | I mean, not how, but why | |
| 10:16:46 | bauzas | (sorry, not explaining you're bad, just about the situation) | |
| 10:17:06 | bauzas | we could have specific libvirt objects like you said | |
| 10:17:24 | bauzas | but those objects would only have values | |
| 10:18:16 | gibi | and that is the point I disagree with out object model ^^ | |
| 10:18:47 | gibi | but as I said I rest my case. I don't want to change how nova looks at this point in its lifecycle | |
| 10:22:14 | ralonsoh | hi folks, do you know when we can set 'device_owner': 'compute:FAIL' for a port? Where is this 'compute:FAIL' coming from? | |
| 10:24:43 | gibi | the FAIL part is interesting | |
| 10:25:07 | ralonsoh | yeah, I've never seen that | |