| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-10 | |||
| 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 | |
| 14:23:20 | gibi | if this happens in the same code path as the claim then cool | |
| 14:23:28 | dansmith | this doesn't depend on the periodic running before it's reserved | |
| 14:23:32 | gibi | OK | |
| 14:23:40 | gibi | that was basically my second question | |
| 14:23:44 | dansmith | runs right here in instance_claim() https://github.com/openstack/nova/blob/276685b3db6e8f2ad59c33bc254461c255700ff8/nova/compute/resource_tracker.py#L215 | |
| 14:24:23 | dansmith | that _update() all calls the _update_to_placement() as its first thing, which your first link above | |
| 14:25:47 | sean-k-mooney | dansmith from my perspective anything after self.pci_tracker.claim_instance in that function is ok, that is the earise that doing the reserved=total make sense | |
| 14:25:49 | dansmith | if that all seems reasonable now, I can document all the above details in the spec more thoroughly so that review is easier | |
| 14:26:01 | dansmith | sean-k-mooney: cool | |
| 14:26:12 | sean-k-mooney | althought there are ways we can fail later so it could be later | |
| 14:26:36 | gibi | dansmith: OK I see it now, no race | |
| 14:26:45 | dansmith | sean-k-mooney: I think there's always going to be some compromise, but it seems like when we're doing the pci accounting is the right balance to me | |
| 14:30:00 | sean-k-mooney | dansmith: so what your propsoing i think will work for spawn and unshleve and evacuate | |
| 14:30:29 | sean-k-mooney | i suspect self._update is reusted for cold migration/resize | |
| 14:30:46 | sean-k-mooney | ya it is here https://github.com/openstack/nova/blob/276685b3db6e8f2ad59c33bc254461c255700ff8/nova/compute/resource_tracker.py#L378 | |
| 14:31:08 | sean-k-mooney | so i think that is fine too | |
| 14:31:24 | sean-k-mooney | live migration is out os scope for this this cycle? | |
| 14:33:19 | sean-k-mooney | live migration does use a move claim but the claiming of the device in the nova db happens a littel earlier | |
| 14:34:06 | sean-k-mooney | i think its fine to still do the reserved=total in the same place, im just asking for the scope fo testing/support | |
| 14:34:53 | dansmith | I'll track down the live case, but yeah the cold move case is also good | |
| 14:35:32 | sean-k-mooney | so for cold migration i think we just need to document that the device is burned once we have got to verify_resize | |
| 14:35:36 | dansmith | I think I'm fine with live being out of scope here, especially since we can mark a device as live_migratable=false now, but there's no reason this shouldn't work the same I think, even if we have to do it in a slightly different place on the dest | |
| 14:35:51 | dansmith | sean-k-mooney: yeah once you've touched both devices on either end, both are burned :) | |
| 14:36:21 | sean-k-mooney | exactly, some opt might assuem that happens at confirm | |
| 14:36:25 | sean-k-mooney | so we just need to document that | |
| 14:36:38 | opendevreview | Merged openstack/nova master: Update conductor and filters allowing migration with SR-IOV devices https://review.opendev.org/c/openstack/nova/+/942146 | |
| 14:37:46 | dansmith | sure | |
| 14:40:20 | gibi | dansmith: yeah by default flavor based PCI devices are not live migratable, only exception is via the recently added live_migratable flag in the dev_spec and alias but we don't expect that will be set for OTU devices | |
| 14:40:46 | dansmith | oh, okay well, that's even easier I guess | |
| 14:41:00 | sean-k-mooney | ya the default helps | |
| 14:41:06 | gibi | only neutron based PCI devs are live migrated by default via unplug and plug | |
| 14:41:18 | sean-k-mooney | right but we dont support them in placment yet | |
| 14:41:22 | gibi | yes | |
| 14:41:25 | sean-k-mooney | so we can also ignor them | |
| 14:41:29 | sean-k-mooney | we could add a check | |
| 14:41:29 | gibi | yes | |
| 14:41:39 | sean-k-mooney | that you dont set otu and physical_network | |
| 14:41:56 | sean-k-mooney | since we know thats invalid today | |
| 14:42:09 | sean-k-mooney | but its not strictly required | |
| 14:42:11 | opendevreview | Gaudenz Steinlin proposed openstack/nova master: Make SSH compression for remote copy configurable https://review.opendev.org/c/openstack/nova/+/943377 | |
| 14:44:41 | dansmith | Okay well, I'll add some notes to the spec for this to cover the above details | |
| 15:06:03 | opendevreview | Sylvain Bauza proposed openstack/nova master: doc: mark the maximum microversion for 2025.1 Epoxy https://review.opendev.org/c/openstack/nova/+/943948 | |
| 15:10:22 | opendevreview | Dan Smith proposed openstack/nova-specs master: Add one-time-use-devices spec https://review.opendev.org/c/openstack/nova-specs/+/943486 | |
| 15:11:37 | gibi | dansmith: left couple of comments but I think the direction looks good | |
| 15:11:49 | dansmith | gibi: thanks! | |
| 15:12:34 | gibi | the only think I'm a bit hesitant about it is the long term effect of the decision that we go with placement to solve this while and therefore we are forced not to support OTU VFs. | |
| 15:13:28 | dansmith | gibi: tbh, I'm not really sure when VFs would need to use this, as I suspect any virt-knowing device would be sensitive to exposure between uses (perhaps not) | |
| 15:14:07 | dansmith | that said, I think that from the operator point of view, it doesn't necessarily matter that it's happening in placement (other than the steps to unreserve after cleaning I guess).. but the devspec wouldn't need to change if we started accounting for the one-time-use in, say, PCI tracker | |
| 15:14:20 | gibi | yeah that is a valid point. This is why I'm not against going with the placement route, just hesitant that our udnerstanding of SRIOV VFs are correct | |
| 15:14:53 | gibi | dansmith: yeah the unreserve is the problematic step if we go with the PCI tracker route | |
| 15:14:57 | dansmith | right | |
| 15:15:01 | gibi | nova has no API for that | |
| 15:15:16 | dansmith | an SRIOV NIC is safe for use without cleaning, because you can't update firmware I think | |