| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-12 | |||
| 14:56:53 | cdent | i'd be pretty opposed to include the name of the rp in the allocation unless we really can't come up with something | |
| 14:56:54 | efried | bauzas: Yes, that would be another "solution" that would be ew. | |
| 14:56:59 | cdent | why _not_ pass the summaries? | |
| 14:57:16 | cdent | or why not let the virt driver talk to placement? | |
| 14:57:18 | NewBruce | sean-k-mooney : back | |
| 14:57:26 | cdent | or any of several other options | |
| 14:57:34 | bauzas | I'll upload my change anyway | |
| 14:57:35 | efried | cdent: not pass the summaries because a) way too much information, unless we pare it down to just the summaries in the allocation, and b) the name isn't in the summaries anyway :P | |
| 14:58:07 | cdent | b) right, but putting it in the summaries would make some form of sense: it's a provider _summary_ after all | |
| 14:58:09 | cdent | however | |
| 14:58:18 | cdent | using meaningful names is a really super dangerous territory | |
| 14:58:20 | gibi | bauzas, efried: this is the PCI - RP mapping solution I come up with https://review.openstack.org/#/c/623543/23/nova/compute/manager.py It works as the pci claim can match devices by abitrary tag | |
| 14:58:30 | efried | cdent: we need some way of mapping RP to "real thing". | |
| 14:58:42 | cdent | which isn't placements job | |
| 14:58:42 | efried | I know: arbitrary metadata on resource_provider! | |
| 14:59:00 | cdent | mapping rp to real thing is the either nova or the virt drivers job | |
| 14:59:19 | efried | gibi: Right, so you're storing the mapping locally in memory. | |
| 14:59:41 | gibi | efried: calculating the mapping by calling placement during boot | |
| 14:59:51 | gibi | efried: nothing stored between boots | |
| 14:59:52 | efried | cdent: Which is what I suggested initially as the workaround. I though it was ew, but I'm kind of coming around to the idea. | |
| 15:00:10 | efried | you don't need to store anything between boots; you construct the information in update_provider_tree in any case. | |
| 15:00:15 | efried | that's where you would save the mapping. | |
| 15:00:22 | efried | you can't spawn untile you've update_provider_treeD | |
| 15:00:25 | bauzas | gibi: you're calling placement to get the RP name, right? | |
| 15:00:33 | gibi | bauzas: yes | |
| 15:00:47 | efried | Could pass the provider_tree into spawn. That would let you look up the name. | |
| 15:00:49 | gibi | bauzas: https://review.openstack.org/#/c/623543/23/nova/compute/manager.py@2138 | |
| 15:01:15 | bauzas | gibi: yeah that's what I saw | |
| 15:01:28 | bauzas | it wasn't a question actually, more a clarification :) | |
| 15:01:36 | bauzas | gibi: but here I'm working on the driver side | |
| 15:01:46 | bauzas | I'm not sure I have the report client | |
| 15:01:54 | efried | would have to be a read-only/copy of the provider tree. | |
| 15:01:55 | bauzas | and I think we basically said no | |
| 15:01:55 | gibi | bauzas: I know, you have to pass some extra info to the driver anyhow | |
| 15:02:16 | bauzas | I mean, we agreed on not having the virt driver calling placement for various reasons | |
| 15:02:26 | efried | yeah | |
| 15:02:31 | openstackgerrit | Boris Bobrov proposed openstack/nova master: Add configs for AMD SEV https://review.openstack.org/636318 | |
| 15:02:41 | bauzas | so, I'm stuck with finding a way to pass down the providertree structure down the wire | |
| 15:02:55 | efried | bauzas: You have the provider tree in the compute manager. | |
| 15:02:59 | bauzas | just because I want to know which physical resource is there | |
| 15:03:13 | efried | no need to make calls to placement. | |
| 15:03:18 | kashyap | time git fetch gerrit | |
| 15:03:31 | bauzas | efried: correct, I could amend https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L3106 | |
| 15:04:08 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: [WIP] libvirt: Bump MIN_{LIBVIRT,QEMU}_VERSION for "Stein" https://review.openstack.org/632507 | |
| 15:04:09 | efried | bauzas: Right. Add provider_tree. Get it the same way we get it before update_provider_tree (i.e. make a copy of it). | |
| 15:04:39 | bauzas | there could be race conditions, right? | |
| 15:04:58 | efried | Not that wouldn't cause blowups anyway. | |
| 15:05:27 | efried | E.g. if I remove the pgpu rp between allocating and spawning, I'm effed in any case. | |
| 15:06:37 | gibi | efried: but you cannot do that without removing the allocation as well, which breaks nova as that allocation is created by nova | |
| 15:07:07 | efried | gibi: Swhat I'm saying, I don't see how you could contrive a race condition that would make the provider_tree be out of sync with the allocations without already having broken something else in the process. | |
| 15:07:18 | gibi | efried: I agree | |
| 15:07:32 | efried | I mean, some other unrelated provider could be out of sync I suppose. But I would hope that wouldn't matter. | |
| 15:07:39 | bauzas | efried: to be honest, I don't care of allocations | |
| 15:07:45 | bauzas | efried: I only care of the RP tree | |
| 15:07:53 | efried | well, you need the allocation to determine the rp | |
| 15:08:01 | bauzas | it's already there, and I trust it | |
| 15:08:30 | efried | And if you have more than one vgpu allocated, you need to know that. | |
| 15:08:41 | bauzas | ideally, it'd be waaaaay better and cleaner if I was getting the RP structure from placement as part of the message it returns me | |
| 15:08:42 | efried | I assume/hope you're getting that information from the allocation and not from somewhere else. | |
| 15:08:56 | bauzas | because my cache could be stale | |
| 15:09:04 | efried | bauzas: But what do you mean by "structure"? | |
| 15:09:28 | efried | It doesn't help you simply to know that the provider of the vgpu resource is a "child". You will eventually have to know *which* child. | |
| 15:09:30 | bauzas | efried: the mdev allocation logic is made there https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6251 | |
| 15:09:36 | bauzas | efried: right | |
| 15:09:45 | bauzas | efried: I want to ideally know the PCI device | |
| 15:09:54 | bauzas | but this mapping is under my responsibility | |
| 15:10:03 | efried | right | |
| 15:10:07 | bauzas | so I need to know which RP is allocated | |
| 15:10:17 | bauzas | and then map the PCI ID thanks to the convention | |
| 15:10:30 | efried | Well, you know which *RP* is allocated. But you don't know its name, only its UUID. | |
| 15:10:44 | bauzas | right, that's what we're discussing since 15 mins | |
| 15:10:53 | bauzas | well, 20 even | |
| 15:11:12 | efried | Right, I think we've come full circle. If you had the provider_tree, you would be sorted. Otherwise you need some other way to maintain a mapping. | |
| 15:11:24 | efried | jaypipes: once you've caught up: Would you support adding a provider_tree arg to spawn()? | |
| 15:11:58 | bauzas | I have one thought | |
| 15:12:06 | bauzas | we always push information down to the driver | |
| 15:12:18 | bauzas | can we have the virt driver getting info from the service ? | |
| 15:12:23 | bauzas | I mean a getter | |
| 15:12:48 | bauzas | I don't like changing our interfaces (at least spawn()) for a very specific usecase | |
| 15:13:04 | bauzas | and 99% of deployments won't care of VGPUs | |
| 15:13:24 | bauzas | but I'm not a virt driver specialist | |
| 15:14:22 | efried | you mean introducing a protected static final ComputeDriver.get_provider_tree() method? | |
| 15:14:40 | bauzas | I dunno, thinking out loud | |
| 15:14:51 | gibi | bauzas: for pci we have InstancePciRequest for NUMA we have InstanceNumaTopology for VGPU we have no object to hold this kind of information | |
| 15:14:58 | efried | hell, update_provider_tree could save off its provider tree as an instance var before it returns | |
| 15:15:06 | bauzas | snap, I need to taxi my kids | |
| 15:15:22 | bauzas | efried: that's called a cache | |
| 15:15:35 | efried | so it would be like the virt driver saving a mapping, except it doesn't need to do any special work to construct it - it's already there. | |
| 15:15:37 | bauzas | efried: and we know it can be a problem | |
| 15:15:48 | bauzas | I need to think about this | |
| 15:16:01 | bauzas | anyway, I need to drop for a couple of hours | |
| 15:16:09 | bauzas | \o | |
| 15:20:14 | jaypipes | efried, bauzas, gibi, cdent: I have said for over a year that the virt driver will need to keep a mapping of the RP UUID to whatever name it uses to refer to that RP internally (mdev, PCI address, whatever the heck IBM does, etc) | |
| 15:20:29 | cdent | yup | |
| 15:21:03 | cdent | virt driver's job | |
| 15:21:12 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: [WIP] libvirt: Bump MIN_{LIBVIRT,QEMU}_VERSION for "Stein" https://review.openstack.org/632507 | |
| 15:22:59 | gibi | jaypipes: while I agree it is a bit more complicate if the RP is created outside of the virt driver like when neutron and cyborg creates the RP but the virt driver consumes the RP | |
| 15:23:20 | gibi | clearly for VGPU your suggestion works | |
| 15:24:59 | sean-k-mooney | gibi: well in the cyboge/neutron case nova does not need to consomne the allocation | |
| 15:25:22 | sean-k-mooney | it just needs to pass it back to cyborg/neutorn so can they do the transaltion then | |