| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-06-03 | |||
| 17:24:53 | sean-k-mooney | for get_volume config its just passing the info and doing the if here right https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L1887 | |
| 17:25:31 | lyarwood | right I thought you were suggesting changing the actual get_config methods in the volume drivers | |
| 17:25:56 | lyarwood | this is still going to cause test fallout that I'd rather avoid for backports but I'll give it a go | |
| 17:25:56 | sean-k-mooney | no | |
| 17:26:08 | sean-k-mooney | and for vif.py its just replaceing the returns here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L578-L601 | |
| 17:26:23 | sean-k-mooney | with an asignment to conf and dong the if then returning | |
| 17:26:55 | sean-k-mooney | lyarwood: the rest of the approch im ok with | |
| 17:27:49 | sean-k-mooney | lyarwood: due to the changing of the signiture of an privete funcion _get_volume_config | |
| 17:28:22 | sean-k-mooney | i would hope the fallout from tha twould be relitivly minimal | |
| 17:28:26 | lyarwood | sean-k-mooney: what about get_base_config for the vif? | |
| 17:28:40 | sean-k-mooney | you dont need to modify that | |
| 17:28:53 | sean-k-mooney | you could but you cna do it in get_config | |
| 17:29:29 | sean-k-mooney | lyarwood: that said https://github.com/openstack/nova/blob/cd084aeeb8a2110759912c1b529917a9d3aac555/nova/virt/libvirt/vif.py#L180 | |
| 17:29:47 | sean-k-mooney | it has the flaovr and image_meta | |
| 17:29:57 | sean-k-mooney | and that shoudl be called in all code paths | |
| 17:30:04 | lyarwood | right I was looking at the end of that method | |
| 17:30:07 | sean-k-mooney | so ya you can just add it there | |
| 17:30:30 | sean-k-mooney | lyarwood: to be honest i tought it was already there | |
| 17:31:03 | lyarwood | ah the only issue is checking if sev is enabled from here is going to be awkward | |
| 17:31:12 | sean-k-mooney | you need to do it here though https://github.com/openstack/nova/blob/cd084aeeb8a2110759912c1b529917a9d3aac555/nova/virt/libvirt/vif.py#L192 | |
| 17:31:37 | sean-k-mooney | lyarwood: becuase you dont have aceese to sev_enabled? | |
| 17:31:44 | lyarwood | _sev_enabled yeah | |
| 17:32:03 | sean-k-mooney | that is just https://github.com/openstack/nova/blob/cd084aeeb8a2110759912c1b529917a9d3aac555/nova/virt/libvirt/driver.py#L6716-L6743 | |
| 17:32:18 | sean-k-mooney | you can put that in desinger.py | |
| 17:32:28 | sean-k-mooney | if you wanted too | |
| 17:32:30 | lyarwood | right but that's checking _host | |
| 17:32:31 | sean-k-mooney | well no | |
| 17:32:38 | sean-k-mooney | it need _host | |
| 17:33:18 | sean-k-mooney | ok well we have _host in vif.yp | |
| 17:33:23 | lyarwood | lol | |
| 17:33:42 | sean-k-mooney | but if you want to do this in a follow up then ok | |
| 17:33:54 | sean-k-mooney | i think this would still be a clearer way to do this | |
| 17:34:30 | sean-k-mooney | we likely woudl just want to pass host as a parmater to sev_enabled | |
| 17:35:28 | sean-k-mooney | actully you could put the if check in set_driver_iommu_for_device | |
| 17:35:53 | sean-k-mooney | and pass host, flavor, image_meta and conf | |
| 17:36:47 | sean-k-mooney | anyway i guess leave it for now | |
| 17:37:07 | sean-k-mooney | at some point we really need to get ride of the desginer.py as well | |
| 17:37:32 | lyarwood | tbh this is the first time I've really looked at it | |
| 17:37:58 | sean-k-mooney | the designer.py has always been a pet pev of mine | |
| 17:38:04 | sean-k-mooney | i want driver.py to be smaller | |
| 17:38:12 | sean-k-mooney | but designer.py does not relaly add value today | |
| 17:38:13 | lyarwood | brb baby bedtime | |
| 17:38:47 | sean-k-mooney | it would be beter to merge it with config.py | |
| 17:39:57 | sean-k-mooney | it was seperated out so that config.py could just be the xml object creation and designer.py woudl have the logic to do some of the compostion | |
| 17:40:23 | sean-k-mooney | but we never too the xml logic out of vif.py or the volume modules | |
| 17:40:34 | sean-k-mooney | so really designer just complicated things | |
| 17:42:05 | opendevreview | Merged openstack/nova stable/wallaby: rbd: Get rbd_utils unit tests running again https://review.opendev.org/c/openstack/nova/+/790836 | |
| 18:04:59 | opendevreview | melanie witt proposed openstack/nova stable/victoria: rbd: Get rbd_utils unit tests running again https://review.opendev.org/c/openstack/nova/+/794624 | |
| 18:31:31 | melwitt | fix for intermittent unit test failure if anyone interested https://review.opendev.org/c/openstack/nova/+/794396 | |
| 18:53:01 | sean-k-mooney | melwitt: oh it was somethime slow an now did not match? | |
| 18:53:32 | sean-k-mooney | or something like that that change the behavior of _refresh_associations | |
| 18:53:37 | melwitt | sean-k-mooney: yeah, learned of it cause it hit my patch heh https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_b0d/794186/1/check/openstack-tox-py38/b0dd1dd/testr_results.html | |
| 18:53:58 | sean-k-mooney | that was going to be my next question good catch | |
| 18:55:03 | melwitt | after awhile I realized it's because refresh associations records the time at which it ran, and the test was relying on the fact that that time would be sufficiently soon enough after "now" was recorded prior | |
| 18:56:28 | sean-k-mooney | that looks like now = time.time() was previously unused | |
| 18:56:37 | sean-k-mooney | oh i guess its later in the test | |
| 18:56:53 | sean-k-mooney | ya it is | |
| 18:57:02 | melwitt | no yeah it's used | |
| 18:57:46 | sean-k-mooney | part of me wishes there was a clearn helper funciton or somthing for this but ya this looks correct to me | |
| 18:58:19 | sean-k-mooney | for example mocking time.time via a decorator at the top of the function | |
| 18:59:47 | melwitt | oh yeah, I guess that would work without needing reset of the mocks throughout, cause all you'd change is the return_value and not asserting calls | |
| 18:59:48 | sean-k-mooney | e.g. @mock.patch.object(time, 'time', return_value=time.time()) | |
| 19:00:08 | sean-k-mooney | yep | |
| 19:00:27 | sean-k-mooney | you could still reset the time mock if needed | |
| 19:00:45 | sean-k-mooney | but they dont check that as far as i can see | |
| 19:01:04 | melwitt | yeah. I was hoping to not though | |
| 19:01:18 | sean-k-mooney | i dont think you need too | |
| 19:01:25 | melwitt | yeah, agree | |
| 19:01:54 | sean-k-mooney | @mock.patch.object(time, 'time', return_value=time.time())/def test_refresh_associations_time(self, log_mock,time_mock): | |
| 19:01:56 | sean-k-mooney | shoudl fix it | |
| 19:02:53 | sean-k-mooney | return_value=time.time() shoudl be evaluated before the mock is appied and resovled into a what time.time() returns | |
| 19:05:04 | melwitt | I'll give it a go | |
| 19:28:18 | opendevreview | melanie witt proposed openstack/nova master: Make test_refresh_associations_* deterministic https://review.opendev.org/c/openstack/nova/+/794396 | |
| 19:30:10 | opendevreview | melanie witt proposed openstack/nova master: Make test_refresh_associations_* deterministic https://review.opendev.org/c/openstack/nova/+/794396 | |
| 19:33:54 | opendevreview | melanie witt proposed openstack/nova stable/victoria: zuul: Replace grenade and nova-grenade-multinode with grenade-multinode https://review.opendev.org/c/openstack/nova/+/794674 | |
| 19:37:32 | opendevreview | melanie witt proposed openstack/nova stable/ussuri: zuul: Replace grenade and nova-grenade-multinode with grenade-multinode https://review.opendev.org/c/openstack/nova/+/794675 | |
| 23:53:54 | opendevreview | melanie witt proposed openstack/nova stable/train: zuul: Replace grenade and nova-grenade-multinode with grenade-multinode https://review.opendev.org/c/openstack/nova/+/794686 | |
| 23:57:08 | opendevreview | melanie witt proposed openstack/nova stable/train: zuul: Replace grenade and nova-grenade-multinode with grenade-multinode https://review.opendev.org/c/openstack/nova/+/794686 | |
| 23:59:36 | melwitt | sigh | |
| 23:59:44 | opendevreview | melanie witt proposed openstack/nova stable/train: zuul: Replace grenade and nova-grenade-multinode with grenade-multinode https://review.opendev.org/c/openstack/nova/+/794686 | |
| #openstack-nova - 2021-06-04 | |||
| 07:05:11 | opendevreview | Balazs Gibizer proposed openstack/nova master: Remove unnecessary mocks from unit test https://review.opendev.org/c/openstack/nova/+/794637 | |
| 08:43:26 | kashyap | lyarwood: Mornin'. When you're about, it's not entirley clear to me if the grenade-multinode time-out is your patch's problem: https://review.opendev.org/c/openstack/nova/+/794639/ | |
| 08:43:43 | kashyap | Invalid volume + request time-out | |
| 08:50:27 | lyarwood | it's not | |
| 08:50:43 | lyarwood | this looks like another detach timeout issue | |
| 08:52:12 | lyarwood | so much nicer to debug this now thanks to gibi's rewrite | |
| 08:52:33 | lyarwood | :) | |
| 08:53:12 | lyarwood | so I think we need to add a dump of the instance console when we fail to detach | |
| 08:53:28 | lyarwood | as this might be an issue within the guestOS but without that it's impossible to say | |
| 08:53:41 | lyarwood | n-cpu and libvirt appear to be doing the correct things at the correct time | |
| 08:57:22 | kashyap | Hm | |
| 09:21:50 | lyarwood | kashyap: https://review.opendev.org/c/openstack/tempest/+/794757 I'll try some runs with this to see what the guest is up to | |
| 09:23:43 | kashyap | lyarwood: Nice; that's quick! | |
| 09:56:49 | opendevreview | Lee Yarwood proposed openstack/nova master: libvirt: Set driver_iommu when attaching virtio devices to SEV instance https://review.opendev.org/c/openstack/nova/+/794639 | |
| 09:56:50 | opendevreview | Lee Yarwood proposed openstack/nova master: DNM testing tempest volume detach failure capture of console https://review.opendev.org/c/openstack/nova/+/794766 | |
| 10:18:58 | opendevreview | Merged openstack/placement master: Add 'cryptography' package to test-requirements.txt https://review.opendev.org/c/openstack/placement/+/787289 | |
| 10:20:03 | opendevreview | Merged openstack/nova master: Fix RequestLevelParams persistence handling in RequestSpec https://review.opendev.org/c/openstack/nova/+/791502 | |
| 12:40:55 | kashyap | sean-k-mooney: Hey, I'm trying to add a new video device model ... "bochs" display | |
| 12:41:19 | kashyap | sean-k-mooney: It is recommended by the QEMU folks for UEFI guests as a safe alternative | |
| 12:41:59 | opendevreview | Kashyap Chamarthy proposed openstack/nova master: Allow 'bochs' as a display device option https://review.opendev.org/c/openstack/nova/+/794796 | |