| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-09 | |||
| 20:23:55 | mriedem | idk this is where i thought we'd generate the event https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7775 | |
| 20:24:24 | prometheanfire | ok, I have to go for a bit, but will be back | |
| 20:24:34 | dansmith | well, | |
| 20:24:45 | dansmith | I did some debugging on plug stuff with the godaddy people a month or so ago, | |
| 20:24:51 | dansmith | and read through all the neutron code related to this | |
| 20:24:59 | dansmith | and I was surprised to learn that actually what happens is, | |
| 20:25:06 | dansmith | something creates an interface with the right name, | |
| 20:25:17 | dansmith | a periodic in the neutron agent notices, hooks it up and sends the event | |
| 20:25:30 | dansmith | so it's a little less connected to us than I would have thought | |
| 20:26:04 | mriedem | i knew the linuxbridge-agent does polling only b/c sean mooney explained that when we had the issue with waiting for hard reboot events for linuxbridge in the gate | |
| 20:26:20 | mriedem | ovs agent listens for an actual event from ovs itself | |
| 20:26:46 | dansmith | maybe that's why we can win it in the gate, since there was a polling loop | |
| 20:27:04 | mriedem | we use ovs in the gate | |
| 20:27:07 | mriedem | for most everything | |
| 20:27:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Handle binding_failed vif plug errors on compute restart https://review.openstack.org/587498 | |
| 20:27:13 | dansmith | I thought we had a LB job? | |
| 20:27:14 | mriedem | neutron has a couple of lb jobs i think | |
| 20:27:32 | dansmith | regardless, you did a LB migration job and it passed a few times at least | |
| 20:27:34 | mriedem | i had rigged up a patch that ran linuxbridge multinode to test sahid's patch | |
| 20:27:41 | mriedem | yeah, could have won the race though right? | |
| 20:27:43 | Sundar | efried: Please ping me when you have a moment | |
| 20:28:07 | efried | Hi Sundar, what's up? | |
| 20:28:16 | dansmith | I guess because of the polling loop we have a decent chance | |
| 20:28:32 | mriedem | and all of our controller services on a single node slowing things down maybe, idk | |
| 20:28:38 | mriedem | don't know what prometheanfire's env setup is like | |
| 20:28:38 | dansmith | because pre-dest is pretty long before we get to the event wait, which seems ...crazy to ever win it | |
| 20:28:59 | dansmith | if so, presumably this patch broke live migration for anyone with a fast system | |
| 20:29:17 | mriedem | god i hope so | |
| 20:29:19 | mriedem | that would be magical | |
| 20:29:32 | dansmith | so, sahid has been arguing to make the timeout non-fatal, reportedly because someone was using a custom driver or something | |
| 20:29:39 | dansmith | but I wonder if it's actually because this is actually totes broken | |
| 20:29:54 | Sundar | We were discussing the relative roles of plugins and drivers. It seems to me that that the distinction is not a hard one. We just need some extension agent with clear APIs for both os-acc and Cyborg. It could be the same module providing two sets of APIs. Does that work for you? | |
| 20:30:20 | efried | Sundar: Absolutely. | |
| 20:30:33 | dansmith | mriedem: melwitt: tbh, if we really think this is that broken (and sounds like it is) then we should probably revert it from the release immediately | |
| 20:31:06 | mriedem | well the other thing i was saying above was, | |
| 20:31:16 | Sundar | Great. I was trying to delineate the two and it was getting rather ambiguous. Good to have this sorted out. Will send out the os-acc spec with this update | |
| 20:31:16 | melwitt | oof | |
| 20:31:20 | mriedem | even if we get the TimeoutError, we raise MigrationError or whatever, | |
| 20:31:32 | mriedem | but i *think* by that point we've already started the guest transfer | |
| 20:31:43 | efried | Sundar: Well, we should still delineate the two, if they're going to be *able* to be separate modules. | |
| 20:31:43 | mriedem | b/c we call _live_migration_operation | |
| 20:31:44 | mriedem | the nwait | |
| 20:31:44 | dansmith | correct | |
| 20:32:07 | mriedem | so starting the guest transfer and then timing out on the watier b/c we registered late kills the nova live migration option but not what's in the hypervisor | |
| 20:32:09 | mriedem | right? | |
| 20:32:18 | mriedem | s/option/operation/ | |
| 20:32:29 | dansmith | mriedem: um, what? | |
| 20:32:52 | mriedem | if we get the timeout and raise MigrationError | |
| 20:32:56 | mriedem | but have already started the guest transfer | |
| 20:33:07 | mriedem | the only thing that happens in nova is we call the _rollback_live_migration code, | |
| 20:33:16 | mriedem | we don't attempt to kill any live migration job that's running in libvirt | |
| 20:33:17 | Sundar | efried: There will certainly be two sets of APIs: one for instance half of the attach and one for the device half. But both can do device-specific, platform-specific and vendor-specific actions. | |
| 20:33:17 | mriedem | right? | |
| 20:33:31 | efried | Sundar: I dig it. | |
| 20:33:49 | efried | Sundar: And I like the idea of being able to supply that code in one module or two. | |
| 20:33:54 | dansmith | lemme look | |
| 20:34:00 | mriedem | in other words, | |
| 20:34:10 | mriedem | nova will say "migratoin failed" but the guest might actually get transferred | |
| 20:34:17 | mriedem | just really f'ing slowly | |
| 20:34:52 | dansmith | well, yeah, I mean, the point of this code was to not raise the speed limit until it came | |
| 20:35:24 | mriedem | so did sahid want the timeout to just log and we'd have a finally that always set the bw back up? | |
| 20:35:54 | dansmith | yes | |
| 20:36:01 | mriedem | given what seems to be a pretty easy race to fail, that seems like it would have been better | |
| 20:36:04 | dansmith | which means you let it go to the other side but without networking | |
| 20:36:20 | Sundar | efried: So, it may be superfluous to have two separate modules, which are separately loaded by Stevedore. os-acc would have to load both, and the distinction in terms of what each module does seems to come down to APIs, rather than anything else. So, we might as well define two sets of APIs, and have one module do both. Internally, of course, the module may have separate packages/sub-modules for different functionalities. | |
| 20:36:36 | dansmith | mriedem: but the goal of the patch wasn't to "maybe catch the plug event", so if it never came in, it really should stop | |
| 20:36:51 | dansmith | mriedem: so I think it should cancel | |
| 20:37:02 | dansmith | which I said on the patch a couple of times, but I guess we never even got it that far | |
| 20:37:16 | Sundar | We may also provide common functions in os-acc for specific hypervisors | |
| 20:37:29 | mriedem | dansmith: ok, well the waiter is in the wrong place then, and https://review.openstack.org/#/c/558001/ was the right thing, | |
| 20:37:31 | mriedem | but not backportable | |
| 20:37:38 | Sundar | which any driver/plugin/module can invoke | |
| 20:37:43 | efried | Sundar: Offhand I don't see a problem with that. Is it ever going to be the case that you need to run one but not both (i.e. a driver but not its corresponding plugin, or vice versa) on a given system? | |
| 20:37:53 | sean-k-mooney | efried: Sundar provided there is a well defiend versioned interface its ok but Sundar i dont thin os-acc should be able to alter the hypervior context | |
| 20:38:18 | efried | Sundar: Let's move to #openstack-cyborg so we're not cross-talking with the others. | |
| 20:38:21 | sean-k-mooney | e.g. just like os-vif os-acc should not be able to modify the libvirt xml | |
| 20:38:27 | dansmith | mriedem: yeah, I was just looking through compute manager wondering why the fsck it was in there too | |
| 20:38:46 | dansmith | mriedem: does that not work for LB for some reason? | |
| 20:39:20 | mriedem | does what not work? | |
| 20:39:22 | mriedem | https://review.openstack.org/#/c/558001/ ? | |
| 20:39:22 | openstackgerrit | Merged openstack/nova stable/queens: Reload oslo_context after calling monkey_patch() https://review.openstack.org/589249 | |
| 20:39:26 | dansmith | yeah | |
| 20:39:29 | openstackgerrit | Merged openstack/nova stable/queens: Fix message for unexpected external event https://review.openstack.org/589505 | |
| 20:39:34 | mriedem | prometheanfire is failing in pike | |
| 20:39:36 | mriedem | https://review.openstack.org/#/c/558001/ is rocky | |
| 20:39:41 | openstackgerrit | Merged openstack/nova master: Trivial fix on migration doc https://review.openstack.org/589028 | |
| 20:39:45 | mriedem | b/c we backported sahid's patch | |
| 20:39:48 | openstackgerrit | Merged openstack/nova master: Add a prelude release note for the 18.0.0 Rocky GA https://review.openstack.org/589303 | |
| 20:39:49 | Sundar | sean-k-mooney: os-acc may provide device-specific XML snippets, for example, which libvirt driver would compose into a domain XML. | |
| 20:40:02 | Sundar | efried: Sure, joined #openstack-cyborg | |
| 20:40:03 | dansmith | mriedem: no, I realize that | |
| 20:40:24 | sean-k-mooney | Sundar: i really hope not or that code should live in the nova tree | |
| 20:40:33 | efried | sean-k-mooney: Can you join us in -cyborg? | |
| 20:40:40 | sean-k-mooney | efried: sure | |
| 20:40:41 | dansmith | mriedem: what I'm saying is, because the event gets triggered from pre-migration, the wait should really be up a level in compute manager, which you added in rocky | |
| 20:41:04 | dansmith | mriedem: and I'm asking if there's some reason why the wait in compute manager can't work with LB | |
| 20:41:32 | dansmith | mriedem: so we like just rip sahid's stuff out of everywhere and make sure that you're including events in the compute manager wait | |
| 20:42:18 | sean-k-mooney | efried: #openstack-cyborg? | |
| 20:42:27 | efried | yes | |
| 20:44:28 | mriedem | dansmith: it should work for LB as far as i know | |