| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-10-18 | |||
| 13:09:39 | bauzas | cool | |
| 13:39:31 | bauzas | dansmith: https://paste.opendev.org/show/bBKFG3Vg7YTQBHPz11Za/ (at the bottom) is what you need to do in order to configure nova to use mtty | |
| 13:40:07 | bauzas | tl;dr: instead of specifying a pci address for the type named 'mtty-1', you just write 'mtty' | |
| 13:40:41 | sean-k-mooney | ya so i woudl be happy with sbauza-dev2_computer being sbauza-dev2_mtty | |
| 13:40:41 | bauzas | but in order to pass the pci address check we have, I hardcoded that string (plus the other module names) in https://review.opendev.org/c/openstack/nova/+/898100/2/nova/virt/libvirt/driver.py#245 | |
| 13:41:10 | bauzas | and sean-k-mooney had concerns about hardcoding that list | |
| 13:41:18 | bauzas | which I can understand | |
| 13:41:37 | sean-k-mooney | well partly becasue you dont even supprot all the modules that dan is compiling in the plugin | |
| 13:41:58 | dansmith | we can make up that list from looking in sysfs right? | |
| 13:42:04 | sean-k-mooney | so at a minium i want all of them to be valid but i woudl prefer it to be a config option in the [devices] section | |
| 13:42:14 | sean-k-mooney | dansmith: yes we can do that too | |
| 13:42:22 | dansmith | I guess if the operator might want to limit some devices to not be picked up by nova, then there's a legit reason to have a conf option | |
| 13:42:41 | bauzas | sean-k-mooney: ack I can try to change https://review.opendev.org/c/openstack/nova/+/898100/2/nova/virt/libvirt/driver.py#8065 for not using the "parent" name | |
| 13:42:43 | dansmith | I'm just a bit over having to config everything by hand that we should be able to discover | |
| 13:42:59 | bauzas | me too | |
| 13:43:08 | sean-k-mooney | dansmith: so unless we enable an mdev type form the device | |
| 13:43:09 | bauzas | so basically I let you guys decide | |
| 13:43:16 | sean-k-mooney | we shoudl not be reporting the mtty devices anyway | |
| 13:43:23 | sean-k-mooney | so im not sure we need to configure things at all | |
| 13:43:59 | sean-k-mooney | basically if you dont have | |
| 13:44:03 | sean-k-mooney | [devices] | |
| 13:44:04 | sean-k-mooney | enabled_mdev_types = mtty-1 | |
| 13:44:06 | sean-k-mooney | [mdev_mtty-1] | |
| 13:44:08 | sean-k-mooney | device_addresses = mtty | |
| 13:44:19 | sean-k-mooney | then the mtty rp shoudl not be created right | |
| 13:44:20 | bauzas | so, shall we just do an exec on lsmod and see whether mtty is loaded ? | |
| 13:44:22 | dansmith | ah right right | |
| 13:44:29 | dansmith | bauzas: don't be silly | |
| 13:44:31 | bauzas | yeah, correct | |
| 13:44:31 | sean-k-mooney | bauzas: hell no | |
| 13:44:50 | bauzas | about the fact that no RPs are created until you configure nova.confg | |
| 13:44:59 | sean-k-mooney | do me the fact device_addresses is not a pci adress shoudl be all you need | |
| 13:45:06 | dansmith | sean-k-mooney: so discover the parents but still obviously let them configure them specifically | |
| 13:45:07 | sean-k-mooney | no need for hardcoded sentineal | |
| 13:45:29 | dansmith | configure the devices specifically I mean | |
| 13:45:30 | bauzas | sean-k-mooney: if we do this, we would remove the pci check and let anyone do fatfingering | |
| 13:45:40 | bauzas | I'm not sure I like that | |
| 13:46:05 | bauzas | https://review.opendev.org/c/openstack/nova/+/898100/2/nova/virt/libvirt/driver.py#7974 is where we parse config | |
| 13:46:31 | sean-k-mooney | what i was orginally thinkgig is as follows | |
| 13:46:41 | sean-k-mooney | if device_addresses does not container a pci adress | |
| 13:46:52 | dansmith | being only able to handle pci devices seems too specific to me anyway | |
| 13:48:19 | bauzas | dansmith: so you'd be in favor of lifting this all ? | |
| 13:48:38 | sean-k-mooney | check if the value of device_addresses is in /sys/devices/virtual/<device_address> | |
| 13:48:50 | bauzas | tbc, there are some assumptions where we say that we have a PCI address | |
| 13:49:14 | bauzas | sean-k-mooney: ah, good idea | |
| 13:49:34 | sean-k-mooney | if that folder does not exsit we can raise that as a config error in init_host | |
| 13:49:44 | bauzas | that's a readonly call, we don't need to privsep call it | |
| 13:50:02 | bauzas | and that's a cheap I/O call only made at startup | |
| 13:50:09 | bauzas | wfm so | |
| 13:50:16 | sean-k-mooney | ya its listraly just a file exist check | |
| 13:50:21 | dansmith | bauzas: but is that really ideal? I mean, it's a bit of a reach, but there are other busses on other systems, and I can totally imagine this sort of virt-friendly hardware someday exposing something totally different there | |
| 13:50:44 | dansmith | (that == being so tied to pci addresses specifically) | |
| 13:50:45 | sean-k-mooney | dansmith: you are correct that mdev does not require pci device parents | |
| 13:51:00 | bauzas | dansmith: I don't disagree with you long-term, but fwiw, the only vfio-mdev devices I've seen rely on being PCI addresses | |
| 13:51:02 | sean-k-mooney | that is an atritifal limiation in novas code base today to simplfy the code | |
| 13:51:12 | dansmith | yeah | |
| 13:51:32 | bauzas | so I'd please defer that PCI untanglement until some real hardware comes up which isn't PCI based | |
| 13:51:39 | bauzas | serial or whatever it would be | |
| 13:51:44 | sean-k-mooney | bauzas: tehre are once that use cpx? compute express link what ever the acfronum is | |
| 13:51:56 | sean-k-mooney | ah CXL | |
| 13:51:58 | dansmith | so the plan is to stick to pci addresses for now, but validate them by checking in sysfs and tolerate non-pci things if they're in /virtual/<addr> ? | |
| 13:52:35 | bauzas | sean-k-mooney: I haven't yet asked to add CXL into nova and I'll stick with playing the ostrich for now :) | |
| 13:52:44 | bauzas | dansmith: yeah | |
| 13:52:46 | sean-k-mooney | actully my orgianll proposal a few days ago was for device_adress to be set to the subpath under /sys/devices | |
| 13:52:58 | sean-k-mooney | so that it woudl work in any buss | |
| 13:53:29 | sean-k-mooney | bauzas: i have talked about CXL for nova several times and we have also said not yet | |
| 13:53:30 | bauzas | only for mediated devices | |
| 13:53:38 | dansmith | sean-k-mooney: ah yeah that's interesting | |
| 13:53:46 | bauzas | mdev capable devices won't show up in /sys/devices | |
| 13:54:05 | sean-k-mooney | bauzas: they will under /sys/devices/pci/ | |
| 13:54:17 | bauzas | ah yeah you're right | |
| 13:54:37 | bauzas | yay, another good idea worth implementing it someday | |
| 13:55:07 | sean-k-mooney | so bacially for now i think the minimal change would be "if not a pci adress assume its a virual device" | |
| 13:55:41 | sean-k-mooney | in the futre we can add a sysfs check if we want, but if we plan do do that in the future we shoudl consider upgraades now | |
| 13:55:43 | bauzas | I can try to write it the way it prepares ourselves to admit that pci devices are just a subset of all the devices that can create mdevds | |
| 13:56:17 | bauzas | sean-k-mooney: I think reading sys https://review.opendev.org/c/openstack/nova/+/898100/2/nova/virt/libvirt/driver.py#7975 is cheap and worthwhile | |
| 13:56:59 | sean-k-mooney | bauzas: it is an you can resue the funciton for the cpu ofline fucntions | |
| 13:57:12 | sean-k-mooney | bauzas: i also suggested this a few days ago | |
| 13:57:51 | bauzas | yup, and there are multiple things we can do this cycle about how we do the liaison | |
| 13:58:10 | bauzas | I'm still having this enhancement on me about using libvirt for creating the mdev | |
| 13:58:43 | sean-k-mooney | speaking of which i think its fair to say htat after the ptg wheree we should discuss some of this we should likely capture the plan in a spec | |
| 13:59:54 | sean-k-mooney | but for sysfs reading in genal plwad use https://github.com/openstack/nova/blob/master/nova/filesystem.py#L28 if you do implement this | |
| 14:00:05 | bauzas | sean-k-mooney: yup, I'll | |
| 14:01:02 | sean-k-mooney | besided the fact this should have a spec, the main reason i want one is to defien the scoep and what will be doen in which order and wthat will eb defered to a future release | |
| 14:01:14 | sean-k-mooney | partly so we dont forget soemthing | |
| 14:02:36 | bauzas | don't disagree | |
| 14:11:06 | dansmith | sean-k-mooney: bauzas I didn't save a link to the bug filed yesterday about the upgrade check, but was there any discussion there about revert vs. change to warning? | |
| 14:11:39 | bauzas | dansmith: I very briefly looked at your convo this morning, can you explain it again please ? | |
| 14:11:52 | bauzas | iirc, it was because of non-slurp | |
| 14:11:56 | dansmith | no | |
| 14:12:23 | dansmith | bauzas: basically sean-k-mooney strong-armed me into adding a status check for comptue-object-ids in this last cycle, which raised an error if you hadn't completed the upgrade (which you wouldn't have yet anyway) | |
| 14:12:59 | dansmith | I begged for mercy but there was no mercy given so I added the check and then an op showed up yesterday wondering why I had ruined his day | |
| 14:13:06 | dansmith | (I may be exaggerating here) | |
| 14:13:24 | dansmith | anyway, the status check really should be *next* cycle saying "you haven't finished your homework from last time" | |
| 14:13:52 | dansmith | so we have two options: 1. just revert out the check entirely from 2023.2 or 2. convert it to a warning result | |
| 14:14:20 | sean-k-mooney | fyi https://paste.opendev.org/show/bGuYFgfWt70oGOHcAuH9/ | |
| 14:14:28 | sean-k-mooney | it looks like its working for me too | |
| 14:14:37 | sean-k-mooney | but i have not tried to boot any vms with those mdevs yet | |
| 14:15:37 | sean-k-mooney | i have allcoated 4 mdevs in this case | |
| 14:15:38 | bauzas | sean-k-mooney: I went down to create and even sshing to the cirros guest, the serial was present | |