| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-19 | |||
| 12:58:30 | openstack | Launchpad bug 1820851 in OpenStack Compute (nova) "Please do cell mapping without periodic task or active admin involvment" [Wishlist,Opinion] | |
| 13:00:08 | mdbooth | frickler: That's awesome. Thanks. | |
| 13:04:07 | kashyap | aspiers: Hmm. Still thinking on the nested dictionary of arch–machine type mapping ... | |
| 13:04:17 | aspiers | ok | |
| 13:04:22 | kashyap | aspiers: Can you put what you just wrote in the review, please? | |
| 13:04:27 | kashyap | As a "consciousness strem" | |
| 13:04:29 | kashyap | s/strem/stream/ | |
| 13:14:41 | openstackgerrit | Boris Bobrov proposed openstack/nova master: Extract SEV-specific bits on host detection https://review.openstack.org/636334 | |
| 13:14:41 | openstackgerrit | Boris Bobrov proposed openstack/nova master: Fix memtune parameters according to libvirt docs https://review.openstack.org/636301 | |
| 13:14:42 | openstackgerrit | Boris Bobrov proposed openstack/nova master: Add configs for AMD SEV https://review.openstack.org/636318 | |
| 13:14:42 | openstackgerrit | Boris Bobrov proposed openstack/nova master: Add iommu driver to devices https://review.openstack.org/644564 | |
| 13:14:43 | openstackgerrit | Boris Bobrov proposed openstack/nova master: [wip] Driver changes for sev https://review.openstack.org/644565 | |
| 13:16:44 | sean-k-mooney | kashyap: nested dict of arch machine type mappings? in the nova.conf or image. i dont think i makes sense in the image and we basically aready have that in the nova.conf | |
| 13:17:34 | kashyap | sean-k-mooney: For more context, check out the "returns" here: https://review.openstack.org/#/c/633855/11/nova/virt/libvirt/host.py@706 | |
| 13:17:49 | kashyap | sean-k-mooney: The context is to separate out the getDomainCapabilities() method from that file | |
| 13:18:26 | mdbooth | Hmm. Unless I can differentiate between workloads which are simply loading Nova classes to gather metadata and workloads which actually plan to execute Nova code, I'm going to have to remove the monkey patching asserts :/ | |
| 13:18:36 | mdbooth | Although the former really shouldn't be monkey patching at all | |
| 13:18:48 | mdbooth | But they always have been, so I guess it doesn't matter in practise | |
| 13:19:13 | sean-k-mooney | kashyap will a machine type ever be suppoted by two archs? i dont think so right. so it could jsut be a dict of machine_type to capablities | |
| 13:20:44 | sean-k-mooney | if your going with the nested structure however i would strongly think of make really OVO however instead of dicts of dicts of capablity dicts | |
| 13:22:03 | kashyap | sean-k-mooney: So far we have AArch64 that has 'virt' machine type -- now there is upstream QEMU talk to introduce something similar for x86_64 | |
| 13:22:40 | kashyap | sean-k-mooney: Please note your point on the change pointed earlier. | |
| 13:22:47 | sean-k-mooney | kashyap: ah and the capablitys fo the virt machine type would be different for aarch64 vs x86 | |
| 13:23:03 | kashyap | Exactly | |
| 13:25:14 | mriedem | yay this is old https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1549-L1552 | |
| 13:26:09 | sean-k-mooney | kashyap: when i +1'd it previously it retruned a config.LibvirtConfigDomainCaps object by the way | |
| 13:26:31 | sean-k-mooney | kashyap: so i have not reviewed it since this changed to a dict of dicts of objectss | |
| 13:27:41 | sean-k-mooney | im not sure how the change to detect sev suport which will only work on amd x86 some how morphed to include generic multi arch multi machiche type capablity discovery code | |
| 13:28:27 | sean-k-mooney | that proably should be in a different patch or the commit needs to be updated as we have conflated two different change in this one patch | |
| 13:28:28 | kashyap | sean-k-mooney: We've had some back and forth on the change. I still need to fully read the lengthy design doc myself (although I pointed out a potential blocker) | |
| 13:29:10 | sean-k-mooney | well it would not be a blocker for sev | |
| 13:29:22 | sean-k-mooney | as sev will only ever work on x86 amd cpus | |
| 13:29:23 | kashyap | sean-k-mooney: This is not the blocker, a different topic | |
| 13:30:18 | kashyap | sean-k-mooney: Check out the points from Dan Berrange and myself on the re-proposed spec: https://review.openstack.org/#/c/641994/ | |
| 13:30:29 | sean-k-mooney | sure but my point is that adding the more complex logic to this patch mean we basicaly block sev support until we figure out this multi arch multi machine type proablem which is not germain to enabling sev suppot | |
| 13:32:43 | kashyap | The machine types figuring out is a lesser problem than the more thorny blocker identified in the re-proposed spec. | |
| 13:32:46 | sean-k-mooney | the memory hard limit was previously discussed. we determined that it woudl only be needed if you did not use hugepages i think. the sev limt is a new factoide but that can be handeled via a placment inventory. that is new work. | |
| 13:33:07 | sean-k-mooney | kashyap: what is the issue you are refering to? | |
| 13:33:51 | kashyap | Hard-coding a memory value. | |
| 13:34:11 | sean-k-mooney | ya but we dont need to do that if we use hugepages which i suggested last cycle | |
| 13:34:25 | kashyap | Also each AMD machine has some limit on the number of VMs. That is also something that needs to be worked into the spec | |
| 13:35:01 | sean-k-mooney | ya that is new. it was not raised previously but that is simple enough to fix with a inventory of sev context in placement | |
| 13:35:20 | sean-k-mooney | assuming we can auto discover that limit form either libvirt or sysfs | |
| 13:35:54 | kashyap | I don't think this is just limited to hugepages. The core point about protecting the host from getting DoSed needs to be addressed. | |
| 13:36:00 | sean-k-mooney | if we really needed too we could have a config option for it. that shoudl be in the spec i agree but i still dont see a hard blocker here | |
| 13:36:02 | kashyap | Please see the cgroups idea DanPB mentioned in his remark. | |
| 13:36:59 | kashyap | I think this is hard blocker. I'm even more convinced of it (that's why I gave a "-1", FWIW) after reading DanPB's message and talking to a developer who implemented SEV bits in libvirt | |
| 13:37:16 | kashyap | Let's work it out on the spec. IRC is too real-time for me sometimes :-) | |
| 13:37:31 | sean-k-mooney | kashyap: well if you require the use of hugepage then you disable over commit and dont need to use a hard memory limit as the vm memory is preallcoated and fixed | |
| 13:37:51 | kashyap | sean-k-mooney: So you're saying when using SEV one should "mandate" huge pages? | |
| 13:37:56 | sean-k-mooney | yes | |
| 13:38:09 | kashyap | Hmm, I don't know the full implications of it. Need to think more. | |
| 13:38:13 | sean-k-mooney | i suggested it last cycle to not need to set the hardlimit | |
| 13:38:35 | sean-k-mooney | the main implication is no over commit | |
| 13:38:48 | sean-k-mooney | and an implict numa topology | |
| 13:39:36 | sean-k-mooney | the memory for sev i think i memlocked so it cant be swapped anyway so you would have disabled over commit already | |
| 13:39:59 | sean-k-mooney | so requireing hugepages would remove some of the complexity of this feature and give a performacne boost | |
| 13:40:10 | kashyap | Okay, I see the pinning the memory by using `hw:mem_page_size=large` point in the spec. | |
| 13:41:33 | mriedem | efried: cdent: edleafe: semi interesting but also for an older release bug about forced live migration failing during PUT /allocations https://bugs.launchpad.net/nova/+bug/1820612 | |
| 13:41:34 | openstack | Launchpad bug 1820612 in OpenStack Compute (nova) "Logging is hard to read if there is a problem with resources during live migration" [Undecided,Incomplete] | |
| 13:41:35 | kashyap | sean-k-mooney: Please respond to Dan on the review, for the discussion to be "sticky". | |
| 13:41:42 | sean-k-mooney | ya i didnt push too hard because it would be nice to not require hugepages but it make this a hole lot eaiser as not useing htem has other accounting issues | |
| 13:41:58 | mriedem | efried: cdent: edleafe: i've pieced together that they are forcing a live migration to a dest host before queens where we'd move the source node allocations from the instance to the migration record, | |
| 13:42:20 | sean-k-mooney | kashyap: sure. | |
| 13:42:21 | mriedem | so in this code, it's doubling up the allocations for the instance consumer by putting the same source node allocations to the source provider and the new allocations to the dest provider, | |
| 13:42:31 | mriedem | but placement pukes on the source provider PUT | |
| 13:42:52 | mriedem | which makes me wonder if placement is trying to make a new allocation rather than replace it | |
| 13:43:48 | mdbooth | This monkey patching change is a hydra :( | |
| 13:45:01 | mriedem | cdent: anyway if they were queens+ they wouldn't have this problem, but that isn't a very helpful response to someone at ocata or pike | |
| 13:45:18 | mriedem | if we know about a bug fix in placement though i could say 'yup this is fixed here, backport it' | |
| 13:45:21 | kashyap | mdbooth: Seems like it is turning into "cthulhu patching" | |
| 13:45:26 | cdent | mriedem: I'm still digesting. | |
| 13:45:32 | edleafe | mriedem: I don't remember ever doubling the resources on the source when migrating to a different host | |
| 13:46:01 | mdbooth | kashyap: Eventlet is just awful. | |
| 13:46:50 | kashyap | mdbooth: Nod, I only hear about it (I didn't have to faff with it much so far). Skimmed your e-mail; thanks for the write-up! | |
| 13:46:58 | mdbooth | I still don't understand what problem it solves, tbh. Seems to me it just makes your multi-threading even jankier. | |
| 13:47:50 | edleafe | mdbooth: One of my first tasks in OpenStack was to replace the Twisted code with eventlet, because people were having so much trouble tracking down bugs in Twisted :) | |
| 13:47:51 | kashyap | mdbooth: I saw your second email about using "Python's APIs except when not possible" for multi-threading | |
| 13:48:08 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add detection of SEV support from QEMU/AMD-SP/libvirt on AMD hosts https://review.openstack.org/633855 | |
| 13:48:26 | mdbooth | edleafe: Hah. There's always something worse :) | |
| 13:48:35 | edleafe | mdbooth: Yup | |
| 13:48:53 | mdbooth | edleafe: But at a top level, why not just use python's multi-threading? | |
| 13:49:32 | cdent | mriedem: Is there any chance there's a race. Something else ate a bit more more RAM during the transaction. That shouldn't be possible (at least not in more recent allocation handling) but it's very hard to keep track of changes... | |
| 13:49:51 | edleafe | mdbooth: I really don't remember. The original nova code used Twisted for async, and it caused a lot of problems, so someone suggested eventlet, and... | |
| 13:49:51 | mdbooth | Python already had fake concurrency, so why implement fake concurrency on top of fake concurrency? | |
| 13:49:58 | kashyap | aspiers: Thanks for addressing the docs bit. You saw what I "discovered" with my comment :D Writing that out helped me understand it better | |
| 13:50:15 | mriedem | cdent: it's possible, this was pre-generations | |
| 13:50:20 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Cleanup comments around claim_resources method https://review.openstack.org/644576 | |
| 13:51:12 | mriedem | cdent: i guess one way to find out is write a functional regression test with at-capacity source compute and then try to live migrate off it | |
| 13:51:26 | mriedem | tests against at-capacity computes is not our strong suit in nova | |
| 13:51:32 | cdent | quite | |
| 13:51:51 | mriedem | anyway, while in here i've cleaned up a bunch of old comments https://review.openstack.org/644576 | |
| 13:52:23 | cdent | huzzah | |
| 13:54:32 | sean-k-mooney | aspiers: kashyap i replied in the spec reproposal | |
| 13:55:45 | aspiers | kashyap: yep, that extra clarity helped me too. Have just replied inline in the review | |
| 13:55:54 | aspiers | sean-k-mooney: thanks | |
| 14:04:54 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add new "supports_amd_sev" capability to libvirt driver https://review.openstack.org/638680 | |
| 14:05:21 | mriedem | alex_xu: replied in https://review.openstack.org/#/c/644293/ - thanks for those questions | |
| 14:05:51 | efried | mriedem: This ready to go? https://review.openstack.org/#/c/644423/ I didn't want to rubber-stamp it if you wanted another look. | |
| 14:06:07 | mriedem | efried: looking | |