| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-08-13 | |||
| 16:12:13 | sean-k-mooney | we are intenrlaly making this a proprty/singelton and early outing already | |
| 16:12:19 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/926218/1/nova/virt/libvirt/host.py#802 | |
| 16:13:54 | artom | I'll be honest, I understand gibi_'s concern, but the lru_cache thing is confusing me | |
| 16:14:26 | sean-k-mooney | pretent i didn mention it | |
| 16:14:41 | artom | You -1'ed the patch and are asking for it :P | |
| 16:14:53 | sean-k-mooney | i -1 because the rest of the patch is wrong | |
| 16:14:55 | sean-k-mooney | even without htat | |
| 16:15:15 | artom | Is addressing gibi_'s comment enough to fix it? | |
| 16:15:27 | sean-k-mooney | yep | |
| 16:15:30 | artom | \o/ | |
| 16:15:52 | sean-k-mooney | we just neeed to make sure this work if you restart the agent multiple times | |
| 16:22:31 | sean-k-mooney | from an upstream point of view i think we can more or less vlaidate this with unit/funcitonal tests | |
| 16:22:44 | sean-k-mooney | we cant test this in tempest without modifyting the cpu toplogy fo the nodepool vms | |
| 16:22:52 | sean-k-mooney | we could chat to infa about that | |
| 16:23:13 | sean-k-mooney | we just need to hw_cpu_sockets=1 hw_cpu_threads=1 | |
| 16:23:33 | sean-k-mooney | on the nodepool images so that we do not have multipel sockets per numa node | |
| 16:23:51 | sean-k-mooney | to fully test it we would also need multiple numa nodes and maybe pci | |
| 16:24:09 | sean-k-mooney | so realistically i think we are going to have to follow up with third party ci and manual testing | |
| 16:24:32 | sean-k-mooney | and jsut test this with unit/functional test for now | |
| 16:25:24 | sean-k-mooney | we may want to consier if we shoudl alter the libvirt fixutre to emulate the observed behavior but im not sure if that is warrented or not | |
| 16:25:30 | gibi_ | a functional test for upstream is OK to me and we can manual test / whitbox test is downstream | |
| 16:25:35 | sean-k-mooney | i feel like that will be overly invasive | |
| 16:26:08 | gibi_ | sean-k-mooney: yeah, growing this logic into the fixture is too much, just mock the part of the fixture in the test case to simulate it oncec | |
| 16:27:44 | sean-k-mooney | ack we would have to modify https://github.com/openstack/nova/blob/master/nova/tests/fixtures/libvirt.py#L808-L822 | |
| 16:27:50 | sean-k-mooney | to be aware of the online state | |
| 16:28:12 | opendevreview | Stephen Finucane proposed openstack/nova master: hardware: Correct log https://review.opendev.org/c/openstack/nova/+/926223 | |
| 16:28:16 | sean-k-mooney | ... this https://github.com/openstack/nova/blob/master/nova/tests/fixtures/libvirt.py#L808-L847 | |
| 16:28:22 | stephenfin | sean-k-mooney: gibi: dead simple log fix there ^ | |
| 16:29:15 | sean-k-mooney | stephenfin: ah you also explaine what -2 is | |
| 16:29:30 | sean-k-mooney | for a second i tough you were just droping kb | |
| 16:29:49 | stephenfin | yeah, I've just copied what we do in the success case | |
| 16:30:03 | gibi | stephenfin: I have a bugreport for that https://bugs.launchpad.net/nova/+bug/2075959 | |
| 16:30:04 | sean-k-mooney | +1 while i wait of ci results | |
| 16:32:22 | opendevreview | Stephen Finucane proposed openstack/nova master: hardware: Correct log https://review.opendev.org/c/openstack/nova/+/926223 | |
| 16:32:32 | stephenfin | gibi: Sweet. Updated with a link | |
| 16:33:27 | gibi | stephenfin: +2. It feels like you read my mind and fixed my low hanging todo :) | |
| 16:35:05 | gibi | Thanks! | |
| 16:41:43 | opendevreview | Artom Lifshitz proposed openstack/nova master: POC: Work around lbvirt reporting offline cores as being on socket 0 https://review.opendev.org/c/openstack/nova/+/926218 | |
| 16:42:07 | artom | gibi, sean-k-mooney, so something like that ^^? A whole lot still missing, obviously, but the basic idea. | |
| 16:43:40 | artom | I wonder if we should be remembering what CPUs were offline? I feel like when we offline the dedicated ones later, that's enough? | |
| 16:45:02 | gibi | artom: the current logic should offline all the unused pcpus, so we can relay on that without remembering explicitly | |
| 16:45:24 | artom | OK, that keeps thing tidy | |
| 16:47:03 | gibi | artom: at a first read what you proposed seems OK to me. I need to look at it once more with a fresh set of eyes probably after we have some test results too | |
| 16:47:25 | artom | For sure :) Good to know that nothing obvious stands out | |
| 16:47:43 | artom | And there's a whitebox DNM patch running, so that should weed out any issues | |
| 16:53:52 | sean-k-mooney | artom: you proably should not be touching cores outside fo cpu_dedicated_set | |
| 16:54:39 | sean-k-mooney | we only powermanage cores in cpu_dedicated_set and its a hard error for cores in cpu_shared_set to be offline | |
| 16:55:03 | sean-k-mooney | but any cores out side of those two sets are valid for the admin to ower on/off as they desire | |
| 16:55:05 | artom | Yeah... I guess the only edge case where it might make sense is if the host has been trained, and nova-compute restarted to change the cpu_dedicated_set | |
| 16:55:08 | sean-k-mooney | as nova is not ment to use them | |
| 16:55:17 | artom | s/trained/drained/ | |
| 16:55:40 | artom | And so some of the offline cores could be outside the _current_ dedicated set, but part of the _previous_ dedicated set | |
| 16:55:48 | sean-k-mooney | well not you missing the fact that core outside fo cpu_shared_set adn cpu_dedicated_set can be used for other usecases | |
| 16:56:43 | sean-k-mooney | artom: so it would be incorrect for use to turn on any cores not in the currnt cpu_dedicated_set | |
| 16:57:11 | sean-k-mooney | as long as the offline cores are not added to cpu_shared_set we dont care if they were formally in cpu_dedicated_set | |
| 16:57:28 | artom | We do if they're offline and we read their socket as 0 :) | |
| 16:57:34 | sean-k-mooney | no we dont | |
| 16:57:40 | sean-k-mooney | because nova will never use them | |
| 16:58:23 | artom | Oh... yeah? NUMA affinity but with shared CPUs is not a thing? | |
| 16:58:43 | sean-k-mooney | it is but we have a sperat check for cpu_shared_set to ensure they are online | |
| 16:58:59 | sean-k-mooney | we dont allow you to have any offlien cores in cpu_shared_set | |
| 16:59:25 | artom | Ah ok, in that case it's even tidier, I don't need the new power_up_unconditionally() | |
| 17:00:34 | sean-k-mooney | right you can resue the existing _power_up although you might want to drop the _ or add a "public" wraper | |
| 17:03:28 | artom | Yeah | |
| 17:48:58 | opendevreview | Merged openstack/nova master: Functional test test_boot_reschedule_with_proper_pci_device_count https://review.opendev.org/c/openstack/nova/+/760354 | |
| 18:09:42 | opendevreview | Artom Lifshitz proposed openstack/nova master: POC: Work around lbvirt reporting offline cores as being on socket 0 https://review.opendev.org/c/openstack/nova/+/926218 | |
| 18:33:02 | opendevreview | sean mooney proposed openstack/nova master: Skip new image format tests https://review.opendev.org/c/openstack/nova/+/926215 | |
| #openstack-nova - 2024-08-14 | |||
| 07:16:18 | opendevreview | Tobias Urdin proposed openstack/nova master: Fix retried resize operation failing with FileNotFound https://review.opendev.org/c/openstack/nova/+/926251 | |
| 07:29:02 | opendevreview | Tobias Urdin proposed openstack/nova master: libvirt: Fix retried resize operation failing with FileNotFound https://review.opendev.org/c/openstack/nova/+/926251 | |
| 13:10:10 | opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Launch instances with SEV-ES memory encryption https://review.opendev.org/c/openstack/nova/+/926106 | |
| 15:39:40 | opendevreview | Viktor Křivák proposed openstack/nova master: Ensure claims to placement are freed after evacuate complete Improve resource_tracker delete_allocation_for_evacuated_instance to handle non-cached compute hosts Add test to prevent regression https://review.opendev.org/c/openstack/nova/+/926292 | |
| 16:07:09 | opendevreview | Markus Hentsch proposed openstack/nova master: Use os-brick for volume attachment passphrases https://review.opendev.org/c/openstack/nova/+/926326 | |
| 16:09:29 | opendevreview | Elod Illes proposed openstack/python-novaclient master: DNM: gate health test https://review.opendev.org/c/openstack/python-novaclient/+/926334 | |
| 17:02:49 | sean-k-mooney | gibi: i think responding to https://review.opendev.org/c/openstack/nova/+/710848 ate what little brain power i had for today but i tried to find where the save was happeing im just not sure which path it is | |
| 17:03:11 | sean-k-mooney | i think its the call to the conductor that trigering the save with the refeshed content | |
| 17:03:47 | sean-k-mooney | artom: fyi ^ | |
| 17:04:26 | sean-k-mooney | we could try moving the refresh to here https://review.opendev.org/c/openstack/nova/+/710848/6/nova/compute/manager.py#2371 | |
| 17:04:53 | sean-k-mooney | which i think would result in it being saved here instead https://review.opendev.org/c/openstack/nova/+/710848/6/nova/compute/manager.py#2260 | |
| 17:05:28 | sean-k-mooney | which would be better i think | |
| 17:05:42 | sean-k-mooney | can you try that locally | |
| 17:05:52 | artom | sean-k-mooney, I saw the comments but since I didn't write the actual patch I have to start from scratch to figure out what's going on | |
| 17:06:41 | sean-k-mooney | ok im going to finish in 5 mins but ill try to just move that locally and see if it passes your test case | |
| 17:37:02 | sean-k-mooney | ... we have a seperate sqlalchemy 2.0 issue | |
| 17:44:18 | opendevreview | sean mooney proposed openstack/nova master: test fixing this when allocation fails. https://review.opendev.org/c/openstack/nova/+/926350 | |
| 17:44:34 | sean-k-mooney | artom: ^ passes locally if i run the funciontal tests | |
| 17:44:40 | sean-k-mooney | but the serise also need a rebase | |
| 17:45:18 | sean-k-mooney | ... | |
| 17:45:30 | sean-k-mooney | or you know i could not do that on patchset 6... | |
| 17:45:49 | artom | Yeah, that relation chain is... special :) | |
| 17:48:20 | opendevreview | sean mooney proposed openstack/nova master: test fixing this when allocation fails. https://review.opendev.org/c/openstack/nova/+/926350 | |
| 17:49:41 | sean-k-mooney | ok that should be better | |
| 17:50:07 | sean-k-mooney | gibi: ^ i think that is a better fix | |
| 17:50:29 | sean-k-mooney | ... or it would be if it didnt fail | |
| 17:53:46 | sean-k-mooney | ok im not sure why that didnt work but it did not so im just not goign to look at that today | |
| 17:54:38 | opendevreview | Merged openstack/nova master: Skip new image format tests https://review.opendev.org/c/openstack/nova/+/926215 | |
| 17:55:04 | sean-k-mooney | oh actully i think i do no | |
| 17:55:14 | sean-k-mooney | that not where the excption is being raised | |
| 17:59:10 | opendevreview | sean mooney proposed openstack/nova master: test fixing this when allocation fails. https://review.opendev.org/c/openstack/nova/+/926350 | |
| 17:59:25 | sean-k-mooney | ^ actully passes locally | |
| 18:06:41 | sean-k-mooney | artom: gibi i have left some comments to explain why i think this also work and is cleaner but also why the patch i have is still not ideal | |