Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-12
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 ^
16:59:32 lyarwood dansmith: so just to confirm, you'd be okay with https://review.openstack.org/#/c/551302/ as a backportable hack if the partial was moved down and we followed up with something different in master?
16:59:52 dansmith lyarwood: yeah
17:00:10 lyarwood dansmith: kk, I'll move that down now then, thanks
17:03:10 stephenfin mriedem, alex_xu: Any chance you could take a look at this again as some point this week? https://review.openstack.org/#/c/631038/
17:04:51 openstackgerrit Lee Yarwood proposed openstack/nova master: Avoid redundant initialize_connection on source post live migration https://review.openstack.org/551302
17:05:20 lyarwood dansmith: ^ there we go, thanks again
17:06:17 prometheanfire does nova require pci_passthough_whitelist to be in the [pci] section in rocky or can it remain in [DEFAULT]?
17:10:05 stephenfin prometheanfire: It will be read from DEFAULT but it should be moved as we'll eventually remove the aliases
17:10:47 stephenfin prometheanfire: You'll get a warning. Also, the option in the '[pci]' group is simply called 'passthrough_whitelist', in case you'd missed that
17:11:04 prometheanfire ya
17:11:24 prometheanfire I'm working on a os_nova backport to stable/rocky, stein is already updated
17:11:26 sean-k-mooney it was technically moved in queens i think
17:11:35 prometheanfire ya, docs show it in pike too
17:11:44 prometheanfire https://docs.openstack.org/nova/pike/admin/pci-passthrough.html
17:12:15 sean-k-mooney prometheanfire: oh it was move in ocata
17:12:36 prometheanfire no one uses that :P
17:13:11 sean-k-mooney you would be surprised how many people are on Newton still
17:13:41 sean-k-mooney there are still people on liberty but yes few people use ocata
17:13:53 sean-k-mooney its EOL upstream anyway
17:14:19 prometheanfire it was a joke :P
17:14:48 sean-k-mooney but anyway https://github.com/openstack/nova/blob/master/nova/conf/pci.py#L84
17:15:14 sean-k-mooney we still support both but we really shoudl drop support for [DEFAULT] at this point
17:15:28 prometheanfire ya
17:16:26 sean-k-mooney prometheanfire: if you are moving it yyou shoudl also move the pci alias too
17:17:07 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/conf/pci.py#L23
17:18:34 prometheanfire alias wasn't even defined/used until stein (I'm adding it in the backports)
17:19:03 sean-k-mooney they predate kolla by years
17:19:22 sean-k-mooney you could use them via the config override mechanisium
17:20:14 sean-k-mooney the main issue was you could only have 1 alsi tuse to how the config_merger ansible plugin worked
17:20:35 sean-k-mooney it does not support MultiStrOpt fields
17:21:17 sean-k-mooney unlike the whitelist the alais does not support a json list format
17:30:05 prometheanfire does nova support out of tree drivers? (xenxl)
17:33:01 melwitt o/
17:33:18 sean-k-mooney prometheanfire: nova does nto support out of tree drivers in general no
17:36:22 prometheanfire sean-k-mooney: ok, just curious
17:44:06 stephenfin sean-k-mooney, adrianc: Finished SR-IOV series review and left some comments. Ping me if/when there's more to look at.
17:44:32 sean-k-mooney stephenfin: cool i will try to adress them later today
17:47:00 sean-k-mooney stephenfin: it can wait till tomorow but we need to review https://review.openstack.org/#/c/636061/ tool the ironic and neutron spec were approved after m2 so apparently this is intending to target stien.
17:48:45 sean-k-mooney stephenfin: so 1 we need to validate it makes sense and 2 merge it and releas a new os-vif version before teh 27th. i was hoping to do a new release on friday week anyway but i would have prefered to at least have a nova/os-vif blueprint for this earlier
17:48:46 prometheanfire sean-k-mooney: they don't think they'd be able to support the driver fully upstream is the reason I asked
17:49:40 sean-k-mooney prometheanfire: so technically you can use out of tree drivers but they are not offically supported by nova (since they are out of tree) and nova may break them at any time
17:50:04 sean-k-mooney historically i dont know how volitile that interface has been
17:50:22 openstackgerrit Surya Seetharaman proposed openstack/nova master: Plumbing required in servers ViewBuilder to construct partial results https://review.openstack.org/635146
17:50:23 openstackgerrit Surya Seetharaman proposed openstack/nova master: API microversion 2.69: Handles Down Cells https://review.openstack.org/591657
17:50:23 openstackgerrit Surya Seetharaman proposed openstack/nova master: API microversion 2.69: Handles Down Cells Documentation https://review.openstack.org/635147
17:50:34 sean-k-mooney prometheanfire: so there is noting preventign kolla from supproting deploying them but they may break
17:52:15 sean-k-mooney prometheanfire: being out of tree means that maintaining comptablity with nova fall entirly onto the xenxl team. but yes being intree requires ci and other testing requirement so its non trivial

Earlier   Later