| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-12 | |||
| 17:27:06 | sean-k-mooney | and use the request for sev | |
| 17:27:17 | efried | we could reinvoke the logic we used to decide the first time | |
| 17:27:32 | dansmith | efried: point being, let's store what we need about that decision and not just couple the guts of the virt driver into the placement data | |
| 17:27:35 | efried | by looking at the flavor & image - which we have access to via the Instance obj, right? | |
| 17:27:46 | sean-k-mooney | ya we can compute it form the info in the request spec which these fucntion already have acess too | |
| 17:28:09 | dansmith | because the other thing is, allocations are somewhat tied to a microversion in placement, and while it seems hard to imagine the core structure changing, we've changed a lot of the external data structures of placement since we started | |
| 17:29:45 | sean-k-mooney | so once the comment is address i personally would have check the requestd_resouces object in the request spec https://github.com/openstack/nova/blob/master/nova/objects/request_spec.py#L93-L100 | |
| 17:30:06 | sean-k-mooney | but we also have acess to the image and flavor via the instance yes | |
| 17:31:14 | dansmith | efried: re: [10:27:05] <efried>dansmith: It's not "the only way we can think to know that later" | |
| 17:31:36 | dansmith | efried: I know not the only way, but "the way", collapsed to "only" for dramatic effect | |
| 17:36:56 | sean-k-mooney | so the instance is already passed to all the places we need to check if the instace requested sev | |
| 17:37:47 | sean-k-mooney | so why dont we jsut hav ea single function that takes the instance object and determins if sev is needed and reuse that everywhere we care. | |
| 17:38:22 | sean-k-mooney | that could live in nova.virt.hardware.yp with the numa stuff? | |
| 17:38:58 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: Include direct-physical in compute manager events check https://review.opendev.org/664431 | |
| 17:38:58 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Revert resize: wait for events according to hybrid plug https://review.opendev.org/644881 | |
| 17:38:59 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [DNM] testing bug/1813789 revert resize events https://review.opendev.org/664442 | |
| 17:40:11 | sean-k-mooney | like is_realtime_enabled https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1436-L1438 | |
| 17:40:34 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: ComputeVirtAPI.allocations_for_instance https://review.opendev.org/664986 | |
| 17:40:39 | efried | aspiers: for grins, you can try this out ^ | |
| 17:40:45 | efried | dansmith: this what you had in mind? | |
| 17:41:23 | dansmith | efried: mostly | |
| 17:41:43 | efried | I explained why no caching in the commit message | |
| 17:41:46 | sean-k-mooney | efried: but is there any reason to have to check the allotion the athoritive info is stored in the image/flavor not the allcoation | |
| 17:42:09 | efried | sean-k-mooney: just to not have to duplicate that logic. | |
| 17:42:20 | efried | but in this case, no, not really. | |
| 17:42:24 | sean-k-mooney | an api call is a lot more expensive | |
| 17:42:28 | dansmith | is a single call to that less expensive than calling the placement? | |
| 17:42:29 | efried | Accessing the flavor/image meta from Instance is cheap | |
| 17:42:54 | efried | dansmith: assuming we already have the flavor & image available in the Instance object, it's all local, yeah. | |
| 17:43:00 | efried | well | |
| 17:43:06 | efried | some of it involves pawing at sysfs | |
| 17:43:13 | sean-k-mooney | we do | |
| 17:43:14 | dansmith | it better not, | |
| 17:43:24 | dansmith | because the api nodes' sysfs wouldn't be the same as compute | |
| 17:43:58 | sean-k-mooney | we dont need to look at sysfs for schduling | |
| 17:44:04 | efried | right, the compute pawed at sysfs to expose the inventory, so by virtue of having the MEM_ENC_CTX inventory at all, we know we did the sysfs stuff already. | |
| 17:44:09 | sean-k-mooney | we may or may not for xml generation | |
| 17:44:21 | dansmith | efried: ack, I see | |
| 17:44:25 | efried | but how would I know that on the compute without looking at placement? :P | |
| 17:44:42 | sean-k-mooney | efried: know what? | |
| 17:45:00 | efried | know that I can SEV | |
| 17:45:11 | sean-k-mooney | because its asked for in teh flavor/image | |
| 17:45:17 | efried | no | |
| 17:45:24 | sean-k-mooney | if we got to the compute node we know we were allocated everything we asked for | |
| 17:45:32 | efried | well, yes, okay | |
| 17:45:44 | efried | that's kind of a lot of assuming. | |
| 17:46:00 | efried | including assuming nothing changed on the host that makes it no longer work | |
| 17:46:02 | sean-k-mooney | assuming that if placment could not fulfil a request it returned no allcoation candates? | |
| 17:46:02 | efried | but that's reasonable. | |
| 17:46:23 | sean-k-mooney | that kind of fundemental no? | |
| 17:46:32 | dansmith | definitely don't think that being sent an instance with sev required is good enough to assume we have it | |
| 17:46:36 | dansmith | because we don't know what the scheduler policy is | |
| 17:46:41 | dansmith | or if that node is old | |
| 17:46:49 | efried | or got force migrated? | |
| 17:46:52 | dansmith | or if the request has been sitting in a rabbit queue across config changes | |
| 17:47:04 | sean-k-mooney | well the virt direver is going to check compatiblity | |
| 17:47:14 | sean-k-mooney | right | |
| 17:47:32 | efried | so then yeah, in order to truly fulfill "can and should do SEV" I would need to check the flavor/image *and* sysfs, every time. | |
| 17:47:45 | efried | which is still going to be cheaper than calling placement across the wire | |
| 17:47:58 | sean-k-mooney | well libvirt capablitys api | |
| 17:48:08 | sean-k-mooney | rather then sysfs but same effect | |
| 17:48:30 | efried | yeah, using "sysfs" to encompass "introspecting the host in whatever way is necessary" | |
| 17:48:49 | efried | which should be all "local" one way or another. | |
| 17:49:09 | efried | "local" to the system; not necessarily local to whatever's in process memory already, which would be even better. | |
| 17:51:37 | sean-k-mooney | so back to https://review.opendev.org/#/c/664420/4/nova/scheduler/request_filter.py | |
| 17:52:42 | sean-k-mooney | if we just move thos three funtion into nova/virt/hardware.py wiht all the cpu pinning and numa stuff and just call | |
| 17:52:43 | dansmith | artom: I think maybe too much got removed, but see my comments and hopefully prove me wrong | |
| 17:53:16 | artom | dansmith, looking | |
| 17:53:34 | sean-k-mooney | required_encryped_memory support in both the dirver and filter that shoudl be fine right | |
| 17:53:36 | efried | sean-k-mooney: that and the sysfs pawing part, yes. Not sure if hardware.py is the right place | |
| 17:54:18 | sean-k-mooney | i think this is similarly to hugepages,realtime,cpupinning and numa | |
| 17:54:28 | sean-k-mooney | all of which are in the hardware.py module | |
| 17:54:47 | efried | does hardware.py also do the flavor interpretation bits associated with that? | |
| 17:54:57 | sean-k-mooney | efried: yes | |
| 17:55:04 | efried | I kind of object to hardware.py not being nova/virt/libvirt/hardware.py | |
| 17:55:18 | efried | but that soapbox is gathering dust in my closet at this point. | |
| 17:55:19 | sean-k-mooney | its used in other driver too | |
| 17:55:28 | sean-k-mooney | its used in the hyperv driver | |
| 17:55:33 | sean-k-mooney | and its used in the api | |
| 17:55:39 | sean-k-mooney | and the scheduler | |
| 17:55:56 | aspiers | I can never drop bad habits :-( | |
| 17:56:20 | sean-k-mooney | the trick is to replace them with other sligly less bad habits | |
| 17:56:28 | aspiers | true | |
| 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 | |