| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-01 | |||
| 10:30:54 | kashyap | davidgiluk: Ah, you are here :-) | |
| 10:30:55 | mdbooth | i.e. there's no need to respond to an event from a call which you just made in code you're still executing | |
| 10:31:00 | johnthetubaguy | mdbooth: I keep being told its not syncronus | |
| 10:31:13 | mdbooth | What's not synchronous about it? | |
| 10:31:19 | johnthetubaguy | you just say, please do post copy at end of next iterration, I believe | |
| 10:31:30 | johnthetubaguy | so when we call post copy, it doesn't immediatly happen, I was told | |
| 10:31:31 | kashyap | davidgiluk: So, this is what johnthetubaguy was saying, when I asked him about "What is Nova's stance on postcopy": < johnthetubaguy> support is in, works with majority of neutron back ends, but not all, AFAIK | |
| 10:32:13 | johnthetubaguy | there can be a small network outage with some backends, during the post copy process (which ironically speeds up the live-migrate, of course) doing work to avoid that | |
| 10:32:24 | mdbooth | johnthetubaguy: In _live_migration_monitor we do: | |
| 10:32:24 | openstack | Launchpad bug 1605016 in OpenStack Compute (nova) "Post copy live migration interrupts network connectivity" [High,In progress] https://launchpad.net/bugs/1605016 - Assigned to Sivasathurappan Radhakrishnan (siva-radhakrishnan) | |
| 10:32:24 | davidgiluk | kashyap: Yeh so my understanding was it was waiting on a fix to lp 1605016 for the others, with https://review.openstack.org/#/c/413021/9 being a proposed fix | |
| 10:32:32 | mdbooth | libvirt_migrate.trigger_postcopy_switch(guest, | |
| 10:32:32 | mdbooth | previous_data_remaining, migration.status)): | |
| 10:32:32 | mdbooth | info.memory_iteration, info.data_remaining, | |
| 10:32:32 | mdbooth | libvirt_migrate.should_switch_to_postcopy( | |
| 10:32:32 | mdbooth | if (is_post_copy_enabled and | |
| 10:32:33 | mdbooth | instance, | |
| 10:32:35 | mdbooth | migration) | |
| 10:32:51 | johnthetubaguy | mdbooth: its the libvirt call that I am told is async | |
| 10:33:07 | johnthetubaguy | mdbooth: it just request post copy at some point in the future, and returns right away, before actually triggering it | |
| 10:33:12 | mdbooth | johnthetubaguy: Well we might switch a few milliseconds early | |
| 10:33:16 | johnthetubaguy | mdbooth: pkoniszewski has the details on that | |
| 10:33:19 | mdbooth | Rather than a few milliseconds late | |
| 10:33:39 | johnthetubaguy | mdbooth: I was told more like seconds, but yeah, I was betting on milliseconds myself | |
| 10:33:46 | davidgiluk | the postcopy transition happens at the end of a pass, and libvirt gets involved before starting the destination | |
| 10:34:03 | johnthetubaguy | mdbooth: either way, I vote for the approach siva_krishnan has in the works already, just need to iron things out | |
| 10:34:12 | johnthetubaguy | davidgiluk: sorry, whats waiting on the fix/ | |
| 10:34:23 | johnthetubaguy | s/ / / ? / | |
| 10:34:32 | davidgiluk | johnthetubaguy: The support for other networking configs | |
| 10:35:05 | johnthetubaguy | davidgiluk: that bug fix should help reduce the network outage some networking configs currently see, yes | |
| 10:35:43 | davidgiluk | johnthetubaguy: < johnthetubaguy> would love to trigger of QEMU events to get more exact timing of when the post copy occurs, but the basic structure is going in the above change | |
| 10:35:48 | davidgiluk | johnthetubaguy: We already emit the events | |
| 10:36:20 | johnthetubaguy | davidgiluk: agreed its omitted, we first need all the structures in place to call neutron at the right time, the current patch | |
| 10:36:34 | johnthetubaguy | oops emitted | |
| 10:36:43 | davidgiluk | generate :-) | |
| 10:37:32 | mdbooth | Well, this merged: https://review.openstack.org/#/c/402318/ :( | |
| 10:37:49 | mdbooth | The libvirt driver becomes slightly less fathomable for no appreciable gain. | |
| 10:38:11 | davidgiluk | johnthetubaguy: you should probably loop in jdenemar to check what it looks like for you from ontop of libvirt - but you should have all the events you need to know when postcopy kicks in | |
| 10:38:35 | kashyap | mdbooth: Propose a revert? | |
| 10:38:47 | johnthetubaguy | davidgiluk: thats the plan for the follow up fix | |
| 10:38:54 | mdbooth | kashyap: I proposed not merging it, which didn't work. | |
| 10:39:01 | kashyap | mdbooth: From glancing at your comments on the review | |
| 10:39:06 | davidgiluk | johnthetubaguy: OK, great; if you do need anything more from qemu, just ask | |
| 10:39:09 | johnthetubaguy | mdbooth: sorry, got pulled into meetings after that one, so I didn't get chance to delay it | |
| 10:39:10 | kashyap | mdbooth: No one has bothered to respond with a counter-argument | |
| 10:39:17 | johnthetubaguy | mdbooth: I can fast revert it if you want | |
| 10:39:36 | kashyap | Then, you have by all means, a right to revert it, and restart the discussion, and make whoever proposed it to respond | |
| 10:39:48 | mdbooth | johnthetubaguy: If you wouldn't mind. I'm assuming, btw, that there are no real users being impacted by this. | |
| 10:40:40 | johnthetubaguy | mdbooth: no idea, the SUSE folks proposed this, and I believe they support users on Xen + Libivrt + Nova, so I was assuming yes | |
| 10:41:15 | mdbooth | It's been broken forever though, right? | |
| 10:41:36 | johnthetubaguy | mdbooth: sure, I assumed they were trying to change that for one of their users that are complaining | |
| 10:42:09 | mdbooth | I just don't want to add more sticky tape without investigating fixing the root cause. | |
| 10:42:25 | mdbooth | I don't see any evidence anybody looked for the root cause. | |
| 10:43:17 | johnthetubaguy | mdbooth: I added my assumption of the cause in the review, well the root cause is Xen being very picky about the device names | |
| 10:43:55 | mdbooth | We need to either be picky earlier, or translate it earlier. | |
| 10:44:18 | johnthetubaguy | mdbooth: the problem is, you don't know if its Xen until it hits the compute node | |
| 10:44:19 | mdbooth | Sticking a band-aid at the lowest possible level is just going to be a source of bugs | |
| 10:44:42 | mdbooth | Yeah, that occurred to me. No immediate answer to that as I haven't given it any thought. | |
| 10:44:53 | mdbooth | Seems that nobody else has either, though. | |
| 10:45:46 | mdbooth | Might be something to be resolved in driver.spawn, and attach/detach volume? | |
| 10:46:03 | mdbooth | That way it's resolved at the time it's created | |
| 10:47:55 | mdbooth | Do those names have any semantic meaning at the api level beyond 'the Nth disk' anyway? I don't think we support multiple controller types, do we? | |
| 10:48:24 | johnthetubaguy | we support multiple hypervisors in a single cloud, at least in theory | |
| 10:48:38 | mdbooth | If ^^^ is a valid assumption, ComputeManager could rewrite it to a number, and the driver could change it back to whatever it wanted. | |
| 10:48:58 | johnthetubaguy | I don't think we even respect the number any more, but it could | |
| 10:49:01 | mdbooth | johnthetubaguy: It's in a single instance. i.e. could we have both sda and vda in the same instance? | |
| 10:49:36 | johnthetubaguy | sda would always fail to attach, all disks need to be vd* I believe, or xvd* | |
| 10:50:17 | mdbooth | Right, so computemanager would see it got 'sda' from the api, translate that to '0', and the driver would change it to 'sda', 'vda', or 'xvda' as required. | |
| 10:51:08 | johnthetubaguy | seems like a good refactor to do on top of the bug fix | |
| 10:51:54 | mdbooth | johnthetubaguy: The bug fix makes the existing code worse, though, and I see no evidence that anybody looked for a root cause. | |
| 10:52:47 | mdbooth | Given that it's obviously not urgent, I don't see why we would choose to take more tech debt for that. | |
| 10:53:28 | mdbooth | Anyway, it's not that important in itself. | |
| 10:53:44 | mdbooth | Just death by 1,000 cuts. | |
| 10:53:55 | johnthetubaguy | it breaks a pretty fundamental API feature, that seemed fairly important to be from a parity point of view, but I duno really. | |
| 10:56:54 | mdbooth | johnthetubaguy: Yeah, I know. But it's been broken a long time and I'd have preferred to see an attempt to fix it without adding tech debt first. | |
| 10:57:34 | johnthetubaguy | mdbooth: that my preference too, but its a balancing act | |
| 10:58:11 | johnthetubaguy | I was very boarderline on -1 +2 myself | |
| 10:58:14 | mdbooth | If it's important to somebody, they'll fix it. You even suggested where to look, and I agreed. | |
| 10:58:48 | bauzas | lyarwood: nice catch, +2d https://review.openstack.org/#/c/427267/2 | |
| 10:59:13 | sfinucan | bauzas: Could you take a look at https://review.openstack.org/#/c/392643/, please? :) | |
| 10:59:14 | bauzas | johnthetubaguy: in case you wanna fix a nasty bug with an easy peasy fix ^ | |
| 10:59:35 | bauzas | sfinucan: roger | |
| 10:59:39 | mdbooth | johnthetubaguy: Now ^^^ is a nasty bug :) | |
| 10:59:56 | mdbooth | Ah, not that one. | |
| 11:00:13 | mdbooth | The one before which bauzas mentioned. | |
| 11:02:03 | kashyap | sfinucan: Your friendly monthly reminder: If you put commit IDs in single quotes, then 'clickability" is lost :-) | |
| 11:02:10 | sfinucan | lyarwood: Finally done, though it seems bauzas pipped me to the mark :) | |
| 11:02:23 | sfinucan | nasty old bug indeed | |
| 11:02:24 | kashyap | sfinucan: Just reminding for next time. | |
| 11:02:26 | lyarwood | bauzas: thanks! | |
| 11:02:33 | sfinucan | sorry, kashyap :'( | |
| 11:02:33 | lyarwood | sfinucan: cheers, np :) | |
| 11:11:21 | openstackgerrit | Sergey Nikitin proposed openstack/nova-specs: Restore V3 Diagnostics info https://review.openstack.org/427637 | |
| 11:15:59 | sfinucan | johnthetubaguy: What are we doing about https://review.openstack.org/#/c/357190 ? | |
| 11:16:41 | johnthetubaguy | sfinucan: not sure what you mean? | |
| 11:16:53 | johnthetubaguy | oh, its that damm mox things | |
| 11:17:01 | sfinucan | aye :) | |
| 11:18:24 | johnthetubaguy | sfinucan: these bits hurt my eyes a little "mock_domain.undefineFlags.assert_has_calls([mock.call(1)])" | |
| 11:18:45 | johnthetubaguy | assert_called_once_with(1) would be much better | |
| 11:19:13 | sfinucan | derekh - you around? | |