| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-14 | |||
| 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 | |
| 16:20:51 | bauzas | yeah | |
| 16:20:54 | mriedem | wfm | |
| 16:21:02 | bauzas | i'm done with complex cache invalidation mechanisms | |
| 16:21:13 | mriedem | are you SO done with them? | |
| 16:25:21 | bauzas | mriedem: /me tries to not think hard of AZs | |
| 16:25:34 | bauzas | or server groups | |
| 16:25:47 | mriedem | hey if you like azs i have some good news for you | |
| 16:25:48 | mriedem | https://review.openstack.org/#/c/509206/ | |