Earlier  
Posted Nick Remark
#openstack-nova - 2019-03-05
19:46:59 mriedem it led to a whole messy situation with dealing with that and reboots for things like opendaylight
19:47:46 dansmith artom: neutron sends us vif-plugged when it sees an appropriately-named interface show up on the system,
19:47:48 mriedem lemme dig up a ghost for you
19:48:00 dansmith which is usually because we have created it as part of starting a vm
19:48:11 mriedem artom: have fun with this https://review.openstack.org/#/c/553035/
19:48:26 dansmith artom: there's also some plug terminology around hooking that interface up to a bridge
19:48:40 dansmith and some around the work nova does to create an interface in the first place like during spawn
19:48:42 artom dansmith, ok, that answers my "ulterior motive" questions, which was - can we wait for vif-plugged in the compute manager. The answer being: no. Because the VM needs to start.
19:48:50 aspiers mriedem: Yes that's correct, there are more changes by my colleague breton which build on https://review.openstack.org/#/c/638680/ and modify the guest config to activate SEV functionality. Actually they aren't far from code complete but I think still need a bit of polish here and there. 2 or 3 are already in Gerrit and there are maybe 2 or 3 to come after that.
19:49:07 dansmith artom: in the spawn case, that's correct
19:49:45 dansmith artom: which is why we start the instance paused, wait for the interface to show up, neutron to notice, notify us, and then we unpause the guest
19:49:48 dansmith artom: before, we would start the instance right away, and it might time out waiting for dhcp before all that happened
19:49:53 openstackgerrit melanie witt proposed openstack/nova master: Populate InstanceMapping.user_id during migrations and schedules https://review.openstack.org/638574
19:49:53 openstackgerrit melanie witt proposed openstack/nova master: Add user_id field to InstanceMapping https://review.openstack.org/633350
19:49:54 openstackgerrit melanie witt proposed openstack/nova master: Add get_counts() to InstanceMappingList https://review.openstack.org/638072
19:49:54 openstackgerrit melanie witt proposed openstack/nova master: Add online data migration for populating user_id https://review.openstack.org/633351
19:49:55 openstackgerrit melanie witt proposed openstack/nova master: Use instance mappings to count server group members https://review.openstack.org/638324
19:49:55 openstackgerrit melanie witt proposed openstack/nova master: Count instances from mappings and cores/ram from placement https://review.openstack.org/638073
19:49:57 dansmith which was the initial point of these events
19:50:32 artom dansmith, hrmm, so I've understood the problem wrong then
19:50:50 aspiers mriedem: We've been mostly pushing them in a linear sequence, but there has been a bit of parallelism on the base work not specific to SEV, to avoid superfluous dependencies slowing things down.
19:50:56 artom It's not just that we're not listening for the event when it arrives
19:51:26 aspiers mriedem: I'm more than happy to draw a dependency graph summarising all patches if that helps?
19:51:46 dansmith artom: the initial problem was that some not-insignificant amount of time, we would start the instance (which created the vif) and the instance would be stillborn without networking, because it got to the OS part of the boot that waits for DHCP before neutron noticed, wired us up, setup dhcp for us etc
19:51:52 aspiers I thought I'd seen launchpad doing that actually, but maybe that was just for dependencies between bps
19:51:59 artom In case of spawn, we can't ever hit that situation because we start listening before we start the VM
19:52:07 dansmith artom: in the boot case, we're definitely waiting at the right time, and that's why we don't have 10% DOA instances in the gate since like 2013
19:52:14 dansmith artom: exactlyu
19:52:15 artom The problem arises in other cases, where the interface is already present on the host
19:52:33 dansmith artom: right, which is because we've possibly blindly applied this to other cases like moves
19:52:45 artom Took me 2
19:52:46 dansmith artom: which have similar issues, of course, but potentially different timing
19:52:52 artom Took me 2 days, but I got there
19:53:38 artom So it's not a blanket "let's start waiting before we poke Neutron" kinda of thing
19:53:45 artom I need to take it case by case
19:54:06 artom See when we poke Neutron, whether the interface is present when we do, and when do we start waiting
19:54:09 artom Fuuun
19:54:15 dansmith artom: that's why in the previous discussion, I was careful to say "before we do the thing that will trigger the event"
19:54:28 dansmith which may or may not be talking to neutron (in fact, it usually isn't I think)
19:54:58 dansmith and also why I said bringing the trigger under the umbrella context is the right thing, for whichever operations that isn't already the case
19:55:13 artom dansmith, a very carefully worded non-specificity ;)
19:56:35 artom And apparently what Neutron does also depend on the VIF type?
19:57:30 dansmith AFAIK, all of this depends on vif type
19:57:40 dansmith which is why we don't wait for events for certain vif types
19:57:53 dansmith and for certain vif types during certain operations, IIRC (i.e. move vs. spawn)
19:58:22 artom Where's the code that decides that?
19:59:54 dansmith there's _get_neutron_events() in libvirt driver, but that doesn't have the vif type filter I was thinking of
19:59:55 dansmith oh,
20:00:08 dansmith sahid wrote a patch that introduced one vif type filter that we reverted
20:00:09 artom Yeah, I was looking at that
20:00:11 dansmith so that's no longer in there
20:00:17 dansmith there's one other place I remember, ahgn on
20:00:21 artom Unless it's implicit in the 'ACTIVE' check
20:01:46 dansmith hmm, I thought there was also a vif type filter in mriedem's more generic check for live migration, but I don't see it there either
20:01:48 artom And actually in libvirt _create_domain_and_network is the only time we call wait_for_instance_event
20:01:48 dansmith so maybe I'm just thinking about that patch from sahid that we reverted
20:01:59 dansmith ...in the libvirt driver, yes
20:03:04 artom Ah, there's _get_neutron_events_for_live_migration in the compute manager
20:03:17 artom No vif type filter though
20:03:51 dansmith yeah, I really thought that one had a filter on it, maybe mriedem can comment when he sees this
20:03:58 dansmith lemme dig up the sahid patch to prove there was one at least
20:04:24 mriedem vif type == linuxbridge?
20:04:34 mriedem vif type goes out the window because we can't rely on it
20:04:44 mriedem ala https://review.openstack.org/#/c/553035/
20:05:01 mriedem opendaylight is ovs vif type but doesn't send network-vif-plugged on vif plugging, only port binding
20:05:16 mriedem aspiers: no i don't need a graph of the code patches,
20:05:17 dansmith artom: https://review.openstack.org/#/c/497457/30/nova/virt/libvirt/driver.py@5536
20:05:50 mriedem aspiers: the reason i was asking is because there doesn't seem to be a reason, to me, to land the sev discovery / exposing the capability from the host change (which jaypipes is +2 on) until we actually have code that can plumb the guest
20:05:56 dansmith mriedem: ah, did we have a check for that before we had to remove because of ODL?
20:06:00 artom dansmith, hrmm, why'd that get reverted out?
20:06:07 dansmith artom: for so many reasons
20:06:12 dansmith artom: amazingly broken :)
20:06:28 mriedem artom: for one thing it was setting up the waiter *after* we'd plugged vifs on the dest
20:06:32 mriedem and the waiter code was waiting on the source
20:06:34 mriedem so a race and all that
20:06:53 artom I was thinking more narrowly in terms of the VIF type logic
20:07:08 dansmith artom: https://review.openstack.org/#/q/Ibf2b5eeafd962e93ae4ab6290015d58c33024132
20:07:17 artom IOW, is "Currently we only use the mechanism of waiting for neutron events during live-migration for linux-bridge" false?
20:08:01 dansmith artom: it turned out that neutron was sending those events, this just excluded waiting for them because of other bugs.. papering over the real problem
20:08:13 dansmith it's been a while, but there was a LOT of analysis and discussion
20:08:30 mriedem artom: true that is false :)
20:08:35 aspiers mriedem: well one reason is that we (SUSE) will need to carry our own downstream backport to Rocky, so the more which lands in Stein the easier it will get for us ... but if that is in conflict with upstream goals then of course I would entirely understand
20:08:48 mriedem artom: https://docs.openstack.org/nova/latest/configuration/config.html#compute.live_migration_wait_for_vif_plug
20:08:59 mriedem ^ we wait for networking events during live migration based on config
20:09:01 mriedem since rocky
20:09:07 mriedem and part of that was backported
20:09:14 mriedem by mr smith
20:09:48 mriedem aspiers: heh, sure - i just don't think we want the sev capability showing up automatically in stein on the compute node resource provider when there is really nothing you can do with it
20:10:33 dansmith ah right, artom, the Note:: in that doc explains why the vif_type thing isn't usable and why you have to config your way out of this for certain backends
20:10:34 dansmith which sucsk
20:10:48 aspiers mriedem: yeah, totally understood ... we were really hoping to get all the patches ready in time for stein but it looks like we just weren't quite quick enough
20:11:33 mriedem aspiers: no problem, you got that capabilities patch landed which i'd abandoned essentially, which is pretty good for your first big contribution to nova
20:11:52 artom dansmith, I get all of it except for "without an accompanying port binding:host_id"
20:11:54 mriedem plus you had that acid trip of a venn diagram
20:12:06 mriedem artom: that's what i was saying earlier,
20:12:15 dansmith artom: ever get the feeling you stepped into a hornet's nest?
20:12:17 mriedem ODL will only send network-vif-plugged for port binding changes to the host
20:12:34 mriedem if the host doesn't change on the port binding, but we plug/unplug vifs or whatever, ODL doesn't send the event
20:12:36 aspiers mriedem: thanks :) feels good to be giving to upstream not just "taking" (i.e. asking for our own pet feature to be reviewed and merged)
20:12:52 mriedem artom: hence why it had to be reverted from that hard reboot change

Earlier   Later