| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-05 | |||
| 11:44:50 | sean-k-mooney | Sundar: has all the cyborg code merged? | |
| 11:45:33 | sean-k-mooney | Sundar: if not we should add it to the first patch that calls cyborg directly | |
| 11:45:47 | sean-k-mooney | Sundar: which i think would be https://review.opendev.org/#/c/631244/38 | |
| 11:47:31 | sean-k-mooney | Sundar: by the way can we abandon https://review.opendev.org/#/q/topic:bp/nova-cyborg-interaction+(status:open+OR+status:merged)+project:opendev/sandbox | |
| 11:48:03 | Sundar | sean-k-mooney: Good point, I'll add the dependencies. Some Cyborg patches are close to merging, but the patches together are testable. The tempest scenarios are possible with one exception of device profile deletion in some circumstances. But that doesn't affect VM launch. | |
| 11:48:23 | artom | aspiers, overriding isn't cool either | |
| 11:48:47 | sean-k-mooney | Sundar: ok it would be good to ad a testing patch that runs the tempst job on nova | |
| 11:48:51 | artom | aspiers, validate_flavor_image() can't be in the driver itself because we don't want to call to the individual driver instance | |
| 11:49:05 | artom | Which is why get_machine_type is in libvirt_utils | |
| 11:49:20 | sean-k-mooney | Sundar: do you mind if i abandon the two patches against the sandbox repo to clean up the gerrit topic | |
| 11:49:31 | artom | Because it's libvirt'y, but doesn't depend on a specific host | |
| 11:49:51 | artom | aspiers, if you can find a way to abstract that ^^ for all drivers, I think it'd be the best solution | |
| 11:50:17 | sean-k-mooney | artom: the validate flavor stuff belongs in hardware.py | |
| 11:50:30 | Sundar | sean-k-mooney: I wasn;t even aware of these patches! They seem very preliminary. I don't know the author. But, IMHO, they can be abandoned. | |
| 11:50:50 | sean-k-mooney | Sundar: the sandbox repo is the one for teaching people how to use gerrit | |
| 11:51:11 | sean-k-mooney | so they are not but they have the topic which is confusing | |
| 11:51:20 | sean-k-mooney | ill close them so | |
| 11:51:23 | Sundar | sean-k-mooney: Re. "testing patch that runs the tempst job on nova", I believe that would be https://review.opendev.org/#/c/670999/ | |
| 11:51:28 | sean-k-mooney | given they are 6 mohts old | |
| 11:51:30 | artom | sean-k-mooney, yeah but aspiers has a problem with circular dependencies | |
| 11:51:38 | sean-k-mooney | artom: not any more | |
| 11:51:39 | aspiers | artom, sean-k-mooney: so the suggestion is a common validation interface across all drivers? | |
| 11:51:45 | aspiers | sean-k-mooney: no I do | |
| 11:51:56 | aspiers | sean-k-mooney: I was wrong (again) | |
| 11:52:19 | sean-k-mooney | hardware specifc tratit valdiation lives in hardare.py | |
| 11:52:19 | aspiers | sean-k-mooney: https://review.opendev.org/#/c/680065/ has circular dependencies even before sev_enabled is added | |
| 11:52:21 | luyao | stephenfin: comments addressed , and thanks for your review. https://review.opendev.org/#/q/topic:bp/virtual-persistent-memory+(status:open) | |
| 11:53:02 | aspiers | sean-k-mooney, artom: http://paste.openstack.org/show/771288/ | |
| 11:53:17 | aspiers | sean-k-mooney: so unfortunately your suggestion wasn't enough to solve it | |
| 11:53:20 | sean-k-mooney | s/triatis/flavor and image/ its where all the numa, pinning, hugepages and pci checks live | |
| 11:54:04 | aspiers | sean-k-mooney: right but until now it never cared about machine types | |
| 11:54:33 | aspiers | maybe moving get_machine_type to a separate class is the only way | |
| 11:54:37 | aspiers | s/class/file/ | |
| 11:55:08 | sean-k-mooney | no that wont work | |
| 11:55:16 | aspiers | right it won't | |
| 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 | |