Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-08
15:26:35 mriedem artom: then i'm pretty sure it's what we've talked about before, finish_revert_resize on the source host calls migrate_instance_finish which updates the port binding to point at the source host, which triggers an event
15:26:40 mriedem *before* the driver registers the callback
15:27:28 artom mriedem, right, but there's another thing at play here - Neutron will only wire the ports after we plug them
15:27:33 artom ... I think
15:28:35 mriedem yes i can read his comments and i think they align with what i just said,
15:28:56 mriedem "Nova then asks neutron to bind port on compute-0 again, at it happens:" - that's the migrate_instance_finish call on the source finish_revert_resize to update the port binding
15:29:15 mriedem "And after that, L2 agent on compute-0 wire port again:" - i assume that's the plug_vifs call from the driver's finish_revert_migration on the source host
15:30:35 mriedem artom: how recreateable is this? it would be a lot more clear if we just had some logging on the source that said, "updating port bindings to point at the source host" and then "finish revert migration in the driver which will plug vifs"
15:30:53 mriedem because if we saw the event come between those 2 messages we know it's the port binding change that is triggering the event before we're ready to wait for it
15:30:54 artom mriedem, like, 70% in our CI?
15:31:06 mriedem are you able to test with patches from upstream?
15:31:21 artom I'd need to check with the CI guys, but I could provide them test builds, yeah
15:31:55 kashyap aspiers: You about...?
15:32:43 kashyap aspiers: When you are -- at the risk of adding more work ... wonder if we should simply split out the addition of getDomainCapablities() method into its own patch.
15:33:06 mriedem artom: ew do you have to patch an rpm or something?
15:33:13 kashyap It is just my OCD of "one logical change per-patch thing". And it allows quicker merge, too. As it'll be easier on the reviewers eyes
15:33:14 artom mriedem, scratch build
15:33:35 artom Backport a patch, build RPMs with that
15:34:04 mriedem artom: actually you should be able to determine this with existing logs
15:34:11 kashyap (A "scratch build" is something that is short-lived, and will be "scratched" from the build system)
15:34:54 artom (Backport a patch, build a SRPM without pushing anything, scratch build, I should have said)
15:35:04 mriedem artom: you should see this on the source https://github.com/openstack/nova/blob/4f9bc724010f0c935bf83a6d19bdd805e86b7086/nova/network/neutronv2/api.py#L3355
15:35:23 mriedem with binding:host_id changing to point at the source host
15:35:43 mriedem and then you should see this from the driver https://github.com/openstack/nova/blob/4f9bc724010f0c935bf83a6d19bdd805e86b7086/nova/virt/libvirt/driver.py#L8965
15:35:47 artom mriedem, ack, looking
15:36:43 mriedem granted there is some other stuff that happens in the driver after that before unplug_vifs happens, so it's a tight window
15:42:25 aspiers kashyap: yeah could do, I kind of like there being an incentive to get SEV stuff merged though ;-)
15:42:54 aspiers when do we fork for stein?
15:43:27 kashyap aspiers: Yeah, I hear you. But as we both know ... it can be used for multiple features :-)
15:43:57 kashyap (I won't insist on it, though. But if you appetite...)
15:43:59 aspiers we already hit feature freeze, right?
15:44:03 aspiers https://wiki.openstack.org/wiki/Nova/Stein_Release_Schedule
15:44:04 kashyap aspiers: Yes, yesterday
15:44:10 kashyap mriedem: ^ Right?
15:45:04 mriedem yes
15:45:13 mriedem please don't be approving anything that's not already approved
15:45:14 kashyap aspiers: Also think of it this way: splitting it out allows it to be merged while SEV bits get reviewed :-)
15:45:39 aspiers I guess
15:45:40 kashyap mriedem: This (AMD SEV work) was already approved for Stein
15:45:55 aspiers I think mriedem means W+1
15:45:58 kashyap aspiers: But sorry to be "that guy"; you're allowed to hate me for 5 minutes.
15:46:04 aspiers haha
15:46:21 sean-k-mooney aspiers: we will fork stien at RC1 so master is feature frozen for the next 2 ish weeks until that is done i think
15:46:42 aspiers sean-k-mooney: OK thanks
15:47:04 kashyap aspiers: This work will spill over into "Train", yes?
15:47:10 sean-k-mooney rc1 will be around march 21st
15:47:26 aspiers kashyap: https://review.openstack.org/#/c/641994/
15:47:43 aspiers Oh, you already saw that :)
15:47:46 aspiers I forgot
15:47:53 kashyap Yeah, no worries.
15:48:33 aspiers Anyway, ultimately I need to do whatever you guys think is best :)
15:48:39 openstackgerrit Balazs Gibizer proposed openstack/nova master: Documentation for bandwidth support https://review.openstack.org/642064
15:48:42 aspiers If splitting it out helps then let's do that
15:49:14 kashyap aspiers: Yeah, it also will be less "daunting" to those who don't normally dwell on this area of code
15:49:28 aspiers Sure
15:49:29 mriedem what are you talking about splitting out?
15:49:45 aspiers the call to getDomainCapabilities
15:50:13 kashyap mriedem: Yeah, the adding of getDomainCapablities() in libvirt/host.py
15:50:34 aspiers kashyap: I was also thinking about splitting this code move out https://review.openstack.org/#/c/633855/11/nova/virt/libvirt/utils.py
15:50:49 mriedem aspiers: ok, i see you already put the -W back on your own change that i don't want merged in stein at this point, so you know where i am on this
15:50:56 kashyap mriedem: This bit (line 680): https://review.openstack.org/#/c/633855/11/nova/virt/libvirt/host.py
15:51:34 mriedem sure, split that out even if to just make that change smaller and easier to grok
15:51:40 mriedem too much code in one patch == hard to review
15:51:43 aspiers mriedem: yes, that's also why I submitted https://review.openstack.org/#/c/641994/
15:51:44 kashyap (Yeah, I agree with mriedem on AMD SEV for Stein, I'm afraid.)
15:52:15 kashyap aspiers: Yes. The machine_type_mappings() in a separate change is good, too.
15:52:21 aspiers There's not even a debate on that, since feature freeze was yesterday :)
15:52:31 mriedem aspiers: you'd be surprised
15:52:56 aspiers Haha OK, well I'm sure some vendors are insane but not us
15:54:10 kashyap It's the same story with upstream _kernel_ as well :D
15:54:20 aspiers Yup
15:54:25 aspiers SUSE knows all about that ...
15:54:27 kashyap Once they start cutting long-term 'stable' release, everyone starts running around headless-chickens
15:54:41 kashyap ... "we need to get this into the "long-term" release!"
15:54:56 sean-k-mooney kashyap: honestly i think that is better the feature backports
15:55:00 gibi mriedem, melwitt: I'm wondering about how to close the https://blueprints.launchpad.net/nova/+spec/bandwidth-resource-provider the spec talks about the server move operations that was not implemented in Stein so if we use that as a scope then the bp cannot be marked implemented
15:55:18 kashyap sean-k-mooney: Not saying it's bad :-) Just the reality and the incentive structure I guess
15:55:31 mriedem gibi: i think we can complete the blueprint in stein and have a separate blueprint for move operations in train
15:55:39 aspiers OK I'm done for the week. I'll look at splitting that out into 3 reviews next week
15:55:56 aspiers Have a good w/e all! o/
15:56:16 kashyap aspiers: Just a heads-up, next week the first three days I'm travelling; so, if I'm slow on responding ... you know why :-)
15:56:20 kashyap Have a good one!
15:56:22 aspiers OK np :)
15:56:22 gibi mriedem: shall I remove the move operation from the old spec when I update that now?
15:56:45 mriedem gibi: are you going to amend the stein spec for the microversion and all the other stuff that came up?
15:57:07 mriedem i guess i'm not sure what you're asking me :)
15:57:51 gibi mriedem: yeah I'm plannig to amend the spec
15:57:58 gibi mriedem: so I can remove the move from the scope there
15:58:53 mriedem ok
15:59:01 gibi mriedem: ok
16:03:01 gibi mriedem: there are two trailing patche for the bandwidth series both can be considered bugfixes, https://review.openstack.org/#/c/638711/ and https://review.openstack.org/#/c/639608/ what do you think, shall I push them forward before RC1?
16:09:33 openstackgerrit guang-yee proposed openstack/nova master: pass endpoint interface to Ironic client https://review.openstack.org/640879
16:27:09 mriedem gibi: on https://review.openstack.org/#/c/639608/ i think that's already a potential bug because of not handling errors from claim_resources
16:27:27 mriedem so it's low priority for me, but i think it's a bug we can report and work on
16:27:57 mriedem as i said before that whole while loop needs to be refactored at some point (that whole damn build_instances method actually)
16:28:46 mriedem gibi: https://review.openstack.org/#/c/638711/ isn't really a bug per se, but it's a performance optimization, so again i don't know that we need to rush it, but it's still good to have
16:29:14 mriedem you could report a low priority bug for tracking that if you wanted - as a perf issue
16:29:28 mriedem like i said, if i'm multi-creating 100 instances in one request, that's 100 separate GET /allocations calls
16:29:49 mriedem although, can you multi-create with a pre-created port...?
16:29:55 mriedem i'm not sure that is supported

Earlier   Later