| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-05 | |||
| 19:09:42 | dansmith | aside from the consistency gain of not calling the same external binary manually in multiple places | |
| 19:10:04 | cfriesen | mriedem: yes, just working on tests | |
| 19:10:11 | melwitt | ok, kewl | |
| 19:14:05 | efried | whoops, guess I should have been paying attention here before I +A'd that sucker. | |
| 19:16:31 | melwitt | it's np | |
| 19:16:50 | efried | okay, can pull it if necessary. | |
| 19:17:26 | melwitt | nah, it's ok. looking at it further, it would take me longer to understand all the qemu-img stuff anyway | |
| 19:28:51 | artom | sean-k-mooney, around? I need your Neutron and/or os-vif expertise | |
| 19:32:00 | mriedem | efried: edmondsw: i'm assuming this is ok for powervm since the parameter is just ignored https://review.openstack.org/#/c/613039/4/nova/virt/powervm/driver.py | |
| 19:32:08 | mriedem | maybe could help optimize something with powervm volume extend though | |
| 19:32:34 | efried | mriedem: powervm oot will need to do the same, so thanks for alerting edmondsw | |
| 19:33:10 | efried | generally I like to see a ML post when we change things like the ComputeDriver interface, since OOT drivers will need to conform. | |
| 19:34:07 | mriedem | ooo as will hyperv http://git.openstack.org/cgit/openstack/compute-hyperv/tree/compute_hyperv/nova/driver.py#n224 | |
| 19:34:20 | efried | mriedem: but yeah, the change itself should be harmless. | |
| 19:37:40 | efried | And I don't see how powervm could take advantage of it to help anything. | |
| 19:39:37 | mriedem | yeah me neither | |
| 19:41:13 | artom | Is the "plug" verb overloaded in Neutron and os-vif? | |
| 19:42:18 | artom | Looking at https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L5653-L5656, I could be forgiven for thinking that our call to plug_vifs() is what causes the network-vif-plugged event to eventually reach | |
| 19:42:27 | artom | But that's incorrect... right? | |
| 19:42:47 | artom | Way earlier we call Neutron via setup_networks_on_host(), and *that's* how we get the event | |
| 19:43:14 | artom | Because Neutron plugs the port | |
| 19:43:58 | artom | ... is that the distinction? Neutron plugs *ports* on the host (ie, sets up host networking) | |
| 19:44:18 | artom | And then Nova plugs the *VIF* to whatever interface Neutron has made for us... | |
| 19:45:11 | artom | ... except the event we get from Neutron is vif-plugged, not port-plugged | |
| 19:46:16 | mriedem | i don't think that's accurate | |
| 19:46:36 | mriedem | because we may or may not get a network-vif-plugged event during hard reboot where the port binding host doesn't change | |
| 19:46:46 | mriedem | but only certain networking backends in neutron will send the event in that case | |
| 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: Add user_id field to InstanceMapping https://review.openstack.org/633350 | |
| 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:54 | openstackgerrit | melanie witt proposed openstack/nova master: Add online data migration for populating user_id https://review.openstack.org/633351 | |
| 19:49:54 | openstackgerrit | melanie witt proposed openstack/nova master: Add get_counts() to InstanceMappingList https://review.openstack.org/638072 | |
| 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:55 | openstackgerrit | melanie witt proposed openstack/nova master: Use instance mappings to count server group members https://review.openstack.org/638324 | |
| 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 | 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 | |