opendevreview | Amit Uniyal proposed openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/881457 | 06:08 |
---|---|---|
opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | 06:08 |
opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | 06:55 |
gibi | dansmith: re https://www.youtube.com/watch?v=dLjNzwEULG8 so I should tatoo some instance.update notification to my body :) | 08:07 |
opendevreview | Amit Uniyal proposed openstack/nova-specs master: Adds cleanup to remove dangling volumes https://review.opendev.org/c/openstack/nova-specs/+/878757 | 08:14 |
gibi | id we changed cirros guest image recently on master? Yesterday and today | 08:29 |
gibi | I started to see guest block io failures | 08:29 |
gibi | see https://review.opendev.org/c/openstack/nova/+/873216/7#message-4584b616820a42414995f77c6e495aafd6499ebc | 08:30 |
gibi | and | 08:30 |
gibi | https://review.opendev.org/c/openstack/nova/+/873216/7#message-0306974232385dedf7a9553ad65459e4f8849e28 | 08:30 |
gibi | and one more https://review.opendev.org/c/openstack/nova/+/885352/3#message-0ed9f92e5069e79cb7b60724c048ca038c7c094e | 08:52 |
gibi | bauzas: I have questions in https://review.opendev.org/c/openstack/nova/+/885352 | 09:04 |
bauzas | gibi: will look in a sec once I eventaually send the email I have in my inbox for a while :D | 09:05 |
bauzas | there | 09:11 |
bauzas | gibi: ack, thanks for your points, have like 2-min for discussing them . | 09:14 |
bauzas | ? | 09:14 |
gibi | yepp I have | 09:14 |
bauzas | cool | 09:16 |
bauzas | so, I don't really want to avoid providing a FileNotFound exception if you want to set a governor | 09:16 |
bauzas | if so, operators would see that their OS platform is not supporting the governors | 09:17 |
gibi | OK. so it is intentional that the FileNotFound exception propagates | 09:17 |
gibi | does it make the nova-compute fail to start if governor is requested in the config but not available on the hypervisor? | 09:18 |
gibi | or only the intance operaton that calls set_governor will fail? | 09:18 |
frickler | gibi: iiuc the GPT messages are normal due to the way cirros works. the vdb issue would be an additional volume failing, not related to cirros? | 09:26 |
frickler | also no change on cirros image yet, I want to get the new version cached in infra images first | 09:27 |
frickler | https://review.opendev.org/c/openstack/project-config/+/885005 | 09:27 |
gibi | frickler: thanks | 09:28 |
gibi | frickler: then I need to check deeper about the volume issue | 09:28 |
bauzas | gibi: sorry, was looking at something else | 09:30 |
gibi | bauzas: as far as I see if the configured governor is not available then the instance operation will fail | 09:31 |
bauzas | gibi: yeah, if the ops wants to set a governor strategy, then we'll try to set it at startup | 09:31 |
bauzas | gibi: so the service restart will fail | 09:31 |
gibi | bauzas: could you point to it? | 09:31 |
bauzas | gibi: https://review.opendev.org/c/openstack/nova/+/885352/3/nova/tests/functional/libvirt/test_power_manage.py#244 | 09:32 |
gibi | looking | 09:35 |
gibi | ahh it goes through power_down_all_dedicated_cpus | 09:36 |
gibi | during init_host | 09:36 |
bauzas | yes | 09:36 |
gibi | cool then | 09:36 |
bauzas | and that's why it failed previously | 09:36 |
bauzas | well, not exactly | 09:36 |
bauzas | it failed in the validate_cpu() method because we were comparing the governors if you set cp_state strategy | 09:37 |
gibi | previously it is failed on the governor property | 09:37 |
gibi | yeah | 09:37 |
gibi | and there now we return None | 09:37 |
bauzas | that's a proposal I made | 09:37 |
gibi | so the len of the governors will be 1 in this case and that check passses | 09:37 |
gibi | then the power_down_all_dedicated_cpus will be called and fail | 09:37 |
bauzas | I'm open to other ideas like dansmith's one about the 'none' string | 09:37 |
gibi | I'm OK with None | 09:38 |
bauzas | gibi: depending on the strategy, yes it will fail if governor is set | 09:38 |
gibi | as we are only using that value to collect the different actual governors across the pcpus | 09:39 |
bauzas | an alternative would be to raise an InvalidConfiguration exception instead of a somehow meaningless FileNotFound | 09:39 |
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 |
bauzas | we do return this InvalidConf exception if you want to change the governors but you have some CPUs offline | 09:40 |
bauzas | gibi: yeah I was torn | 09:40 |
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:41 |
gibi | I'm OK as is | 09:43 |
gibi | summarized our discussion in the review now. Thanks for clearing this up for me | 09:44 |
bauzas | gibi: np and thanks for the review | 09:45 |
opendevreview | Sylvain Bauza proposed openstack/nova master: cpu: make governors to be optional https://review.opendev.org/c/openstack/nova/+/885352 | 09:55 |
sean-k-mooney | bauzas: i have not read back fully but it should be a hard error if you have enabled the govenors | 10:56 |
sean-k-mooney | but if you have enabeld cpu state then the lack of the cpu frequency govoner should not break things | 10:57 |
sean-k-mooney | bauzas: gibi what os was this on by the way | 11:04 |
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:05 |
sean-k-mooney | hum q35 vms? | 11:07 |
sean-k-mooney | https://www.phoronix.com/news/KVM-CPUFreq-RFC-Patches that might changegoing forward | 11:10 |
gibi | sean-k-mooney: It was theorethical based on reading the code :) | 11:19 |
gibi | but bauzas helped me understand it. | 11:19 |
sean-k-mooney | ok well it wont work in vms currently | 11:20 |
sean-k-mooney | so that is the refernce plathform :) | 11:20 |
gibi | so a configured but missing governor will be a hard failure on nova-compute startup | 11:20 |
sean-k-mooney | well a configured but missing govoner should be a hard fail | 11:20 |
gibi | it is :) so everything is good | 11:20 |
sean-k-mooney | but what would not be ok is if the cpu state part didnt work | 11:20 |
gibi | eypp | 11:21 |
gibi | yepp | 11:21 |
sean-k-mooney | if you configure that just because the govoner is not supported | 11:21 |
sean-k-mooney | so thats the bug right | 11:21 |
gibi | yes, and it is being fixed ow | 11:21 |
gibi | now | 11:21 |
sean-k-mooney | ya i looked at the patch quickly | 11:21 |
*** blarnath is now known as d34dh0r53 | 12:42 | |
dansmith | bauzas: replied | 13:25 |
bauzas | dansmith: ok, so you would prefer to have an empty string ? | 13:41 |
bauzas | instead of None | 13:42 |
dansmith | bauzas: I would, but don't hold this up on me.. None is fine. | 13:42 |
dansmith | seriously, three people are fine with None, it's fine | 13:42 |
bauzas | okidoki | 13:42 |
dansmith | bauzas: sorry I thought there was still a style thing to fix, I +2d now | 14:14 |
opendevreview | Merged openstack/nova master: Process unlimited exceptions raised by unplug_vifs https://review.opendev.org/c/openstack/nova/+/879350 | 14:16 |
*** ralonsoh__ is now known as ralonsoh | 14:26 | |
opendevreview | Merged openstack/nova master: Fix failed count for anti-affinity check https://review.opendev.org/c/openstack/nova/+/873216 | 14:35 |
opendevreview | Merged openstack/nova master: db: Don't rely on branched connections https://review.opendev.org/c/openstack/nova/+/880663 | 16:54 |
opendevreview | Merged openstack/nova master: db: Remove unnecessary 'insert()' argument https://review.opendev.org/c/openstack/nova/+/880664 | 16:54 |
opendevreview | Merged openstack/nova master: tests: Pass parameters to sqlalchemy.text() as bindparams https://review.opendev.org/c/openstack/nova/+/880665 | 16:54 |
opendevreview | Merged openstack/nova master: tests: Add missing args to sqlalchemy.Table https://review.opendev.org/c/openstack/nova/+/880667 | 16:54 |
opendevreview | Merged openstack/nova master: cpu: fix the privsep issue when offlining the cpu https://review.opendev.org/c/openstack/nova/+/885293 | 21:37 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!