| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-12 | |||
| 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 | gibi | bauzas: I know, you have to pass some extra info to the driver anyhow | |
| 15:01:55 | bauzas | and I think we basically said no | |
| 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 | |
| 15:25:44 | gibi | sean-k-mooney: for sriov ports, the virt driver still responsible to select the PCI VF | |
| 15:26:08 | sean-k-mooney | yes this is true but currently they are not in placement | |
| 15:26:08 | gibi | sean-k-mooney: and the PCI VF should come from the same RP | |
| 15:26:18 | gibi | as the bandwidth | |
| 15:26:22 | sean-k-mooney | the same RP as the bandwith i am assuming | |
| 15:26:24 | sean-k-mooney | ya | |
| 15:26:38 | gibi | so transitively the virt driver needs to know which RP is used for the bandwidth | |
| 15:27:10 | sean-k-mooney | well the bandwith woudl be an inventor on teh PF RP and we soudl similar have an inventory of VFs | |
| 15:28:05 | sean-k-mooney | so as long as the VF resust and bandwith request are in teh same requst group we shoudl be ok the issue is that would require 2 services to manage different inventoies in the same RP | |
| 15:28:13 | sean-k-mooney | which we dont support today | |
| 15:28:30 | gibi | sean-k-mooney: yeah, it is solved when VF inventory is moved to the placement | |
| 15:29:17 | sean-k-mooney | i personally had assumed that the resouce track would evolve to sotre the placement RP mappings | |
| 15:29:59 | efried | jaypipes: So that mapping has to be created in update_provider_tree (and could in fact *be* the provider tree created therein) and retrievable from spawn. | |
| 15:30:51 | efried | If we state it that way ^, then I guess we can leave it up to each virt driver how it wants to create and store the mapping. Shoving the provider tree into an instance var seems like as easy a way as any. | |
| 15:33:55 | jaypipes | efried: it could, yes. though there's nothing in what I said that *requires* it be that way. After all, the PCI device management code (gawd save us all), external Neutron agents (per gibi's series) and other things could easily keep a map of UUID -> internal name in whatever way they wanted. Shit, I believe I proposed at some point that a simple YAML file on disk storing the map of UUID -> internal device/provider name would be just fine | |
| 15:34:40 | openstackgerrit | Ralf Haferkamp proposed openstack/nova master: compute: reset instance events on reload https://review.openstack.org/420026 | |
| 15:43:45 | sean-k-mooney | jaypipes: the other option which im not a huge fan of would be just to set the service specific internal name in the RP name field | |
| 15:44:40 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support server create with ports having resource request https://review.openstack.org/636360 | |
| 15:44:40 | sean-k-mooney | jaypipes: that does not really work well in the general case and dose not work at all in the inter service case hence why im not a huge fan of that. | |
| 15:45:00 | kashyap | mdbooth: If you get some time, I'd really appreciate some libvirt driver and sensible eyes in general on this: https://review.openstack.org/#/c/632507/ | |
| 15:45:18 | kashyap | mdbooth: I know it not fully complete, but the serial / char dev code is tripping me up a bit. | |
| 15:46:01 | kashyap | (Funnily enough, for me, the bug that is referenced in the code that's being deleted ... was a bug I filed 5 years ago) | |
| 15:46:11 | sean-k-mooney | in the sriov case if the RP name was the PF adress neutron and nova would agree on it at least | |
| 15:46:59 | sean-k-mooney | but really that is just "bit stealing" rather and a convention rather then a dedicated mechanium | |
| 15:48:15 | jaypipes | sean-k-mooney: that's basically what gibi's doing in https://review.openstack.org/#/c/623543/23/nova/compute/manager.py@2138 | |