| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-04 | |||
| 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 | gibi | bauzas: except server delete | |
| 16:06:45 | bauzas | yeah, I'm seeing with https://review.openstack.org/#/c/636360/19/nova/api/openstack/common.py | |
| 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 | 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:24:56 | gibi | mriedem: even after the 2.72 | |
| 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 | |
| 16:25:10 | bauzas | good then | |
| 16:25:46 | mriedem | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/evacuate.py#L123 400 | |
| 16:25:54 | bauzas | so that works, at least if neutron agent is upgraded with compute service, which is usually what's done | |
| 16:26:13 | mriedem | if the neutron agent is upgraded before nova-compute it could be reporting inventory before the compute is ready to handle those types of allocations... | |
| 16:26:18 | mriedem | but i'm not sure how realistic that is | |
| 16:26:41 | gibi | bauzas: even if they ugrade the neutron agent they can skip the bw config in the agent until nova-compute is upgraded | |
| 16:26:45 | bauzas | worth a note at least | |
| 16:26:54 | gibi | bauzas: the whole neutron bw inventory is config based | |
| 16:26:57 | bauzas | yeah | |
| 16:27:12 | mriedem | how much of the compute-level stuff is virt-driver specific? | |
| 16:27:16 | bauzas | so, ops can upgrade whenever they want, and once they're done, they set configs | |
| 16:27:17 | mriedem | just the multi-VF on same PF thing? | |
| 16:27:37 | bauzas | gibi: planned to write some ops docs btw. ? | |
| 16:27:51 | bauzas | that would also help me reviewing all of this :) | |
| 16:27:55 | gibi | bauzas: yeah, rubasov already started it for neutron and I will chime in | |
| 16:28:19 | bauzas | I'd be more than happy to review it | |
| 16:28:23 | gibi | mriedem: good question, I think the SRIOV + bandwidth support is now become virt driver specify | |
| 16:28:41 | gibi | bauzas: here is a neutron WIP doc patch https://review.openstack.org/#/c/640390/ | |
| 16:28:45 | dansmith | mriedem: hmm, aren't the two things in runway slot 2 and 3 part of the same effort? | |
| 16:28:59 | mriedem | dansmith: yeah, spot instances | |
| 16:29:02 | dansmith | yeah | |
| 16:29:04 | mriedem | separate blueprints for whatever reason | |
| 16:29:10 | dansmith | hrm | |
| 16:29:37 | dansmith | they're part of the same stack in gerrit, so.. kinda seems like that's one thing:) | |
| 16:29:42 | gibi | mriedem: if there is a InstancePCIRequest then it will have the new tag included even if the compute host only has a single PF | |
| 16:29:48 | mriedem | gibi: but maybe a compute without a supporting virt driver wouldn't configure the neutron agent to report that inventory | |
| 16:29:52 | artom | mriedem, I wasn't checking the database instance_numa_topology | |
| 16:29:58 | artom | So the pins are updated and correct | |
| 16:30:14 | gibi | mriedem: SRIOV without bw is kept intact | |
| 16:30:15 | artom | But Windriver are telling me the DB isn't updated, despite apply_migration_context() being called | |
| 16:30:43 | gibi | mriedem: so if no resource request in the sriov port then we don't need the special key from the virt driver | |
| 16:30:57 | mriedem | artom: are you saving the instance changes after calling apply_migration_context()? | |
| 16:31:04 | mriedem | b/c ^ doesn't persist the changes, it just updates the object fields | |
| 16:31:15 | artom | mriedem, wait, seriously? | |