| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-29 | |||
| 17:44:54 | melwitt | fghaas: cfriesen might be the person to ask about that | |
| 17:45:24 | jmlowe_ | I was really hoping somebody just forgot to slip in a _update_port_binding_for_instance call | |
| 17:45:42 | mriedem | you're looking at the wrong method | |
| 17:45:47 | mriedem | the binding stuff is called from setup_instance_network_on_host | |
| 17:46:22 | mriedem | setup is called on unshelve | |
| 17:46:24 | mriedem | b/c you're on a new host | |
| 17:46:34 | mriedem | cleanup is called from shelve offload b/c you're leaving that host | |
| 17:46:59 | mriedem | jmlowe_: so likely all we need to do is in https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L2517, get the ports for the instance, and clear their device_owner | |
| 17:47:08 | mriedem | since that's tied to the az which is tied to the host | |
| 17:47:15 | fghaas | melwitt: thanks, I'll keep my eyes peeled for cfriesen's thoughts then :) | |
| 17:47:43 | mriedem | jmlowe_: what i'm not sure about, is if that will screw with neutron if the device_owner is null but the binding:host_id is still set | |
| 17:47:57 | mriedem | https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L515 | |
| 17:48:14 | mriedem | we basically want ^ but we want to retain the port.device_id | |
| 17:48:44 | cdent | mriedem: why is it when you find and fix a race that race starts showing up all over the damn place (when it wasn't beore)? | |
| 17:49:05 | mriedem | because software is terrible | |
| 17:49:24 | mriedem | why doesn't shelve work after 7 years? | |
| 17:49:44 | mriedem | jmlowe_: report a bug and we can go from there | |
| 17:50:04 | jmlowe_ | ok, will do | |
| 17:51:14 | mriedem | jmlowe_: as noted, the other solution is changing _update_port_binding_for_instance to update the device_owner when the host binding changes | |
| 17:51:19 | mriedem | couple ways to skin this cat | |
| 17:52:01 | mriedem | jmlowe_: kind of need to know if the instance.availability_zone also changed when you unshelved it | |
| 17:52:11 | mriedem | looks like it must have? | |
| 17:52:17 | jmlowe_ | it did | |
| 17:52:37 | mriedem | ok yeah, then _update_port_binding_for_instance is probably all we need to update | |
| 17:52:41 | mriedem | which is called on unshelve | |
| 17:53:07 | jmlowe_ | I really don't care what it used to be, I need to be damn sure on startup it overwrites whatever the value is null, old, whatever with what nova now knows to be true | |
| 17:54:33 | mriedem | yup | |
| 17:54:54 | mriedem | my point being, i don't think we need to also worry about cleanup_instance_network_on_host | |
| 17:55:12 | mriedem | i'm asking in -neutron if we should/could unbind the port while the instance is shelved but leave it attached | |
| 17:55:20 | mriedem | since that seems like sane book keeping | |
| 17:55:35 | openstackgerrit | Artom Lifshitz proposed openstack/nova-specs master: NUMA-aware live migration https://review.openstack.org/552722 | |
| 17:55:52 | jmlowe_ | I am, will try to resist the inevitable air rage as I walk past Jeremy in his first class seats | |
| 17:56:02 | artom | jaypipes ^^ since you were so close, I feel comfortable bugging you on IRC ;) | |
| 17:56:20 | jaypipes | artom: absolutely. lemme re-review right quick. | |
| 17:56:39 | jaypipes | artom: yikes. whitespace! :P | |
| 17:56:46 | artom | Eh? | |
| 17:57:04 | artom | Did failing to run pep8 in my impatience come back to bite me? | |
| 17:57:07 | mriedem | https://review.openstack.org/#/c/552722/10/specs/rocky/approved/numa-aware-live-migration.rst@173 | |
| 17:57:17 | jaypipes | artom: hold up on pushing again.. I have other comments | |
| 17:57:35 | cdent | jmlowe_: that jeremy fellow should be regularly shunned | |
| 17:57:50 | artom | An evil squit, to be clear | |
| 17:57:51 | mriedem | artom: feel free to bug me about that spec (later) since i want to check the mechanics on the conductor / mixed compute version negotiation stuff | |
| 17:58:26 | artom | mriedem, sure, thanks :) Do you mean the implementation though? The spec doesn't really spell it out, other than "the conductor is going to do it" | |
| 17:58:31 | dansmith | efried: ...oh | |
| 17:58:40 | fungi | er, flies | |
| 17:59:05 | jmlowe_ | whack the underside of his tray to send his early drink flying | |
| 17:59:25 | mriedem | artom: i'll see | |
| 17:59:37 | dansmith | oh in the thread, that's why I didn't see it | |
| 18:00:39 | jaypipes | artom: k, reviewd. | |
| 18:02:47 | cfriesen | dansmith: do we currently have a mechanism to purge entries from the nova_cell0 db? | |
| 18:03:07 | openstackgerrit | Artom Lifshitz proposed openstack/nova-specs master: NUMA-aware live migration https://review.openstack.org/552722 | |
| 18:03:20 | cdent | fungi: jmlowe_ works with one of my oldest friends, who happens to travel a lot, and thus is not in the cheap seats | |
| 18:03:22 | artom | jaypipes, thank you! And right back to you :D | |
| 18:05:11 | mriedem | cfriesen: yes | |
| 18:05:24 | dansmith | cfriesen: yes, nova-manage db purge --all-cells, or without that and pointing at something with just cell0 configured | |
| 18:05:32 | dansmith | s/something/a config/ | |
| 18:05:38 | mriedem | https://docs.openstack.org/nova/latest/cli/nova-manage.html#nova-database | |
| 18:05:54 | openstackgerrit | Merged openstack/nova master: Fix issue for pep8 on py3 https://review.openstack.org/557633 | |
| 18:05:54 | mriedem | you have to archive first | |
| 18:05:59 | cfriesen | mriedem: thx | |
| 18:06:39 | mriedem | cfriesen: note that purge CLI is new in rocky | |
| 18:07:05 | mriedem | wonder if we should have a 'since' tag on our CLI docs | |
| 18:08:13 | fungi | cdent: i figured. i tend not to even track airline miles because it depresses me to think about how much i travel (and i don't even travel that much compared to a lot of people in this community) | |
| 18:10:20 | openstackgerrit | Merged openstack/nova master: VMware: add log message for VIF info details https://review.openstack.org/557256 | |
| 18:10:27 | openstackgerrit | Merged openstack/nova master: Start untangling network utilities. https://review.openstack.org/557542 | |
| 18:10:36 | openstackgerrit | Merged openstack/nova master: Move two more generic network utilities to a move obvious place. https://review.openstack.org/557543 | |
| 18:13:52 | openstackgerrit | Merged openstack/nova master: vmware: Fixes _detach_instance_volumes method https://review.openstack.org/557377 | |
| 18:15:47 | kashyap | dansmith: When you get a moment, unit test question: this "works", but can you tell if that deceptively "simple" 'mock_warn' is correct: http://paste.openstack.org/show/717935/ | |
| 18:16:58 | kashyap | (Catching this warning: https://review.openstack.org/#/c/534384/20/nova/virt/libvirt/driver.py@3871) | |
| 18:17:17 | jaypipes | artom: back at ya ;) | |
| 18:17:23 | dansmith | mriedem: yawna nak my version argument in the doc so I can update or not and resubmit | |
| 18:17:24 | dansmith | / | |
| 18:17:25 | dansmith | / | |
| 18:18:46 | dansmith | kashyap: that works and is done elsewhere, but I also don't care that you assert that the warn was called, but that you executed that code. If you had a typo in anything inside that if, you wouldn't hit it in your tests unless you cause the if to be true, which is the real goal | |
| 18:19:11 | openstackgerrit | Artom Lifshitz proposed openstack/nova-specs master: NUMA-aware live migration https://review.openstack.org/552722 | |
| 18:19:42 | kashyap | dansmith: Yep; the typo inside the 'if' is taken care by the config "choices", right | |
| 18:19:49 | dansmith | kashyap: no | |
| 18:19:50 | kashyap | That's why we moved it there | |
| 18:19:52 | dansmith | no | |
| 18:20:00 | kashyap | Err, I mean, let me rephrase | |
| 18:20:07 | dansmith | if condition: LOG.warrrrrning('foo') | |
| 18:20:17 | dansmith | that would not fail tests unless you cause condition to be true | |
| 18:20:18 | jaypipes | artom: aaaaannndddd... +2. :) | |
| 18:20:32 | artom | jaypipes, yey! Thank you | |
| 18:21:01 | dansmith | kashyap: no failure: https://pastebin.com/A2muKWxU | |
| 18:21:24 | mriedem | dansmith: is that "you want to" or "you going to"? | |
| 18:21:27 | artom | I was worried we'd "hit" an asymptote and never get there ;) | |
| 18:21:35 | mriedem | i can't parse that southern drawl | |
| 18:21:48 | kashyap | dansmith: Okido; noted. | |
| 18:21:49 | dansmith | mriedem: you want to, although I assume you're going to | |
| 18:22:18 | mriedem | dansmith: i already -1ed for the other stuff | |
| 18:22:24 | mriedem | oh you replied | |
| 18:22:25 | mriedem | sec | |
| 18:24:26 | kashyap | dansmith: Alright; to make you double-happy, I'll slap in another test for the 'host-passthrough'. As we have two conditions there | |
| 18:24:54 | dansmith | kashyap: if it's not related to your change, then save it for another patch | |
| 18:25:05 | dansmith | kashyap: but yes as I noted, we don't have tests for the other conditions there | |
| 18:25:07 | kashyap | dansmith: It's actually related, because: | |
| 18:25:09 | mriedem | dansmith: replied in the docs patch | |
| 18:25:32 | dansmith | mriedem: tanks | |
| 18:25:39 | mriedem | that's also why i called out my specific points for the -1 | |