| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-24 | |||
| 10:40:52 | sean-k-mooney | ack | |
| 10:40:57 | sean-k-mooney | that is what i was wondering | |
| 10:41:10 | sean-k-mooney | i was hoping that we woudl not remove RPs if they had allcoations | |
| 10:41:13 | gibi | the commit message has an edge case described when nova will fail to start though https://review.opendev.org/c/openstack/nova/+/852397/5//COMMIT_MSG | |
| 10:41:54 | sean-k-mooney | this else branch https://review.opendev.org/c/openstack/nova/+/846470/15/nova/compute/pci_placement_translator.py#320 | |
| 10:42:02 | gibi | but other than that the PCI RP will be kept until the PCIDevice is in the nova DB | |
| 10:42:18 | sean-k-mooney | is for the case where its in the pci tracker with a claim agaisnt it but removed form the config right | |
| 10:42:54 | gibi | at that point in the series we have no allocations against PCI RPs. so we just ignore the device without spec | |
| 10:43:14 | sean-k-mooney | we ignore removign it | |
| 10:43:35 | sean-k-mooney | so the rp stays there whiel the device is not deleted in the pci tracker | |
| 10:44:01 | sean-k-mooney | i guess it will just stay there | |
| 10:44:09 | sean-k-mooney | ok ill review what you have later anyway | |
| 10:44:20 | sean-k-mooney | since you have a patch for the case i was really worried about | |
| 10:45:27 | sean-k-mooney | but ya until we have allocations it really does not matter anyway | |
| 10:45:32 | gibi | later in the series we have the patch for reconf with allocations | |
| 10:45:36 | gibi | https://review.opendev.org/c/openstack/nova/+/852397/5/nova/compute/pci_placement_translator.py#419 | |
| 10:46:30 | sean-k-mooney | perfect you even tell them how to fix it in the warning | |
| 10:48:06 | gibi | and here is a test case for it https://review.opendev.org/c/openstack/nova/+/852397/5/nova/tests/functional/libvirt/test_pci_in_placement.py#760 | |
| 10:48:39 | gibi | below that there is the test case for reconfiguring by removing a PF but adding its VFs. That is a hard stop | |
| 10:49:46 | gibi | that would lead to a dependent device config which we explicitly not support | |
| 10:56:47 | sean-k-mooney | can we add a test for creating an instnace then removing the device form the config and restarting the agent | |
| 10:57:06 | sean-k-mooney | that shoudl allow the agent to start but complain loudly if we want to keep the old behaivr | |
| 10:57:15 | sean-k-mooney | or fail to start | |
| 10:57:30 | sean-k-mooney | but in either case the RP should not be updated and the allcoation should be kept in placment | |
| 11:01:33 | gibi | yes here is the test case that warns https://review.opendev.org/c/openstack/nova/+/852397/5/nova/tests/functional/libvirt/test_pci_in_placement.py#760 | |
| 11:01:46 | gibi | and here is the case where nova refuse to start | |
| 11:01:47 | gibi | https://review.opendev.org/c/openstack/nova/+/852397/5/nova/tests/functional/libvirt/test_pci_in_placement.py#802 | |
| 11:01:57 | gibi | as the reconf would create dependent device situation | |
| 11:02:36 | gibi | a simple config removal only cause a warning but RP and allocation is kept | |
| 11:02:58 | gibi | a config change that removes an allocated PF and configures its VF will be a hard stop | |
| 11:03:26 | sean-k-mooney | ack that sound like the semantics we want to have | |
| 11:03:46 | sean-k-mooney | the warning behavior is the same as we had today | |
| 11:03:57 | sean-k-mooney | and you are also catching a failure mode we dont prevent today | |
| 11:04:07 | sean-k-mooney | well it will be prevented differntrly | |
| 11:04:18 | sean-k-mooney | the current case if you add the pf and remove the vfs | |
| 11:04:24 | sean-k-mooney | i think will result in the pf goign to unaviaable | |
| 11:04:36 | sean-k-mooney | since the child device is claimed | |
| 11:04:52 | sean-k-mooney | so its a slight behavior delta but not nessisarly a bad one | |
| 11:05:21 | gibi | it is a delta becase we explicitly not support dependent device config with PCI in placement and that will be documented :) | |
| 11:05:59 | gibi | this is the delta https://review.opendev.org/c/openstack/nova/+/846435/19/doc/source/admin/pci-passthrough.rst#370 | |
| 11:06:34 | gibi | I probably need to mention the extra case in the doc about reconf | |
| 11:07:04 | gibi | ohh I did https://review.opendev.org/c/openstack/nova/+/852397/5/doc/source/admin/pci-passthrough.rst#407 | |
| 12:15:01 | gmann | gibi: dansmith_ : re on RBAC, I am +2 on dansmith_ patch https://review.opendev.org/c/openstack/nova/+/848021/2 | |
| 12:15:31 | gmann | but little confused with the error in my patch, https://review.opendev.org/c/openstack/nova/+/849209/4 | |
| 12:20:47 | opendevreview | Merged openstack/nova master: Add source dev parsing for vdpa interfaces https://review.opendev.org/c/openstack/nova/+/841016 | |
| 12:20:54 | opendevreview | Merged openstack/nova master: Fix suspend for non hostdev sriov ports https://review.opendev.org/c/openstack/nova/+/841017 | |
| 12:21:33 | sean-k-mooney | yay | |
| 12:22:00 | sean-k-mooney | stephenfin: when you have time could you review the last vdpa patch https://review.opendev.org/c/openstack/nova/+/853704/7 | |
| 13:06:17 | Uggla | sean-k-mooney, gibi is https://review.opendev.org/c/openstack/nova/+/854355 ok for you. Or you absolutely don't want to change NovaPersistentObject ? I think it is clearer in the way, but I don't mind changing if you think it is better. | |
| 13:06:39 | gibi | gmann: left a note about the test failure in https://review.opendev.org/c/openstack/nova/+/849209/4 | |
| 13:09:59 | gmann | gibi: that is what I am not getting why it is failing as there is no reference of PROJECT_ADMIN. locally all test passing | |
| 13:11:15 | gmann | PS3 passed successfully which was rebased with no change in that patch | |
| 13:12:18 | opendevreview | Ghanshyam proposed openstack/nova master: Remove system scope from all APIs https://review.opendev.org/c/openstack/nova/+/848021 | |
| 13:12:27 | opendevreview | Ghanshyam proposed openstack/nova master: Keep legacy admin behaviour in new RBAC https://review.opendev.org/c/openstack/nova/+/849209 | |
| 13:12:36 | gmann | rebasing on master, let's see | |
| 13:15:04 | gibi | Uggla: I made some notes in https://review.opendev.org/c/openstack/nova/+/854355 . I can accept the direction you are proposing. | |
| 13:16:23 | Uggla | gibi, thx I'm gonna look at the comments. | |
| 13:16:42 | gibi | gmann: that feels like some weird rebase issue then in zuul | |
| 13:17:09 | gmann | gibi: yeah seems so. it should pass now, let's see | |
| 13:17:33 | gibi | yeah we will see | |
| 13:47:49 | sean-k-mooney | Uggla: i have not done a full review but that is proably workable providere there is not api or db impact | |
| 13:48:32 | opendevreview | Rico Lin proposed openstack/nova master: Add locked_memory extra spec and image property https://review.opendev.org/c/openstack/nova/+/778347 | |
| 13:48:33 | opendevreview | Rico Lin proposed openstack/nova master: libvirt: Add vIOMMU device to guest https://review.opendev.org/c/openstack/nova/+/830646 | |
| 13:48:33 | opendevreview | Rico Lin proposed openstack/nova master: Add traits for viommu model https://review.opendev.org/c/openstack/nova/+/844507 | |
| 13:50:27 | Uggla | sean-k-mooney, yep I just want the minimal change. Mostly to avoid next contributors "confusion" creating new objects. | |
| 14:58:21 | auniyal__ | Hi All | |
| 14:58:26 | auniyal__ | please review these | |
| 14:58:31 | auniyal__ | https://review.opendev.org/c/openstack/nova/+/852171 | |
| 14:59:23 | auniyal__ | https://review.opendev.org/c/openstack/nova/+/853811 | |
| 14:59:35 | auniyal__ | https://review.opendev.org/c/openstack/nova/+/853812 | |
| 16:33:29 | opendevreview | Ghanshyam proposed openstack/nova master: Keep legacy admin behaviour in new RBAC https://review.opendev.org/c/openstack/nova/+/849209 | |
| 16:35:21 | gibi | sean-k-mooney: am I correct that in nova there is no implemented preference between consuming a PF (for a direct-phyiscal port) that has VFs over consuming a PF that has no VFs? | |
| 16:38:06 | gibi | there is a single func test that assumes that we consume the PF without the VFs instead of the PF with VFs. But I think it was a simple coincidence that nova actually consumed that | |
| 16:39:04 | gibi | with the pools splitted the order of the pools changed and that test fails now. | |
| 16:39:29 | gibi | I will try to trick nova to change the order back... | |
| 16:47:59 | gmann | gibi: 849209 should pass now, actually unshelve_to_host policy merged in between and RBAC patches were not rebases to master. after rebasing to master the unshelve_to_host new policy shows up in gerrit display which needs to be corrected in RBAC changes. | |
| 16:48:07 | gmann | gibi: I fixed that and it in gate now | |
| 16:49:25 | gibi | gmann: ahh, true, that explains it. I always forget that zuul rebases a patch even in the check queue | |
| 16:49:55 | gmann | yeah | |
| 17:20:05 | gmann | gibi: dansmith: unit/functional test pass (means things are ok now) on RBAC patch, please review https://review.opendev.org/c/openstack/nova/+/849209 | |
| 17:20:23 | gmann | thanks sean-k-mooney for review, will check your query/feedback tomorrow | |
| 17:23:14 | gibi | dansmith: if you can look at ^^ that would be awesome as you probably have the context. Otherwise I can try to look at it tomorrow morning with fresh brain | |
| 17:23:43 | dansmith | gibi: I'm on my way to an appointment now.. so maybe later | |
| 17:23:53 | gibi | dansmith: ack | |
| 17:40:39 | opendevreview | Balazs Gibizer proposed openstack/nova master: Heal missing simple PCI allocation in the resource tracker https://review.opendev.org/c/openstack/nova/+/851359 | |
| 17:40:40 | opendevreview | Balazs Gibizer proposed openstack/nova master: Heal PCI allocation during resize https://review.opendev.org/c/openstack/nova/+/852396 | |
| 17:40:40 | opendevreview | Balazs Gibizer proposed openstack/nova master: Handle PCI dev reconf with allocations https://review.opendev.org/c/openstack/nova/+/852397 | |
| 17:40:41 | opendevreview | Balazs Gibizer proposed openstack/nova master: Allow enabling PCI tracking in Placement https://review.opendev.org/c/openstack/nova/+/850468 | |
| 17:40:41 | opendevreview | Balazs Gibizer proposed openstack/nova master: Generate request_id for Flavor based InstancePCIRequest https://review.opendev.org/c/openstack/nova/+/853835 | |
| 17:40:42 | opendevreview | Balazs Gibizer proposed openstack/nova master: Create RequestGroups from InstancePCIRequests https://review.opendev.org/c/openstack/nova/+/852771 | |
| 17:40:42 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support resource_class and traits in PCI alias https://review.opendev.org/c/openstack/nova/+/853316 | |
| 17:40:43 | opendevreview | Balazs Gibizer proposed openstack/nova master: Map PCI pools to RP UUIDs https://review.opendev.org/c/openstack/nova/+/854118 | |
| 17:40:43 | opendevreview | Balazs Gibizer proposed openstack/nova master: Make allocation candidates available for scheduler filters https://review.opendev.org/c/openstack/nova/+/854119 | |
| 17:40:45 | opendevreview | Balazs Gibizer proposed openstack/nova master: Filter PCI pools based on Placement allocation https://review.opendev.org/c/openstack/nova/+/854120 | |
| 17:40:45 | opendevreview | Balazs Gibizer proposed openstack/nova master: Store allocated RP in InstancePCIRequest https://review.opendev.org/c/openstack/nova/+/854121 | |
| 17:40:47 | opendevreview | Balazs Gibizer proposed openstack/nova master: Func test for PCI in placement scheduling https://review.opendev.org/c/openstack/nova/+/854122 | |
| 17:40:47 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support move operations with PCI tracking in placement https://review.opendev.org/c/openstack/nova/+/854247 | |
| 17:40:49 | opendevreview | Balazs Gibizer proposed openstack/nova master: Split PCI pools per PF https://review.opendev.org/c/openstack/nova/+/854440 | |
| 17:40:49 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support same host resize with PCI in placement https://review.opendev.org/c/openstack/nova/+/854441 | |
| 18:05:46 | sean-k-mooney | gmann: non of it needs to be adressed in those patches | |