| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-06-07 | |||
| 09:32:11 | bauzas | gibi: https://review.opendev.org/c/openstack/nova/+/885352/3/nova/tests/functional/libvirt/test_power_manage.py#244 | |
| 09:35:28 | gibi | looking | |
| 09:36:05 | gibi | ahh it goes through power_down_all_dedicated_cpus | |
| 09:36:10 | gibi | during init_host | |
| 09:36:11 | bauzas | yes | |
| 09:36:11 | gibi | cool then | |
| 09:36:24 | bauzas | and that's why it failed previously | |
| 09:36:45 | bauzas | well, not exactly | |
| 09:37:07 | bauzas | it failed in the validate_cpu() method because we were comparing the governors if you set cp_state strategy | |
| 09:37:08 | gibi | previously it is failed on the governor property | |
| 09:37:12 | gibi | yeah | |
| 09:37:24 | gibi | and there now we return None | |
| 09:37:36 | bauzas | that's a proposal I made | |
| 09:37:37 | gibi | so the len of the governors will be 1 in this case and that check passses | |
| 09:37:58 | gibi | then the power_down_all_dedicated_cpus will be called and fail | |
| 09:37:58 | bauzas | I'm open to other ideas like dansmith's one about the 'none' string | |
| 09:38:44 | gibi | I'm OK with None | |
| 09:38:46 | bauzas | gibi: depending on the strategy, yes it will fail if governor is set | |
| 09:39:28 | gibi | as we are only using that value to collect the different actual governors across the pcpus | |
| 09:39:51 | bauzas | an alternative would be to raise an InvalidConfiguration exception instead of a somehow meaningless FileNotFound | |
| 09:40:27 | gibi | if FileNotFound explains the missing file path then I think that is enough for the deployer to figure out what is the issue | |
| 09:40:33 | bauzas | we do return this InvalidConf exception if you want to change the governors but you have some CPUs offline | |
| 09:40:56 | bauzas | gibi: yeah I was torn | |
| 09:41:45 | bauzas | gibi: between raising the FileNotFound exception that would show the filename, or a better direct exception that would directly tell 'sorry, you want to change the governors but your OS doesn't support it" | |
| 09:43:57 | gibi | I'm OK as is | |
| 09:44:38 | gibi | summarized our discussion in the review now. Thanks for clearing this up for me | |
| 09:45:22 | bauzas | gibi: np and thanks for the review | |
| 09:55:08 | opendevreview | Sylvain Bauza proposed openstack/nova master: cpu: make governors to be optional https://review.opendev.org/c/openstack/nova/+/885352 | |
| 10:56:25 | sean-k-mooney | bauzas: i have not read back fully but it should be a hard error if you have enabled the govenors | |
| 10:57:00 | sean-k-mooney | but if you have enabeld cpu state then the lack of the cpu frequency govoner should not break things | |
| 11:04:28 | sean-k-mooney | bauzas: gibi what os was this on by the way | |
| 11:05:43 | sean-k-mooney | you are right that not all plathform will have this but any majour linux disto on x86 should so im wondering what exotic config was being used to find this | |
| 11:07:51 | sean-k-mooney | hum q35 vms? | |
| 11:10:57 | sean-k-mooney | https://www.phoronix.com/news/KVM-CPUFreq-RFC-Patches that might changegoing forward | |
| 11:19:46 | gibi | sean-k-mooney: It was theorethical based on reading the code :) | |
| 11:19:57 | gibi | but bauzas helped me understand it. | |
| 11:20:03 | sean-k-mooney | ok well it wont work in vms currently | |
| 11:20:10 | sean-k-mooney | so that is the refernce plathform :) | |
| 11:20:12 | gibi | so a configured but missing governor will be a hard failure on nova-compute startup | |
| 11:20:30 | sean-k-mooney | well a configured but missing govoner should be a hard fail | |
| 11:20:44 | gibi | it is :) so everything is good | |
| 11:20:58 | sean-k-mooney | but what would not be ok is if the cpu state part didnt work | |
| 11:21:08 | gibi | eypp | |
| 11:21:10 | gibi | yepp | |
| 11:21:11 | sean-k-mooney | if you configure that just because the govoner is not supported | |
| 11:21:13 | sean-k-mooney | so thats the bug right | |
| 11:21:21 | gibi | yes, and it is being fixed ow | |
| 11:21:22 | gibi | now | |
| 11:21:33 | sean-k-mooney | ya i looked at the patch quickly | |
| 13:25:05 | dansmith | bauzas: replied | |
| 13:41:55 | bauzas | dansmith: ok, so you would prefer to have an empty string ? | |
| 13:42:01 | bauzas | instead of None | |
| 13:42:14 | dansmith | bauzas: I would, but don't hold this up on me.. None is fine. | |
| 13:42:40 | dansmith | seriously, three people are fine with None, it's fine | |
| 13:42:54 | bauzas | okidoki | |
| 14:14:58 | dansmith | bauzas: sorry I thought there was still a style thing to fix, I +2d now | |
| 14:16:12 | opendevreview | Merged openstack/nova master: Process unlimited exceptions raised by unplug_vifs https://review.opendev.org/c/openstack/nova/+/879350 | |
| 14:35:53 | opendevreview | Merged openstack/nova master: Fix failed count for anti-affinity check https://review.opendev.org/c/openstack/nova/+/873216 | |
| 16:54:02 | opendevreview | Merged openstack/nova master: db: Don't rely on branched connections https://review.opendev.org/c/openstack/nova/+/880663 | |
| 16:54:10 | opendevreview | Merged openstack/nova master: db: Remove unnecessary 'insert()' argument https://review.opendev.org/c/openstack/nova/+/880664 | |
| 16:54:19 | opendevreview | Merged openstack/nova master: tests: Pass parameters to sqlalchemy.text() as bindparams https://review.opendev.org/c/openstack/nova/+/880665 | |
| 16:54:26 | opendevreview | Merged openstack/nova master: tests: Add missing args to sqlalchemy.Table https://review.opendev.org/c/openstack/nova/+/880667 | |
| 21:37:32 | opendevreview | Merged openstack/nova master: cpu: fix the privsep issue when offlining the cpu https://review.opendev.org/c/openstack/nova/+/885293 | |
| #openstack-nova - 2023-06-08 | |||
| 06:47:19 | mnederlof | Hi all, would someone be able to help me to review the change i made? (https://review.opendev.org/c/openstack/nova/+/884595) | |
| 06:50:52 | frickler | mnederlof: based on the bug you reference (which was marked as invalid for nova) maybe sean-k-mooney might want to have a look. but also next week the summit is happening so things might be slow until some time after that | |
| 06:58:11 | mnederlof | yeah it was marked invalid, because it needed to be a feature, so i created a bp for it as well. but i can imagine the summit takes some preparation time this week as well :) | |
| 08:18:22 | opendevreview | Merged openstack/nova-specs master: Re-propose using extend volume completion action for 2023.2 https://review.opendev.org/c/openstack/nova-specs/+/877233 | |
| 09:57:17 | sean-k-mooney | mnederlof: frickler due to budget and other considerations im actully not attendign this summit. i had planned to go to the last physical ptg but it was canceled | |
| 10:00:10 | sean-k-mooney | mnederlof: placign the config option in workarounds implies it will be deleted at some point. so it likely should be elsewhere | |
| 10:01:12 | sean-k-mooney | mnederlof: you can get the same effect already without modifying nova | |
| 10:01:23 | sean-k-mooney | well almost | |
| 10:01:45 | sean-k-mooney | if you disabel the access to the diretl storage url in glance and exposing multipel locations | |
| 10:02:08 | sean-k-mooney | then nova cant detect that its the same ceph cluseter and we will download the image via glance and flatten it | |
| 10:35:15 | mnederlof | sean-k-mooney: yeah you could do that with glance, but then you lose the speed improvement when using a clone/flatten by ceph directly (which is significant) | |
| 10:36:13 | mnederlof | and thanks for the review, i will continue to work on these other points you mentioned | |
| 10:37:16 | sean-k-mooney | you will need to attend the nova team meeting and ask for the specless bluepritn to be approves as a specless blueprint by the way. its possibel that you may be asked to provdie a spec for this | |
| 10:38:01 | sean-k-mooney | we have a deadline for approvals in each cycle and in this case its july 6th | |
| 10:38:36 | sean-k-mooney | the next team meeting will be canceled due to the ptg so the next operturntiy to do this will be tuesday the 20th | |
| 10:39:59 | sean-k-mooney | bauzas: without doing a full review (feel free to do one if you want) do you think https://review.opendev.org/c/openstack/nova/+/884595 would need a spec | |
| 10:41:10 | sean-k-mooney | there si no api change which means we have some flexiblity and given its config driven and only in the libvirt driver the question really is what is the upgrade impact | |
| 10:41:45 | sean-k-mooney | mnederlof: have you spent any time thinking about what happens if you cahnge this option after the cloud is deployed | |
| 10:42:21 | sean-k-mooney | i.e. you have many vms using not flattend storage possibley with existign snapshots. then you enable flattening | |
| 10:42:43 | sean-k-mooney | will you retoractivaly flatten all the existign snapshots or just the new one | |
| 11:29:05 | mnederlof | sean-k-mooney: only new ones will be flattened, as this happens during instance create procedure, so if you change it after deployment, a operator needs to do the flattening of the previously created instances manually, using `rbd flatten <nova-pool>/<instance uuid>_disk` | |
| 11:29:35 | mnederlof | which is something he is already doing sometimes, if he wants to delete a glance image which has children | |
| 11:48:04 | sean-k-mooney | "{{ hostvars[inventory_hostname]['ansible_facts']['default_ipv4']['address'] }}" | |
| 11:49:46 | sean-k-mooney | https://opendev.org/zuul/zuul-jobs/src/commit/839de7f8996838162ae0de6a9f6ba28f968381bc/roles/nimble/defaults/main.yaml#L4 | |
| 11:54:53 | opendevreview | Amit Uniyal proposed openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/881457 | |
| 11:54:54 | opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | |
| 13:24:25 | sean-k-mooney | anyone care to review this backport. i just realised its still open https://review.opendev.org/c/openstack/nova/+/829804 | |
| 13:49:32 | dansmith | sean-k-mooney: can you hit this? https://review.opendev.org/c/openstack/nova/+/885352 | |
| 13:50:57 | dansmith | sean-k-mooney: how is that backport not just a feature? it even says so in the reno | |
| 13:51:16 | dansmith | I know it has as bug and bug-like description in the commit message, but it seems like a feature | |
| 13:51:35 | dansmith | and it also went straight to wallaby? | |
| 13:52:03 | dansmith | I guess I missed the discussion, so I'll go look | |
| 13:54:06 | dansmith | hmm, I'm not finding it | |
| 13:56:16 | dansmith | oh, feb 2022, not 2023 so was this already in yoga? | |
| 14:03:11 | sean-k-mooney | dansmith: where was some patalogical behvior which affected amd host that this helped adress | |
| 14:03:27 | sean-k-mooney | i need to join a meeting be let me loop back with the context i dont have it to hand | |
| 14:03:35 | dansmith | ack | |