| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-04 | |||
| 15:16:01 | gibi | any feedback are welcome | |
| 15:16:41 | artom | mriedem, well, *my* crap is going to remain unrushed, and will smell delicious by the time it lands ;) | |
| 15:17:14 | artom | efried, btw, I'm +1 on https://review.openstack.org/#/c/635440/9, but I left a question inline | |
| 15:20:22 | mriedem | umm, what actually puts the instance into ERROR state in ^? | |
| 15:22:02 | artom | Gremlins? | |
| 15:22:24 | jroll | .v 37 | |
| 15:22:29 | jroll | blurhg, sorry | |
| 15:22:35 | kashyap | :D | |
| 15:23:11 | cfriesen | sean-k-mooney: is it expected that PCI aliases will be configured in the nova-api nova.conf the same as all compute nodes? | |
| 15:23:26 | sean-k-mooney | yes | |
| 15:23:35 | cfriesen | mriedem: there you go ^ | |
| 15:25:26 | mriedem | isn't that....weird? | |
| 15:25:46 | mriedem | the computes can have different pci devices right? so wouldn't the alias config be per-compute? | |
| 15:29:37 | cfriesen | mriedem: I assume the idea is that a given alias should mean the same thing across the whole cloud, even if not all computes have that device. | |
| 15:31:21 | mriedem | ok i guess it's true https://docs.openstack.org/nova/latest/admin/pci-passthrough.html#configure-nova-api-controller | |
| 15:31:25 | mriedem | and documented that way | |
| 15:31:32 | sean-k-mooney | mriedem: it is weird but its needed for reasons | |
| 15:31:43 | sean-k-mooney | im trying to rememebr why | |
| 15:31:51 | sean-k-mooney | stephenfin: do you rememebr ? | |
| 15:32:23 | sean-k-mooney | i think i had something to do with either hardwar offloaded ovs or pci numa policies | |
| 15:33:09 | stephenfin | sean-k-mooney: Why they have to be specified on the API node? | |
| 15:33:16 | sean-k-mooney | cfriesen: it was related to scheduling. we needed the content of teh alias to aloow the schulers to make desissions | |
| 15:33:21 | sean-k-mooney | stephenfin: yes | |
| 15:33:47 | sean-k-mooney | i think it was so the numa toplogy filter could take the pci_numa policy into effect | |
| 15:34:08 | cfriesen | I'm not complaining, it's helpful for the flavor/image validation. :) | |
| 15:34:11 | stephenfin | Nah, it's because it's needed for move operations | |
| 15:34:16 | stephenfin | See b4ce2d9f12ef6d50837e4133dff09fa43fd152d2 | |
| 15:35:38 | sean-k-mooney | stephenfin: well the move operations e.g. cold migration need it for schudleing | |
| 15:37:05 | sean-k-mooney | mriedem: the alias does not contain the pci addresses. so it works independenly form the compute nodes for most usecause | |
| 15:38:09 | sean-k-mooney | it does mean if you put the pci vendor and product id in the alias that it has to match across the compute nodes but you are better off haveing 1 alias per device model anyway | |
| 15:41:45 | mriedem | sure | |
| 15:42:13 | mriedem | artom: btw, how is your downstream numa live migration whitebox tempest testing stuff passing if the intel people testing it are finding issues? just different issues from what the CI would hit? | |
| 15:42:45 | sean-k-mooney | mriedem: i responded on the mailing list | |
| 15:43:03 | sean-k-mooney | mriedem: they were using virsh edit to view the xml instead of virsh dumpxml | |
| 15:43:15 | sean-k-mooney | virsh dumpxml shows the current state of the vm | |
| 15:43:32 | sean-k-mooney | virsh edit shows the xml that the vm would have it you were to reboot it | |
| 15:44:00 | sean-k-mooney | it looks like when we update the xml as part of a migration virsh edit still shows the original xml | |
| 15:44:33 | sean-k-mooney | so that is the reason for the delta. the whitebox test use virsh dumpxml which i belive is correct | |
| 15:52:53 | mriedem | (8:39:26 AM) stephenfin: mriedem: Just tell me what I need to blindly +W | |
| 15:53:05 | mriedem | stephenfin: don't go blind on this, but https://review.openstack.org/#/c/623543/ | |
| 15:53:18 | mriedem | since gibi abandoned https://review.openstack.org/#/c/625311/ | |
| 15:53:32 | stephenfin | mriedem: Yup, already on it (gibi pinged us) | |
| 15:53:57 | mriedem | ok that's a big change so i'll hold off until someone more familiar with the pf/vf/pci stuff has gone through it | |
| 15:58:11 | gibi | mriedem: sorry about the size of that patch, it organicly grow to that | |
| 15:59:25 | sean-k-mooney | gibi: is your current plan to support but the manual tag in the whitelist + the auto disovery of the tag? | |
| 16:00:37 | sean-k-mooney | gibi: im ok with that by the way as a fallback mechanisum ignoring the similar nameing of pf_interface_name and devname | |
| 16:01:23 | sean-k-mooney | oh you have abandoned it never mind im looking at the wrong patch | |
| 16:01:27 | bauzas | gibi: I just started reviewing https://review.openstack.org/#/c/623543/ and I have a concern | |
| 16:01:51 | gibi | sean-k-mooney: I dropped the whitelist support | |
| 16:01:54 | bauzas | gibi: do we agreed on having a pci claim providing a reschedule ? | |
| 16:02:03 | bauzas | (for this I mean) | |
| 16:02:27 | gibi | bauzas: fixing the PCI filter would be a really big work as the filters doesn't know about allocation candidates | |
| 16:02:38 | sean-k-mooney | gibi: cool that was my perference but i could live with it if it was just a fallback for if we could not auto detect it. | |
| 16:02:51 | bauzas | gibi: sure, I'm just saying we haven't this in the spec, right? | |
| 16:02:57 | gibi | sean-k-mooney: libvirt autodetection was pretty small change so I went for it | |
| 16:03:20 | gibi | bauzas: yeah, devil is in the details so this edge case surfaced during implementation | |
| 16:03:27 | gibi | bauzas: I'm planning to retrofit the spec | |
| 16:03:48 | bauzas | gibi: okay, my point is that I want to make sure people are okay with this | |
| 16:04:03 | gibi | bauzas: sure, I also want to make sure, hence the big commit message | |
| 16:04:15 | bauzas | (fwiw, I have the same problem with any NUMA affinity for vGPUs ;) ) | |
| 16:04:26 | bauzas | changing the filter is difficult | |
| 16:04:58 | gibi | if we end up fearing the re-schedule then I can drop this patch and document the limitation | |
| 16:05:33 | bauzas | gibi: I'm not really bad with this | |
| 16:05:54 | bauzas | also, https://review.openstack.org/#/c/636360/19 is the API microversion that really creates the new feature, right? | |
| 16:06:18 | gibi | bauzas: yes, everything is rejected until https://review.openstack.org/#/c/636360/19 | |
| 16:06:37 | gibi | bauzas: everything involving ports with resource request | |
| 16:06:45 | bauzas | yeah, I'm seeing with https://review.openstack.org/#/c/636360/19/nova/api/openstack/common.py | |
| 16:06:45 | gibi | bauzas: except server delete | |
| 16:07:06 | gibi | bauzas: yepp, that is the key | |
| 16:15:17 | stephenfin | gibi: Left a series of nits plus a suggestion to s/pf_interface_name/parent_ifname/, but otherwise +2 on https://review.openstack.org/#/c/623543/ Thanks for tackling that | |
| 16:16:21 | bauzas | gibi: just to make it clear, if you solve https://review.openstack.org/#/c/623543/39//COMMIT_MSG by the compute claim, that means that you check the compute version for all the move ops ? | |
| 16:17:05 | gibi | stephenfin: I'm OK to rename the key to parent_ifname | |
| 16:17:20 | stephenfin | and I'm OK to re-review if you do :) | |
| 16:17:25 | bauzas | don't see any of this in https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/bandwidth-resource-provider.html#upgrade-impact | |
| 16:17:50 | gibi | bauzas: I think you found a problem | |
| 16:18:18 | gibi | bauzas: we did not planned to check the compute version | |
| 16:19:08 | gibi | bauzas: right now if you boot a server and the scheduler select a host that is still Rocky then the pci claim fails as the new pf_interface_name key will be missing from the pci spec | |
| 16:19:25 | gibi | bauzas: so no resource inconsistency but more re-schedule will happen | |
| 16:20:41 | bauzas | gibi: wait | |
| 16:20:42 | gibi | bauzas: ohh no. the InstancePCIRequest will not request the new key either if the request lands on an old compute | |
| 16:20:58 | bauzas | gibi: yeah that | |
| 16:21:08 | bauzas | it's old code so it doesn't know about anything | |
| 16:21:31 | gibi | bauzas: yeah, the InstancePCIRequest change is also in the compute/manager | |
| 16:22:40 | gibi | BUT if we have an old compute then the old compute will not have bandwidth inventory | |
| 16:22:49 | gibi | so placement will not select it | |
| 16:23:07 | gibi | (assuming that nova-compute is updated along with the neutron agents) | |
| 16:23:10 | bauzas | gibi: that was the original reason why we didn't need a check | |
| 16:23:22 | bauzas | because placement checks it for free | |
| 16:23:23 | bauzas | but | |
| 16:23:49 | bauzas | for a classic move operation, you can end up with the late-check not being read | |
| 16:24:03 | mriedem | there is no move support for this in stein | |
| 16:24:30 | gibi | bauzas: ^^ | |
| 16:24:35 | mriedem | gibi: and we enforce that in the api right? | |
| 16:24:39 | mriedem | yeah i remember | |
| 16:24:47 | gibi | mriedem: yes, the moves are rejected | |
| 16:24:56 | gibi | mriedem: even after the 2.72 | |
| 16:24:56 | bauzas | mriedem: what happens with my instance ? am I able to migrate it with dropping the bandwidth check, or do i get some ERROR ? | |
| 16:25:07 | mriedem | you get a 400 or 409 | |
| 16:25:08 | bauzas | ah, that's an API enforcement | |
| 16:25:09 | mriedem | i can't remember which | |