| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-12 | |||
| 17:57:49 | sean-k-mooney | anyway it could go somewhere else | |
| 17:57:51 | artom | Agile self-improvement ;) | |
| 17:58:34 | efried | aspiers: not sure if you've been following along, but I think the consensus is to centralize the methods that do a) flavor/image parsing/validation, and b) host capability introspection; and call those from all the places (update_provider_tree calls b to expose inventory; request filter calls a to add to the request; virt driver lifecycle ops call a & b to decide whether SEV-y things should be done or not) | |
| 17:59:05 | aspiers | OK | |
| 17:59:28 | efried | but if you want to just play around, I made this for you: https://review.opendev.org/664986 | |
| 18:00:13 | efried | ...for lifecycle ops to use to look for MEM_ENC_CTX in the alloc to bypass at least a). | |
| 18:02:36 | mriedem | umm, i just got back | |
| 18:02:53 | mriedem | not going to read all of the scrollback | |
| 18:03:09 | mriedem | without knowing more, i'd say i don't want to replicate allocation data per instance in nova | |
| 18:03:31 | mriedem | source of truth is the external service and all that | |
| 18:05:46 | sean-k-mooney | mriedem: the context was how to determin if sev is need. efried summerised where we landed a few lines up | |
| 18:07:00 | mriedem | isn't sev a required trait on the flavor? | |
| 18:07:16 | sean-k-mooney | its a resouce class in the flavor | |
| 18:07:17 | mriedem | if so, that is already information stored on the instance | |
| 18:07:23 | mriedem | ok either way | |
| 18:07:35 | efried | "stored on the instance" you mean via the flavor (and/or image in this case)? | |
| 18:07:42 | sean-k-mooney | yes efried didnt want to duplicate the logic between the filter an the virt driver | |
| 18:07:48 | mriedem | not the image | |
| 18:07:52 | mriedem | if it's a resource class it's not on the image | |
| 18:08:00 | mriedem | but yes if it's on the flavor it's embedded on the instance | |
| 18:08:03 | efried | The flavor doesn't use a resource class directly | |
| 18:08:06 | mriedem | instance.flavor.extra_spec | |
| 18:08:10 | mriedem | it's an extra spec | |
| 18:08:11 | mriedem | right? | |
| 18:08:15 | efried | In this case we decide whether to SEV based on flavor extra spec *or* image meta prop | |
| 18:08:17 | mriedem | resources:SEV=1 | |
| 18:08:25 | mriedem | what image meta prop? | |
| 18:08:30 | mriedem | i thought the image only had traits? | |
| 18:08:33 | efried | no, no explicit placement-ese in flavor or image | |
| 18:08:52 | efried | hw_memory_encryption=true <= img | |
| 18:08:52 | efried | hw:memory_encryption=true <= flavor | |
| 18:08:55 | mriedem | i have been deliberately ignoring / avoiding the sev talk going on in here for the last several months so idk wtf we are now | |
| 18:09:04 | efried | (I may have spelled it wrong, but that's the idea) | |
| 18:09:17 | efried | request filter interprets those into placement-ese | |
| 18:09:28 | mriedem | as a trait? or a resource class? | |
| 18:09:29 | mriedem | or both? | |
| 18:09:33 | openstackgerrit | Eric Fried proposed openstack/nova master: Raise if flavor and image disagree on hide_hypervisor_id https://review.opendev.org/663365 | |
| 18:09:37 | efried | a resource class | |
| 18:09:46 | efried | required=MEM_ENCRYPTION_CONTEXT:1 | |
| 18:09:46 | mriedem | with what amount? 1 | |
| 18:09:47 | mriedem | ? | |
| 18:09:59 | sean-k-mooney | yes | |
| 18:10:03 | efried | we got here because there's a limited number of SEVs you can do on a host | |
| 18:10:09 | efried | and | |
| 18:10:11 | mriedem | gimme all the sevs | |
| 18:10:29 | efried | we want the solution to be generic for memory encryption technologies, so e.g. MKTME can play in this space. | |
| 18:10:43 | mriedem | is mktme that band from the 90s? | |
| 18:10:47 | mriedem | with the cool album covers? | |
| 18:10:49 | efried | yes | |
| 18:10:50 | aspiers | :) | |
| 18:11:15 | aspiers | whereas SEV is just terrible Euro-pop | |
| 18:11:19 | mriedem | let me guess, mktme is intel something | |
| 18:11:35 | aspiers | yes, Intel multi-key total memory encryption | |
| 18:12:27 | edleafe | which sed | |
| 18:12:33 | edleafe | doh! | |
| 18:12:48 | sean-k-mooney | hehe this is one of the intel acronym that i dont auto expand when i see it | |
| 18:13:32 | efried | or Right SEV Fred, who's too sexy for his RAM | |
| 18:14:44 | aspiers | OK so are we still here? -> <efried> aspiers: not sure if you've been following along, but I think the consensus is to centralize the methods that do a) flavor/image parsing/validation, and b) host capability introspection; and call those from all the places (update_provider_tree calls b to expose inventory; request filter calls a to add to the request; virt driver lifecycle ops call a & b to decide | |
| 18:14:46 | aspiers | whether SEV-y things should be done or not) | |
| 18:15:30 | efried | unless mriedem has a different opinion on the other options discussed. | |
| 18:15:37 | efried | he already -1'd allocations-on-Instance | |
| 18:15:45 | efried | but has not weighed in on virtapi.allocations_for_instance | |
| 18:15:56 | efried | but jumped right to "go look at the flavor" | |
| 18:16:09 | efried | so that seems like the path | |
| 18:16:16 | aspiers | What about https://review.opendev.org/#/c/664420/4/nova/scheduler/request_filter.py@223 | |
| 18:16:20 | sean-k-mooney | i would emulate this https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L259 | |
| 18:16:40 | aspiers | alex_xu seems to have a reasonable point there | |
| 18:16:56 | sean-k-mooney | or maybe this is a little less scary https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L171 | |
| 18:17:08 | efried | aspiers, alex_xu: How is this different from what's happening in require_tenant_aggregate ? | |
| 18:17:15 | aspiers | sean-k-mooney: BTW you know that we have gitea running on opendev now :) | |
| 18:17:40 | sean-k-mooney | aspiers: i do and the last time i used it it nova almost crashed it | |
| 18:17:43 | efried | aspiers: Commenting on the review | |
| 18:17:57 | aspiers | sean-k-mooney: interesting, gitea seems a lot more lightweight on my browser than github | |
| 18:18:04 | aspiers | efried: thanks | |
| 18:18:07 | sean-k-mooney | that was because they messed up a tempelate which is now fixed but still | |
| 18:18:53 | aspiers | efried: right, I copied the raise RequestFilterFailed() from that | |
| 18:19:39 | sean-k-mooney | anyway im feeling really tired today so im going to go lie down. o/ talk to people tomorow | |
| 18:19:40 | aspiers | sean-k-mooney: so what exception should I throw if the SEV extra spec and image property conflict with each other? | |
| 18:19:52 | sean-k-mooney | am | |
| 18:20:09 | sean-k-mooney | it depend on where you are thowing it | |
| 18:20:23 | aspiers | well, you are suggesting to put it in hardware.py? | |
| 18:20:26 | sean-k-mooney | alex asked you to add some validation in the api as well | |
| 18:20:27 | aspiers | IIUC | |
| 18:20:35 | aspiers | it needs to be in a shared library, right? | |
| 18:20:58 | sean-k-mooney | ya so nova.virt.hardware.py is called everywhere | |
| 18:20:59 | aspiers | so that it can be called from the request_filter, from the driver, and maybe also from the api | |
| 18:21:07 | sean-k-mooney | its in the virt dirver schduler and api already | |
| 18:21:43 | aspiers | I'm talking about the exception currently at https://review.opendev.org/#/c/664420/4/nova/scheduler/request_filter.py@223 | |
| 18:21:52 | aspiers | if I move it to a shared library, the exception needs to be more generic | |
| 18:22:06 | sean-k-mooney | we have generally beein add new excpetion as needed for each check | |
| 18:22:06 | aspiers | InvalidRequest? | |
| 18:22:18 | aspiers | OK | |
| 18:22:25 | sean-k-mooney | like this | |
| 18:22:27 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1192-L1198 | |
| 18:23:02 | aspiers | that sounds fine to me | |
| 18:23:06 | aspiers | I can add one to exceptions.py | |
| 18:23:14 | aspiers | I mean exception.py | |
| 18:23:27 | aspiers | efried: does that work for you? | |
| 18:23:47 | openstackgerrit | Eric Fried proposed openstack/nova master: update constraints url https://review.opendev.org/664771 | |
| 18:24:40 | dansmith | efried: fwiw, my unmattering vote would be for just checking image+flavor for the sev thing and not doing the virtapi bit until we come up with a good reason | |