| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-03 | |||
| 19:42:06 | artom | Eventually | |
| 19:42:19 | mriedem | can i get another core to approve this simple refactor https://review.opendev.org/#/c/679651/ | |
| 19:49:16 | mriedem | dansmith: btw on https://review.opendev.org/#/c/633293/46 i gave a paste for what it would look like to use the existing prep_resize | |
| 19:50:27 | mriedem | at this point we can defer caring about that series until U i guess | |
| 19:55:54 | artom | dansmith, btw, I have a func test for RPC pinning, and it did find 1 bug. Wonder why it didn't pick up drop_move_claim() raising an RPC version error | |
| 19:56:31 | dansmith | artom: I dunno, probably worth looking though | |
| 19:56:39 | artom | Yep. | |
| 19:56:57 | dansmith | mriedem: okay, sorry I didn't really mean to block everything on that, I just wanted to ponder a bit but I never got anywhere | |
| 19:57:32 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Provider Config File: Update provider tree with new custom resources https://review.opendev.org/676522 | |
| 20:03:31 | sean-k-mooney | ok i think im following what ye are suggesting | |
| 20:03:51 | sean-k-mooney | i also like mriedem function name but pep8 would hate it | |
| 20:21:02 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: NUMA live migration support https://review.opendev.org/634606 | |
| 20:21:03 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Deprecate CONF.workarounds.enable_numa_live_migration https://review.opendev.org/640021 | |
| 20:21:03 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Functional tests for NUMA live migration https://review.opendev.org/672595 | |
| 20:21:38 | artom | dansmith, mriedem, ^^ and at this point I need to done for a bit, because it's dad taxi and supper time | |
| 20:22:24 | artom | But if there's anything I'll try to address it later tonight | |
| 20:26:09 | sean-k-mooney | aspiers: som of the sev functional test are asserting behavior im not sure is correct or rather is failing when in integrate my image-metadata-prefilter series | |
| 20:26:44 | sean-k-mooney | aspiers: you seam to be asserting the ablit for the sev support to be removed while the agent is running. | |
| 20:27:19 | sean-k-mooney | the commen stats that thsi coudl happen as a result of a reconfiguration or kernel downgrade which woul nomally inlvoe the agent being resarted | |
| 20:28:45 | sean-k-mooney | aspiers: it looks like you are adding the sev tratis to the existing cpu tratis function which i am now cacheing as a property | |
| 20:28:58 | sean-k-mooney | https://review.opendev.org/#/c/666915/15/nova/virt/libvirt/driver.py | |
| 20:29:34 | sean-k-mooney | i can fix your functional test by reseting the property when you simulate teh restart of the compute service | |
| 20:29:52 | sean-k-mooney | but i wan to confim that you dont intend this to change outside of an agent restart | |
| 20:36:37 | sean-k-mooney | aspiers: im going to proceed with the assumetion i have interpred the test coorectly based on the comments and you are simulating an agent restart and just reset the chached traits. if you get a chance to review and confim that woudl be good. | |
| 21:12:49 | efried | nova FYI https://review.opendev.org/#/c/679862/ | |
| 21:14:20 | dansmith | +1 | |
| 21:24:19 | openstackgerrit | sean mooney proposed openstack/nova master: libvirt: use domain capabilities to get supported device models https://review.opendev.org/666915 | |
| 21:24:20 | openstackgerrit | sean mooney proposed openstack/nova master: Add transform_image_metadata request filter https://review.opendev.org/665775 | |
| 21:27:34 | mriedem | har har jk lol | |
| 21:30:51 | dansmith | heh | |
| 21:37:00 | aspiers | sean-k-mooney: the comments which explicitly say it's simulating agent restart are correct | |
| 21:37:15 | aspiers | https://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/libvirt/test_report_cpu_traits.py#L105 | |
| 21:37:22 | aspiers | https://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/libvirt/test_report_cpu_traits.py#L163 | |
| 21:38:14 | aspiers | efried: you around? need some quick advice on https://review.opendev.org/#/c/644565 | |
| 21:40:15 | efried | aspiers: ... | |
| 21:40:22 | aspiers | I think https://review.opendev.org/#/c/644565/49/nova/virt/libvirt/driver.py@5089 needs to be added to utils.py instead | |
| 21:40:46 | aspiers | since it turns out that blockinfo.py also needs to call it | |
| 21:41:14 | aspiers | or more precisely, blockinfo.py needs to know the same machine type, and in both cases SEV needs to be taken into account | |
| 21:41:37 | efried | aspiers: You want my advice for this why? | |
| 21:42:08 | aspiers | Well, just a sanity check that sounds reasonable, because that would mean passing more info into blockinfo.py | |
| 21:42:19 | aspiers | Adding SEV checks to the machine type calculation requires extra info | |
| 21:42:28 | aspiers | e.g. the Host object to check whether SEV is supported | |
| 21:42:41 | aspiers | or the sev_enabled variable if already calculated by the caller | |
| 21:43:07 | efried | At a glance, I agree it looks weird for a method with this generic a name to need anything sev-specific. | |
| 21:43:38 | efried | but I've repeatedly punted on this machine type stuff for a reason. | |
| 21:43:56 | aspiers | The key case here is when there is no machine type specified in the image | |
| 21:44:08 | aspiers | Maybe I can hassle kashyap about it | |
| 21:44:26 | efried | you need to default the machine type differently depending on whether SEV is enabled?? | |
| 21:44:30 | aspiers | Yes | |
| 21:44:38 | efried | that... doesn't sound right | |
| 21:44:40 | aspiers | Normally 'pc' for x86_64, but 'q35' if SEV is enabled | |
| 21:44:53 | aspiers | That's in the spec | |
| 21:45:21 | aspiers | Ahh, actually the spec is out of date on this point | |
| 21:45:47 | efried | What sounds more likely to me is: | |
| 21:45:47 | efried | If the machine type is not specified, we default it to X. | |
| 21:45:47 | efried | X doesn't work for SEV, so if you do that, SEV will be disabled. | |
| 21:45:47 | efried | SEV requires machine type Y, so if you want to SEV, you have to specify machine type Y as well as {other stuff to get SEV} | |
| 21:45:48 | aspiers | No, the spec is technically correct | |
| 21:46:34 | aspiers | AFAICS your proposal would have a big (negative) impact on usability of SEV | |
| 21:46:37 | efried | put another way, the "is SEV enabled" check gets an additional condition for machine type. And the above is how that's reflected in the documentation. | |
| 21:46:55 | aspiers | It would make it impossible to turn SEV on or off just by choosing a different flavor | |
| 21:47:09 | aspiers | You'd have to also choose an SEV-specific image to go alongside it | |
| 21:47:18 | efried | um | |
| 21:47:30 | efried | you don't have to do that anyway? | |
| 21:47:36 | aspiers | No | |
| 21:47:47 | aspiers | You can have one image which can be used SEV or non-SEV | |
| 21:47:54 | efried | can you not specify machine type via flavor? | |
| 21:48:32 | aspiers | I don't think so | |
| 21:49:10 | aspiers | https://opendev.org/openstack/nova/src/branch/master/nova/virt/libvirt/utils.py#L543 | |
| 21:49:11 | efried | sorry, but if I'm understanding this correctly, I'm afraid that has to be the right call. | |
| 21:49:26 | efried | what are the reasons you would specify a machine type in your image? | |
| 21:49:37 | efried | possibly more importantly, what are the reasons you wouldn't? | |
| 21:49:53 | aspiers | Well, having the choice of SEV is one reason not to | |
| 21:50:09 | aspiers | Well I suppose it could be set to q35 and work non-SEV | |
| 21:50:32 | efried | and crucially, are we breaking anyone by setting a default different than the default they're expecting? | |
| 21:50:55 | aspiers | There are different levels of default | |
| 21:51:09 | aspiers | There's arch-specific defaults hardcoded into nova | |
| 21:51:16 | aspiers | There's the default per arch in nova.conf | |
| 21:51:56 | aspiers | There's even a QEMU default, which is changing to q35 at some point | |
| 21:52:04 | aspiers | (hence https://bugs.launchpad.net/nova/+bug/1780138) | |
| 21:52:05 | openstack | Launchpad bug 1780138 in OpenStack Compute (nova) "Don't assume the guest machine type to be of 'pc'" [Medium,Confirmed] - Assigned to Kashyap Chamarthy (kashyapc) | |
| 21:52:38 | efried | forgive me for having swapped all of this out, but do we have some flavor/image cross-validation code for this in place? | |
| 21:53:10 | aspiers | That's what I was in the process of adding | |
| 21:53:11 | efried | So if their image explicitly says machine type X and their flavor asks for SEV, we can punt? | |
| 21:53:22 | aspiers | Yes, that's what I agreed this morning with kashyap and sean-k-mooney | |
| 21:53:29 | efried | okay | |
| 21:53:33 | aspiers | I'm adding an InvalidMachineType exception | |
| 21:53:53 | aspiers | as per https://review.opendev.org/#/c/644565/49/nova/virt/libvirt/driver.py@5105 | |
| 21:54:24 | efried | and this is one where we're doing a request filtery thing (except no longer in request filter - now it's a RequestSpec loader thing?) to translate/add placement-isms | |
| 21:54:42 | aspiers | No, that's a different set of checks | |
| 21:54:58 | efried | yeah, I know, but I mean that's another part of this feature | |
| 21:55:19 | aspiers | It's part of the same series, yeah | |
| 21:55:49 | efried | and how many machine types will sev work with? | |
| 21:55:54 | aspiers | just q35 | |
| 21:55:56 | efried | beaut | |
| 21:56:06 | efried | so why don't we do this: | |
| 21:56:06 | efried | In the request spec loader thingy, if image meta doesn't specify a machine type, shove one in there. | |
| 21:56:11 | aspiers | I suppose SEV-capable images already need hw_firmware_type=uefi, so adding a requirement for another property isn't terrible | |
| 21:56:26 | aspiers | Yeah could do | |
| 21:56:38 | aspiers | See https://review.opendev.org/#/c/666616/41/doc/source/admin/configuration/hypervisor-kvm.rst@585 | |