Earlier  
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 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
15:25:44 gibi sean-k-mooney: for sriov ports, the virt driver still responsible to select the PCI VF
15:26:08 gibi sean-k-mooney: and the PCI VF should come from the same RP
15:26:08 sean-k-mooney yes this is true but currently they are not in placement
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 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:44:40 openstackgerrit Balazs Gibizer proposed openstack/nova master: Support server create with ports having resource request https://review.openstack.org/636360
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

Earlier   Later