| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-11 | |||
| 18:01:37 | sean-k-mooney | that not how it was intended to work | |
| 18:02:13 | gibi | I have to drop now but we can discuss it tomorrow | |
| 18:03:05 | gibi | https://github.com/openstack/nova/blob/0d484ce37d86e989c8abdf57aec5e334f68206ef/nova/pci/request.py#L172 | |
| 18:03:19 | gibi | this is very nova appends more spec to the same alias | |
| 18:03:52 | gibi | s/very/where/ | |
| 18:04:02 | sean-k-mooney | ya i just found that | |
| 18:04:21 | sean-k-mooney | i can tell you tha tthat is very problematic | |
| 18:05:33 | sean-k-mooney | sriov live migraiton and the gpu live migration depend on that never happeinging | |
| 18:06:31 | sean-k-mooney | it looks like that was added before i started workign on nova in the very early days of pci support | |
| 18:06:59 | sean-k-mooney | but that breaks other assumtions made over the years | |
| 18:09:20 | sean-k-mooney | i guess it does not break sriov live migration because of the hotplug | |
| 18:09:42 | sean-k-mooney | the vgpu live migraiton does not expect this nor does pci in placment | |
| 18:12:30 | gibi | yeah pci in placement explicitly forbid this, just not with the proper response hence the bug and reproduce above | |
| 18:12:55 | sean-k-mooney | we need to add a check to prevent merging aliais with diffent resouce classes | |
| 18:13:11 | sean-k-mooney | like https://github.com/openstack/nova/blob/0d484ce37d86e989c8abdf57aec5e334f68206ef/nova/pci/request.py#L168-L170 | |
| 18:13:49 | gibi | we have a clear reject in the pci in placement code already https://github.com/openstack/nova/blob/0d484ce37d86e989c8abdf57aec5e334f68206ef/nova/objects/request_spec.py#L504-L528 | |
| 18:14:14 | gibi | just the exception is not translated properly hence the 500 return from nova-api | |
| 18:14:31 | sean-k-mooney | that failign for a diffent reason | |
| 18:14:38 | sean-k-mooney | the check is vlaid there | |
| 18:14:50 | sean-k-mooney | but this really should be a config error in other case | |
| 18:14:57 | sean-k-mooney | but ya lets pick this up again tomorrow | |
| 18:15:48 | sean-k-mooney | there are several thinkg our config parsing has allwoed over the years that i woudl condier invliad an dthis is right at the top of the list for me | |
| 18:16:19 | sean-k-mooney | specificly merging 2 alisie with a diffent product_id | |
| 18:17:21 | sean-k-mooney | merging none conflictign values is kind of ok | |
| 18:17:41 | sean-k-mooney | but the orign behavior is not in my opipion | |
| 18:48:07 | opendevreview | Dan Smith proposed openstack/nova master: Support "one-time-use" PCI devices https://review.opendev.org/c/openstack/nova/+/943816 | |
| 20:46:07 | opendevreview | mitya-eremeev-2 proposed openstack/nova master: Add more thorough handling of evacuated instances https://review.opendev.org/c/openstack/nova/+/944058 | |
| #openstack-nova - 2025-03-12 | |||
| 06:27:40 | opendevreview | Amit Uniyal proposed openstack/nova master: DNM: enable NFS backend job https://review.opendev.org/c/openstack/nova/+/944008 | |
| 06:51:50 | opendevreview | Rajesh Tailor proposed openstack/nova master: Add support for showing instance-action finish_time https://review.opendev.org/c/openstack/nova/+/928933 | |
| 08:05:26 | opendevreview | Michael Still proposed openstack/nova master: libvirt: Add extra spec for sound device. https://review.opendev.org/c/openstack/nova/+/926126 | |
| 08:05:27 | opendevreview | Michael Still proposed openstack/nova master: Protect older compute managers from sound model requests. https://review.opendev.org/c/openstack/nova/+/940770 | |
| 08:05:27 | opendevreview | Michael Still proposed openstack/nova master: libvirt: Add extra specs for USB redirection. https://review.opendev.org/c/openstack/nova/+/927354 | |
| 08:22:14 | gibi | sean-k-mooney: I have a feeling that the pool matching logic in stats.py also intentionally handles the multi spec case. See https://github.com/openstack/nova/blob/0d484ce37d86e989c8abdf57aec5e334f68206ef/nova/pci/utils.py#L75 it basically implements an OR between the multiple specs | |
| 08:22:49 | sean-k-mooney[m] | it did in the really early days | |
| 08:23:02 | sean-k-mooney[m] | but i tough twe removed this like 10+ years ago | |
| 08:23:21 | gibi | appreanlty we did not | |
| 08:23:51 | gibi | PCI in Placement never supported it though I just broke the error reporting leading to http 500 | |
| 08:23:55 | sean-k-mooney[m] | so the vfio-variant dirver live migration depend on 1:1 alias mappings | |
| 08:24:12 | sean-k-mooney[m] | at least when not using pci in placement | |
| 08:24:56 | gibi | sean-k-mooney: do you mean that the live migration will only work if the same exact device is on both sides and therefore having specs in OR relationship with different product id would not work? | |
| 08:25:15 | sean-k-mooney[m] | that is my understanding | |
| 08:25:20 | gibi | yeah I agree | |
| 08:25:44 | sean-k-mooney[m] | i dont se how it could work any other way when the deviece is directly exposed to the guest kernel as a vf | |
| 08:26:21 | sean-k-mooney[m] | i think we need to at least restict when oring can happen | |
| 08:26:48 | gibi | the unplug/plug live migration for neutron requested PCI probably works as that is not real live migration of the device, but the variant driver based will lead to very sad guest kernels | |
| 08:27:07 | sean-k-mooney[m] | yep | |
| 08:28:25 | sean-k-mooney[m] | so i know why they orginally did this which is they were trying to make differnt configurations of intels QAT device look the same\ | |
| 08:28:39 | sean-k-mooney[m] | and allow you to use ask for QAT | |
| 08:28:47 | sean-k-mooney[m] | without careing which one you got | |
| 08:29:29 | sean-k-mooney[m] | that only works if you never have to deal with live migration | |
| 08:29:59 | sean-k-mooney[m] | so i think we need to disable this when live_migratable is set to true or you use pci_in_placment | |
| 08:30:15 | sean-k-mooney[m] | either should result in a config error on startup | |
| 08:31:20 | sean-k-mooney[m] | we can allow it for non llive migratbale device or when not using PCI in placement for backward compatibality | |
| 08:58:23 | opendevreview | Rajesh Tailor proposed openstack/nova master: Add support for showing instance-action finish_time https://review.opendev.org/c/openstack/nova/+/928933 | |
| 09:12:16 | sean-k-mooney | it is day 2 or alt world where my fundemental core belife that "their should only be 1 7:30 in a day and it should happen after noon" is being chllanged by the concept of 7 am... | |
| 09:13:27 | sean-k-mooney | its currntly 09:13 which is still an hour before im useually awake | |
| 11:43:09 | opendevreview | ribaudr proposed openstack/nova master: FUP improve comment accuracy and variable naming for tag removal https://review.opendev.org/c/openstack/nova/+/943124 | |
| 11:43:10 | opendevreview | ribaudr proposed openstack/nova master: FUP Remove unnecessary PCI check https://review.opendev.org/c/openstack/nova/+/944105 | |
| 11:43:10 | opendevreview | ribaudr proposed openstack/nova master: FUP improve and add integration tests for PCI SR-IOV servers https://review.opendev.org/c/openstack/nova/+/944106 | |
| 11:52:58 | andrewbonney | sean-k-mooney: in case you have time today for another look, https://review.opendev.org/c/openstack/nova/+/919961 seems fine in CI | |
| 12:13:32 | sean-k-mooney | andrewbonney: ill review it propelry shortly. if you need to respin it might be nice to add a fixes release note | |
| 12:13:40 | sean-k-mooney | but thats not strictly required | |
| 12:13:56 | andrewbonney | thanks | |
| 12:13:58 | sean-k-mooney | so ya ill take a look shortly | |
| 12:23:17 | sean-k-mooney | andrewbonney: +2 form me | |
| 12:24:02 | sean-k-mooney | andrewbonney: this proably wont merge for RC1 but it can merge and be backported as normal after | |
| 12:24:27 | andrewbonney | sure, just trying to get it ticked off before I head off for extended leave | |
| 12:24:50 | sean-k-mooney | ack | |
| 12:26:07 | opendevreview | Merged openstack/nova master: Fix serial console for ironic https://review.opendev.org/c/openstack/nova/+/942575 | |
| 13:44:08 | gibi | folks, Why do we only use the move claim during live migration if the VM is NUMA aware? context https://github.com/openstack/nova/blob/f71a0a6204744b7774caf85c16f27964a4f851db/nova/compute/manager.py#L9057-L9066 | |
| 13:45:06 | gibi | based on my local testing that src_supports_numa_live_migration check there is only true if the VM is NUMA aware | |
| 13:45:43 | gibi | so it seems like we doesn't do any claim for the instance during live migration if the instance is not NUMA aware (except claiming things outside of the Claim object like PCI) | |
| 13:48:43 | Uggla | @artom, can you have a look at gibi's question above. | |
| 13:50:01 | artom | gibi, I don't remember :( | |
| 13:50:11 | artom | But I'm pretty sure it's not _just_ NUMA-aware VMs | |
| 13:50:17 | artom | Other code added later started using that too | |
| 13:50:26 | artom | VPMEM I think? | |
| 13:51:04 | artom | And src_supports_numa_live_migration was there to handle the rolling upgrade case at the time of implementation | |
| 13:51:18 | gibi | if you look at the code linked above there is a check that looks for src_supports_numa_live_migration in the migrate data, that condition is false for me locally for a non NUMA aware VM | |
| 13:51:22 | artom | We can _probably_ remove it now, with the usual caveats around RPC | |
| 13:51:30 | Uggla | @artom, look at https://review.opendev.org/c/openstack/nova/+/944105 you left a comment that may remember you things. | |
| 13:52:48 | artom | Oh yeah, there was the SRIOV live migration thing as well | |
| 13:52:53 | artom | It's handled by a separate code path | |
| 13:53:06 | Uggla | yep below in the manager | |
| 13:53:12 | gibi | yeah the PCI stuff claimed differently, but what about the rest of the resources? | |
| 13:53:31 | artom | You mean non-Placement resources? | |
| 13:55:14 | Uggla | @gibi this is claimed at scheduler step before no ? | |
| 13:55:58 | gibi | artom: yea | |
| 13:56:19 | gibi | all the normal things we do during an instance claim or during a move claim in other ops | |
| 13:56:59 | artom | I _think_ as live migration stuff got added they all stated using the MovaClaim? | |
| 13:57:00 | gibi | obviously what is not done today does not casue any highly visible issue so maybe those things does not need to be claimed during live migration, but it is very strange | |
| 13:57:50 | gibi | yeah using the MoveClaim would be logical during live migration, but my observation is that for a non NUMA aware VM the code does not use any claim | |
| 13:58:14 | gibi | (except the PCI one outside of the Claim obj) | |
| 13:59:05 | artom | I agree with you that it's strange | |
| 13:59:21 | artom | But it's been so long (Train!) that I've lost most context for the decisions that were made | |
| 13:59:49 | gibi | yeah I understand you don't have a context. I will try to charachterize the my feelings more | |
| 14:00:12 | artom | I talk about it here: https://specs.openstack.org/openstack/nova-specs/specs/train/implemented/numa-aware-live-migration.html#resource-claims | |
| 14:00:15 | gibi | best would be to find a way how this could break our resource tracking and the move from there | |
| 14:00:32 | gibi | I need to jump on a call, I will be back in 30 mins (hopefully) | |
| 14:01:14 | artom | OK | |