| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-11 | |||
| 16:07:45 | efried | okay, but the virt driver knows how to twiddle the guest config based on a) mem_encryption flavor/image prop, and b) fact I'm on an AMD host | |
| 16:08:21 | efried | If there are user-/admin-configurable options beyond "yes" or "no", we should talk about where those should go. | |
| 16:08:33 | efried | But my understanding was that from the admin/user pov it was "yes" or "no". | |
| 16:08:39 | aspiers | It is | |
| 16:08:52 | efried | and if there are any variations, it's discovered based on specifics of the system | |
| 16:08:53 | efried | okay | |
| 16:08:59 | efried | so the virt driver knows b) | |
| 16:09:04 | efried | that's how it knows it's doing SEV | |
| 16:09:20 | aspiers | It sounds like you are saying that the logic should go in the virt driver and it should be "if the current context has MEM_ENCRYPTION_CONTEXT=1 and the current host is SEV, then add the SEV bits" | |
| 16:09:50 | efried | okay, yes | |
| 16:10:06 | aspiers | OK, thanks - think I know how to write it now | |
| 16:10:51 | efried | I suppose if you're being careful, you should raise a nice exception if you get MEM_ENCRYPTION_CONTEXT without being SEV-capable (regardless of AMD-ness) | |
| 16:11:17 | aspiers | Yes can do. BTW SEV is AMD-only | |
| 16:11:29 | aspiers | Very much a vendor-specific thing | |
| 16:12:00 | aspiers | You probably knew that and I'm just misinterpreting the point of your last clause in parentheses | |
| 16:14:51 | efried | Yes, what I'm saying is, theoretically there should have been no way you got this far if the host wasn't SEV-capable, but, I don't know, if someone switched off the capability between allocation and spawn, you should fail hard. | |
| 16:15:37 | aspiers | Yep, absolutely | |
| 16:15:50 | aspiers | It was the "(regardless of AMD-ness)" bit I didn't quite get | |
| 16:15:57 | efried | stephenfin: fixes it py27 | |
| 16:16:07 | stephenfin | sweet | |
| 16:16:18 | stephenfin | I'll submit that as a fix to pycodestyle | |
| 16:17:59 | efried | stephenfin: fixes in py36 as well | |
| 16:18:13 | efried | stephenfin: why is self.lines[-1] correct? | |
| 16:19:38 | stephenfin | It's not, actually, now that you mention it | |
| 16:21:11 | stephenfin | Or maybe it is. Needs more tests | |
| 16:21:34 | aspiers | efried: bbobrov is on vacation so I'm taking over his SEV patches, and thinking about rebasing them all into a single linear commit series (even though this won't accurately reflect the dependencies between them) - does that make sense? | |
| 16:22:03 | efried | aspiers: Yes it does. I was kind of wondering why they weren't already in one line. | |
| 16:23:04 | aspiers | efried: Well, I gave a few reasons for that in our previous discussions on this, but an additional reason is weak coordination between me and him ... :) | |
| 16:23:18 | efried | ack | |
| 16:23:38 | aspiers | One downside is that if an earlier patch fails CI, it bottlenecks merging of anything higher up | |
| 16:24:11 | aspiers | but if it makes life easier for reviewers then that probably overrules any other considerations | |
| 16:24:48 | efried | It's not just that. It's that ultimately you're going to need a patch that ties everything together. And as previously discussed, you can't have a dep tree that's upside down. | |
| 16:24:56 | aspiers | Sure | |
| 16:25:03 | aspiers | Another downside is that it will significantly increase the number of rebases | |
| 16:25:10 | efried | So yes, the fact that some patches *could* be merged independently if they were separated - it's just something that we live with. | |
| 16:25:10 | aspiers | but again I can live with those | |
| 16:25:33 | aspiers | Definitely a nice future enhancement for Gerrit | |
| 16:25:39 | aspiers | One can dream ... | |
| 16:26:29 | aspiers | I guess I'll rebase all his stuff on top of mine, since it's the next phase | |
| 16:26:57 | aspiers | My stuff is all about feature detection and scheduling; his is about guest config and booting | |
| 16:27:17 | aspiers | Then the series will roughly match the order of work items | |
| 16:27:31 | aspiers | as listed http://specs.openstack.org/openstack/nova-specs/specs/train/approved/amd-sev-libvirt-support.html#work-items | |
| 16:29:46 | efried | aspiers: | |
| 16:29:46 | efried | > It was the "(regardless of AMD-ness)" bit I didn't quite get | |
| 16:29:46 | efried | I just meant "being on AMD isn't enough; other stars must align for SEV to be supported" | |
| 16:29:46 | efried | So your check in the virt driver should just reuse the same "am I SEV capable" method that it used to expose the MEM_ENCRYPTION_CONTEXT inventory and the HW_CPU_X86_AMD_SEV trait in the first place. Which is why there should be no way for that check to fail - unless something changed in between update_provider_tree and spawn. | |
| 16:31:06 | efried | and yes, I agree with your ordering, especially since your patches are already 2/3 with one +2 and the last one very close to same, but the other ones are still awaiting your nod before I even dig into them. | |
| 16:37:27 | aspiers | efried: OK thanks, we're 100% aligned :) | |
| 16:40:26 | aspiers | efried: Oh, one extra detail regarding the above - yes the check can just look at the supports_amd_sev instance variable, but it also needs to check extra_specs for resources:MEM_ENCRYPTION_CONTEXT=1 to know that encrypted memory was requested, right? | |
| 16:40:43 | aspiers | If *both* are true then it should add all the SEV guest config tweaks | |
| 16:42:10 | efried | s/check extra_specs/check the allocation/ but otherwise, ja, genau | |
| 16:44:11 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: flatten rbd images when unshelving an instance https://review.opendev.org/457886 | |
| 16:49:06 | aspiers | efried: OK thanks, I guess that will become self-evident when I look into how to check allocations | |
| 16:49:51 | efried | aspiers: Well, I would rather you check the allocations rather than duplicating the logic you just put in your request filter to check and validate the flavor & image | |
| 16:50:12 | efried | The allocations are passed to spawn. They're just a dict. See the placement API reference for the format. | |
| 16:50:26 | aspiers | Cool, thanks! | |
| 16:50:54 | aspiers | I'm beginning to feel that this is nearing code-completion and maybe can be added to the runway queue soon | |
| 16:51:00 | efried | yup | |
| 16:51:15 | aspiers | I do want to test the whole series on real SEV hardware first though | |
| 16:51:48 | aspiers | (which I have available to me right now) | |
| 16:58:05 | efried | dustinc: sdk release 0.31.0 is in u-c now, so I'd like to respin the series to bump that and fix up that other thing we talked about yesterday. Let me know when you're clear of local edits. | |
| 17:44:22 | dustinc | efried: Hmm...might be a bit, go for it and I can deal with merging my local edits | |
| 18:03:16 | spatel | sean-k-mooney: any idea about this setting https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1724614 ? | |
| 18:03:17 | openstack | Launchpad bug 1724614 in linux (Ubuntu) "[KVM] Lower the default for halt_poll_ns to 200000 ns" [Medium,Triaged] | |
| 18:03:59 | spatel | i have default value but i heard it gives good boost to CPU if you play with this setting | |
| 18:05:24 | mriedem | efried: do we have a bug for the test_check_doubled_words py27 failures? | |
| 18:09:41 | sean-k-mooney | spatel: i have not had experience with tweaking that specific tunable in the past. | |
| 18:10:35 | spatel | even i am running in default, i heard from someone but may be its application specific. | |
| 18:10:43 | sean-k-mooney | spatel: i would proably try to find some infomation on it form nfv/hpc tuning guide but there are man paramaters that can be tweaked not all of which will help for all workloads | |
| 18:11:01 | spatel | +1 | |
| 18:11:53 | spatel | but i am seeing sometime my ksm cpu usage going high on compute node | |
| 18:12:10 | spatel | [ksmd] | |
| 18:12:29 | sean-k-mooney | you proably want to read this https://www.kernel.org/doc/Documentation/virtual/kvm/halt-polling.txt | |
| 18:17:56 | sean-k-mooney | spatel: ibm seam to be recommendign a value fo 80000 ns based on https://www.ibm.com/support/knowledgecenter/en/linuxonibm/liaag/wkvm/wkvm_c_tune_kvm.htm but this seams to be a tradeoff between powersavings and latency which translated to higher throughput at the cost of cpu efficenty | |
| 18:19:51 | spatel | it seem that setting will schedule in very less time which increase power usage | |
| 18:20:29 | spatel | anyway i am not going to change until i have solid reason to do | |
| 18:37:06 | mriedem | coreycb: is this the right change log to be looking at for ubuntu bionic py27? http://changelogs.ubuntu.com/changelogs/pool/main/p/python2.7/python2.7_2.7.15~rc1-1ubuntu0.1/changelog | |
| 18:38:34 | mriedem | efried: https://bugs.launchpad.net/nova/+bug/1832392 | |
| 18:38:35 | openstack | Launchpad bug 1832392 in OpenStack Compute (nova) "nova.tests.unit.test_hacking.HackingTestCase.test_check_doubled_words failing in py27 jobs since June 10" [High,Confirmed] | |
| 18:40:04 | efried | mriedem: Tracking it against bug/1804062 | |
| 18:40:16 | efried | mriedem: https://review.opendev.org/664622 is merging to disable the test | |
| 18:40:32 | coreycb | mriedem: that is the latest update for the bionic-security pocket but bionic-updates is more recent. you can see the versions here: https://launchpad.net/ubuntu/+source/python2.7 | |
| 18:40:42 | efried | mriedem: and stephenfin is working on a patch to pycodestyle to get closer to fixing the core issue. | |
| 18:41:54 | mriedem | coreycb: ah ok here is the june 10 change then https://launchpad.net/ubuntu/+source/python2.7/2.7.15-4ubuntu4~18.04 | |
| 18:43:41 | coreycb | mriedem: yep | |
| 18:44:38 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: Revert resize: wait for events according to hybrid plug https://review.opendev.org/644881 | |
| 18:44:38 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: Include direct-physical in compute manager events check https://review.opendev.org/664431 | |
| 18:45:00 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [DNM] testing bug/1813789 revert resize events https://review.opendev.org/664442 | |
| 18:46:27 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Revert resize: wait for events according to hybrid plug https://review.opendev.org/644881 | |
| 18:46:28 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: Include direct-physical in compute manager events check https://review.opendev.org/664431 | |
| 18:46:28 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [DNM] testing bug/1813789 revert resize events https://review.opendev.org/664442 | |
| 18:46:35 | artom | Fscked up my rebases | |
| 18:48:02 | mriedem | ok so probably no point in rechecking anything | |
| 18:48:24 | mriedem | or working on nova for a few days... | |
| 18:49:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Refresh instance network info on deletion https://review.opendev.org/660761 | |
| 19:03:06 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420 | |
| 19:03:16 | aspiers | efried: there ya go ^^^ | |
| 19:03:21 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Fix default values in update_cell command https://review.opendev.org/662383 | |
| 19:04:26 | aspiers | artom: reflog is your friend ;-) | |
| 19:04:54 | artom | aspiers, wasn't that bad | |