| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-10 | |||
| 10:51:15 | kashyap | stephenfin: What do you mean "straight to the source"? You mean using directly the firmware descriptor files shipped by OVMF/EDK2? | |
| 10:51:21 | stephenfin | yes | |
| 10:51:26 | kashyap | stephenfin: Agree, on removing the hard-coded list | |
| 10:52:33 | kashyap | stephenfin: A detail: each distro (depending on the distro) ships them as part of either EDK2 or OVMF package. I did it for Fedora; and filed bugs for Debian and Ubuntu: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=932269 | |
| 10:52:34 | openstack | Debian bug 932269 in ovmf "Ship the firmware "descriptor files" as part of the 'ovmf' package" [Normal,Fixed] | |
| 10:52:54 | stephenfin | I checked. Ubuntu 20.04 has them, as does Debian now | |
| 10:53:04 | kashyap | Excellent; by now they should | |
| 10:53:06 | stephenfin | And I found references to them for SUSE | |
| 10:53:17 | stephenfin | All in the standard locations too, thankfully | |
| 10:53:34 | kashyap | stephenfin: Yes, I've worked w/ SUSE folks too in the past to get it worked in | |
| 10:58:40 | stephenfin | bauzas: Friendly reminder that the UEFI secure boot series is all green now, save for errant gate failures, and ready for your attention | |
| 11:00:36 | artom | stephenfin, is that... bad? | |
| 11:01:08 | stephenfin | It's unexpected, no? | |
| 11:01:33 | artom | Well, from the nodepool VMs, where we would expect everything to be in one node and socket, yeah | |
| 11:01:36 | stephenfin | Though I guess the gate jobs are VMs and therefore not really representative of real hardware | |
| 11:03:01 | artom | stephenfin, well, see here: https://zuul.opendev.org/t/openstack/build/a19ff22927a045df9f8982d7ef678238/log/controller/logs/screen-n-cpu.txt#892-901 | |
| 11:03:25 | artom | It put each CPU in its own socket | |
| 11:03:36 | artom | So my expectation is clearly wrong :) | |
| 11:04:28 | stephenfin | \o/ | |
| 11:30:43 | bauzas | stephenfin: yup, I was looking at other series, but I'll review them again this afternoon | |
| 11:40:37 | yonglihe | sean-k-mooney: I try to work out your concern and that bug is about LM/resize, that's out of scope. refer to comments, there are some idea about that. | |
| 11:43:33 | yonglihe | sean-k-mooney: I did not dig into that bug, just say: It's seems that's data out of sync somehow, change how to store the data won't help? (fix me) | |
| 11:54:36 | alex_xu | sean-k-mooney1: yonglihe I looked those two bugs, hope I understand that correctly https://review.opendev.org/c/openstack/nova/+/771363/11/nova/objects/network_request.py#42 | |
| 12:07:36 | sean-k-mooney1 | artom: nova default to creating 1 socket per vcpu | |
| 12:08:11 | sean-k-mooney1 | so in the gate vms you will have 8 socket and 1 numa node generated implcitly by qemu/libvirt | |
| 12:08:19 | sean-k-mooney1 | even for non numa vms | |
| 12:09:16 | sean-k-mooney1 | stephenfin: by the way passing the function form the driver.py to host.py goes away in a later patch where i actully move the fucntions too host.py | |
| 12:09:29 | stephenfin | sean-k-mooney1: could you do that first? | |
| 12:09:31 | sean-k-mooney1 | we dont actully use the function that take the callback until that patch | |
| 12:09:56 | sean-k-mooney1 | maybe but it will be a bit of a pain to split back out the patches | |
| 12:10:24 | sean-k-mooney1 | i can do that it more a question of time | |
| 12:11:09 | sean-k-mooney1 | artom: would you be ok removing the numa toplogy object form the pci tracker and adding in a numa to socket map instead | |
| 12:12:11 | sean-k-mooney1 | although if we have to account for the many socket to 1 numa node of the gate i guess we need to retink that abit | |
| 12:12:42 | sean-k-mooney1 | in the gate all pci devices will be connected to the first socket | |
| 12:12:50 | sean-k-mooney1 | we only have 1 pcie root | |
| 12:13:05 | sean-k-mooney1 | and there is also only 1 numa node by default | |
| 12:13:53 | sean-k-mooney1 | a hack would be to have that numa to socket dict be int->list[int] and for you to just use the first socket form the list | |
| 12:14:08 | sean-k-mooney1 | we did discuss that at one point | |
| 12:17:32 | sean-k-mooney | alex_xu: its the same bug just upstream and downstream version | |
| 12:17:46 | sean-k-mooney | alex_xu: the bug is not related to claiming the pci devics | |
| 12:18:14 | sean-k-mooney | alex_xu: we correctly claim the pci device the issue is we dont update the neutron port profile correctrly as part of unshelve | |
| 12:18:34 | sean-k-mooney | alex_xu: and we generate the xml with the pci address stored in the profile | |
| 12:18:48 | sean-k-mooney | so we use the pci address form its previous host | |
| 12:19:00 | sean-k-mooney | so the vm we are unsleving is the one using the incorrect device | |
| 12:19:31 | alex_xu | sean-k-mooney: yes, so for cyborg, we should update the port with new arq uuid | |
| 12:19:39 | sean-k-mooney | yes | |
| 12:20:04 | alex_xu | so I'm thinking we need the network_request.arq_uuid again to pass that new arq from conductor to compute | |
| 12:20:15 | sean-k-mooney | and we should update the profile with the claimed pci address for sriov too we just dont today | |
| 12:20:25 | alex_xu | yea | |
| 12:20:40 | sean-k-mooney | alex_xu: well network_request is not stored in the db right | |
| 12:20:53 | alex_xu | it is fine, we needn't the old arq | |
| 12:20:56 | sean-k-mooney | im uncomfortable with having the neutron port binding be the only place we persit that | |
| 12:21:09 | sean-k-mooney | we need it for things like hard reboot no? | |
| 12:21:30 | alex_xu | we needn't, I check that also, we build network_info from the neutron directly | |
| 12:21:59 | sean-k-mooney | we do and store it in the network info cache | |
| 12:22:14 | sean-k-mooney | my point is i dont think we should be using the value sotre in neutron to generate the xml | |
| 12:22:28 | sean-k-mooney | we should be tryign to do it without using the network info cache | |
| 12:22:49 | sean-k-mooney | so that user cant acidentally alter the port profile in a way that can brake nova | |
| 12:22:51 | alex_xu | but all the action is using that network info, even for boot instance | |
| 12:23:04 | sean-k-mooney | right and that is a design flaw | |
| 12:23:13 | sean-k-mooney | that info can be currpted | |
| 12:23:43 | sean-k-mooney | i have seen it happen in customer deployment many times | |
| 12:24:03 | alex_xu | I'm thinking if the user done that, whether it means there already have other thing goes wrong | |
| 12:24:03 | sean-k-mooney | generally it can be recoverd but we should really treat that as write only | |
| 12:24:45 | sean-k-mooney | its possible yes | |
| 12:25:01 | alex_xu | so binding profile is only for neutron use, but nova should have it own copy | |
| 12:25:44 | alex_xu | the hard thing is if the nova copy is different with neutron one, what should nova do | |
| 12:25:54 | sean-k-mooney | binding profile was intended to be used to pass info from nova to neutorn in one direction | |
| 12:26:05 | sean-k-mooney | alex_xu: use the nova one | |
| 12:26:11 | artom | sean-k-mooney, yeah, that is a good idea | |
| 12:26:13 | alex_xu | ok | |
| 12:26:29 | alex_xu | then correct the neutron info | |
| 12:26:30 | artom | And to internalize the function that generates that map into the NUMATopology object itself | |
| 12:26:34 | sean-k-mooney | alex_xu: https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/portbindings.py#L31-L34 | |
| 12:26:43 | artom | Not sure we have enough time before FF though | |
| 12:26:50 | sean-k-mooney | alex_xu: correct field that differ ideally | |
| 12:27:04 | sean-k-mooney | but we cant do that based on the info cache we have today | |
| 12:27:08 | artom | sean-k-mooney, also, if those _filter functions are really meant to be purely functional, then @classmethod is not the way to do it - you put them outside the class entirely :) | |
| 12:27:26 | artom | I'll see what I can hack up today | |
| 12:27:45 | sean-k-mooney | artom: why class method are prefectly valid ways to do that | |
| 12:28:02 | alex_xu | for yongli's patch, should we block it for that issue? | |
| 12:28:04 | sean-k-mooney | you coudl put them outside the class entirely or even make them static | |
| 12:28:23 | alex_xu | I feel it is hard for yongli to fix that, since it is boarder issue | |
| 12:28:26 | sean-k-mooney | alex_xu: am no we could proceed and clean this up next cycle | |
| 12:28:38 | alex_xu | yea | |
| 12:28:48 | sean-k-mooney | alex_xu: im just uncomfortable realying on that | |
| 12:29:05 | sean-k-mooney | i was hoping we could ask cybrog for the arq using the neutorn port uuid | |
| 12:29:18 | sean-k-mooney | and have cyborg be the souce of truth | |
| 12:29:40 | alex_xu | but nothing we can do it now, I don't think system metadata is good place, since we said before, we shouldn't anything to it, since it without version control | |
| 12:30:18 | sean-k-mooney | alex_xu: actully i thought the preferece was to use it for things linke this over adding more tables | |
| 12:30:29 | sean-k-mooney | alex_xu: we can have version contol by using ovos | |
| 12:30:48 | sean-k-mooney | it allow use to store addtional info without db migrations | |
| 12:31:03 | sean-k-mooney | but ok lets put this issue assign for now and maybe add a todo | |
| 12:31:42 | sean-k-mooney | alex_xu: can we maybe add a test to vaoidate unshelve and ensure that the port profile is updated with the new arq? | |
| 12:32:32 | alex_xu | sean-k-mooney: I don't yongli implement that, I remember he said unshelve is out of scope for the spec | |
| 12:32:53 | sean-k-mooney | didnt we add support for shelve with cyborg recently | |
| 12:33:06 | sean-k-mooney | if its not supproted with neutron port then we would need to block it yes | |
| 12:33:48 | alex_xu | I think when yongli's spec merged, the shelve with cyborg doesn't merge yet | |
| 12:34:21 | alex_xu | I think the missing part is update the new arq uuid | |
| 12:34:45 | sean-k-mooney | yep | |