| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-12 | |||
| 12:37:18 | sean-k-mooney | if so then i would have it validate them and raise an exception if there is a conflict | |
| 12:37:31 | aspiers | sean-k-mooney, alex_xu: I just did something very similar for SEV | |
| 12:37:40 | aspiers | https://review.opendev.org/#/c/664420/4 | |
| 12:37:49 | sean-k-mooney | alex_xu: am we could just parse it there yes | |
| 12:37:51 | alex_xu | aspiers: nice | |
| 12:38:04 | alex_xu | sean-k-mooney: yea | |
| 12:38:14 | sean-k-mooney | aspiers: your using a request prefilter | |
| 12:38:26 | aspiers | yes | |
| 12:39:16 | alex_xu | at least request prefilter is better than mixing everything ResourceRequest.from_extra_specs | |
| 12:39:54 | sean-k-mooney | right the ohter option is to do it here https://github.com/openstack/nova/blob/master/nova/scheduler/utils.py#L387 | |
| 12:40:08 | sean-k-mooney | and in resources_from_request_spec | |
| 12:40:19 | sean-k-mooney | but the prefilter i think are cleaner | |
| 12:40:33 | sean-k-mooney | both would work | |
| 12:40:40 | alex_xu | yes, agree | |
| 12:40:47 | alex_xu | we can add that for pmem | |
| 12:41:48 | sean-k-mooney | alex_xu: as aspiers has done you can do your validation like this https://review.opendev.org/#/c/664420/4/nova/scheduler/request_filter.py@197 | |
| 12:42:05 | sean-k-mooney | to ensure there is no conflict | |
| 12:42:27 | aspiers | right | |
| 12:42:29 | alex_xu | sean-k-mooney: then the instance is going to error status? | |
| 12:42:52 | sean-k-mooney | yes i think so | |
| 12:42:57 | alex_xu | I guess people hate the instance is going to error status.... | |
| 12:43:02 | sean-k-mooney | we will refuse to sechdule | |
| 12:43:34 | sean-k-mooney | well if we did not validate it would still go into error | |
| 12:43:41 | sean-k-mooney | but it woudl be a no valid host error | |
| 12:43:46 | alex_xu | we can validate that in the API layer | |
| 12:43:54 | sean-k-mooney | as teh livbrt driver would presuably fail to swapn it | |
| 12:44:01 | alex_xu | the request prefilter is only do the placement translation | |
| 12:44:23 | sean-k-mooney | where in the api laryer? | |
| 12:44:44 | sean-k-mooney | on flavor setting properties on teh flaovr or on instance spawn | |
| 12:45:07 | alex_xu | like somewhere we validate the hw:numa_* extra specs | |
| 12:45:14 | sean-k-mooney | we dont | |
| 12:45:27 | sean-k-mooney | well the first validation happens in the numa toplogy filter | |
| 12:46:07 | sean-k-mooney | but its not really validation we try to use the values to filter a host and that either work or it doesnt | |
| 12:46:34 | sean-k-mooney | so i think doing it in the prefilter which is even earlier in scheduling is fine | |
| 12:46:51 | alex_xu | wo do, let me find the link | |
| 12:47:28 | alex_xu | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L659 | |
| 12:48:37 | sean-k-mooney | oh ok i really never knew that was there | |
| 12:48:47 | sean-k-mooney | in that case sure you can add the same validate for pmem | |
| 12:48:49 | alex_xu | so we can resolve the conflict of hw:pmem and resource:* somewhere like that | |
| 12:49:00 | alex_xu | \o/ finally find something sean-k-mooney doesn't know | |
| 12:49:58 | alex_xu | so I can override resources:* when hw:pmem present. | |
| 12:49:59 | sean-k-mooney | hehe :) there are plenty of things i dont know like how to use a mac without breaking it or how to spell litrally everything | |
| 12:50:06 | aspiers | XD | |
| 12:50:12 | alex_xu | hah | |
| 12:50:46 | aspiers | sean-k-mooney: maybe not, but I bet you can help me with this problem | |
| 12:50:53 | alex_xu | the last question is what should I do for the user only specify the resources:*. so I can prase the resources:* and put them into instance.vpmems. | |
| 12:50:55 | aspiers | https://review.opendev.org/#/c/644565/5/nova/virt/libvirt/driver.py@1625 | |
| 12:51:26 | aspiers | I need to change _sev_required() so that it checks the resource allocations for the instance (the MEMORY_ENCRYPTION_CONTEXT class) | |
| 12:51:53 | aspiers | but the callers don't have access to the resource allocations | |
| 12:51:58 | aspiers | AFAICS | |
| 12:51:58 | sean-k-mooney | alex_xu: i think we shoudl require uses to use hw:pmem personally as we might want to use the resouce class for something else in the future | |
| 12:52:23 | aspiers | they have access to the request context and the instance, but not the allocations | |
| 12:52:25 | sean-k-mooney | alex_xu: im thinking of the case where we might want to back guest ram by PMEM | |
| 12:52:34 | sean-k-mooney | e.g. as an alternive to hugepages | |
| 12:52:39 | sean-k-mooney | or file backed memeory | |
| 12:53:07 | sean-k-mooney | aspiers: looking | |
| 12:53:19 | alex_xu | good idea | |
| 12:53:34 | aspiers | I found _instance_to_allocations_dict() in scheduler/client/report.py which maybe could extract the allocations from the instance, but I don't know if that's right | |
| 12:53:47 | alex_xu | do you mean reject pmem resource class in extra spec resource:*? | |
| 12:53:55 | aspiers | also, nothing uses that method any more, since Iec02942d38 removed the last callers of it | |
| 12:54:46 | sean-k-mooney | alex_xu: we could but i was thinking just not populating the instance.vpmems from it | |
| 12:55:34 | alex_xu | that will do an allocation in the placement, but in the end, the instance won't attach the pmem | |
| 12:55:39 | sean-k-mooney | aspiers: do you need to cehck the resouce allocation or jsut the flavor | |
| 12:55:46 | aspiers | sean-k-mooney: the allocation | |
| 12:55:54 | aspiers | that's what efried said yesterday | |
| 12:56:37 | sean-k-mooney | im not sure why checking the request_sepc or flavor/image would not be enough | |
| 12:56:52 | aspiers | sean-k-mooney: http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-06-11.log.html#t2019-06-11T16:49:51 | |
| 13:00:08 | aspiers | sean-k-mooney: but maybe when efried wrote that he didn't know that _sev_required() needs to be called from multiple locations in the driver code which don't have access to allocations | |
| 13:00:18 | aspiers | I guess I could set another instance variable | |
| 13:00:22 | sean-k-mooney | the instance contains the request_sepc and the request spec contains the request_resouces | |
| 13:00:29 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/objects/request_spec.py#L98 | |
| 13:01:04 | aspiers | but requested_resources is different from allocations, no? | |
| 13:01:12 | sean-k-mooney | it is | |
| 13:01:24 | sean-k-mooney | and it does not currenly hav the info you want see the comment above | |
| 13:01:43 | aspiers | right | |
| 13:01:48 | sean-k-mooney | im just wondering is that better to use in the future | |
| 13:01:51 | aspiers | in fact efried wants me to help fix that | |
| 13:02:29 | aspiers | I'm guessing that code which wants to know whether to add SEV bits to the libvirt XML config should be checking what got allocated, not just what got requested | |
| 13:02:50 | sean-k-mooney | they should be the same thing | |
| 13:03:10 | sean-k-mooney | if placemnet returned an allcoation candatae then eveything that was requested was allcoated | |
| 13:03:32 | aspiers | I guess in the MEM_ENCRYPTION_CONTEXT case where only 1 unit is requested they have to be the same | |
| 13:03:34 | sean-k-mooney | looking at the instance object i dont see the allcoation stored anywhere | |
| 13:03:43 | aspiers | yeah I couldn't either | |
| 13:05:15 | aspiers | I found https://opendev.org/openstack/nova/src/branch/master/nova/compute/manager.py#L2434 | |
| 13:07:18 | aspiers | that passes allocations to spawn() via https://opendev.org/openstack/nova/src/branch/master/nova/compute/manager.py#L2203 | |
| 13:08:01 | sean-k-mooney | ya we do pass them to the driver | |
| 13:08:03 | sean-k-mooney | https://opendev.org/openstack/nova/src/branch/master/nova/compute/manager.py#L2207 | |
| 13:08:16 | aspiers | that's 4 lines below my link :) | |
| 13:08:19 | aspiers | same thing | |
| 13:08:43 | aspiers | but then allocs doesn't get passed around to all the smaller methods called by spawn() | |
| 13:09:06 | sean-k-mooney | right its only used to get teh mdevs | |
| 13:09:07 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L3237 | |
| 13:09:49 | aspiers | so maybe I can set an _sev_required instance variable in spawn() | |
| 13:10:26 | sean-k-mooney | well you could jsut pass the allcoation to _get_guest_xml | |
| 13:10:26 | aspiers | or somewhere underneath that | |
| 13:10:45 | aspiers | that's a good idea | |
| 13:10:57 | aspiers | I guess I only need it for constructing XML | |
| 13:11:02 | aspiers | so that would scope it correctly | |
| 13:11:20 | aspiers | hopefully efried will come online before I disappear down this rathole :) | |
| 13:12:33 | sean-k-mooney | its still a little messy | |