| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-07 | |||
| 16:09:45 | gibi | https://github.com/openstack/nova/blob/276685b3db6e8f2ad59c33bc254461c255700ff8/nova/compute/manager.py#L2615C1-L2617C40 | |
| 16:09:57 | gibi | we load allocation from placement before the instance_claim call | |
| 16:10:40 | gibi | so if you see the allocation in placement then the same data should be in the allocation dict in the claim call | |
| 16:10:42 | dansmith | yes, the allocations get made in placement, and the pci device has the rp_uuid in the extra_info, it's just never getting merged | |
| 16:11:52 | gibi | I suggest to trace how we read the allocations from placement before the instance_claim call | |
| 16:12:03 | dansmith | the device that we get back from the pci claim also doesn't get the new key in extra_info that I'm putting in device_spec | |
| 16:12:49 | dansmith | gibi: okay I thought those were passed down via rpc and not re-loaded in compute, but I could be wrong | |
| 16:13:39 | dansmith | but yeah I see that get_allocations_for_consumer() call.. it's almost like that's filtering for just our RP or something | |
| 16:14:18 | gibi | dansmith: for the devspec => dev data move you need to extend this https://github.com/openstack/nova/blob/276685b3db6e8f2ad59c33bc254461c255700ff8/nova/pci/devspec.py#L418 | |
| 16:15:14 | dansmith | ah, okay I added the key in the update, like this: https://review.opendev.org/c/openstack/nova/+/942144/4/nova/objects/pci_device.py | |
| 16:15:35 | dansmith | and I see it in the "final resource view" when n-cpu starts, but not when I get it back from the pci claim | |
| 16:15:45 | dansmith | I'll add it there too | |
| 16:16:26 | gibi | I need to drop. It was a long week. I will try to check back later today but more likely only on monday | |
| 16:16:45 | dansmith | no prob, thanks | |
| 16:19:05 | dansmith | yep, that did it | |
| 16:23:45 | gibi | ohh if you see the allocation in placement but not during claim then it must be the allocation healing that creates the allocation in placement and not the scheduler. pci_in_placement flag needs to be true in the config not just report_in_placement | |
| 16:32:59 | dansmith | hmm | |
| 17:15:56 | opendevreview | Merged openstack/python-novaclient master: Update master for stable/2025.1 https://review.opendev.org/c/openstack/python-novaclient/+/943764 | |
| 17:22:51 | dansmith | gibi: aha, yep, that does it, thanks | |
| 17:36:37 | opendevreview | Merged openstack/nova stable/2024.2: Reproduce bug/2097359 https://review.opendev.org/c/openstack/nova/+/942085 | |
| 18:09:57 | opendevreview | Merged openstack/nova stable/2024.2: Update InstanceNUMACell version after data migration https://review.opendev.org/c/openstack/nova/+/942086 | |
| 18:11:46 | opendevreview | sean mooney proposed openstack/nova master: only show standard image properties in server show. https://review.opendev.org/c/openstack/nova/+/942413 | |
| 18:13:20 | opendevreview | Merged openstack/nova stable/2024.2: Update InstanceNUMACell version in more cases https://review.opendev.org/c/openstack/nova/+/942087 | |
| 18:16:05 | opendevreview | sean mooney proposed openstack/nova master: only show standard image properties in server show. https://review.opendev.org/c/openstack/nova/+/942413 | |
| 18:17:23 | sean-k-mooney | gibi: ^ this is a less dramtic rewite tha tavoid round triping the data via the object when we are composing the responce | |
| 18:17:45 | sean-k-mooney | gibi: so it will keep the keys and values matching what we have in the system_metadta_table | |
| 18:18:38 | sean-k-mooney | of couse if we normalised the values before we saved them there is noting we can do at this point | |
| 18:22:03 | opendevreview | sean mooney proposed openstack/nova master: only show standard image properties in server show. https://review.opendev.org/c/openstack/nova/+/942413 | |
| 18:22:17 | sean-k-mooney | fixed pep8 issue | |
| 18:22:40 | sean-k-mooney | am im going to finish now ish and i assume you already have so no rush with ^ | |
| 18:30:38 | opendevreview | Dan Smith proposed openstack/nova master: WIP: Reserve OTU devices https://review.opendev.org/c/openstack/nova/+/943816 | |
| 18:30:45 | dansmith | shockingly, that seems to work ^ | |
| 18:31:29 | sean-k-mooney | that looks shorter then im expecting | |
| 18:31:59 | dansmith | well, I wasn't thinking it would be huge given it's technically a simple thing to do, | |
| 18:32:02 | sean-k-mooney | oh | |
| 18:32:03 | dansmith | but yeah | |
| 18:32:27 | sean-k-mooney | your doing it in update_provider_tree | |
| 18:32:42 | dansmith | yeah so I'm adding some review comments about why | |
| 18:33:00 | sean-k-mooney | ok | |
| 18:33:14 | sean-k-mooney | we just make sure that that is not only working because of the perodic | |
| 18:33:53 | sean-k-mooney | or we would have a gap where if we spawned and delete right after each other it might not run | |
| 18:34:15 | dansmith | it's not, but obviously tests | |
| 20:17:50 | opendevreview | Merged openstack/nova master: Update manager to allow vfio pci device live migration https://review.opendev.org/c/openstack/nova/+/942145 | |
| #openstack-nova - 2025-03-08 | |||
| 01:23:11 | opendevreview | melanie witt proposed openstack/nova master: unified limits: discover service ID and region ID https://review.opendev.org/c/openstack/nova/+/943507 | |
| #openstack-nova - 2025-03-10 | |||
| 05:05:05 | opendevreview | Callum Dickinson proposed openstack/nova master: Add image meta to libvirt XML metadata https://review.opendev.org/c/openstack/nova/+/942766 | |
| 11:15:38 | sean-k-mooney | melwitt: would you mind readding your +2 adn approving https://review.opendev.org/c/openstack/nova/+/939411/10 the only differnce since you last reviewd was the addtion of a release note | |
| 11:16:29 | sean-k-mooney | bauzas: this ^ was a regression breaking rebuild last cycle so we will have to backport it anyway but still nice ot have before rc1 | |
| 12:13:37 | opendevreview | Merged openstack/nova master: libvirt: fix maxphysaddr passthrough dom parsing https://review.opendev.org/c/openstack/nova/+/942371 | |
| 12:16:52 | opendevreview | Yaguang Tang proposed openstack/nova stable/2024.2: Fix device type when booting from ISO image https://review.opendev.org/c/openstack/nova/+/943924 | |
| 13:19:10 | bauzas | sean-k-mooney: ack, stephenfin already sent it to the gate fwiw | |
| 13:19:32 | bauzas | sean-k-mooney: I started a new etherpad for rc tracking https://etherpad.opendev.org/p/nova-epoxy-rc-potential | |
| 13:20:11 | bauzas | sean-k-mooney: you can as usual add some changes if you think those are important before RC1 in the right section 'bugfixes requiring attention before RC1' | |
| 13:32:52 | sean-k-mooney | bauzas: ack, this is/was a latent issue form the ironic leeasee serise so not super urgent. i.e. if it missed rc 1 it woul dnot be a new regression intoduced in epoxy, just nice to have sooner rtahter then later | |
| 13:33:00 | sean-k-mooney | stephenfin: thanks for reviewing :) | |
| 13:33:39 | sean-k-mooney | bauzas: as it stand im not really aware of anyting super urgent for RC1 outside of the mechanical patches we do every release | |
| 13:46:39 | opendevreview | Merged openstack/nova master: api: project/tenant and user IDs are not UUIDs https://review.opendev.org/c/openstack/nova/+/940369 | |
| 13:46:45 | opendevreview | Merged openstack/nova master: api: Address TODO in microversion v2.99 https://review.opendev.org/c/openstack/nova/+/943253 | |
| 13:54:28 | opendevreview | Yaguang Tang proposed openstack/nova stable/2024.2: Handle iso+gpt detections https://review.opendev.org/c/openstack/nova/+/943940 | |
| 14:09:23 | opendevreview | Merged openstack/nova master: Fix parameter order in add_instance_info_to_node https://review.opendev.org/c/openstack/nova/+/939411 | |
| 14:10:00 | gibi | dansmith: o/ regarding https://review.opendev.org/c/openstack/nova/+/943816/1/nova/compute/pci_placement_translator.py | |
| 14:11:02 | gibi | that palce exists in the pci in placement code to implement missing allocation healing and not intended to remain there for ever. It is more like a transitional code to support upgrade and gradual enablement of the featureh | |
| 14:11:28 | dansmith | orly, okay | |
| 14:11:35 | gibi | in general pci allocation should only happen during scheduling in the scheduler / conductor. | |
| 14:12:01 | gibi | this allocation healing is like the old healing periodic when we introduced placement | |
| 14:12:09 | gibi | 5+ years ago | |
| 14:12:27 | dansmith | right, well, this is not really allocation and I think the consensus (me, sean, sylvain) is that the reservation should be in the compute node | |
| 14:12:28 | gibi | I think you ended up looking at this code as friday in your env the allocation happened in this code | |
| 14:12:42 | gibi | yeah I know that your code is not allocation | |
| 14:12:51 | dansmith | but yeah I traced the placement update stuff looking for a good place to do it and this looked perfect :) | |
| 14:12:54 | gibi | but the code path seem to only exists for allocation | |
| 14:12:56 | gibi | so far | |
| 14:13:06 | dansmith | I had originally written my own inventory update in the RT | |
| 14:13:45 | dansmith | so I can go back to that approach, which it sounds like would be better so this healing can be ripped out later yeah? | |
| 14:13:57 | gibi | I might be mistake. I have to dig a bit deeper where we do the actual inventory reporting of PCI in placement | |
| 14:14:45 | gibi | https://github.com/openstack/nova/blob/5f3133efc04dde15d64679e1e7564d2e575549db/nova/compute/resource_tracker.py#L1325-L1342 | |
| 14:14:53 | dansmith | okay, tbh, this looked like it was intended to be permanent.. it seems related to making sure placement inventory gets healed if our resources ever changed | |
| 14:14:54 | gibi | I'm reacting based on this comment | |
| 14:15:49 | dansmith | okay, so the rest of update_to_placement() seems to intent to stay there long-term right? | |
| 14:16:02 | dansmith | you're just thinking the pci part is not permanent? | |
| 14:16:03 | gibi | https://github.com/openstack/nova/blob/5f3133efc04dde15d64679e1e7564d2e575549db/nova/compute/pci_placement_translator.py#L554-L556 OK this says both inventory and allocation handling so maybe this is both at the moment and then it means it will remain the same | |
| 14:16:41 | dansmith | right, it's doing both | |
| 14:17:28 | dansmith | gibi: this is the "don't do it here because..." feedback I wanted before going further, but if you need to dig in deeper, feel free to punt until you have time to do proper thinking | |
| 14:20:10 | gibi | in short, there is allocation healing in that code path which will go away. There is inventory generation there which remains there. So extending the inventory generation with this feature might be OK. | |
| 14:20:34 | gibi | so maybe it is OK | |
| 14:20:47 | dansmith | okay | |
| 14:20:49 | gibi | independenty I'm not sure how racy this is with the pci_claim | |
| 14:21:06 | dansmith | the pci claim on the compute node happens before this | |
| 14:21:07 | gibi | if this happens independently from the pci_claim in the periodic then we have a race window | |
| 14:21:38 | dansmith | the benefit here is that if you mark a device as OTU after it has already been allocated to an instance, it will be "healed" to reserved state | |
| 14:21:44 | dansmith | s/the/a benefit/ | |
| 14:21:47 | gibi | I see | |
| 14:21:52 | dansmith | I'm not sure what race you mean, | |
| 14:22:12 | dansmith | there's a small window where we could fail before we do the reserve, but if that happens, the instance will not boot | |
| 14:22:17 | dansmith | so it's safe and the window is very small | |
| 14:22:25 | dansmith | I don't think there's a race | |
| 14:22:45 | dansmith | this happens synchronously at the end of the instance_claim() method | |
| 14:22:52 | dansmith | (*and* in the periodic) | |
| 14:22:57 | gibi | I mean the pci claim claims an OTU device and returns to the compute manager spawn a VM and a periodic update_available_resouces runs after a while and sees the allocated OTU device and reserve it | |
| 14:23:10 | dansmith | no, | |
| 14:23:13 | dansmith | it happens synchronously | |