| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-19 | |||
| 16:51:12 | mdbooth | I'm going to move the monkey patching points back to where they were, *and* add a guard to explicitly avoid double monkey patching | |
| 16:52:29 | stephenfin | mdbooth: I think you might also be interested in some of Armin Ronacher's most recent presentations on all things Python http://lucumr.pocoo.org/talks/ | |
| 16:52:39 | sean-k-mooney | mdbooth: i tryed the guard locally and it did not seam to help | |
| 16:52:40 | stephenfin | he of Flask fame | |
| 16:54:09 | stephenfin | ...openstack/nova/doc/source/reference/notifications.rst:142:Unknown directive type "automodule". | |
| 16:54:14 | stephenfin | So we are using autodoc | |
| 16:58:00 | stephenfin | mdbooth: Try adding this to doc/source/conf.py | |
| 16:58:07 | stephenfin | autodoc_mock_imports = ['nova.tests'] | |
| 16:59:21 | mdbooth | stephenfin: That's still really flaky, though, because it doesn't explain why we don't import the nova.cmd or nova.api.openstack.wsgi_app entry points | |
| 16:59:39 | mdbooth | stephenfin: I'd prefer to either go from flaky to non-flaky, or just leave well alone | |
| 17:00:09 | mdbooth | flaky->different flaky isn't worth the churn, I think | |
| 17:00:45 | sean-k-mooney | mdbooth: im flipped the condition on my local gard to never monkey patch and that fixes it | |
| 17:00:59 | sean-k-mooney | so the issue is the docs only build if we never monkeypatch | |
| 17:01:05 | stephenfin | Oh, I didn't realize we did that there | |
| 17:01:34 | sean-k-mooney | stephenfin: we monkey patch on the __init__.py in nova/cmd | |
| 17:01:40 | stephenfin | Yup, see that now | |
| 17:01:45 | stephenfin | weirdddd | |
| 17:02:03 | stephenfin | mdbooth: Can you share the debug logs from Sphinx? | |
| 17:02:19 | sean-k-mooney | ya anywya as i said above the issue is the sphinx only works if we do not do eventlet.monkeypatch | |
| 17:02:24 | mdbooth | stephenfin: In what configuration? | |
| 17:02:39 | mdbooth | stephenfin: FWIW, this doesn't fail for me on F29, even when monkey patched | |
| 17:03:02 | mdbooth | Although apparently it does fail on Centos7 and Ubuntu, so I'm guessing it's some library version | |
| 17:03:02 | stephenfin | Oh, wherever it's failing | |
| 17:03:11 | stephenfin | I assume that's only on Ubuntu so? | |
| 17:03:12 | stephenfin | Ack | |
| 17:03:36 | sean-k-mooney | stephenfin: jangutter mentioned its broken on centos 7 too | |
| 17:03:56 | sean-k-mooney | mdbooth: are you running on py36 on fedora 29 or py37 | |
| 17:04:07 | sean-k-mooney | ubuntu and centos 7 use py36 | |
| 17:04:16 | stephenfin | Yeah, we need debug logs for this stuff | |
| 17:04:19 | mdbooth | 37 | |
| 17:04:39 | sean-k-mooney | ok so it could be a 36 vs 37 thing | |
| 17:05:08 | openstackgerrit | Lance Bragstad proposed openstack/nova master: Give the policy vision document a facelift https://review.openstack.org/644615 | |
| 17:05:36 | sean-k-mooney | ill install 3.7 and check | |
| 17:05:54 | lbragstad | mriedem lumped it all in there ^ lemme know if you want that broken up | |
| 17:06:00 | lbragstad | cc efried ^ | |
| 17:07:19 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Misc cleanups https://review.openstack.org/644616 | |
| 17:10:27 | sean-k-mooney | so looks like ti works fine on python 3.7 with the gauard | |
| 17:10:45 | sean-k-mooney | ill revert to a clean copy of the patch and confrim its fine without the guard too | |
| 17:13:19 | mdbooth | Hehe, with vim's standard highlighting in nova.monkey_patch, I see: NOTE, NOTE, TODO, NOTE, NOTE, NOTE, NOTE | |
| 17:13:27 | aspiers | sean-k-mooney: we got Brijesh from AMD to respond to your hugepages suggestion | |
| 17:13:37 | sean-k-mooney | mdbooth: stephenfin so yes this only happens on python3.6 which is why it passes on fedora and not on ubuntu or centos which default to python 3.6 | |
| 17:13:50 | mdbooth | Well, at least they're all in the same place, now | |
| 17:13:53 | sean-k-mooney | aspiers: oh ok i havent read it yet | |
| 17:14:06 | sean-k-mooney | aspiers: was it favorable or not? | |
| 17:14:20 | aspiers | sean-k-mooney: I don't think so. Bottom of https://review.openstack.org/#/c/641994/2 | |
| 17:15:02 | aspiers | SEV by design prevents the kernel copying encrypted pages around | |
| 17:15:34 | aspiers | he says using hugepages would pin it within the hugepages pool, but not pin each page to a single address | |
| 17:15:44 | aspiers | I'm on a call with AMD now | |
| 17:16:48 | sean-k-mooney | aspiers: ya dan berrange replied and said hugepages may not be sufficent too as the roms and uefi pflash would not be pinned | |
| 17:16:55 | aspiers | right | |
| 17:18:04 | aspiers | sean-k-mooney: how does it work for device passthrough? | |
| 17:18:47 | aspiers | apparently that's a similar scenario in terms of pinning | |
| 17:20:34 | sean-k-mooney | the iommu maps the device mmio region to a fixed set of addresses that are maped to the memory regions listed in the pci VF config space i think. but honestly i tought that a hugepage was similarly fixed and could not migrate when we use explcit not transparent hugepages | |
| 17:21:30 | mdbooth | It seems there's no need for a double-import guard unless you borked the python path, because python doesn't appear to import a second time anyway | |
| 17:21:39 | mdbooth | stephenfin: Is ^^^ your understanding? | |
| 17:21:53 | sean-k-mooney | mdbooth: we dont need the import guard correct | |
| 17:22:20 | stephenfin | mdbooth: That sounds reasonable, aye | |
| 17:22:31 | stephenfin | Based on my understanding | |
| 17:22:55 | sean-k-mooney | mdbooth: the issue is we never monkeypatched the docs before and under python 3.6 only it breaks things if we do. apparently | |
| 17:24:20 | mdbooth | Incidentally, that likely only breaks because we didn't patch earlier... | |
| 17:24:34 | mdbooth | Perhaps we should update sphinx to always monkey patch ;) | |
| 17:25:38 | sean-k-mooney | mdbooth: you joke but we could proably write a tiny sphinx extenion to monky patch earlir. that said didnt you plane to patch in nova/__init__.py | |
| 17:26:34 | sean-k-mooney | oh your not doing that in that patch | |
| 17:27:29 | mdbooth | sean-k-mooney: Yeah, that breaks everything for different reasons. | |
| 17:27:58 | aspiers | sean-k-mooney: yeah, I get the impression Brijesh (AMD) thinks otherwise but TBH I'm out of my depth here :) | |
| 17:27:59 | sean-k-mooney | ah ok :) well it is eventlets | |
| 17:28:09 | aspiers | definitely not my area of expertise | |
| 17:28:32 | sean-k-mooney | aspiers: well we still lock the memory expiclitly when we use hugepages | |
| 17:28:45 | aspiers | ah | |
| 17:28:53 | sean-k-mooney | i.e. we set the locked element https://libvirt.org/formatdomain.html#elementsMemoryBacking | |
| 17:29:08 | openstackgerrit | Matthew Booth proposed openstack/nova master: Eventlet monkey patching should be as early as possible https://review.openstack.org/626952 | |
| 17:29:10 | mdbooth | Thar she blows. | |
| 17:29:18 | sean-k-mooney | or maybe we only do that for realtime i can check | |
| 17:29:28 | mdbooth | Lets see if that passes muster. | |
| 17:30:03 | aspiers | sean-k-mooney: ah OK, is that also done for device pass-through? | |
| 17:30:47 | sean-k-mooney | no its not done for device pass-though | |
| 17:32:37 | sean-k-mooney | aspiers: this is the only time we current lock the memory explcitly whic is for realtime guests | |
| 17:32:39 | sean-k-mooney | http://git.openstack.org/cgit/openstack/nova/tree/nova/virt/libvirt/driver.py#n4810 | |
| 17:34:20 | sean-k-mooney | the way dpdk work i am prtty sure the hugepage does not migrate to different phyical pages on the host as it is used for dma transfer for the vhost-user nics | |
| 17:34:54 | sean-k-mooney | aspiers: in anycase if you have hardware you can test it on it would be worth testing with hugepages | |
| 17:36:04 | sean-k-mooney | aspiers: but if dan's and brijesh's assertion regarding the rom+pflash issue are correct it may not be enough | |
| 17:36:04 | aspiers | sean-k-mooney: AMD already tried hugepages in one of their earliest iterations | |
| 17:36:15 | aspiers | that's what I just heard | |
| 17:36:49 | aspiers | they are also saying that mlock() is just a hint not a guarantee | |
| 17:37:31 | sean-k-mooney | ok well in that case i dont know what the best path forword is on the hardlimit issue | |
| 17:37:55 | aspiers | one suggestion was to expose the right hardlimit via QEMU / libvirt | |
| 17:38:01 | aspiers | so nova could just query it | |
| 17:39:29 | sean-k-mooney | aspiers: if qemu/libvirt can provide it that sounds resonable but it would make the relevent libvirt/qemu version that provides that the minium qemu/libivrt for that feature | |
| 17:39:47 | aspiers | correct, there's already a minimum version requirement anyway | |
| 17:40:06 | aspiers | I think everyone is aware of the issue now though, so hopefully Dan and Brijesh can figure something out :) | |
| 17:45:47 | sean-k-mooney | aspiers: so reading dan's comment again if we just remove the hard_limmit entirly then that may be suffienct to adress his orignial comment here https://review.openstack.org/#/c/641994/2/specs/train/approved/amd-sev-libvirt-support.rst@167 | |
| 17:46:32 | sean-k-mooney | ah but that wont work as that is how you are pinning the memory | |
| 17:46:47 | aspiers | right | |
| 17:49:15 | aspiers | bryan_stephenson: catch up on the conversation via http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-03-19.log.html#t2019-03-19T17:13:27 | |
| 17:50:22 | sean-k-mooney | aspiers: honestly looking at the documentation of hard_limit i think relying on it for the behavior you desire is undocumented and not part of the contract at least form the libvirt perspective | |
| 17:50:56 | sean-k-mooney | aspiers: hard_limit does not guarntee teh meoeoy will not be swapped or that it will be preallocated | |
| 17:51:37 | sean-k-mooney | aspiers: thos poperties are conntoeld via allocation and locked in the memory backing | |
| 17:53:20 | bbobrov | lets just ask on the libvirt mailing list | |
| 17:53:31 | bbobrov | (hi) | |
| 17:53:37 | aspiers | I agree with bbobrov | |