| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-12 | |||
| 16:01:45 | efried | if you unplug one that has allocations against it, you can expect problems; but otherwise... | |
| 16:02:07 | efried | and even then, I think we've accounted for that by setting reserved=total. | |
| 16:02:37 | sean-k-mooney | well there were edgecases like this https://review.openstack.org/#/c/626381/ | |
| 16:03:12 | sean-k-mooney | if the device you remove had allocation against it we cant remove it form placemetn until those allocation are also removed | |
| 16:03:48 | sean-k-mooney | so the auto updating is greate but there are also dragons lurking there | |
| 16:05:09 | efried | right, like I said, that case is accounted for (at least I definitely remember this code in the xen vgpu patches) where, if a card goes away, but has allocations against it, we set reserved=total so no futher allocations can be made against it, until all the existing allocations go away, at which point we can delete the provider. | |
| 16:06:12 | sean-k-mooney | ya so we have solution for this edgecase and likely other but its a little more involved then you would assume at first glance | |
| 16:06:12 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Calculate RequestGroup resource provider mapping https://review.openstack.org/616239 | |
| 16:06:12 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Fill the RequestGroup mapping during schedule https://review.openstack.org/619528 | |
| 16:06:13 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Pass resource provider mapping to neutronv2 api https://review.openstack.org/616240 | |
| 16:06:13 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Recalculate request group - RP mapping during re-schedule https://review.openstack.org/619529 | |
| 16:06:14 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Send RP uuid in the port binding https://review.openstack.org/569459 | |
| 16:06:14 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Test boot with more ports with bandwidth request https://review.openstack.org/573317 | |
| 16:06:15 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Remove port allocation during detach https://review.openstack.org/622421 | |
| 16:06:15 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Record requester in the InstancePCIRequest https://review.openstack.org/625310 | |
| 16:06:16 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add pf_interface_name tag to passthrough_whitelist https://review.openstack.org/625311 | |
| 16:06:16 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Ensure that bandwidth and VF are from the same PF https://review.openstack.org/623543 | |
| 16:06:17 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support server create with ports having resource request https://review.openstack.org/636360 | |
| 16:07:09 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support server create with ports having resource request https://review.openstack.org/636360 | |
| 16:09:39 | mriedem | lyarwood: mdbooth: +2 on https://review.openstack.org/#/c/551302/ | |
| 16:09:43 | mriedem | dansmith: ^ | |
| 16:11:24 | mdbooth | mriedem: Thanks! | |
| 16:15:00 | lyarwood | mriedem: cheers | |
| 16:16:03 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support server create with ports having resource request https://review.openstack.org/636360 | |
| 16:17:20 | efried | aspiers, mriedem: Were either of you planning to resurrect https://review.openstack.org/#/c/538498/ ? | |
| 16:17:54 | sean-k-mooney | mriedem: when you have a sec can you let me know if the responces in https://review.openstack.org/#/c/624842/8//COMMIT_MSG make sense. if you are happy with them i can resping with that info added to the commit and your other comments addressed in an hour or so | |
| 16:18:21 | mriedem | efried: i'm not working on it | |
| 16:19:45 | stephenfin | efried: Comments on https://review.openstack.org/#/c/582392/ | |
| 16:19:51 | efried | ... | |
| 16:19:52 | mriedem | sean-k-mooney: i guess.... | |
| 16:20:05 | mriedem | setting the vif mac to 0s works depending on the underlying driver | |
| 16:20:08 | mriedem | is the summary i'm getting | |
| 16:20:15 | sean-k-mooney | yes | |
| 16:20:19 | mriedem | i'm not sure how the minimum libvirt version plays into any of that | |
| 16:20:37 | sean-k-mooney | oh with libvirt 3.2 it takes care of all of this | |
| 16:20:42 | mriedem | set_vf_interface_vlan doesn't document it's expected exit codes at all either so i had no idea | |
| 16:20:49 | sean-k-mooney | so we can delete this code | |
| 16:23:57 | aspiers | efried: I started to rebase it and got confused by the conflicts through lack of familiarity with the code, so I installed git-irebase and started to incrementally rebase it one commit at a time, so that the conflicts are smaller and there's more chance of me understanding how to handle them correctly | |
| 16:24:15 | efried | aspiers: tldr you're working on it? | |
| 16:24:23 | aspiers | well, trying ;-) | |
| 16:24:39 | aspiers | I reserve the right to give up through incompetence though | |
| 16:25:02 | sean-k-mooney | mriedem: the behavior chagne 2 weeks ago in https://github.com/openstack/nova/commit/13e283cc82e7cab564850501f9781385fb7b3f68 ill have to respin anyway now. | |
| 16:25:12 | aspiers | efried, mriedem: however I've just taken a brief pause to review the SEV patches my colleague just posted | |
| 16:25:54 | efried | aspiers: Just lmk if you do decide to give up on it, and I'll take over. I want it to get finished one way or another. (No particular hurry, just don't want it stalled indefinitely.) | |
| 16:25:57 | mriedem | sean-k-mooney: behavior change? | |
| 16:26:02 | aspiers | efried: sure thing | |
| 16:26:09 | efried | and thank you. | |
| 16:26:13 | aspiers | yw | |
| 16:27:08 | mriedem | efried: have you taken a gander at https://review.openstack.org/#/c/616239/ yet? that's gibi's "Calculate RequestGroup resource provider mapping" change which i figure you're the best person to review that | |
| 16:27:12 | sean-k-mooney | mriedem: actully no never mind the exit_code = [0, 2, 254] is now donw internally in another fucntion instead of pased in | |
| 16:27:38 | sean-k-mooney | never mind i dont need to respin due to a behavior change but i will to adress you comments | |
| 16:27:51 | efried | mriedem: It's on my list, but I admit my list is currently more of a... cloud. I can bump it. | |
| 16:28:02 | mriedem | same | |
| 16:28:47 | dansmith | mriedem: lyarwood mdbooth: Tell me why we have to do the partial() and I'll remove my -1, otherwise that seems nuts to me | |
| 16:29:12 | mriedem | b/c the virt driver calls the post method | |
| 16:29:16 | mriedem | and doesn't have the source bdms | |
| 16:29:23 | mriedem | it's a callback | |
| 16:30:10 | dansmith | mriedem: right, but we can modify that interface | |
| 16:30:22 | mriedem | and update all virt drivers to lookup and pass the bdms? | |
| 16:30:54 | dansmith | eff yeah, otherwise you're just stacking partial()s on top of each other until the end of time | |
| 16:30:58 | openstackgerrit | Eric Fried proposed openstack/nova master: Use tox 3.1.1 fixes https://review.openstack.org/582392 | |
| 16:30:58 | openstackgerrit | Eric Fried proposed openstack/nova master: tox: Don't write byte code (maybe) https://review.openstack.org/636378 | |
| 16:31:07 | dansmith | and nobody actually knows how/where all those things come from | |
| 16:31:12 | mriedem | dansmith: actually it's already too late | |
| 16:31:17 | mriedem | once the virt driver calls the post method, | |
| 16:31:24 | mriedem | the bdms have been modified via pre_live_migration on the dest | |
| 16:31:31 | mriedem | which is why he's looking them up before call pre_live_migration | |
| 16:31:46 | dansmith | mriedem: too late for stacking the partials you mean right? | |
| 16:31:47 | efried | stephenfin: fixed, good suggestion. | |
| 16:32:24 | mriedem | too late for the virt driver on the source host to lookup the bdms to pass to _post_live_migration because that happens *after* pre_live_migration has modified the bdms | |
| 16:32:34 | dansmith | mriedem: all I'm saying is pass these things to the method you want to have pass them back, instead of wrapping | |
| 16:32:40 | dansmith | mriedem: I'm not saying have the virt driver look them up | |
| 16:32:52 | efried | mriedem: don't forget to send a courtesy note to the ML so OOT drivers aren't blindsided :* | |
| 16:32:53 | mriedem | so pass through the driver.live_migration method | |
| 16:33:08 | dansmith | mriedem: right | |
| 16:33:54 | mriedem | i don't feel strongly either way, but this has to be backported and as noted that interface change will require all drivers to be updated lockstep | |
| 16:34:12 | mriedem | so if we were going to change the interface, i'd do it in a follow up which isn't backported | |
| 16:34:22 | dansmith | mriedem: well, then backport this and fix it after if you want | |
| 16:34:33 | lyarwood | ack, I can take that if mdbooth can't | |
| 16:34:44 | dansmith | I would also define the partial right before you make the call to the driver, | |
| 16:34:53 | mdbooth | dansmith: TBH, I actually like the partial better, and in this context I'd partial all the things. | |
| 16:35:00 | dansmith | mdbooth: I have no doubt that you do | |
| 16:35:12 | mriedem | rollback still has to be fixed as well | |
| 16:35:13 | mriedem | for the same issue | |
| 16:35:29 | mdbooth | dansmith: The reason is that is simplifies the interface between compute and the driver by removing arguments which are purely local the the calling method. | |
| 16:35:45 | mdbooth | So the callback method only passes relevant data, and everything else remains local. | |
| 16:36:00 | mdbooth | However, that isn't why I used one here. Here's it's just so I didn't have to change the interface. | |
| 16:36:11 | dansmith | mdbooth: yeah, I get the obvious argument for it, I just think it hurts readability and understandability if you're just focused on that one method | |
| 16:36:44 | dansmith | it'd be slightly better if you define the partial and all the extra things you're going to pass down right before you make the call to the driver, | |
| 16:36:54 | openstackgerrit | Merged openstack/nova stable/pike: Add functional regression test for bug 1794996 https://review.openstack.org/623358 | |
| 16:36:55 | openstack | bug 1794996 in OpenStack Compute (nova) pike "_destroy_evacuated_instances fails and kills n-cpu startup if lazy-loading flavor on a deleted instance" [High,In progress] https://launchpad.net/bugs/1794996 - Assigned to Matt Riedemann (mriedem) | |
| 16:37:03 | openstackgerrit | Merged openstack/nova stable/pike: Fix InstanceNotFound during _destroy_evacuated_instances https://review.openstack.org/623359 | |
| 16:37:13 | openstackgerrit | Merged openstack/nova master: Trivial: reorder hashes according to object_hashes.txt https://review.openstack.org/636261 | |
| 16:37:19 | dansmith | but if I were to add something else in a year, we could easily end up with a partial(partial(thing)) because someone didn't read 50 lines up | |
| 16:38:27 | dansmith | using it to make a backport cleaner is exactly the hacky situation where I think the benefit outweighs the cost, but going forward in master, it's just a confusing hack, IMHO | |
| 16:58:07 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Don't set bandwidth limits for vhostuser, hostdev interfaces https://review.openstack.org/635170 | |
| 16:58:07 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove get_config_vhostuser https://review.openstack.org/565471 | |
| 16:58:08 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Validate bandwidth configuration for other VIF types https://review.openstack.org/636383 | |
| 16:58:08 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Further de-dupe os-vif VIF tests https://review.openstack.org/636384 | |
| 16:58:34 | stephenfin | jangutter, sean-k-mooney: Per our discussion on IRC last week ^ | |