| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-04 | |||
| 20:09:10 | aspiers | melwitt: ahhh maybe that's it | |
| 20:10:09 | aspiers | melwitt: maybe it's missing the flavor or image_meta | |
| 20:10:39 | melwitt | aspiers: so sometimes what happens is you're working on a new test whose code path will access an attribute that's not stubbed out/filled in, and then it falls back on a read from the db, and if you're inherited from NoDBTestCase, that error will raise | |
| 20:11:18 | aspiers | no, just adding a call to self._stub_instance_get() is enough to trigger this db-related failure, *even* if I don't use the returned fake instance | |
| 20:11:31 | aspiers | so it's not my test code which is triggering the error | |
| 20:11:54 | melwitt | yeah if you look in https://github.com/openstack/nova/blob/master/nova/tests/unit/fake_instance.py#L45 you can see what's been filled in | |
| 20:11:59 | aspiers | yeah I spotted that | |
| 20:12:04 | melwitt | oh, hm | |
| 20:12:12 | aspiers | but this makes no sense, because other tests happily call that same method | |
| 20:13:40 | aspiers | melwitt: uploading now | |
| 20:13:52 | openstackgerrit | Adam Spiers proposed openstack/nova master: Reject live migration and suspend on SEV guests https://review.opendev.org/680158 | |
| 20:13:56 | aspiers | there ^^^ | |
| 20:14:40 | aspiers | melwitt: https://review.opendev.org/#/c/680158/1/nova/tests/unit/api/openstack/compute/test_migrate_server.py@592 causes this db error | |
| 20:14:51 | aspiers | why do other identical lines in the same file not cause it? | |
| 20:16:22 | melwitt | at a glance, I notice the other tests calling _stub_instance_get are also mocking the live_migrate method in compute_api | |
| 20:16:35 | aspiers | right, that's what I was saying earlier | |
| 20:16:43 | aspiers | they mock the meat of the code path | |
| 20:17:19 | aspiers | but the call to _stub_instance_get() isn't inside the mocking?! | |
| 20:17:49 | aspiers | so it can't be that | |
| 20:17:57 | aspiers | super weird | |
| 20:18:06 | openstackgerrit | Adam Spiers proposed openstack/nova master: Reject live migration and suspend on SEV guests https://review.opendev.org/680158 | |
| 20:18:28 | melwitt | you're saying the _stub_instance_get call itself will not return and raises that error? | |
| 20:18:33 | aspiers | yes | |
| 20:18:39 | melwitt | O.o | |
| 20:18:53 | melwitt | ok, that is unexpected | |
| 20:19:24 | aspiers | oh no wait | |
| 20:19:32 | aspiers | OK I get it now | |
| 20:19:42 | aspiers | _stub_instance_get() has side effects of course | |
| 20:19:47 | aspiers | it patches stuff | |
| 20:19:53 | aspiers | so it doesn't matter if I don't use the return value | |
| 20:20:55 | aspiers | yeah it's the lookup of instance.image_meta which fails | |
| 20:21:33 | mriedem | dansmith: artom: ack, probably going to have to be in the morning at this point, just got off a call | |
| 20:21:42 | melwitt | aspiers: where's the lookup? I'm curious | |
| 20:22:04 | aspiers | melwitt: https://review.opendev.org/#/c/680158/2/nova/compute/api.py@218 | |
| 20:22:13 | aspiers | melwitt: I think my mocking of that hardware method is broken probably | |
| 20:22:24 | aspiers | mocking in the wrong place is my guess | |
| 20:22:44 | aspiers | need to mock in nova.compute.api | |
| 20:22:54 | melwitt | ok, I thought you said _stub_instance_get was blowing up itself | |
| 20:22:59 | aspiers | yeah I was wrong | |
| 20:23:03 | melwitt | and I was curious where in there it accesses db | |
| 20:23:04 | melwitt | ok | |
| 20:24:44 | aspiers | ohh doh | |
| 20:25:05 | aspiers | patching hardware.get_mem_encryption_constraint doesn't stop the calculation of the parameters being passed to it | |
| 20:25:16 | aspiers | so I have to patch instance.image_meta too, or just set it | |
| 20:31:29 | dansmith | mriedem: np | |
| 20:41:39 | aspiers | melwitt: got it working finally | |
| 20:41:48 | aspiers | melwitt: thanks for your help :) | |
| 20:41:51 | melwitt | yay | |
| 20:43:03 | openstackgerrit | Merged openstack/nova master: Modernize nova-lvm job https://review.opendev.org/674356 | |
| 20:45:55 | openstackgerrit | Adam Spiers proposed openstack/nova master: Reject live migration and suspend on SEV guests https://review.opendev.org/680158 | |
| 20:46:17 | aspiers | melwitt: https://review.opendev.org/#/c/680158/2..3/nova/tests/unit/api/openstack/compute/test_suspend_server.py@47 | |
| 20:47:09 | openstackgerrit | Adam Spiers proposed openstack/nova master: Enable booting of libvirt guests with AMD SEV memory encryption https://review.opendev.org/666616 | |
| 20:48:10 | melwitt | aspiers: oh, so you didn't need to stub compute_api | |
| 20:48:19 | aspiers | melwitt: right, it was just image_meta | |
| 20:48:43 | aspiers | I didn't want to stub compute_api because that's where the desired exception gets thrown | |
| 20:49:15 | melwitt | yeah, makes sense | |
| 20:55:27 | openstackgerrit | Matt Riedemann proposed openstack/nova master: FUP for I66d8f06f19c5c631e33208580428aa843abb38d2 https://review.opendev.org/678951 | |
| 20:57:14 | aspiers | oh crap, circular imports | |
| 21:01:38 | aspiers | what's the ASCII smiley for a facepalm? | |
| 21:09:34 | aspiers | efried: make sense if I move sev_enabled() from libvirt.utils to virt.hardware to break the circular import? | |
| 21:10:04 | efried | aspiers: without looking specifically at the patch, that sounds like a reasonable thing. | |
| 21:10:17 | aspiers | I don't think it's possible to move get_mem_encryption_constraint() and its helpers in the other direction | |
| 21:10:24 | aspiers | which is a shame, cause they're libvirt-specific | |
| 21:10:29 | efried | except that I still resent that nova.virt.hardware is libvirt-specific | |
| 21:10:35 | aspiers | right | |
| 21:10:36 | efried | yeah | |
| 21:10:42 | aspiers | thing is, they depend _get_flavor_image_meta which is shared | |
| 21:10:43 | efried | that ship has sailed though. | |
| 21:10:49 | aspiers | OK | |
| 21:11:06 | aspiers | efried: I suggested to kashyap yesterday that we actually move the machine type stuff to a separate machine_types file | |
| 21:11:30 | aspiers | libvirt.utils is a kinda dumb bag of random shit | |
| 21:11:34 | efried | if you're just grasping for shit to do, you could factor the libvirt-specific stuff out of nova.virt.hardware and into a nova.virt.libvirt.hardware module, leaving anything that's actually generic (assuming there is anything) in nova.virt.hardware. | |
| 21:12:06 | efried | but we probably have more important things to do | |
| 21:12:13 | aspiers | but I dunno, maybe I can | |
| 21:12:33 | aspiers | however I'm very conscious that once this runway window closes, that's my last chance to land SEV in Train gone | |
| 21:13:11 | aspiers | anyway yet again it's 10pm and I haven't had dinner | |
| 21:13:17 | aspiers | time to tear myself away | |
| 21:14:12 | aspiers | efried: parting thought - once I fix the circular import, in *theory* the patch series is code complete (haha how many times have I said that before) | |
| 21:15:08 | efried | aspiers: That refactor would have nothing to do with the SEV series, runways, or indeed probably Train, as we would likely prefer to wait until ussuri opens to land it. | |
| 21:15:37 | aspiers | efried: cool | |
| 21:15:45 | aspiers | what do you think about the machine_types.py idea? | |
| 21:16:09 | aspiers | nova.virt.machine_types | |
| 21:16:16 | aspiers | I don't even know if it's libvirt-specific actually | |
| 21:16:18 | aspiers | I think not | |
| 21:16:35 | aspiers | but specific to QEMU so ... | |
| 21:17:23 | aspiers | yeah, qemu-kvm -machine help lists them all | |
| 21:24:08 | donnyd | melwitt: dansmith booya | |
| 21:24:12 | donnyd | https://usercontent.irccloud-cdn.com/file/OdTlIVXX/image.png | |
| 21:24:34 | dansmith | donnyd: chunk size or something else? | |
| 21:24:36 | donnyd | chunk size numbers are looking much mo betta | |
| 21:24:44 | dansmith | sweet | |
| 21:24:52 | dansmith | donnyd: let me get you an address where you can send the check... | |
| 21:25:08 | donnyd | LOL | |
| 21:26:18 | donnyd | 5x faster | |
| 21:26:37 | donnyd | that is a pretty big win | |
| 21:27:09 | melwitt | yeah, that's awesome. I didn't know about glance chunk size until today | |
| 21:28:24 | dansmith | donnyd: that was with 16MiB chunks? | |
| 21:28:28 | donnyd | yea | |
| 21:28:40 | dansmith | cool | |
| 21:28:47 | donnyd | not sure if I should turn it up anymore or not | |