Earlier  
Posted Nick Remark
#openstack-nova - 2019-02-12
15:58:35 efried as opposed to just... removing the hardware and everything works.
15:59:01 sean-k-mooney that is not really a thing today
15:59:14 efried neither is generic numa affinity
15:59:18 sean-k-mooney i mean i "might" work in some cases
15:59:45 efried Seems like all of this is stuff we're striving for.
16:00:16 efried The whole reason update_provider_tree happens on a loop is so that it can, in the future, automatically adjust the provider tree when stuff like this happens.
16:00:27 sean-k-mooney i think https://etherpad.openstack.org/p/nova-ptg-train everyting from lin 7-30 is very related
16:01:35 efried In fact, I think that it might "just work" after Sylvain's vgpu stuff lands to hot-(un)plug a GPU.
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

Earlier   Later