| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-11-02 | |||
| 17:51:45 | bauzas | dansmith: I definitely appreciate your help on that one | |
| 17:52:00 | bauzas | and I really want to give you review credits back on your own series | |
| 17:57:51 | sean-k-mooney | dansmith: i have some concerns with allow config with out pci adresses | |
| 17:58:11 | sean-k-mooney | i.e. if you allow some of your gpus to be used for pci passthogu and other for vgpu | |
| 17:58:36 | dansmith | right, that's a good reason *to* do it, but not a good reason to *require* it, IMHO | |
| 17:58:59 | sean-k-mooney | if you dont use device_address in that case we coudl passthough a device (that was allowed in the pci devspec) but not disalloed in the mdev config section | |
| 17:59:37 | sean-k-mooney | dansmith: i dont know i always felt that its a psudo security isseu to not explcitly list devices we can expose to a guest | |
| 17:59:45 | sean-k-mooney | i get that it sucks to need to be explict | |
| 17:59:54 | sean-k-mooney | but that the trade off | |
| 18:00:40 | dansmith | but in the mdev case you're not just blindly exposing them, | |
| 18:00:42 | sean-k-mooney | to be clear we have this tradeoff in the pci tracker too | |
| 18:00:50 | dansmith | you've already opted into "all devices with this mdev type" right? | |
| 18:01:12 | sean-k-mooney | yep so its the same as just putting vendor_id and product_id in the pci dev_spec | |
| 18:01:22 | sean-k-mooney | aka the pci whitelist | |
| 18:01:28 | dansmith | I don't think that's the same really | |
| 18:01:45 | opendevreview | Stephen Finucane proposed openstack/nova master: docs: Add documentation on server groups https://review.opendev.org/c/openstack/nova/+/899979 | |
| 18:01:46 | dansmith | well, if you do both vendor *and* product I guess it is | |
| 18:01:55 | sean-k-mooney | to me there is no diffence betwen mdev type and *vendor_id and product_id) | |
| 18:01:56 | dansmith | but not like worrying that 8086 is going to allow your root bridge to go through or something | |
| 18:02:16 | dansmith | but I really don't see the problem with that level of specificity | |
| 18:02:34 | stephenfin | bauzas: melwitt: https://review.opendev.org/c/openstack/nova/+/899979 came up as a result of an OpenShift question. I figure you'd both be interested in that addition to our docs | |
| 18:03:03 | sean-k-mooney | dansmith: i think its more an issue with how the config options are arranged more then anything esle | |
| 18:03:08 | stephenfin | I've also proposed helper alias for OSC to make requesting server groups slightly nicer https://review.opendev.org/c/openstack/python-openstackclient/+/899975 | |
| 18:03:12 | stephenfin | *a helper | |
| 18:03:36 | sean-k-mooney | dansmith: we split the mdevs into multipel dynmaic config sections to influcne this | |
| 18:03:45 | bauzas | sean-k-mooney: if we can ease your pain, we can document the fact that max_instances automatically passes *all* GPUs to nova | |
| 18:03:48 | sean-k-mooney | which to me make it harder to know if a device will be allowed or not | |
| 18:04:31 | bauzas | sean-k-mooney: and we could mention to use device_addresses as an alternative for people afraid of passing the whole GPUs | |
| 18:04:37 | sean-k-mooney | bauzas: just make -1 or None the default and use it as a sentanical to mean all. | |
| 18:04:55 | bauzas | lemme provide the patch first | |
| 18:05:08 | sean-k-mooney | bauzas: well i thing we shoudl be recommending device_address as the way peopel shoudl do it in general | |
| 18:05:24 | dansmith | sean-k-mooney: that doesn't work right? that gives us the wrong behavior we have now | |
| 18:05:56 | sean-k-mooney | dansmith: it doesn but i think bauzas was going to have it be wrong out of the box too | |
| 18:06:35 | sean-k-mooney | unless i missunderstood | |
| 18:06:38 | bauzas | tbc I was documenting in the config doc helper that max_instances should be set to the number of mediated devices a device can create | |
| 18:07:24 | sean-k-mooney | but where you going to recommend usign device_addresses as what peopel shoudl do by default? | |
| 18:07:53 | bauzas | we could | |
| 18:08:23 | sean-k-mooney | personally i woudl prefer reuqiring you do do "device_adddress=*" to have use any device with this mdev type behavior by the way | |
| 18:08:42 | sean-k-mooney | or saying you must set | |
| 18:08:51 | sean-k-mooney | either device_addres or max_instances | |
| 18:10:39 | bauzas | yeah those options are conflicting, for sre | |
| 18:10:58 | bauzas | I can mark those this way with oslo.config | |
| 18:11:19 | sean-k-mooney | but can you also make the mutally exclive group required | |
| 18:11:29 | sean-k-mooney | and the dynmic config section requried | |
| 18:12:01 | bauzas | we're on same page | |
| 18:12:32 | sean-k-mooney | well there is one thing im not sure you and i agree on | |
| 18:12:33 | bauzas | [mdev_nvidia-XXX] would become mandatory if we merge https://review.opendev.org/c/openstack/nova/+/899406 | |
| 18:12:40 | sean-k-mooney | ah | |
| 18:12:42 | sean-k-mooney | ya that | |
| 18:12:54 | bauzas | what's inside this section is mutually exclusive | |
| 18:13:04 | bauzas | either device_addresses or max_instances | |
| 18:13:07 | bauzas | wfy ? | |
| 18:13:17 | sean-k-mooney | so your ok with requiring that if an mdev type is in enabled_mdev_types | |
| 18:13:22 | sean-k-mooney | we must have the config section | |
| 18:13:32 | bauzas | that's what we agreed at the PTG | |
| 18:13:34 | sean-k-mooney | [mdev_<mdev name>] | |
| 18:13:48 | sean-k-mooney | ok then ya with that condition i think im good with your propsoal | |
| 18:13:57 | bauzas | in order to ease people's lifes, I'm just gonna add max_instances as a mutually exclusive option | |
| 18:14:29 | bauzas | if you're SR-IOV based, we'll pick N VFs for you | |
| 18:14:40 | bauzas | if you're not SRIOV, we'll just pick N devices for you | |
| 18:15:09 | melwitt | stephenfin: cool, thanks. I'll check it out | |
| 18:15:53 | sean-k-mooney | bauzas: we could just make max_instances apply to sriov case | |
| 18:16:04 | sean-k-mooney | since the non sriov case just works | |
| 18:16:35 | bauzas | with the non-sriov gpus, you need to specify all the cards | |
| 18:16:49 | sean-k-mooney | in the device list | |
| 18:16:52 | bauzas | giving them a way to say "I don't care" | |
| 18:16:58 | bauzas | seems good to me | |
| 18:17:07 | sean-k-mooney | ya so just add * to device_addreses | |
| 18:17:34 | sean-k-mooney | well im flexible on how we do that | |
| 18:18:09 | bauzas | we need implementation patches and then we could do anything we want | |
| 18:18:09 | sean-k-mooney | im ok to have a away to say "expose all devices of this type" i just woudl like it to be a little more explict then it is today | |
| 18:18:28 | bauzas | btw. I'm on PTO tomorrow | |
| 18:18:29 | sean-k-mooney | right os if we want to defer the details of how we do that for now | |
| 18:18:31 | sean-k-mooney | that works for me | |
| 18:18:41 | sean-k-mooney | ya im on pto tomorrow and monday | |
| 18:18:59 | bauzas | cool | |
| 18:19:10 | bauzas | I'll try if I can draft something before I leave | |
| 18:19:37 | bauzas | now that I have a machine, this is waaaay easier than before | |
| 18:19:57 | sean-k-mooney | yep hardware both sucks and make life simpler | |
| 18:36:04 | bauzas | sean-k-mooney: I made the wrong assumption, for non-sriov, max_instances is indeed meaningless | |
| 18:36:26 | sean-k-mooney | cool | |
| 18:38:09 | bauzas | so yeah we maybe need a wilcard | |
| 18:38:34 | bauzas | that said, we captured the opposite https://etherpad.opendev.org/p/nova-caracal-ptg#L669 | |
| 18:38:57 | bauzas | but I recently discovered that every single day is full of adventures and news when it comes to nvidia | |
| 19:44:34 | opendevreview | sean mooney proposed openstack/nova master: add pyproject.toml to support pip 23.1 https://review.opendev.org/c/openstack/nova/+/899753 | |
| 20:23:21 | pas-ha[m] | <bauzas> "dansmith: fwiw, there is also..." <- bauzas: I don't think I force anything with this patch, it should be 100% backward compatible - if no explicit devices are defined together with the single enabled mdev type, everything should work as before. | |
| 20:23:48 | pas-ha[m] | at least that was my intention, not sure I actually tested for that :-) | |
| 20:24:38 | sean-k-mooney | with a single mdev type bu tif you had 2 or more then the config section would be requried for each correct | |
| 20:26:14 | sean-k-mooney | hum so https://review.opendev.org/c/openstack/nova/+/899406/1/nova/virt/libvirt/driver.py#7933 | |
| 20:26:27 | sean-k-mooney | is just always regestring the config options | |
| 20:26:33 | sean-k-mooney | but not nessiarly enforicing it | |
| 20:26:53 | sean-k-mooney | because you still defalt it here https://review.opendev.org/c/openstack/nova/+/899406/1/nova/virt/libvirt/driver.py#7958 | |
| 20:27:34 | sean-k-mooney | pas-ha[m]: so ya i think your version is tecnically still backward compatible | |
| 20:28:33 | sean-k-mooney | pas-ha[m]: the fact your chagne has not unit test chagne means either its backward compatiable or we have no test coverage for this | |
| 20:28:59 | pas-ha[m] | sean-k-mooney: I bet the latter :-D | |
| 20:29:12 | sean-k-mooney | unfortunetly you might be right | |
| #openstack-nova - 2023-11-03 | |||
| 06:45:24 | bauzas | pas-ha[m] : correct, you're right my bad, we are not changing the behavior for operators not using the groups, we just do this for those who want to use the groups while they only use one type | |
| 08:02:30 | bauzas | folks I'm off today see you on Monday | |
| 14:20:17 | dansmith | melwitt: have you seen vnc console test fails recently? | |
| 14:20:32 | dansmith | melwitt: I've seen this a couple times now: https://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_fa0/899375/5/gate/nova-grenade-multinode/fa011a2/testr_results.html | |