Earlier  
Posted Nick Remark
#openstack-nova - 2019-02-14
15:18:19 bauzas it's a PCI device
15:18:26 gibi stop the host, remove the pci device, start the host
15:18:32 bauzas ah that, then yes
15:18:40 bauzas but honestly, how many people would do this ?
15:18:48 bauzas I can add a documentation of course
15:18:56 gibi bauzas: yeah
15:18:58 bauzas actually, I need to do it
15:19:13 bauzas and remove it by https://review.openstack.org/#/c/636591/
15:19:34 gibi bauzas: it could be problem in future when we start supporting different device types per pgpu, but our RP inventory does not match the physical usage of the pgpu
15:19:42 gibi bauzas: I feel this inconsistency is dangerous
15:19:51 bauzas but lemme use one type that gives 1 VGPU per pGPU
15:20:13 bauzas gibi: right, I said it, we don't have a problem for the moment
15:20:22 bauzas gibi: until we have multiple types
15:20:32 bauzas where two pGPUs could have different types
15:20:45 bauzas I documented it tho
15:20:49 gibi bauzas: but if we release this code, then there will be deployments where the allocation in placement will not reflect that usage on the host, and fixing those deployments will be painful
15:21:49 gibi bauzas: the fix will be like moving allocations between RPs based on parent child relationship
15:21:58 gibi of the mdev and the pgpu
15:22:25 gibi I agree that it is not a problem today, but it will cause pain in the future if we are not handling it now
15:22:41 bauzas gibi: actually that's a good point, we should need to reshape allocations :(
15:25:11 gibi bauzas: it will be even more complicated if a server requests two VGPUs in a numbered groups, and therefore it needs to be filled from one pgpu which is represented as on RP in placement. But the RP in placement won't have an exact inventory for a single pgpu
15:25:38 bauzas gibi: we don't accept requests for > 1 vGPU atm
15:26:16 bauzas for many reasons, one being at least that nvidia driver doesn't support more than one mdev attached to a guest per pGPU
15:26:33 gibi bauzas: I see
15:26:39 gibi bauzas: that hepls
15:26:42 gibi helps
15:26:49 bauzas ideally, the patch is ready
15:27:03 gibi bauzas: still I think we should fix the mapping now to avoid the pain of reshape in the future
15:27:07 bauzas but the problem is about the RP UUID > name transtive relationship
15:27:13 bauzas yeah
15:27:25 bauzas so, I need to find a quick and clean solution for this
15:27:45 bauzas I should probably ask for a hangout session if people agree
15:27:52 bauzas to get feedback
15:28:19 gibi bauzas: yeah that would make sense. Especially if we already have different solution proposals to choose from
15:28:35 bauzas gibi: the alternative would be asking operators to rebuild :p
15:28:57 gibi bauzas: hm, that could be option N+1 ;)
15:29:15 bauzas gibi: actually, lemme look at the driver code and see what I can do
15:29:22 bauzas we already have the providertree info
15:29:32 bauzas now the big concern I have is caching it
15:29:42 bauzas so that the method could use it
15:29:59 bauzas instead of passing straight the tree down to that method
15:30:03 bauzas which I dislike
15:30:16 gibi at least we have options
15:30:33 gibi In general I don't like caching as that is hard to make it right
15:41:55 mriedem stephenfin: replies in https://review.openstack.org/#/c/633620/
15:42:08 bauzas gibi: yup me too
15:47:25 stephenfin mriedem: right back at ya (tl;dr: you do need to drop that line)
15:49:07 mriedem ok updating
15:52:38 openstackgerrit Matt Riedemann proposed openstack/nova master: Make VolumeAttachmentsSampleV249 test other methods https://review.openstack.org/633621
15:52:38 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix deps for api-samples tox env https://review.openstack.org/633620
15:52:39 openstackgerrit Matt Riedemann proposed openstack/nova master: Add microversion to expose virtual device tags https://review.openstack.org/631948
15:56:08 openstackgerrit Takashi NATSUME proposed openstack/nova master: Remove duplicate cleanup in functional tests https://review.openstack.org/636996
16:11:21 bauzas efried: thanks for commenting https://review.openstack.org/#/c/636591/1/nova/virt/libvirt/driver.py
16:11:49 efried fo sho, look forward to seeing that patch.
16:11:52 stephenfin mriedem: Said it to lyarwood but I've backported that PYTHONDONTWRITEBYTECODE setting all the way back to queens. If you could take a look, I'd appreciate it https://review.openstack.org/#/q/topic:PYTHONDONTWRITEBYTECODE+status:open
16:12:21 bauzas efried: given we update the provider tree at compute startup, I just wonder if it's safe enough to assume that the libvirt in-memory cached attribute for the tree would be there anyway when an instance is spawned ?
16:12:29 bauzas mriedem: thoughts on that too ?
16:12:49 bauzas mriedem: context is https://review.openstack.org/#/c/636591/1/nova/virt/libvirt/driver.py@6347
16:13:57 efried bauzas: We're pretty much *relying* on there being a match between upt and spawn
16:14:26 bauzas efried: there could be a race tho
16:14:28 efried bauzas: Also, pretty sure we run upt right before spawn, not just in the periodic, I could be wrong. So it should be pretty durn recent by then.
16:14:46 bauzas efried: where spawn() could run *before* upt
16:14:48 bauzas yeah
16:14:56 efried bauzas: But like I've said already (uh, possibly about something else): if there's a race, it's going to f up more than just this.
16:15:06 mriedem we run update_available_resource on compute start
16:15:08 mriedem which calls upt
16:15:11 bauzas i know
16:15:13 mriedem that's before the service is ready for spawn
16:15:15 mriedem so what's the issue
16:15:18 bauzas okay
16:15:33 bauzas I thought we were allowing instances to spawn, while compute was starting
16:15:41 mriedem f i hope not
16:15:46 mriedem https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1259
16:15:57 mriedem https://github.com/openstack/nova/blob/master/nova/service.py#L180
16:16:08 bauzas oh good catch this is pre hook
16:16:12 bauzas perfect
16:16:35 bauzas mriedem: you're not worried by libvirt keeping a copy of the provider tree structure ?
16:16:47 mriedem i haven't been following along
16:17:07 bauzas mriedem: nah, it's just because we need to find the name of a RP by its uuid
16:17:08 mriedem sounds like more cache-tastic fun
16:17:24 bauzas mriedem: as we only get RP UUID from the allocations list we pass over spawn
16:17:28 mriedem which RP?
16:17:30 efried mriedem: TL;DR: the virt driver needs to be responsible for maintaining a mapping between RP and "real thing". The provider tree already has that information, so don't bother building something special, just save it off during upt.
16:17:47 bauzas mriedem: the RP which was allocated a VGPU
16:17:55 mriedem so the pgpu?
16:17:58 mriedem the child provider
16:18:01 bauzas yup
16:18:15 bauzas so that libvirt can use or create a mdev from *this* pgpu
16:18:25 bauzas http://paste.openstack.org/show/745109/
16:18:44 mriedem what's the alternative? the driver holding it's own mapping of rp_uuid -> rp_name? which is something ProviderTree already does
16:18:49 efried yes ^
16:18:56 efried (to both)
16:18:56 bauzas above is the fact that when we create a new instance, we use a random mdev which is unrelated to the pgpu which was allocated by placement
16:19:25 mriedem i don't have a strong feeling either way
16:19:46 mriedem ProviderTree is convenient albeit heavy weight and hopefully someone doesn't abuse it somehow, but doing our own mapping dict is redundant
16:19:49 mriedem so shrug
16:20:07 bauzas yeah me too, but I'm opinionated enough to make it the smoothiest
16:20:30 bauzas and caching the value in the libvirt driver object seems the smoothiest approach
16:20:45 mriedem so the provider tree would just get reset on every upt call?
16:20:47 mriedem if so, then sure

Earlier   Later