Posted	Nick	Remark
#openstack-nova - 2019-06-11
15:17:50	artom	This is not something I have the energy to debate
15:17:59	dansmith	artom: don
15:18:00	dansmith	artom: don't hedge :)
15:18:44	efried	stephenfin: I'm stumped. Are you getting anywhere? If not, I'm going to call time and just disable the test for now.
15:18:54	sean-k-mooney	dansmith: why in the api module and not here https://github.com/openstack/nova/blob/master/nova/network/model.py#L460
15:18:58	stephenfin	efried: Yeah, I'm down in pycodestle
15:19:02	stephenfin	*pycodestyle
15:19:17	dansmith	sean-k-mooney: I said nova/network/something would be fine too
15:19:47	artom	I wasn't aware of the "must call neutron API" rule
15:20:04	efried	stephenfin: Was that "yeah I'm getting somewhere" or "yeah disable the test"?
15:20:14	dansmith	artom: I'm aware of no such rule
15:20:25	sean-k-mooney	artom: im not sure it a rule but that module is nova.network.neutronv2.api
15:20:37	stephenfin	efried: Disable it. I'll keep looking
15:20:37	artom	sean-k-mooney, actually yeah, with the new abstraction suggested by dansmith (plug-time vs bind-time), it fits in model as well
15:20:43	stephenfin	Here's our issue though https://github.com/PyCQA/pycodestyle/blob/2.0.0/pycodestyle.py#L1639
15:20:57	stephenfin	We shouldn't be getting there but I don't know yet what's changed
15:20:59	sean-k-mooney	pretty much everything it does interacts with the neutron api in one way or another
15:21:30	dansmith	sean-k-mooney: we have constants defined in that module, and this detail is completely related to "what to expect from the neutron api" so I have no problem saying it should (or can) be in there
15:21:34	stephenfin	Things like this are why I was trying to get us onto a newer hacking version. Things invariably break
15:21:49	sean-k-mooney	artom: plugtime and bind time is more or less what i was thinking about in the neutron spec too so that makes sense to me
15:21:49	efried	stephenfin: aren't we on a newer hacking version at this point?
15:22:03	stephenfin	not really. Hacking is relying on a really old version of flake8
15:22:08	artom	kawn sen suss!
15:22:11	stephenfin	Like, super old
15:23:09	stephenfin	and I can't update because they broke all the things in flake8 3.x :( https://gitlab.com/pycqa/flake8/issues/545
15:23:46	sean-k-mooney	ok well its not where i would instintivly look for them but i can live with it.
15:25:05	sean-k-mooney	artom: to your orginal question https://github.com/openstack/nova/blob/master/nova/network/api.py isthe old nova networks api and the neton one extends it
15:25:16	sean-k-mooney	i think you can ignore https://github.com/openstack/nova/blob/master/nova/network/api.py at this point
15:25:48	artom	sean-k-mooney, ack, thanks for looking :)
15:26:04	artom	(Though tbf my original question was rendered moot when we agreed to put it in model)
15:27:31	sean-k-mooney	artom: :) by the way i proposed https://review.opendev.org/#/c/664442/ to test this
15:28:13	artom	sean-k-mooney, yep, thanks for that
15:30:17	openstackgerrit	Eric Fried proposed openstack/nova master: Skip test_check_doubled_words hacking check UT https://review.opendev.org/664622
15:30:20	efried	stephenfin: ^
15:35:24	efried	dansmith: mind unblocking the gate ^
15:45:26	openstackgerrit	Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420
15:45:39	aspiers	efried: vastly improved patchset based on your feedback ^^^
15:47:31	openstackgerrit	Sylvain Bauza proposed openstack/nova-specs master: Proposes NUMA affinity for vGPUs https://review.opendev.org/650963
15:48:26	efried	aspiers: looking.
15:49:51	stephenfin	efried, cdent: Here's what's happening
15:50:12	stephenfin	Because of https://bugs.python.org/issue33899, we now insert a NEWLINE token at the end of all lines
15:50:26	stephenfin	that manifests itself in pycodestyle
15:50:45	stephenfin	which is interating through all the tokens on the test line
15:50:46	stephenfin	https://github.com/PyCQA/pycodestyle/blob/2.0.0/pycodestyle.py#L1616
15:51:45	stephenfin	previously, when we got to the end of the line we were falling through to this case https://github.com/PyCQA/pycodestyle/blob/2.0.0/pycodestyle.py#L1650-L1652
15:52:09	stephenfin	but now, because we see this newline token, we hit this case instead https://github.com/PyCQA/pycodestyle/blob/2.0.0/pycodestyle.py#L1632-L1635
15:52:26	stephenfin	note that that case doesn't include a call to 'check_physical'
15:52:33	stephenfin	so our test never gets run
15:53:06	stephenfin	I don't know how to fix that. It feels like this is now a bug in pycodestyle
15:54:08	stephenfin	I note that the latest version of pycodestyle does pretty much the same thing too https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2104-L2107
15:54:33	cdent	stephenfin: sorry stephenfin, I thought most of what you said above was already understood: and it being a bug on pycodestyle was known
15:54:49	cdent	or rather: pycodestyle's continued behavior is the root
15:54:56	stephenfin	Um, whut? :)
15:55:02	stephenfin	Clearly I didn't read far enough
15:55:37	cdent	Or maybe I just reached that conclusion based on the discussion on https://bugs.python.org/issue33899 which if I recall right suggests the same thing
15:56:37	stephenfin	Yup, now that I search for pycodestyle I see the line of course. Dang it
15:56:47	stephenfin	Their fix is rather odd though https://github.com/PyCQA/pycodestyle/pull/801/files
15:58:06	openstackgerrit	Adam Spiers proposed openstack/nova master: Pass extra_specs to flavor in vif tests https://review.opendev.org/662556
16:01:45	aspiers	efried: What's supposed to happen when hw:mem_encryption=True but trait:HW_CPU_X86_AMD_SEV=required is missing?
16:02:05	efried	aspiers: How could that happen?
16:02:12	efried	oh
16:02:18	aspiers	efried: Should the filter automatically add it for now, but then later error if we add support for MKTME?
16:02:32	efried	No, we should never add the trait.
16:02:40	stephenfin	efried: If you apply this to '.tox/py27/lib/python2.7/site-packages/pycodestyle.py' it fixes things, yeah? http://paste.openstack.org/show/752752/
16:03:01	aspiers	OK, so then we need to error if it's missing, which somewhat reduces the user-friendliness benefit of hw:mem_encryption
16:03:03	efried	aspiers: You're assured of landing on capable hosts because they're the only ones that can satisfy the MEM_ENCRYPTION_CONTEXT inventory.
16:03:05	efried	No
16:04:05	efried	if I don't care which, I leave the trait out and land on either one.
16:04:05	efried	if I care that it's MKTME, I specify the MKTME trait and land on an Intel host
16:04:05	efried	if I care that it's SEV, I specify the SEV trait and land on an AMD host
16:04:05	efried	if the image/flavor prop is specified, it means I want mem encryption
16:04:21	aspiers	Sure, but I'm talking about the last case
16:04:35	aspiers	In that case, there needs to be logic *somewhere* in the code which says "OK, the user wants encrypted memory, and the only way to achieve that currently is SEV"
16:04:47	aspiers	so that it triggers usage of all the SEV bits
16:04:52	efried	I never land on a non-capable host, because only capable hosts have MEM_ENCRYPTION_CONTEXT inventory.
16:04:52	efried	No.
16:05:13	efried	Today the only hosts exposing that inventory will be SEV-capable hosts, so that falls into place.
16:05:15	aspiers	Sure
16:05:22	aspiers	but where does that logic go? ^^^
16:05:40	efried	It's implicit. It's free. It doesn't "go" anywhere.
16:05:45	aspiers	It's not enough to land on the right host
16:05:55	aspiers	nova needs to know to add all the extra bits to the guest config
16:06:11	efried	extra bits
16:06:21	aspiers	yes, like <launchSecurity /> and <locked />
16:06:28	efried	always the same if I'm SEV, right?
16:06:28	aspiers	and a whole bunch of iommu stuff
16:07:13	aspiers	Not sure I understand the question
16:07:28	aspiers	That extra config varies depending on a whole bunch of things
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)

Earlier   Later