| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-05 | |||
| 11:55:26 | sean-k-mooney | it because importing nova/virt/libvirt/__init__.py has sideffects | |
| 11:55:26 | aspiers | well, not unless it is moved outside nova.virt.libvirt | |
| 11:55:29 | aspiers | yes | |
| 11:55:51 | sean-k-mooney | we should remove this https://github.com/openstack/nova/blob/master/nova/virt/libvirt/__init__.py#L17 | |
| 11:55:56 | aspiers | that sounds good | |
| 11:56:00 | aspiers | why is it even there? | |
| 11:56:11 | sean-k-mooney | we proably moved things and that was a quick hack | |
| 11:56:31 | aspiers | OK so first I have to get rid of that | |
| 11:56:33 | aspiers | sigh | |
| 11:58:26 | sean-k-mooney | remind me why you need to call the fucntion to get the machine type | |
| 11:58:39 | sean-k-mooney | i know its for suspend and live migate | |
| 11:58:46 | aspiers | no | |
| 11:58:53 | aspiers | it's to check for q35 | |
| 11:58:56 | sean-k-mooney | yes | |
| 11:58:59 | aspiers | https://review.opendev.org/#/c/680065/6//COMMIT_MSG | |
| 11:59:05 | aspiers | SEV doesn't work without q35 | |
| 11:59:15 | sean-k-mooney | aspiers: where is that check going to be called form | |
| 11:59:21 | sean-k-mooney | it wont be used in the api | |
| 11:59:28 | aspiers | yes in the API was the idea | |
| 11:59:34 | sean-k-mooney | and it can only be known on the compute node | |
| 11:59:41 | sean-k-mooney | but what api | |
| 11:59:49 | sean-k-mooney | live migrate and suspend? | |
| 11:59:55 | aspiers | no, launch | |
| 12:00:04 | sean-k-mooney | it cant be known at lauch | |
| 12:00:08 | sean-k-mooney | we have not selected the host | |
| 12:00:16 | sean-k-mooney | and the machine type can be set in the hsot config | |
| 12:00:26 | aspiers | right, this is the crux of the problem | |
| 12:00:40 | sean-k-mooney | right so you cant check for q35 in the api | |
| 12:00:41 | aspiers | but we can at least check the image | |
| 12:00:51 | sean-k-mooney | at lest not the create server api | |
| 12:01:02 | aspiers | no we can check for hw_machine_type in the image | |
| 12:01:19 | aspiers | we already require hw_firmware_type=uefi | |
| 12:01:21 | sean-k-mooney | checkign the image does not require calling get default machine tyhpe | |
| 12:01:24 | sean-k-mooney | or whathever | |
| 12:01:42 | sean-k-mooney | if we are jsut checking the image then just check the metadata directly | |
| 12:02:17 | aspiers | that's what I was originally doing https://review.opendev.org/#/c/644565/49/nova/virt/libvirt/utils.py@538 | |
| 12:02:38 | sean-k-mooney | that check should be in hardware.py direclty | |
| 12:02:46 | aspiers | https://review.opendev.org/#/c/644565/49/nova/virt/libvirt/driver.py@5097 | |
| 12:02:54 | aspiers | I was originally only checking the image | |
| 12:02:57 | sean-k-mooney | you can have a second check for the machine type on the host | |
| 12:03:17 | sean-k-mooney | aspiers: for the fast fail on create server i think that is all you can do | |
| 12:03:27 | aspiers | hang on | |
| 12:03:31 | sean-k-mooney | then you can do the late check in the dirver | |
| 12:03:45 | aspiers | either we *require* hw_machine_type=q35 image property or we make it optional | |
| 12:03:49 | aspiers | which are you suggesting? | |
| 12:03:58 | aspiers | I mean in the SEV case | |
| 12:04:01 | sean-k-mooney | we will not require it | |
| 12:04:06 | sean-k-mooney | or should not | |
| 12:04:14 | aspiers | OK so then what's the point of an API check? | |
| 12:04:16 | sean-k-mooney | we can check if hw_machine_type is set | |
| 12:04:27 | sean-k-mooney | and if its not q35 reject | |
| 12:04:32 | aspiers | OK yeah | |
| 12:04:41 | aspiers | then if not set, it will fail later | |
| 12:04:46 | sean-k-mooney | but the api check was not for create it was for live migrate and suspend originaly | |
| 12:04:51 | sean-k-mooney | yep | |
| 12:04:51 | aspiers | perhaps horribly on multiple compute hosts | |
| 12:05:02 | aspiers | no live migrate/suspend is a different topic | |
| 12:05:09 | aspiers | that's not related to machine type | |
| 12:05:22 | aspiers | that relies on checking for hw_mem_encryption | |
| 12:05:30 | sean-k-mooney | we could maybe report machine types as traits in the future | |
| 12:05:36 | sean-k-mooney | but that is not ideal | |
| 12:05:37 | aspiers | that's this one https://review.opendev.org/#/c/680158/3 | |
| 12:05:47 | aspiers | kashyap: didn't we talk about that? | |
| 12:05:59 | aspiers | kashyap: about machine types as traits? | |
| 12:06:08 | aspiers | thought I vaguely remembered some discussion on that | |
| 12:06:10 | aspiers | maybe not | |
| 12:06:12 | sean-k-mooney | i think that is leaking too much info personally | |
| 12:06:33 | aspiers | I don't see a security risk but different rathole | |
| 12:06:43 | aspiers | maybe you're right but let's not go there now | |
| 12:06:44 | sean-k-mooney | no that is not what i ment | |
| 12:07:19 | sean-k-mooney | we could advertise that the host has q35 support | |
| 12:07:24 | aspiers | OK so to summarise, the plan is this: | |
| 12:07:37 | aspiers | hw_machine_type=q35 image prop is optional for SEV | |
| 12:07:57 | aspiers | we have an API check enforcing that if it's there, it has to be q35 family | |
| 12:08:19 | aspiers | but if it's not there then we just hope the scheduler picks a machine with nova.conf defaulting x86_64 to q35 | |
| 12:08:35 | aspiers | if it doesn't it will fail and try other compute hosts | |
| 12:08:44 | sean-k-mooney | and check it in the driver | |
| 12:08:46 | sean-k-mooney | but yes | |
| 12:08:57 | sean-k-mooney | it will fail and rescdule | |
| 12:09:18 | sean-k-mooney | and you can use host aggreates ectra if you want too to reduce the chance of that happening | |
| 12:09:32 | aspiers | so the operator should configure CONF.libvirt.machine_type to x86_64=q35 on all SEV hosts | |
| 12:09:35 | sean-k-mooney | if you dont want to set q35 in the image or the config | |
| 12:09:55 | sean-k-mooney | yes i think that is what we shoudl recommend | |
| 12:10:03 | aspiers | OK | |
| 12:10:05 | aspiers | BUT | |
| 12:10:15 | aspiers | does that fix the import cycle? | |
| 12:10:28 | aspiers | it means there are two types of q35 check | |
| 12:10:37 | aspiers | one image-only, and one including nova.conf | |
| 12:10:42 | sean-k-mooney | yes in hardware.py you dont need to import the utils funciton anymore | |
| 12:10:52 | sean-k-mooney | hardware.py just does the image check | |
| 12:11:11 | sean-k-mooney | and the driver just does the final machine type check using the image+config | |
| 12:11:38 | aspiers | I don't follow | |
| 12:11:40 | sean-k-mooney | so no import of libvirt.utils is need in hardware.py | |
| 12:11:53 | sean-k-mooney | ill comment on the patch | |
| 12:11:57 | aspiers | wait | |
| 12:12:05 | aspiers | which one of the two types of q35 check are you suggesting happens in hardware.py? | |
| 12:12:13 | aspiers | the API one or the driver one? | |
| 12:12:28 | aspiers | the driver one *does* need libvirt.utils | |
| 12:12:34 | aspiers | to read the config | |
| 12:12:58 | aspiers | sean-k-mooney: ^^ (in case you are already commenting on the patch) | |