| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-03 | |||
| 14:09:13 | lyarwood | elod: yeah I appreciate that, some of these should've already been backported as bugfixes tbh | |
| 14:09:26 | lyarwood | elod: but the func changes are required to actually test this correctly | |
| 14:09:35 | lyarwood | elod: and it's all pretty trivial IMHO | |
| 14:11:11 | openstackgerrit | Lee Yarwood proposed openstack/nova master: fakelibvirt: Use versionutils to set min versions found in the driver https://review.opendev.org/749707 | |
| 14:11:11 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Bump MIN_{LIBVIRT,QEMU}_VERSION and NEXT_MIN_{LIBVIRT,QEMU}_VERSION https://review.opendev.org/746981 | |
| 14:11:12 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Remove MIN_LIBVIRT_FILE_BACKED_DISCARD_VERSION https://review.opendev.org/746982 | |
| 14:11:12 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Remove MIN_{LIBVIRT,QEMU}_NATIVE_TLS_VERSION https://review.opendev.org/746983 | |
| 14:11:13 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Remove MIN_LIBVIRT_BETTER_SIGKILL_HANDLING https://review.opendev.org/746984 | |
| 14:11:13 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Remove MIN_LIBVIRT_VIDEO_MODEL_VERSIONS https://review.opendev.org/746985 | |
| 14:11:14 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Remove MIN_{LIBVIRT,QEMU}_PMEM_SUPPORT https://review.opendev.org/746986 | |
| 14:15:30 | elod | lyarwood: ok, i see. I'll try to overcome my taste o:) | |
| 14:16:46 | CeeMac | stephenfin: is there a topic naming convention for branching? | |
| 14:17:13 | artom | sean-k-mooney, wait, we didn't add any functional tests for SRIOV live migration? | |
| 14:17:30 | artom | sean-k-mooney, all I see is the PCI stuff - ie, not Neutron ports, but flavor-driven | |
| 14:17:32 | stephenfin | CeeMac: If it's a bug, bug/<bug-id>. Because you likely want this backported, I'd suggest opening one if you can? | |
| 14:17:32 | sean-k-mooney | artom: not without stephenfin patch set | |
| 14:17:42 | stephenfin | artom: I have it done but it didn't exist previously, no | |
| 14:17:49 | stephenfin | It was a lot of work, in fairness | |
| 14:17:55 | artom | stephenfin, it merged? | |
| 14:17:58 | artom | Link plz? | |
| 14:17:58 | sean-k-mooney | no | |
| 14:18:04 | CeeMac | stephenfin: sure I can open a bug for this | |
| 14:18:07 | sean-k-mooney | its conflicting wiht many things | |
| 14:18:16 | stephenfin | https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:sriov-functional-tests | |
| 14:18:27 | CeeMac | i'll grab a coffee first! | |
| 14:18:40 | sean-k-mooney | artom: specificaly its conflicting with both my live migration bug fix your live migration bug fix and gibi sriov attach/detach series | |
| 14:18:43 | stephenfin | CeeMac: Sure thing :) | |
| 14:19:40 | artom | sean-k-mooney, sounds about right | |
| 14:19:44 | sean-k-mooney | the conflict are not surpriseing since we are all modifying similar parts fo the code | |
| 14:19:51 | sean-k-mooney | and adding tests | |
| 14:19:57 | artom | stephenfin, wait, how does https://review.opendev.org/#/c/746950/6 pass if you're depending on gibi's NUMA SRIOV live migration fix? | |
| 14:20:30 | stephenfin | artom: I'm not. His fix is for a certain edge condition I'm not triggering | |
| 14:20:30 | sean-k-mooney | it does not depend on it | |
| 14:20:51 | stephenfin | I did try to add a test for it but clearly didn't understand the bug enough | |
| 14:20:51 | artom | Err, *you're not | |
| 14:20:54 | stephenfin | *well enough | |
| 14:21:04 | sean-k-mooney | stephenfin: by edgecase you mean the fact we dont consider the numa toplogy of the guest at all right now | |
| 14:21:14 | artom | Also, having NUMA and SRIOV in the same instance is not an edge condition :) | |
| 14:21:19 | artom | Unless I'm way off base on that bug | |
| 14:21:32 | stephenfin | sean-k-mooney: it's to do with the claiming bit right? | |
| 14:21:38 | sean-k-mooney | yes | |
| 14:21:46 | sean-k-mooney | when we claim we pass None currenlty | |
| 14:21:50 | stephenfin | okay, so I have https://review.opendev.org/#/c/749360/ | |
| 14:22:34 | artom | stephenfin, ah, you're not using NUMA | |
| 14:22:36 | artom | Just SRIOV | |
| 14:22:36 | stephenfin | but the guest NUMA topology appears to be getting correctly recalculated to ensure the instance lands on the same host NUMA node as the PCI device | |
| 14:22:42 | artom | Not in the same test, at least | |
| 14:22:47 | stephenfin | I am in that one | |
| 14:22:51 | stephenfin | I'm using CPU pinning | |
| 14:22:55 | stephenfin | that's NUMA | |
| 14:23:14 | stephenfin | https://review.opendev.org/#/c/749360/1/nova/tests/functional/libvirt/test_pci_sriov_servers.py@363 | |
| 14:24:08 | sean-k-mooney | it could be passing jsut out of luck | |
| 14:24:21 | artom | Interesting, I wonder if there's a specific assertion that's missing | |
| 14:24:32 | sean-k-mooney | you would need to force a case where cpus are only avaiable on one node and the pci device is on another | |
| 14:24:38 | sean-k-mooney | with the bug it would boot | |
| 14:24:46 | stephenfin | sean-k-mooney: Yeah, I think that's what I'm missing | |
| 14:24:56 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: compute: Validate a BDMs disk_bus when provided https://review.opendev.org/744552 | |
| 14:25:09 | stephenfin | But I wasn't able to come up with a clever way to do that since conf is global :-( | |
| 14:25:12 | sean-k-mooney | by the way gibi's fix i dont thnk it a full fix | |
| 14:25:53 | sean-k-mooney | since we are claiming the devices at a different point then we are generating the numa toplogy there is always a race with outher vms unless this is all happening under a common lock | |
| 14:26:00 | sean-k-mooney | and i dont think it is | |
| 14:26:16 | sean-k-mooney | gibi's patch makes the code stricly better | |
| 14:26:40 | sean-k-mooney | but it does not make the numa toplogy generation and pci claims effectly atomic | |
| 14:31:08 | stephenfin | I'm getting the impression, based on those resize/cold migrate resize/confirm bugs, that pretty much everything the resource tracker is doing has to be atomic | |
| 14:31:56 | sean-k-mooney | where we dont have tristate logic like the pci tracker yes definetly | |
| 14:32:35 | sean-k-mooney | but in general yes | |
| 14:32:54 | sean-k-mooney | that is why we have the lock | |
| 14:33:45 | artom | The whole periodic using the migrations and flavors and instances is a mess | |
| 14:35:02 | artom | Kinda makes me wonder if there's a still a point to it... | |
| 14:35:22 | artom | We've gotten a lot better at making migrations correctly track resources, especially live migrations | |
| 14:35:34 | artom | Might make more sense to address the remaining gaps, and then remove it altogether | |
| 14:35:40 | artom | Or make it a nova-manager command, with a lock | |
| 14:35:48 | artom | Kinda like the placement audit thing | |
| 14:36:33 | sean-k-mooney | you mean in placment | |
| 14:36:52 | sean-k-mooney | in theory you should be abel to run nova with that periodic disabled | |
| 14:37:04 | sean-k-mooney | in paratics i expect that to be super broken | |
| 14:37:27 | artom | sean-k-mooney, no, I mean fix the remaining problems with resources not being correctly updated | |
| 14:37:33 | artom | And stop running the periodic | |
| 14:38:00 | sean-k-mooney | yes but that means basicaly persit stuff to the db properly | |
| 14:38:30 | sean-k-mooney | so stop usign move cliams and claims in general in memory and actully claim the stuff in the db atomicly | |
| 14:40:08 | artom | Well claims do update the usage eventually | |
| 14:40:24 | sean-k-mooney | part of the reason we have to rebuild the resouce state form flaovr/instance/migration is because we dont store it anywhere but in memory | |
| 14:40:58 | sean-k-mooney | artom: sure but if you restart the agent then you have to rebuild the state again on start up | |
| 14:41:17 | sean-k-mooney | form the migrations and isntance exctra | |
| 14:41:56 | artom | sean-k-mooney, https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L344 | |
| 14:42:14 | sean-k-mooney | yes that update teh compute nodes table | |
| 14:42:21 | sean-k-mooney | that is not the same thing | |
| 14:42:53 | sean-k-mooney | oh thats in the move claim | |
| 14:43:00 | sean-k-mooney | its not updateign the db | |
| 14:43:11 | sean-k-mooney | its updating the resouce tracker | |
| 14:43:11 | artom | How so? | |
| 14:43:33 | artom | sean-k-mooney, https://github.com/openstack/nova/blob/c57d52e1978210fb3260bf8ea442237d548db576/nova/compute/resource_tracker.py#L1185 | |
| 14:44:01 | sean-k-mooney | the compute node table does not containe all the data in the resouce tracker | |
| 14:44:04 | sean-k-mooney | just a summary | |
| 14:44:14 | sean-k-mooney | it has non of the vgpu mdeves for example | |
| 14:44:34 | artom | Aren't those in placement? | |
| 14:44:38 | sean-k-mooney | no | |
| 14:44:52 | sean-k-mooney | the count of how many we can create is | |
| 14:45:28 | artom | But we have to care about the specific individual ones like we do with PCPUs? | |
| 14:45:28 | sean-k-mooney | not the mdev paths for the vgpu instnace | |