Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-06-10
15:44:01 dansmith yeah, always set compute.disabled=True, not make the rpc call if it's down, return 200
15:44:29 mriedem ok i can wordsmith that after tearing up artom's patch here
15:44:43 dansmith by all means, prioritize tearing up anything of artom's
15:44:46 artom ;_:
15:45:07 artom (That's from both emotional and physical pain)
15:48:24 aspiers stephenfin: ok to add you to reviewers for a couple of SEV patches which need more core attention? they're not particularly big or complex
15:48:31 stephenfin sure
15:48:35 aspiers thanks!
16:00:48 mriedem artom: sean-k-mooney: mdbooth: dansmith: comments in https://review.opendev.org/#/c/644881/
16:01:18 artom Whoa, you weren't kidding about tearing up
16:01:37 mdbooth Whoa!
16:01:43 mdbooth mriedem: Thanks, btw
16:02:33 artom sean-k-mooney, correct me if I'm wrong, but I don't believe "The bigger issue I have is the concern that we could have a server with both ovs hybrid plugged ports and ports without that" is possible
16:03:16 sean-k-mooney it is only posible if you have ovs +sriov ports
16:03:43 sean-k-mooney e.g. if they are all ovs ports then it would all be hybrid_plug true or false
16:03:59 artom sean-k-mooney, ah, so ports on which the firewall driver that causes hybrid plug to be used doens't apply
16:04:08 sean-k-mooney but if you have some vswitch prot for say managmenet and some sriov ports for the data plane then you could have a mixx
16:04:10 artom (https://review.opendev.org/#/c/660782/6/playbooks/legacy/nova-multinode/run.yaml@55)
16:06:16 sean-k-mooney right so if you are using sriov there is no firewall
16:06:18 artom So... do we need to handle that? Ie, wait for hybrid plugged ports in the compute manager, and for the sriov ones in the virt driver?
16:06:43 artom Or is there no vif to plug for the latter case, so it's irrelevant?
16:07:19 mriedem or will you get an event regardless b/c of the port binding change, in which case you don't need to filter on ovs hybrid plug in the compute manager
16:07:20 sean-k-mooney that is a good question there should be a vif plugged in that case
16:07:24 sean-k-mooney so i guess we would
16:07:54 mriedem which would take this change back to when artom, dansmith and i talked about it on hangouts 2 months ago
16:09:54 sean-k-mooney we would get an event for the sriov port
16:09:58 artom *sigh*
16:10:10 artom I suppose we better test it
16:10:20 sean-k-mooney but im trying to remeber when
16:10:49 sean-k-mooney plug is a noop for sriov. it is handeled by libvirt.
16:11:10 artom Actually, wait
16:11:27 artom Is resizing an instance with SRIOV supported?
16:11:30 sean-k-mooney yes
16:11:41 sean-k-mooney its kind of buggy but yes
16:12:45 sean-k-mooney althogh https://review.opendev.org/#/q/topic:bug/1809095+(status:open+OR+status:merged) will fix the most recent one
16:16:08 sean-k-mooney this is concerning https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/mech_sriov/mech_driver/mech_driver.py#L131-L143
16:16:32 artom I suppose I could hax _get_neutron_events_for_migration() to be smart about what vifs it returns...
16:16:52 artom That seems fragile as hell, event more than the patch in its current form
16:16:54 sean-k-mooney if the the prot is a PF https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/mech_sriov/mech_driver/mech_driver.py#L163-L168
16:17:03 sean-k-mooney we will send the event when we update the binding
16:17:16 sean-k-mooney if the port if a vf we will send the event when the agent wires it up
16:18:04 artom At point do we just say screw it and start queuing reveived events?
16:18:09 artom *At what
16:18:27 artom Or unload all those problems on Neutron and tell them to send the events consistently?
16:19:13 sean-k-mooney well https://review.opendev.org/#/c/645173/ is ment to help with this
16:19:22 artom Because assuming what sean-k-mooney is saying is True (and I have no reason to doubt it, but it def needs testing), we now have 3 different port "types" that we need to be aware of
16:19:40 sean-k-mooney but i have not been working on it because of other stuff like RDT/CAT
16:20:53 artom sean-k-mooney, yeah, but depending on that spec would make the Nova patch unbackportable, which I'm trying to avoid
16:21:03 artom (Never mind that it would take forever
16:21:05 artom )
16:21:53 sean-k-mooney right also i have not had time to work on it for a month and likely wont in the short term as i have nto figured out how to expresss when the event will be sent yet in an abstct way
16:22:44 artom I guess "do what we did before (wait in libvirt), except for known cases where it doesn't work and we have to do new thing (wait in compute manager)" would be a valid guiding principle
16:23:05 artom I'm gonna go get lunch, then try and see if I can code that in a palatable way
16:23:14 sean-k-mooney you would have to wait if hybrid_plug=true or vnic_type=direct-physical
16:23:20 sean-k-mooney in the manager
16:23:27 sean-k-mooney else waith in the driver
16:23:51 artom sean-k-mooney, so, that latter part hasn't actually come up (yet?), so I don't think I want to include it
16:24:24 artom But I'm going to try to genericize my _uses_hybrid_plug() method a bit to make it extensible
16:24:25 sean-k-mooney we could fix sriov in a sperate patch?
16:24:33 sean-k-mooney artom: mriedem thoughts ^
16:25:02 artom ... and also do as yet unclear things to allow the compute manager to communicate to the virt driver *which VIFs* it needs to listen to events for
16:25:11 artom Or some other way of achieving the same effect
16:25:17 sean-k-mooney artom: we would need to be able to express on a per vf bassis if we should wait which is what i want to do with that sepec anyway
16:25:38 artom sean-k-mooney, yeah, it's basically the same, just without the Neutron part
16:26:34 sean-k-mooney we would have to embed logcici into the vif object to encode "wait in dirver" or what ever instead of using a flag for all vifs yes
16:27:19 artom sean-k-mooney, that's not going to be backportable either, is it?
16:28:58 sean-k-mooney the vif "objects" we store in the networkin info cache are dicts so its not versioned
16:28:58 efried stephenfin: I've got a permalink in nova docs
16:28:58 efried https://opendev.org/openstack/nova/blame/commit/1316c1c2850d2f966f335b628f7f5fe88cef611c/doc/source/user/flavors.rst#L776
16:28:58 efried that I want to use from a reno. How do I do that?
16:29:10 sean-k-mooney but i would have to check the code to determin that
16:29:41 efried stephenfin: I don't think I can just use extra-specs-numbered-resource-groupings_ - because the renos and the docs are in separate repositories?
16:29:42 mriedem efried: https://docs.openstack.org/nova/latest/user/flavors.html#extra-specs-numbered-resource-groupings
16:29:59 mriedem unless nova-doc works in reno builds
16:30:10 stephenfin efried: https://docs.openstack.org/nova/latest/user/flavors.html#extra-specs-numbered-resource-groupings
16:30:13 stephenfin yeah
16:30:30 stephenfin it doesn't because release note builds aren't versioned
16:30:32 efried stephenfin: mriedem: Right, so I have to use the whole URL, okay. Thanks.
16:30:33 artom sean-k-mooney, oh, those VIFs. Well, we have what we need already, no? ovs_hybrid_plug and type
16:30:36 sean-k-mooney artom: ya the vif you are using in that code is an instance of nova.network.model.Vif https://github.com/openstack/nova/blob/master/nova/network/model.py#L380
16:30:48 sean-k-mooney yes we do
16:31:28 sean-k-mooney but if you wanted to add a flag so you did not check bot of those you could add a property to that class that return a vlaue base on if ether of the dict values were set
16:32:46 sean-k-mooney e.g. put the logic to wait into the Vif class and have a property "wait_location" that return "driver" or "manager" based on the vnic_type or hybrid_plug
16:32:59 sean-k-mooney artom: like https://github.com/openstack/nova/blob/master/nova/network/model.py#L461
16:33:14 sean-k-mooney i guess it could be just a fuction too
16:33:23 aspiers efried: according to the SEV spec, our current plan is to add hw:mem_encryption to flavor extra specs and image properties, but I've just noticed that while existing extra specs params use the "hw:" prefix, image properties all use the "hw_" prefix. Presumably I should remain consistent with those conventions?
16:33:51 artom sean-k-mooney, not sure about putting logic in the model itself, but yeah, it's an idea
16:34:07 mriedem sean-k-mooney: so as it stands, artom's patch will do:
16:34:19 sean-k-mooney artom: well we have is_hybrid_plug_enabled in the Vif object
16:34:19 mriedem 1. if all ports are ovs_hybrid_plug=True, wait for all in the manager and you're fine,
16:34:24 artom 0. drive artom to even moar excessive alcoholism
16:34:35 mriedem 2. if all ports are ovs_hybrid_plug=false, wait in the driver and you're fine (tested in the gate),
16:34:39 artom sean-k-mooney, yeah, but that's basically just a getter
16:35:07 mriedem 3. if some ports are ovs_hybrid_plug=True but not all, we'll wait for all events for all ports in the manager and some might not come (at least direct-physical)
16:35:24 sean-k-mooney mriedem: correctr
16:35:40 mriedem ok so based on #3,
16:36:04 mriedem what if we only wait for the known ones in the manager (ovs_hyrbid_plug=True),
16:36:26 sean-k-mooney direct-pysical will be the same as ovs_hyrbid_plug=True
16:36:35 mriedem and if there are others, we pass vifs_already_plugged=False to the driver? that would fail in the ovs_hybrid_plug=true ports b/c we already processed that event yeah?
16:36:58 sean-k-mooney vnic_type=direct and macvatap are teh same as ovs_hyrbid_plug=False and will arrive at the dirver

Earlier   Later