| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-12 | |||
| 18:25:00 | efried | thanks for the input sir | |
| 18:25:00 | dansmith | cool | |
| 18:25:02 | aspiers | wfm | |
| 18:25:43 | efried | aspiers: Yes, the centralized method in hardware.py should raise some kind of generic conflict exception, which should be caught and converted to RequestFilterFailed (or whatever it's called today) in the request filter. | |
| 18:26:15 | aspiers | efried: sean-k-mooney suggested making a new exception. ACK about the conversion to RFF | |
| 18:26:26 | efried | (by "generic" I mean not request filter specific; it should still have the lovely and hard-fought message you've already composed) | |
| 18:26:42 | aspiers | oh right, gotcha | |
| 18:26:46 | sean-k-mooney | aspiers: right just copy this one and change the wording https://github.com/openstack/nova/blob/master/nova/exception.py#L2054 | |
| 18:26:59 | efried | aspiers: you could use this: https://review.opendev.org/#/c/663365/2/nova/exception.py | |
| 18:27:19 | sean-k-mooney | efried: oh ya you were adding a reusable one | |
| 18:27:22 | sean-k-mooney | efried: that works too | |
| 18:27:28 | aspiers | oh perfect | |
| 18:27:45 | aspiers | Well, I might inherit from that one :) | |
| 18:27:50 | aspiers | since my message is more informative | |
| 18:28:13 | aspiers | class SEVFlavorImageConflict(FlavorImageConflict) | |
| 18:28:16 | efried | you should introduce the exception (class name) in your own patch, cause there's no telling if/when ^ will merge | |
| 18:28:24 | efried | and whoever comes second can resolve conflict. | |
| 18:28:42 | sean-k-mooney | aspiers: non of the other conflcit ones inherit for efried new excetion at teh moment | |
| 18:28:44 | efried | but yeah, make the exception name generic, and you can override the message in your method. | |
| 18:29:10 | sean-k-mooney | they inherit for Invalid or Forbidden | |
| 18:29:31 | aspiers | and NotFound | |
| 18:29:47 | efried | Those have HTTP semantic baggage, swhy I avoided them. | |
| 18:29:49 | sean-k-mooney | Forbidden when its disallows by policy and Invalid when the did not specify it correctly | |
| 18:30:11 | sean-k-mooney | e.g mem_encyption_context=yes instead or true | |
| 18:30:17 | aspiers | efried: why introduce the same new exception in two patches? can't we just land yours sooner? | |
| 18:30:26 | aspiers | is it because you can +2 my patches but not your own? ;-) | |
| 18:30:36 | efried | Your API invocation of new centralized message should convert generic exception to something that inherits from Invalid | |
| 18:31:03 | efried | aspiers: I can't +2 my patches, no, and also, there doesn't seem to be an overwhelming consensus that that patch of mine is a good thing to do at all. | |
| 18:31:15 | aspiers | efried: even if it's needed for SEV? | |
| 18:31:17 | efried | so I don't have high confidence that it will land soon/ever. | |
| 18:31:20 | efried | it's not | |
| 18:31:24 | efried | only the exception is | |
| 18:31:29 | aspiers | I'm confused | |
| 18:31:30 | efried | so it should be extracted. | |
| 18:31:34 | aspiers | ohhhhh | |
| 18:31:35 | sean-k-mooney | efried: i think i found a bug in our vgpu code today that might use your patch | |
| 18:31:47 | aspiers | I missed that that patch does other stuff | |
| 18:31:49 | aspiers | OK | |
| 18:32:08 | sean-k-mooney | e.g. without looking too close i think we dont handel stop followed by start proerly | |
| 18:32:11 | efried | yeah, the patch I pointed to has nothing to do with SEV. It has to do with "hide hypervisor ID to make NVIDIA drivers work | |
| 18:32:13 | efried | " | |
| 18:32:20 | aspiers | yeah, got it now | |
| 18:32:37 | aspiers | yikes, feels like I need another 40k spec just to remember all the plans we made in the last hour | |
| 18:32:52 | efried | You don't need to update the spec with any of this. | |
| 18:32:53 | aspiers | thank god for IRC logs | |
| 18:33:13 | aspiers | I know, was just being silly :) | |
| 18:33:32 | aspiers | it's a lot of detail for a newbie to remember, but I think I'll be ok | |
| 18:33:49 | sean-k-mooney | anyway this time im really going to go and rest. o/ | |
| 18:33:52 | aspiers | I guess my right to play the newbie card is gradually fading though | |
| 18:33:58 | aspiers | cya sean-k-mooney, thanks for your help o/ | |
| 18:33:59 | efried | - create two consolidated methods in hardware.py: a) parse/validate flavor/image for SEV yes-or-no; b) introspect host for SEV capability | |
| 18:33:59 | efried | - ^ methods raise exceptions | |
| 18:33:59 | efried | - existing usages of ^ logic cut over to using ^, converting exceptions to contextually-appropriate ones. | |
| 18:34:26 | aspiers | I think b) and callers of b) do not need to change | |
| 18:34:29 | aspiers | it's only a) | |
| 18:34:36 | efried | ight | |
| 18:34:41 | aspiers | b) is all in the driver | |
| 18:35:00 | aspiers | and available via the supports_amd_sev instance variable | |
| 18:35:21 | efried | "instance" == compute driver instance, not "instance" == "nova server" | |
| 18:35:38 | aspiers | sure :) | |
| 18:36:07 | aspiers | and then there's Instance instances too :) | |
| 18:36:35 | aspiers | OK, gonna take a break or probably stop for today | |
| 18:36:44 | aspiers | thanks a lot all | |
| 18:37:46 | efried | dustinc: Not rechecking the ironicclient part of the sdk series for yesterday's doubled_words snafu - pep8 needs resolving on the lowermost one. | |
| 18:39:44 | dustinc | efried: saw that, once I am done with the spec update I am working on I will take care of it - thanks | |
| 18:40:10 | efried | dustinc: no hurry, just letting you know why it's not going to be included in my "recheck everything" sweep :P | |
| 18:40:39 | efried | dustinc: also lmk if at some point you want to talk through providers.yaml | |
| 18:42:16 | dustinc | efried: providers.yaml: will do..I def. want to at some point | |
| 19:10:13 | openstackgerrit | Eric Fried proposed openstack/nova master: Make RequestContext(instance_lock_checked) fail https://review.opendev.org/665003 | |
| 19:10:25 | efried | mriedem: resolves a TODO of yours ^ | |
| 19:11:38 | openstackgerrit | Merged openstack/nova master: Remove unnecessary setUp methods https://review.opendev.org/663179 | |
| 19:12:15 | mriedem | efried: does it or does it not have to do with SEV | |
| 19:13:37 | efried | does not | |
| 19:14:02 | efried | I was looking into removing a hack for glance using get_endpoint_data through _ContextAuthPlugin (fail) and just happened to notice this. | |
| 19:14:19 | efried | mriedem: what was that bug you opened for the doubled_words hacking test yesterday? Did you mark it as a dup of https://bugs.launchpad.net/nova/+bug/1804062 already? | |
| 19:14:21 | openstack | Launchpad bug 1804062 in OpenStack Compute (nova) "test_hacking fails for python 3.6.7 and newer" [High,In progress] - Assigned to Stephen Finucane (stephenfinucane) | |
| 19:15:24 | mriedem | yes | |
| 19:15:33 | mriedem | https://bugs.launchpad.net/nova/+bug/1832392 | |
| 19:15:35 | openstack | Launchpad bug 1804062 in OpenStack Compute (nova) "duplicate for #1832392 test_hacking fails for python 3.6.7 and newer" [High,In progress] - Assigned to Stephen Finucane (stephenfinucane) | |
| 19:15:36 | mriedem | efried: question in your patch | |
| 19:16:28 | melwitt | efried: after the discussion about nested magic 1 in office hours today, how are you feeling about the it vs the nova spec for numa affinity for vgpus? | |
| 19:17:43 | openstackgerrit | Eric Fried proposed openstack/nova master: Make RequestContext(instance_lock_checked) fail https://review.opendev.org/665003 | |
| 19:17:53 | efried | mriedem: done | |
| 19:18:15 | melwitt | I wasn't sure if I understood whether nested magic 1 will take much more effort than previously believed | |
| 19:19:40 | efried | melwitt: Components of it - specifically the ones that allow us to do NUMA modeling/affinity for simple cases like VGPU - are not under conflict. I still think the VGPU numa affinity thing doesn't move us forward in a useful way, and just introduces tech debt we'll have to remove very soon. | |
| 19:19:58 | efried | but I don't feel strongly enough to downvote. | |
| 19:20:09 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Revert resize: wait for events according to hybrid plug https://review.opendev.org/644881 | |
| 19:20:10 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: Include direct-physical in compute manager events check https://review.opendev.org/664431 | |
| 19:20:10 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [DNM] testing bug/1813789 revert resize events https://review.opendev.org/664442 | |
| 19:21:09 | melwitt | efried: ack. and do you know if there are people available to work on nested magic 1 or does it need an owner? | |
| 19:21:24 | melwitt | for the implementation after the spec merges | |
| 19:23:17 | dansmith | artom: ooh, did somebody find a bug while writing their tests? :) | |
| 19:23:35 | artom | dansmith, I don't think so? | |
| 19:23:52 | artom | But parts of the code were untested, yeah | |
| 19:23:52 | dansmith | artom: okay, just looking at your events=None change in the latest patch | |
| 19:24:04 | artom | Oh, err, yeah | |
| 19:24:15 | artom | That may have been a problem :/ | |
| 19:24:19 | dansmith | sweet | |
| 19:24:26 | dansmith | now I feel less bad | |
| 19:27:00 | artom | dansmith, so not only am I making Nova better, I'm also contributing to your mental health | |