| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-06-03 | |||
| 12:19:27 | sean-k-mooney | dont you love how we only support bash globs if the adress is a sting and only support regexs if its a dictionary | |
| 12:20:35 | gibi | nah, the self.is_physical_function handling is worst in my eyes | |
| 12:21:04 | gibi | we write that flag twice based on two different utility function reading the same sysfs location | |
| 12:21:28 | sean-k-mooney | heh of course we do | |
| 12:21:30 | gibi | but yes, that string / dict duality is close second | |
| 12:22:03 | sean-k-mooney | we did it because of the conflict between * in glob and regex meendnin ins * is .* in regex land | |
| 12:22:16 | sean-k-mooney | i just wish we used regex form from the start | |
| 12:22:33 | gibi | yepp I figured that regex was added later and blow up the picture | |
| 12:22:54 | gibi | and of course the whole devname special case is a pain | |
| 12:23:08 | sean-k-mooney | i would not mind devname if it worked for all devices | |
| 12:23:17 | sean-k-mooney | but the fact that its unreliable and only works for nic | |
| 12:23:20 | sean-k-mooney | sometimes | |
| 12:23:25 | sean-k-mooney | is why i hate it | |
| 12:24:29 | sean-k-mooney | once you are done with the placment work i do still think its worth revisigint our config format in a differnt discussion | |
| 12:25:17 | gibi | I agree | |
| 12:25:36 | gibi | I will try to untangle as much of the current parsing code as possible before I add the new things to it | |
| 12:25:37 | sean-k-mooney | like in general can oslo config supprot yaml or can we add a resouces.yaml for all the complext host level resouces | |
| 12:26:15 | sean-k-mooney | ok cool | |
| 12:26:36 | sean-k-mooney | the other thing to be aware of is how we parse physical_network | |
| 12:26:58 | gibi | I'm not there yet :) | |
| 12:27:00 | sean-k-mooney | speicifcliy we are relying on physical_network=null being converted to python None | |
| 12:28:10 | sean-k-mooney | so we are using the json parsing to differnceat between not set, physical_network=null and pyhsical_network="null" or physical_network="None" | |
| 12:28:19 | gibi | nice | |
| 12:28:22 | sean-k-mooney | all 4 of those have differnt meanings | |
| 12:28:39 | sean-k-mooney | the last 2 are just strings that are the name of a phsyical network in neutron | |
| 12:29:24 | sean-k-mooney | pyhical_network=null without quotes is converted to python None and that is used for hardware offloaded ovs with tunneld networks | |
| 12:29:30 | sean-k-mooney | and unset means this is not a nic | |
| 12:29:58 | gibi | I would like to give prizes to deployments naming there physnets None and null :D | |
| 12:30:17 | sean-k-mooney | hehe ya | |
| 12:30:37 | sean-k-mooney | but since the null vaule was not planned to be supported | |
| 12:30:48 | sean-k-mooney | and was a bug that was abused for hardware offloed ovs | |
| 12:30:56 | sean-k-mooney | its posible | |
| 12:32:56 | sean-k-mooney | https://bugs.launchpad.net/nova/+bug/1915282 | |
| 13:35:21 | dansmith | kashyap: so it seems like this heisenbug has receded into the background, even though only one package has changed in fedora since last week (that we install) | |
| 13:35:42 | dansmith | kashyap: so I'm going to formalize my devstack patch so we'll just always capture qemu coredmps so we're ready for next time | |
| 13:36:02 | kashyap | dansmith: Huh, the bugs from hell. Same story w/ that other detach thing. Although we sussed out a latent libvirt bug from it | |
| 13:36:26 | kashyap | dansmith: Yeah, thank you; I also saw that the same path works for Ubuntu as well - I checked, and by extension, Debian too | |
| 13:36:28 | dansmith | scary if that means users can hit them, but yeah :/ | |
| 13:36:41 | dansmith | kashyap: ah cool | |
| 13:36:57 | kashyap | dansmith: I commented w/ a link to evidence your DNM patch | |
| 13:37:06 | dansmith | cool thanks | |
| 14:03:33 | opendevreview | Artom Lifshitz proposed openstack/nova stable/wallaby: fake: Ensure need_legacy_block_device_info returns False https://review.opendev.org/c/openstack/nova/+/843678 | |
| 14:03:34 | opendevreview | Artom Lifshitz proposed openstack/nova stable/wallaby: Add a regression test for bug 1939545 https://review.opendev.org/c/openstack/nova/+/843702 | |
| 14:03:35 | opendevreview | Artom Lifshitz proposed openstack/nova stable/wallaby: compute: Ensure updates to bdms during pre_live_migration are saved https://review.opendev.org/c/openstack/nova/+/843680 | |
| 14:03:36 | opendevreview | Artom Lifshitz proposed openstack/nova stable/wallaby: fup: Make connection_info returned by CinderFixture unique per attachment https://review.opendev.org/c/openstack/nova/+/844594 | |
| 14:03:38 | opendevreview | Artom Lifshitz proposed openstack/nova stable/wallaby: fup: Assert state of connection_info during LM rollback in func tests https://review.opendev.org/c/openstack/nova/+/844595 | |
| 14:15:03 | opendevreview | Artom Lifshitz proposed openstack/nova stable/victoria: compute: Ensure updates to bdms during pre_live_migration are saved https://review.opendev.org/c/openstack/nova/+/843949 | |
| 14:15:04 | opendevreview | Artom Lifshitz proposed openstack/nova stable/victoria: fup: Make connection_info returned by CinderFixture unique per attachment https://review.opendev.org/c/openstack/nova/+/844598 | |
| 14:15:05 | opendevreview | Artom Lifshitz proposed openstack/nova stable/victoria: fup: Assert state of connection_info during LM rollback in func tests https://review.opendev.org/c/openstack/nova/+/844599 | |
| 15:19:09 | opendevreview | Artom Lifshitz proposed openstack/nova stable/ussuri: fake: Ensure need_legacy_block_device_info returns False https://review.opendev.org/c/openstack/nova/+/843950 | |
| 15:19:10 | opendevreview | Artom Lifshitz proposed openstack/nova stable/ussuri: Add a regression test for bug 1939545 https://review.opendev.org/c/openstack/nova/+/843951 | |
| 15:19:11 | opendevreview | Artom Lifshitz proposed openstack/nova stable/ussuri: compute: Ensure updates to bdms during pre_live_migration are saved https://review.opendev.org/c/openstack/nova/+/843952 | |
| 15:19:12 | opendevreview | Artom Lifshitz proposed openstack/nova stable/ussuri: fup: Make connection_info returned by CinderFixture unique per attachment https://review.opendev.org/c/openstack/nova/+/844606 | |
| 15:19:14 | opendevreview | Artom Lifshitz proposed openstack/nova stable/ussuri: fup: Assert state of connection_info during LM rollback in func tests https://review.opendev.org/c/openstack/nova/+/844607 | |
| 15:53:20 | gibi | sean-k-mooney: I just realized that there is antoher edge case in the PCI parsing, the remove managed feature allows PF address and VF product ID in the same device spec test_remote_managed_pf_raises | |
| 15:53:25 | gibi | https://github.com/openstack/nova/blob/ffb810e2ba2fdec9b2a881a88fa6d65cd32f8fa3/nova/tests/unit/pci/test_devspec.py#L501-L513 | |
| 15:57:15 | sean-k-mooney | gibi: that was a prexisting feature | |
| 15:57:21 | sean-k-mooney | they did not add it | |
| 15:57:44 | sean-k-mooney | that is how you whitelisted the VFs of a pf before we added teh glob and regex support | |
| 15:57:58 | sean-k-mooney | so thats been there since like icehouse | |
| 15:58:07 | gibi | it is pretty remote managed specific https://github.com/openstack/nova/blob/d86916360858daa06164ebc0d012b78d19ae6497/nova/pci/devspec.py#L315-L334 | |
| 15:58:29 | sean-k-mooney | no this work for any sriov device | |
| 15:58:39 | sean-k-mooney | they just added a test case for it | |
| 15:58:43 | gibi | I think what you are referring to is the ability to list PF and match VF, but the remote managed extends this to list PF with VF's product_id | |
| 15:59:20 | sean-k-mooney | so you used to be able to use the pf address and vf product id | |
| 15:59:22 | gibi | it has an if self._remote_managed: on top so this does not run for the rest of the caseas | |
| 15:59:32 | sean-k-mooney | and that would allow all the VFs of that pf to be used | |
| 15:59:36 | sean-k-mooney | but not the pf itself | |
| 15:59:49 | sean-k-mooney | just looking at the code you linked now | |
| 16:00:31 | gibi | in the generic case I only see PF address matching for VF devs, but no such logic for vendor / product matching | |
| 16:02:14 | sean-k-mooney | i think its this https://github.com/openstack/nova/blob/d86916360858daa06164ebc0d012b78d19ae6497/nova/pci/devspec.py#L254-L265= | |
| 16:02:20 | sean-k-mooney | but i would have tro look at this carfully | |
| 16:02:50 | sean-k-mooney | we do supprot suign adress=<pf addres> product_id=<vf id> | |
| 16:03:11 | gibi | nope, that is only matching addresses | |
| 16:03:20 | gibi | the PhysicalPciAddress has no info about the vendor / product | |
| 16:03:22 | sean-k-mooney | its match the adress object | |
| 16:03:32 | gibi | yepp | |
| 16:03:36 | gibi | only address matching happens there | |
| 16:04:05 | gibi | vendor / product happens independently https://github.com/openstack/nova/blob/d86916360858daa06164ebc0d012b78d19ae6497/nova/pci/devspec.py#L377-L378 | |
| 16:04:07 | sean-k-mooney | right but that is combiend with the vendro id and prodcut id filter elsewhere | |
| 16:06:28 | sean-k-mooney | yes it does so the way it works is we mach on vendor and product id first | |
| 16:06:41 | sean-k-mooney | and then we match on the adrees or the partent adress | |
| 16:06:51 | sean-k-mooney | https://github.com/openstack/nova/blob/d86916360858daa06164ebc0d012b78d19ae6497/nova/pci/devspec.py#L379-L380= | |
| 16:07:01 | melwitt | sean-k-mooney: I was wondering if you might be able to help see what's wrong with this failure, I think it might be related to ansible. I have written about it on L57 https://etherpad.opendev.org/p/nova-stable-branch-ci tl;dr is we're passing executable=/bin/bash with the ansible command args but it's running with /bin/sh on the remote side. I don't understand it | |
| 16:07:37 | melwitt | have you seen something like that before? | |
| 16:07:54 | sean-k-mooney | melwitt: i was talking to fungi about that yesterday on #opentack-infra | |
| 16:08:04 | gibi | sean-k-mooney: hm, OK, I see it now. But they why on earth we needed to re-implement the same (?) logic specifically for remote_managed at https://github.com/openstack/nova/blob/d86916360858daa06164ebc0d012b78d19ae6497/nova/pci/devspec.py#L322-L334 ? | |
| 16:08:13 | melwitt | sean-k-mooney: oh cool. I'll go check the channel log | |
| 16:09:21 | sean-k-mooney | gibi: remote managed does not otherwise allow the PF to be listed | |
| 16:09:32 | sean-k-mooney | if i recall correctly | |
| 16:10:18 | sean-k-mooney | melwitt: they did not come to a conclution either but were speculating it was related to hoave devstrack get invoke it and the #!/bin/bash lines in the hook script | |
| 16:10:24 | sean-k-mooney | but those point to bash too | |
| 16:10:45 | sean-k-mooney | melwitt: i think the fix is to stop usinging the raw module and actully use shell | |
| 16:11:15 | sean-k-mooney | i think the raw moduel is perhaps using a shle script to run your comamdn via the executabel you specify | |
| 16:11:17 | melwitt | sean-k-mooney: I wondered that exact same thing. the raw module seemed the only potentially suspicious thing to me | |
| 16:11:30 | sean-k-mooney | so raw is not ment to be used in normal code | |
| 16:11:57 | gibi | sean-k-mooney: thanks. I think I have not enough brain power for this right now | |
| 16:12:02 | sean-k-mooney | raw is intended to install the deps that are required for command and shell so that you can bootstrap an ansible target when its mising the basic required pacakages for ansibel to work | |
| 16:12:26 | melwitt | sean-k-mooney: yeah. supposedly saying executable=/bin/bash will make it run under bash and it must have used to but for some reason recently the behavior has changed | |
| 16:13:05 | sean-k-mooney | yes so looking at the ansible docs the args you would normally pass in ansibel you set as varibles in the string when you are doing an adhock command | |