Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-10-25
13:52:26 dansmith correct
13:54:41 sean-k-mooney ok that's what i taught. we may need to pass more the the host-id when creating the new portbinding unless neutron are happy to allow use to update the binding profile with the claimed pci devices when we activate the binding
13:55:07 openstackgerrit Radoslav Gerganov proposed openstack/nova master: VMware: implement get_inventory() driver method https://review.openstack.org/506175
13:55:32 mriedem sean-k-mooney: i thought some of the dvr hackery migration code we have in nova.network.neutronv2.api already dealt with pci devices in the port binding profile?
13:56:37 sean-k-mooney it may i was just trying to think if we need to have the binding profile in the new migrtion object or not.
13:57:47 sean-k-mooney mriedem: that said there is no code to regenerate the interfaces at all currently so if it was updating pci devices it was not updating the xml for sriov devices
13:58:24 mriedem sean-k-mooney: ok - definitely going into moshe / cfriesen territory here
13:59:26 sean-k-mooney mriedem: ya ill check with them, as far as i know i think moshe had patches to make livemigration with macvtap mode work but i dont recall if they ever merged
14:00:54 sean-k-mooney mriedem: vnic_type=direct or direct_physical do not support live migration currently at all as far as i know so this is an edge case of an edge case
14:07:25 mriedem dansmith: better to do a new remotable class method here? https://review.openstack.org/#/c/514904/3/nova/objects/migration.py
14:07:29 mriedem i think that's what we do in InstanceList
14:08:24 dansmith mriedem: the reason we did it for instance is because we had sort_key already and they wanted to replace it with sort_keys
14:08:49 dansmith we do add arguments like this all the time and I think the behavior of them being unset makes sense in this case (i.e. =None means default sort)
14:09:07 mriedem ok
14:09:17 dansmith I shall comment
14:10:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix AttributeError in BlockDeviceMapping.obj_load_attr https://review.openstack.org/514825
14:10:26 mriedem melwitt: i just removed the obj_reset_changes thing in the bdm object here; didn't want that holding up this otherwise simple fix ^
14:20:42 efried dansmith Got it, thanks. I just saw your commentary at https://review.openstack.org/#/c/506420/29/nova/conductor/tasks/migrate.py@61 which confirms some stuff I "discovered" last night as I was trying to figure out how to make flavors take numbered groupings.
14:21:07 dansmith ack
14:21:23 efried dansmith Getting resources from the flavor is going to break down *very* quickly for everything other than scheduling.
14:21:48 efried We're gonna have to get it from placement - but that's a chicken/egg for these scenarios.
14:22:05 dansmith well, that's why I was suggesting we go ahead with passing the allocation into the virt drier,
14:22:12 dansmith instead of setting the precedent of ripping that from the flavor
14:22:32 efried dansmith Does that help, though? Unless compute caches it somewhere...
14:22:56 dansmith help what?
14:23:23 dansmith it helps us not have to reconstruct the request to placement for things like "which device on which numa node should we give them a vgpu from"
14:23:27 efried Help with getting rid of remove_provider_from_instance_allocation
14:23:40 dansmith no, but that's not an issue after my set anyway
14:23:54 efried Let me come at it from the other angle: What all needs to merge for us to get rid of remove_provider_from_instance_allocation ? Just your thing, or also cdent's POST thing?
14:24:18 dansmith yes, and then a cycle has to pass, because we still need that for the old path where we have pike computes doing the old thing
14:24:30 dansmith so we merge my thing, snap queens, then we can remove all that junk
14:24:58 dansmith merging cdent's thing is orthogonal to being able to stop doubling.. it's just a correctness/atomicity thing
14:25:14 efried dansmith Ayee, okay. So we'll still need to figure out how to make remove_provider_from_instance_allocation work with e.g. nested.
14:25:42 dansmith well, it kinda depends on how that turns out
14:25:48 dansmith my first response is to say no, we don't
14:25:59 dansmith because pike computes won't have any nesting
14:26:12 dansmith and just the presence of the nested stuff doesn't break us,
14:26:39 dansmith what would cause a problem is if we could be migrating between a pike and queens node where the queens node has some nested resources that we have to account for in that thing
14:28:33 efried dansmith Presumably the scheduler on the pike node would just request the resources from whatever RPs are available on the pike node, which won't be nested.
14:28:57 mriedem scheduler is global
14:29:12 efried Right, point remains.
14:29:13 mriedem you upgrade scheduler before computes
14:29:15 dansmith efried: we don't do that new behavior until all the computes are upgraded
14:29:32 dansmith efried: because the old ones assume doubled allocations
14:29:48 mriedem and pike computes don't know how to cleanup allocations using the migration uuid
14:29:56 mriedem which is another way of saying what dan just said i think
14:29:59 dansmith right
14:32:03 efried Just so my brain doesn't explode, I need to zoom out a little bit and frame all of this in terms of how to grab numbered resource groupings from the flavor for the sake of scheduling.
14:32:27 openstackgerrit Pavel Gluschak (scsnow) proposed openstack/nova master: Fixed concurrent access to direct io test file https://review.openstack.org/515091
14:33:03 efried I think for now I just need to sum up all the resources and continue to pass in the single dict to remove_provider_from_instance_allocation
14:33:47 cdent efried: overhangs are the best part
14:33:48 efried And only keep them split up for the scheduler side of things, which is resources_from_request_spec
14:34:16 dansmith efried: currently I think that's right yeah
14:34:42 gibi mriedem: As you suggested last week I reached out to the author of bp service-create-destroy-notification and he stated he is still planning to work on the implementation.
14:34:44 efried This keeps us throwing good money after bad, digging deeper into the shared RP tech debt hole, but I don't see an alternative without boiling the ocean.
14:35:00 efried dansmith mriedem Thanks y'all.
14:35:22 cdent bonus points for boiling the ocean
14:35:28 mriedem gibi: cool thanks
14:35:46 efried cdent No penalties for mixing metaphors?
14:36:00 cdent good lord from me? no way.
14:36:06 cdent mixed metaphors are the best
14:37:15 openstackgerrit Sylvain Bauza proposed openstack/nova master: Accept all standard resource classes in flavor extra specs https://review.openstack.org/514942
14:37:15 openstackgerrit Sylvain Bauza proposed openstack/nova master: VGPU_support: add enabled white list https://review.openstack.org/512580
14:37:16 openstackgerrit Sylvain Bauza proposed openstack/nova master: WIP: libvirt: Provide VGPU inventory for a single GPU type https://review.openstack.org/514763
14:41:43 efried How's the new hire stuff going jaypipes? Winding down?
14:43:36 jaypipes efried: it's going.
14:46:03 dansmith jaypipes: that well huh?
14:46:16 mriedem dansmith: sorry for getting to this late, but questions inline https://review.openstack.org/#/c/508595/
14:47:52 dansmith mriedem: standard operating procedure for you, I know
14:48:00 mriedem :(
14:48:05 dansmith mriedem: that comment was added recently by you right? maybe just rebase noise
14:48:11 mriedem yes
14:48:17 mriedem when cleaning out the system_metadata stuff
14:48:33 mriedem i'm not -1 for that, that could be added back on top later
14:48:38 dansmith gonna pull it out of the gate? I'm still reading
14:48:46 mriedem -1 is for the lack of testing with limits, unless i'm missing something
14:48:52 mriedem *build_requests + limits in the sortmaster
14:52:09 dansmith mriedem: you think it's broken or just want more coverage?
14:52:25 dansmith since it's all the same logic (no special limit decrementing) it's not much of a gap
14:52:44 mriedem there was a test for build requests + limits which was removed,
14:52:48 mriedem so i think we want that coverage yeah
14:53:10 mriedem you could replay it on top of this change if you want (not the series, just this change)
14:54:56 dansmith I would rather not keep that test the way it was, since it tests the instance_list module from above the compute api, but I'm happy to slap a limit base test into instance_list
14:55:25 dansmith mriedem: it's fairly deep into the gate (spot 15 out of 42) but if you want to pull it out, I'll add it to that patch, else I can pile the coverage on top
14:55:43 mriedem just put it on top
14:55:44 mriedem that's fine
15:04:23 dansmith did they just restart zuul or something?
15:07:49 jaypipes gibi: please see my explanation on https://review.openstack.org/#/c/377215/53/nova/tests/functional/db/test_resource_provider.py
15:08:05 dansmith mriedem: so, actually, I think we should pull that from the gate, since everything just restarted
15:08:20 dansmith mriedem: I think there might be a bug that this test will uncover
15:08:25 openstackgerrit Chris Dent proposed openstack/nova master: Move project_id and user_id to Allocation object https://review.openstack.org/500410
15:08:26 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Allow _set_allocations to delete allocations https://review.openstack.org/501051
15:08:26 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Limit number of attempts to delete allocations https://review.openstack.org/507224
15:08:27 openstackgerrit Chris Dent proposed openstack/nova master: Include project_id and user_id in AllocationList.get_all_by_consumer_id https://review.openstack.org/512420
15:08:27 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Include project_id and user_id in GET /allocations https://review.openstack.org/512421
15:08:28 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Enable PUT dict to /allocations/{consumer_uuid} https://review.openstack.org/510625
15:08:28 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Document the changes to PUT and GET /allocations/{consumer_uuid} https://review.openstack.org/510626
15:08:28 jaypipes gibi: it's a weird one, for sure, but the mock there isn't actually to make the test work properly. it's to exercise one part of the WHERE clause that exists in that get_all_by_filters() method
15:08:29 openstackgerrit Chris Dent proposed openstack/nova master: [placement] POST /allocations to set allocations for >1 consumers https://review.openstack.org/500073
15:08:29 bauzas dansmith: yup, looks like

Earlier   Later