Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-19
16:39:30 jangutter stephenfin: ah!
16:39:47 stephenfin mdbooth: Good reading, fwiw http://python-notes.curiousefficiency.org/en/latest/python_concepts/import_traps.html
16:39:49 sean-k-mooney mdbooth: we should proably open a bug and fix it later
16:39:56 mdbooth That seems to avoid whatever sphinx is importing
16:40:58 mdbooth sean-k-mooney: For what?
16:41:49 stephenfin mdbooth: It's loading the module. I suspect you'd get the same issue if you left it in 'nova/test.py' and added 'import .test' to 'nova/tests/__init__.py'
16:41:57 stephenfin *'import nova.test'
16:42:45 mdbooth stephenfin: Which module is it loading?
16:43:16 stephenfin It's loading 'nova', which is causing it to load 'nova.tests', I would imagine
16:43:26 sean-k-mooney mdbooth: https://review.openstack.org/#/c/626952/8/nova/monkey_patch.py@55
16:43:38 sean-k-mooney mdbooth: does that comment make sense.
16:43:59 sean-k-mooney actull i test it locally
16:45:29 stephenfin and that's happening because of https://github.com/openstack/nova/blob/master/doc/source/conf.py#L27
16:45:33 mdbooth sean-k-mooney: Right, if the problem is importing twice that would work round it. However, if we previously weren't monkey patching at all by happy coincidence, I'm not sure if that's not just a dirtier hack.
16:45:49 stephenfin though that's clearly not necessary https://github.com/openstack/nova/blob/master/doc/source/conf.py#L22
16:46:55 mdbooth stephenfin: “Never add a package directory, or any directory inside a package, directly to the Python path”.
16:47:17 mdbooth From that traps doc you sent earlier
16:48:08 stephenfin yuuuup
16:48:23 stephenfin In general, messing with the Python path is a bad idea
16:48:39 stephenfin but the alternative is making all these things installable
16:48:44 mdbooth s/(the|path) //
16:49:01 mdbooth g
16:49:17 stephenfin :)
16:49:24 stephenfin I like to live on the edge, personally
16:50:46 mdbooth Ok, I'm going to do both
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 stephenfin Oh, wherever it's failing
17:03:02 mdbooth Although apparently it does fail on Centos7 and Ubuntu, so I'm guessing it's some library version
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?

Earlier   Later