| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-11 | |||
| 11:05:26 | sean-k-mooney | so ya +9-14 lines | |
| 11:07:48 | lyarwood | gibi: https://review.opendev.org/c/openstack/nova/+/735947 - btw I -1'd this earlier, more for my own education and understanding as the code LGTM but the commit wasn't clear on the reasoning for this. | |
| 11:09:40 | gibi | artom: ack, looks good to me | |
| 11:10:39 | gibi | lyarwood: I have to get back to that | |
| 11:12:23 | artom | gibi, mcuh thanks! | |
| 11:12:24 | sean-k-mooney | off the top of my head i dont rememebr all the detail but the highlevle reasoning is that hyperthreading affect the entire host ans sicne cpu are part of the unname greoup these should also be. it would be invalid today to have 2 named groups one that requested HT on and the other HT off for the same host | |
| 11:12:44 | openstackgerrit | Merged openstack/nova master: apidb: Compact Rocky database migrations https://review.opendev.org/c/openstack/nova/+/759405 | |
| 11:13:45 | sean-k-mooney | i rememebr suggesting that we shoudl perhaps only ally HW_CPU_* in general in the unamed group becasue at the time i did not think there was ever a valid usecase for varing them in depently in the named groups | |
| 11:13:51 | sean-k-mooney | lyarwood:^ | |
| 11:15:11 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/735947/5/nova/api/validation/extra_specs/traits.py#26 | |
| 11:21:33 | gibi | I think I agree with sean-k-mooney here | |
| 11:21:36 | sean-k-mooney | lyarwood: actully i remember more of the context now filling it in to the patch | |
| 11:25:34 | lyarwood | k thanks I also need to drop in a min to pick up some new glasses, I'll get back to the review this afternoon | |
| 11:26:20 | sean-k-mooney | lyarwood: https://review.opendev.org/c/openstack/nova/+/735947/5//COMMIT_MSG#13 | |
| 14:01:52 | gibi | yonglihe: finshed reading the main smartnic patch I still have issues. Also I think you missed my earlier request to add a set of functional tests | |
| 14:09:04 | gibi | alex_xu: ^^ fyi | |
| 14:20:17 | gibi | is there any patch that needs my immediate attention? | |
| 14:22:42 | stephenfin | sean-k-mooney: question: is this change necessary? https://review.opendev.org/c/openstack/nova/+/770533/10/nova/virt/libvirt/host.py#1262 | |
| 14:23:10 | stephenfin | I can't figure out what uses it? I don't think we ever finished the NIC capability feature and that's the only thing that code was used for previously | |
| 14:23:29 | sean-k-mooney | the elif? | |
| 14:23:34 | sean-k-mooney | yes the elif is | |
| 14:23:49 | stephenfin | adding the 'vdpa' field to the 'capabilities' | |
| 14:23:52 | stephenfin | dic | |
| 14:23:54 | stephenfin | *dict | |
| 14:24:09 | sean-k-mooney | its used to get the path to add to the xml | |
| 14:24:11 | stephenfin | which will be saved in the 'extra_info' field of the 'PciDevice' table | |
| 14:25:02 | stephenfin | I don't think it is | |
| 14:25:04 | stephenfin | https://review.opendev.org/c/openstack/nova/+/770532/10/nova/virt/libvirt/vif.py#501 | |
| 14:25:05 | sean-k-mooney | when generating the xml for the interface we add the vdpa path not the pci addres or the parent | |
| 14:25:19 | stephenfin | that's calling the 'get_vdpa_device_path' | |
| 14:25:53 | stephenfin | which is pretty self-contained and inspect the XML from libvirt directly https://review.opendev.org/c/openstack/nova/+/770533/10/nova/virt/libvirt/host.py#1333 | |
| 14:26:02 | sean-k-mooney | ach right | |
| 14:26:16 | stephenfin | so is that something you forgot to cleanup? | |
| 14:26:26 | sean-k-mooney | both will work | |
| 14:26:41 | stephenfin | personally, I'd rather we didn't have to pass through an instance of 'Host' | |
| 14:26:46 | stephenfin | if that's an option | |
| 14:27:14 | sean-k-mooney | we do not have the pci request here | |
| 14:27:41 | sean-k-mooney | so we cant get if form the db like i planned orginally | |
| 14:27:54 | sean-k-mooney | thats why im looking it up i think | |
| 14:28:10 | sean-k-mooney | i will need to look at this in context again sorry | |
| 14:28:25 | stephenfin | okay | |
| 14:28:38 | stephenfin | I'll play around with it a little more, see if I can make sense of it :) | |
| 14:30:12 | sean-k-mooney | so we dont have the pci devices form the db in the virt driver | |
| 14:30:40 | sean-k-mooney | and at present we do not have the mapping form the neutorn port to the pci request ourside of the network_request object | |
| 14:31:09 | sean-k-mooney | so to remove passing in host we would have to pass in the network requiest to lookup the requite id then do a db lookup | |
| 14:31:15 | sean-k-mooney | that would be much more expensive | |
| 14:31:45 | stephenfin | can we pass information through as part of the binding profile, like we do for SR-IOV? | |
| 14:31:50 | sean-k-mooney | i personally have been using the vdpa path in the pci_devices table to debug this and ensure the xml is useing the correct path for the device tha was saved | |
| 14:32:01 | sean-k-mooney | stephenfin: i really dont want to do that | |
| 14:32:15 | sean-k-mooney | because i want to stop using the binding procile for the xml generation entirely soon | |
| 14:32:16 | gibi | somebody opened ~80 new bugs to nova based on fault injection. e.g. https://bugs.launchpad.net/nova/+bug/1918340 | |
| 14:32:17 | openstack | Launchpad bug 1918340 in OpenStack Compute (nova) "Fault Injection #1 - improve unit test effectiveness" [Undecided,New] | |
| 14:32:40 | stephenfin | e.g. https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L242-L246 | |
| 14:32:52 | sean-k-mooney | close them all with we do not consider bugs cause by falut injectoin to be valid? | |
| 14:33:13 | sean-k-mooney | stephenfin: ya i want to remove that use of the profile | |
| 14:33:34 | stephenfin | sean-k-mooney: could we do that wholesale though? | |
| 14:33:36 | sean-k-mooney | we never should have done that in the first place it was a hack | |
| 14:33:58 | stephenfin | I'm sure there's a good reason not to do it, but it's a bit weird having two different ways to do things now | |
| 14:34:13 | sean-k-mooney | right now im using that | |
| 14:34:24 | sean-k-mooney | to get the pci addres of the parent vf | |
| 14:34:47 | sean-k-mooney | but in the futre i want to store this info in our own data stucure in nova db somewhere | |
| 14:35:10 | sean-k-mooney | and pass it in to build/attach ectra | |
| 14:35:51 | yonglihe | gibi, then i got work to do. -:) | |
| 14:47:10 | sean-k-mooney | stephenfin: by the way im currently writing two patches. one to block booting vms with ports of type vdpa until its supproted and a second to block unsupported operations in general | |
| 14:47:53 | sean-k-mooney | similar to how we block api calls with cyborg if they are not supported yet with a 403. | |
| 14:49:34 | stephenfin | makes sense | |
| 14:59:21 | kashyap | stephenfin: For later, you might also find this useful to know about: https://github.com/rhuefi/qemu-ovmf-secureboot | |
| 14:59:55 | kashyap | (Fedora and other distros already ship a variant of this tool we wrote in the past.) | |
| 15:16:05 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Add functional tests for UEFI, secure boot https://review.opendev.org/c/openstack/nova/+/776682 | |
| 15:16:06 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Document UEFI secure boot feature https://review.opendev.org/c/openstack/nova/+/776684 | |
| 15:16:06 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Remove duplicated 'start_compute' helper https://review.opendev.org/c/openstack/nova/+/776683 | |
| 15:16:39 | stephenfin | lyarwood: removed that unnecessary ADMIN_API from the functional test ^ | |
| 15:17:28 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: compute: Lock by instance.uuid lock during swap_volume https://review.opendev.org/c/openstack/nova/+/758733 | |
| 15:17:29 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: [stable-only] gate: Skip ceph tests after plugin moved to Octopus release https://review.opendev.org/c/openstack/nova/+/780014 | |
| 15:17:38 | lyarwood | elod / melwitt ; ^ fun times with ceph on stable/train btw | |
| 15:17:41 | lyarwood | stephenfin: ack thanks | |
| 15:21:20 | sean-k-mooney | lyarwood: you could pin the plugin to an older commit no? | |
| 15:22:19 | sean-k-mooney | lyarwood: you just do https://github.com/SeanMooney/ci-sean-mooney/blob/main/zuul.d/jobs.yaml#L36 | |
| 15:22:26 | sean-k-mooney | required-projects: | |
| 15:22:28 | sean-k-mooney | - name: openstack/devstack | |
| 15:22:30 | sean-k-mooney | override-checkout: master | |
| 15:22:40 | sean-k-mooney | but in this case override the ceph plugin | |
| 15:23:05 | sean-k-mooney | so you can continue to deploy what ever it had before | |
| 15:23:37 | elod | lyarwood: yikes. thanks for looking it and fixing! is this blocking train and older branches? | |
| 15:23:42 | lyarwood | yeah the issue is these jobs aren't zuul native on stable/train and manually check out the plugin | |
| 15:24:27 | lyarwood | elod: I assume <=stable/train with the nova-live-migration failures | |
| 15:24:40 | lyarwood | elod: given grenade isn't running prior to stable/train right? | |
| 15:25:03 | lyarwood | sean-k-mooney: that's a neat hack btw, I wansn't aware of it. | |
| 15:25:20 | lyarwood | sean-k-mooney: we could limit the checkout in the hook script to a known good commit to keep ceph coverage I guess | |
| 15:25:22 | sean-k-mooney | we use it in a number of places for stable branches | |
| 15:25:49 | sean-k-mooney | lyarwood: tempest is one case sicne it branchless | |
| 15:26:36 | sean-k-mooney | lyarwood: but yes pinning to a know good commit or actully taging it and pinnging to that tag would proably be the way to go | |
| 15:27:05 | sean-k-mooney | if it was zuul native i think that can be anything that is vaild for a git checkout command | |
| 15:27:19 | sean-k-mooney | so commit tag or branch | |
| 15:27:48 | lyarwood | right, let me do it that way actually so we don't drop coverage | |
| 15:28:15 | sean-k-mooney | where is the job defiend | |
| 15:28:27 | sean-k-mooney | the enable plugin line optionally takes a git ref | |
| 15:28:39 | sean-k-mooney | which is the branch/tag/commit to use | |
| 15:29:09 | sean-k-mooney | you can do it trivally in the lcoal.conf if you can change the enable_plugin line | |
| 15:30:21 | sean-k-mooney | oh ... https://github.com/openstack/nova/blob/stable/train/gate/live_migration/hooks/ceph.sh#L13 | |