| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-05 | |||
| 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 | dansmith | so maybe I'm just thinking about that patch from sahid that we reverted | |
| 20:01:48 | artom | And actually in libvirt _create_domain_and_network is the only time we call wait_for_instance_event | |
| 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 | |
| 20:13:00 | mriedem | this: https://review.openstack.org/#/c/553035/ | |
| 20:13:01 | artom | dansmith, yes | |
| 20:13:25 | aspiers | mriedem: haha, well my first version of that venn diagram was quite different, you can blame^Wthank efried for a lot of that second version | |
| 20:13:37 | mriedem | linuxbridge also had some problems with that in the neutron lb job because something about polling for changes and it would miss them and not send the event | |
| 20:13:42 | melwitt | I concur, thanks aspiers++ | |
| 20:13:48 | artom | mriedem, ah, so the note is just worded poorly to say "ODL will only send us vif-plugged if the host has changed" | |
| 20:13:51 | dansmith | mriedem: ah, yeah that's the thing I was expecting to find in there, forgot that had to be reverted too | |
| 20:13:53 | aspiers | thanks melwitt :) | |
| 20:14:04 | artom | ... which obviously it doesn't on hard reboot | |
| 20:14:05 | mriedem | artom: it is worded perfectly | |
| 20:14:38 | artom | Nuh uh, communication is only perfect if the thickest of idiots understand it. I didn't understand this. QED. | |
| 20:14:49 | mriedem | of course the port binding host *does* change during live migration, | |
| 20:15:03 | mriedem | but that happens after / during the guest transfer, not as a result of plug_vifs during pre_live_migration | |
| 20:15:06 | mriedem | which is what that config is used for | |
| 20:15:53 | mriedem | also, let me say, that i'm afraid sean-k-mooney might be dead or something because this entire conversation is the kind of thing he stays up until 3am for | |
| 20:16:01 | artom | Seriously. | |
| 20:16:23 | aspiers | LOL | |
| 20:16:25 | artom | RIP sean I guess. We'll do a minute of silence i Denver. | |
| 20:18:21 | artom | mriedem, wait, so why is that note even there? It's talking about not sending events when the host binding doesn't change. | |