| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-06-15 | |||
| 14:25:56 | sean-k-mooney | i dont like the idea of duplicating the logic in the driver | |
| 14:27:14 | sean-k-mooney | these are the two main places we care about correct https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L9244= | |
| 14:28:27 | gibi | https://github.com/openstack/nova/blob/93a65f06df67ce39d65827692150c78013c7f6d5/nova/virt/libvirt/driver.py#L8530 | |
| 14:28:44 | gibi | we need to report the inventories in update_provider_tree | |
| 14:29:30 | sean-k-mooney | sorry wrong link https://github.com/openstack/nova/blob/93a65f06df67ce39d65827692150c78013c7f6d5/nova/compute/resource_tracker.py#L1221-L1232= | |
| 14:32:37 | gibi | sean-k-mooney: so you suggest to pass down the pci_tracker to the update_provider_tree? | |
| 14:35:00 | sean-k-mooney | yep | |
| 14:35:19 | sean-k-mooney | can catch the not implemted error and invoke without it if the driver does nto support it for one release | |
| 14:35:21 | sean-k-mooney | with a fixme | |
| 14:35:28 | sean-k-mooney | so that we dont rbeak out of tree drivers | |
| 14:35:52 | sean-k-mooney | for pci in placment we only are adding supprot for libvirt for now | |
| 14:36:00 | opendevreview | Alexey Stupnikov proposed openstack/nova stable/victoria: Test aborting queued live migration https://review.opendev.org/c/openstack/nova/+/845748 | |
| 14:36:13 | opendevreview | Alexey Stupnikov proposed openstack/nova stable/victoria: Add functional tests to reproduce bug #1960412 https://review.opendev.org/c/openstack/nova/+/845753 | |
| 14:36:31 | opendevreview | Alexey Stupnikov proposed openstack/nova stable/victoria: Clean up when queued live migration aborted https://review.opendev.org/c/openstack/nova/+/845754 | |
| 14:38:46 | sean-k-mooney | gibi: do you think that passing it down or a subset of data form it woudl be problematic | |
| 14:39:24 | gibi | it is different than the other resources like CPU of VGPU | |
| 14:39:29 | sean-k-mooney | gibi: the stat pools woudl be all we need | |
| 14:39:59 | sean-k-mooney | well the other way to do it si to import the code form the pci module | |
| 14:40:04 | sean-k-mooney | and filter the list of pci device with it | |
| 14:40:15 | sean-k-mooney | and thel look at all the instnace and there pci usage | |
| 14:40:23 | gibi | we are moving logic around a virt driver boundary so I'm affraid | |
| 14:40:43 | sean-k-mooney | actully for placment | |
| 14:41:08 | sean-k-mooney | we only need to know the set of pci devices and the whitelist | |
| 14:41:35 | sean-k-mooney | but i would be unfortable with using the current set form libvirt as if its passed into a guest i dont know if that will still be in the data form libvirt | |
| 14:42:08 | sean-k-mooney | its also more work to recompute it so i would be much more comfortabel geting the data form the pci tracker | |
| 14:42:16 | frickler | nova is running nova-live-migration-ceph and tempest-integrated-compute-centos-8-stream as non-voting jobs in gate, can someone have a look and clean that up? https://review.opendev.org/840833 | |
| 14:42:58 | frickler | (just noticed because both are failing and I was worried about "my" patch queued behind it) | |
| 14:42:59 | gibi | sean-k-mooney: I will look into passing the pci tracker down to the virt driver but I feel bad about it architecturally | |
| 14:43:17 | sean-k-mooney | well you have anohter option | |
| 14:43:28 | sean-k-mooney | wich is to implemnt this part of the update in the compute manager | |
| 14:44:22 | gibi | that would duplicate the reshape logic :/ | |
| 14:44:40 | gibi | ideally I would like to have all the similar thing is the same place | |
| 14:44:50 | gibi | so if GPU is handled in the virt driver level then PCI should be too | |
| 14:45:13 | gibi | but I'm not sure why we have a pci tracker in the manager level instead of in the virt level | |
| 14:45:32 | gibi | I mean I know a lot of reasons but I probably don't know all of them | |
| 14:45:45 | gibi | so moving the pci tracker down to the virt level is scary | |
| 14:45:57 | gibi | just passing it down will create coupling that is scary too | |
| 14:46:10 | gibi | <can I rewrite nova, please?> | |
| 14:47:12 | sean-k-mooney | well vGPU shoudl not be where it si now | |
| 14:47:39 | sean-k-mooney | and the pci module is ment to be shared across virt driver which is why its where it is | |
| 14:49:55 | gibi | so should there be a mdev_tracker in the resource tracker? | |
| 14:50:11 | gibi | and also a cpu/memory/disk tracker? | |
| 14:50:23 | gibi | then we would not need the update_provider_tree to run in the virt level | |
| 14:50:30 | gibi | it could run on the compute manager level | |
| 14:50:42 | sean-k-mooney | i wanted to track mdevs in the pci tracker or in the resouces table | |
| 14:50:42 | gibi | but for some reasons we introduced update_provider tree down in the virt level | |
| 14:50:49 | sean-k-mooney | that we use for pmem | |
| 14:50:55 | sean-k-mooney | we have multipel resouce tracker in nova already | |
| 14:51:17 | sean-k-mooney | cpus and memory are traced in the hsot numa toplogy blob vai the hardware.py module | |
| 14:51:33 | sean-k-mooney | pmem uses the resouces table | |
| 14:51:41 | sean-k-mooney | pci has its own tracker | |
| 14:51:50 | sean-k-mooney | and medev use the libvirt xml domain files | |
| 14:51:55 | sean-k-mooney | which is proably the worst of them | |
| 14:52:18 | gibi | so when numa (if ever) will be in placement then the cpu inventory reporting in update_provider_tree need to be moved also to the compute manager level? | |
| 14:52:43 | gibi | as most of the numa tracking is in the resource tracker not in the virt level | |
| 14:52:56 | sean-k-mooney | prbably | |
| 14:53:06 | gibi | so this shows that the concept of update_provider_tree is wrong | |
| 14:53:09 | sean-k-mooney | the resouce tracker is were most of the tackign happens | |
| 14:53:11 | gibi | it cannot really update anything | |
| 14:53:22 | sean-k-mooney | well we can compute it | |
| 14:53:28 | sean-k-mooney | the virt driver has the list of cores | |
| 14:53:32 | sean-k-mooney | and the list of instnaces | |
| 14:53:54 | sean-k-mooney | actully its simpelr then that | |
| 14:54:07 | sean-k-mooney | for cpus we jsut need cpu_share_set and cpu_dedicate_set | |
| 14:54:20 | sean-k-mooney | to define the capstiy | |
| 14:54:28 | sean-k-mooney | and all the traits are provided by the virt dirver | |
| 14:54:38 | sean-k-mooney | the same is technially true for pci devices | |
| 14:54:47 | gibi | yepp | |
| 14:54:54 | sean-k-mooney | so you can just import the pci module | |
| 14:55:00 | sean-k-mooney | and pass it the set of hypervior devices | |
| 14:55:06 | sean-k-mooney | and ask it for the set of pools | |
| 14:55:22 | sean-k-mooney | then use the pools to do the update | |
| 14:55:29 | sean-k-mooney | but that is duplciationg the data we already have | |
| 14:55:39 | sean-k-mooney | so it just comes down to efficnecy | |
| 14:56:03 | gibi | in my mind it comes down to coupling today the virt driver does not couple to the pci tracker at all but tomorrow it will | |
| 14:56:17 | gibi | it increase complexity | |
| 14:56:49 | sean-k-mooney | right so really the reosuce tracker and pci tracker are ment to be the source of truth for the resouce that are avlaibel | |
| 14:56:56 | sean-k-mooney | and the virt driver just provide the raw resoucs | |
| 14:57:01 | sean-k-mooney | btu that is not how it works today | |
| 14:57:19 | sean-k-mooney | we have the virt driver direclty updateign the tree | |
| 14:57:23 | gibi | yepp | |
| 14:57:51 | sean-k-mooney | so that is backwards architularly but we did it because how that tree will like is virt dirver depentent because ironic | |
| 14:58:14 | sean-k-mooney | well ironic vmware ectra they look differnt then libvirt or hyperv | |
| 14:58:29 | sean-k-mooney | e.g. clsuterd vs 1:1 drivers | |
| 14:58:51 | gibi | hm it is differnt per driver yes | |
| 14:59:02 | gibi | so that is baad abstraction | |
| 14:59:11 | sean-k-mooney | currently yes | |
| 14:59:21 | sean-k-mooney | if you jsut want to resue the code form the pci module that is fine | |
| 14:59:26 | gibi | we have the generic nova scheduler code that depends on the tree but the tree if virt driver dependent | |
| 14:59:28 | sean-k-mooney | i dont really think we shoudl port it | |
| 14:59:51 | gibi | I will play around more before I decide | |
| 14:59:56 | sean-k-mooney | ack | |
| 15:00:10 | gibi | thanks for talking to me about it | |
| 15:00:16 | sean-k-mooney | gibi: i will be happy to review working code | |
| 15:00:19 | sean-k-mooney | what ever form that takes | |
| 15:00:19 | gibi | sorry If I was sooo negative | |
| 15:00:34 | gibi | shit meeting :/ | |
| 15:00:44 | gibi | I almost made progress today :D | |
| 15:06:41 | opendevreview | Merged openstack/nova stable/yoga: Allow claiming PCI PF if child VF is unavailable https://review.opendev.org/c/openstack/nova/+/840833 | |
| 15:53:49 | dansmith | gibi: sean-k-mooney bauzas: glance is planning to make a change to how image locations are updated (for the tight integration with ceph) | |