| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-12 | |||
| 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 <= flavor | |
| 18:08:52 | efried | hw_memory_encryption=true <= img | |
| 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 | mriedem | with what amount? 1 | |
| 18:09:46 | efried | required=MEM_ENCRYPTION_CONTEXT: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 | aspiers | InvalidRequest? | |
| 18:22:06 | sean-k-mooney | we have generally beein add new excpetion as needed for each check | |
| 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 | |
| 18:24:45 | dansmith | this being, not a good enough reason | |
| 18:24:54 | dansmith | in the future, when said reason exists, that's how we do it, IMHO | |
| 18:24:55 | efried | dansmith: ack, that's where we're going | |
| 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 | |