Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-03-11
17:14:02 dansmith I assume because there could be multiple things matched that have different characteristics
17:14:27 dansmith so you think we need to examine all the matching ones so we can abort if your spec matches a VF?
17:16:09 gibi auch. Yeah you are right, at that point we only have the dev_spec that can be wildcard and match to multiple devices. I'm not sure how the remote managed check below works then as that does some physical function check. Anyhow that means we cannot validate that input in at the device_spec parsing code and need to add it later in the pci_placemenet_translator
17:16:23 dansmith gibi: it just punts for patterns
17:16:36 gibi OK
17:16:40 dansmith gibi: well, my translator code already skips VFs
17:16:42 gibi not nice but meh
17:16:58 dansmith meaning, it won't reserve for VFs.. I could log something there though
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?

Earlier   Later