| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-11 | |||
| 17:17:43 | sean-k-mooney | we determin if its a phsyica function by looking at the libvirt nodedev xml when generiging the set of pci dicts and then we filter those by the devspec regexes ectra | |
| 17:17:45 | gibi | dansmith: yeah I think we can check in the translator that if the OTU is requested for a VF as that is clearly invalid. Currently your code ignoring it but that is missleading as the user expects that the OTU marked device will be reseved but it won't be | |
| 17:18:06 | gibi | so I suggest to raise there and let the deployer know that what is requested not supported | |
| 17:18:09 | dansmith | gibi: you mean the deployer.. the user can't see any of this | |
| 17:18:14 | dansmith | but okay | |
| 17:18:18 | gibi | yeah the deployer | |
| 17:18:19 | gibi | sorry | |
| 17:18:55 | gibi | I think we already have late input validation for the dependend device handling in the translator (when both the PF and its VFs are matched) | |
| 17:19:03 | dansmith | okay | |
| 17:19:43 | gibi | https://github.com/openstack/nova/blob/0d484ce37d86e989c8abdf57aec5e334f68206ef/nova/compute/pci_placement_translator.py#L146-L151 | |
| 17:21:08 | dansmith | gibi: okay so that's where we try to add children, and explode if we already have the parent? so I can just check OTU there and refuse similarly I guess? | |
| 17:21:50 | sean-k-mooney | add child is passed a pcidevce object so it could have also checked dev.dev_type | |
| 17:22:34 | dansmith | but no need to even check because if we're calling add_child and one_time_use=true, then fail right? | |
| 17:24:06 | sean-k-mooney | i mean you can fail there that pretty late | |
| 17:24:39 | dansmith | right but, see above.. when we're checking the devspec early, we don't have enough into to make a determination for patterns | |
| 17:25:02 | sean-k-mooney | dansmith: the devspec cant be used to tell if its a pf | |
| 17:25:15 | dansmith | ...right, I understand | |
| 17:25:23 | sean-k-mooney | but the device we get back form the driver alredy know if its a pf or not | |
| 17:26:44 | sean-k-mooney | i think you cna check in here https://github.com/openstack/nova/blob/0d484ce37d86e989c8abdf57aec5e334f68206ef/nova/pci/manager.py#L111 | |
| 17:28:28 | gibi | yeah you can check it in the pci manager as well. There we also have the both the dev and it's matching dev_spec | |
| 17:28:50 | sean-k-mooney | right roughtly here https://github.com/openstack/nova/blob/0d484ce37d86e989c8abdf57aec5e334f68206ef/nova/pci/manager.py#L135 | |
| 17:29:37 | sean-k-mooney | dev is the list of dicts form the virt driver and pci_dev_spec is the matched spec | |
| 17:29:55 | dansmith | okay | |
| 17:30:15 | sean-k-mooney | so here the dict has been encreced by the tags form the devspec https://github.com/openstack/nova/blob/0d484ce37d86e989c8abdf57aec5e334f68206ef/nova/pci/manager.py#L140 | |
| 17:30:30 | sean-k-mooney | and you can add a check before we add it to the devices list | |
| 17:31:30 | sean-k-mooney | you will need to add onetime use here https://github.com/openstack/nova/blob/master/nova/pci/devspec.py#L419 as well | |
| 17:31:41 | gibi | I would do it in the translator where you filter the VFs out that is the closest to the actual logic assuming no VFs are OTU | |
| 17:32:17 | sean-k-mooney | the translator is not the only place we do filtering but that an option yes | |
| 17:32:36 | gibi | the OTU feature hard depends on the translator | |
| 17:32:49 | gibi | it acutally implementes the OTU reservation | |
| 17:33:21 | sean-k-mooney | ya which is weiered to me because i expect this to really be done in the pci manager | |
| 17:33:25 | gibi | as we cannot put it early in the dev_spec parsing, the second best thing for me would be to put the check to the same place where the reserveation logic is | |
| 17:33:29 | sean-k-mooney | but i get why its doen that way | |
| 17:33:48 | dansmith | gibi: yeah that feels a lot cleaner to me, just because it's so contextually-relevant there | |
| 17:33:57 | gibi | if we implement OTU with the PCI tracker then I would agree with you, but we are going with implementing it in Placement | |
| 17:34:04 | gibi | dansmith: yepp | |
| 17:34:40 | sean-k-mooney | i kind of hate that we have any pci logic out soid eof the pci and hardware.py modules but that ship has sailed... | |
| 17:38:11 | gibi | sean-k-mooney: it is not hard to move the pci_placement_translator under the pci module | |
| 17:39:03 | opendevreview | Balazs Gibizer proposed openstack/nova master: Add functional reproducer for bug 2102038 https://review.opendev.org/c/openstack/nova/+/944061 | |
| 17:39:04 | opendevreview | Balazs Gibizer proposed openstack/nova master: Multiple spec per PCI alias limitation https://review.opendev.org/c/openstack/nova/+/944062 | |
| 17:55:40 | sean-k-mooney | ^ is not valid config | |
| 17:56:54 | sean-k-mooney | gibi: we can have two differntly named aliases refer to the same vedor_id and product_id | |
| 17:57:11 | sean-k-mooney | but you cant have 2 alises with the same name and differnt contett | |
| 18:00:21 | gibi | sean-k-mooney: you can :) | |
| 18:00:32 | gibi | not with PCI in Placement but without it | |
| 18:00:46 | sean-k-mooney | it was a bug if it was ever there | |
| 18:00:51 | sean-k-mooney | it was never inteded to be supproted | |
| 18:00:52 | gibi | the spec in the alias is a list | |
| 18:00:56 | sean-k-mooney | yes | |
| 18:01:03 | sean-k-mooney | but that is so you can define multiple | |
| 18:01:21 | gibi | nope a single named alias can have a list of attached spec dicts | |
| 18:01:24 | sean-k-mooney | it was always ment to map to exactly one value | |
| 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 | |