| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-11 | |||
| 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: tests: Remove duplicated 'start_compute' helper https://review.opendev.org/c/openstack/nova/+/776683 | |
| 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: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 | |
| 15:30:25 | lyarwood | yeah there | |
| 15:30:28 | sean-k-mooney | why were we ever doing that | |
| 15:30:30 | lyarwood | like I said, tis fun | |
| 15:30:38 | lyarwood | not my code ./me runs | |
| 15:30:43 | lyarwood | and it's pre-zuul | |
| 15:30:57 | sean-k-mooney | right but even pre-zuul this was wrong | |
| 15:33:07 | lyarwood | I'm being an idiot, I can just export CEPH_RELEASE | |
| 15:33:44 | sean-k-mooney | ah the plugin supprot that as a configurable | |
| 15:33:58 | sean-k-mooney | you could add --branch to the git clone too | |
| 15:35:28 | lyarwood | sean-k-mooney: the plugin is branchless | |
| 15:35:56 | sean-k-mooney | ya but i tought that worked with commit too | |
| 15:36:06 | sean-k-mooney | it does not | |