| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-08 | |||
| 16:08:23 | sean-k-mooney | neutron then wires up the port when the vm starts but that takes a little tiem to happen | |
| 16:08:47 | sean-k-mooney | the fix is to delegate to the os-vif lib to plug the interface in this edgecase also | |
| 16:09:17 | sean-k-mooney | that way the port will be wired up by neutron before we migrate | |
| 16:09:36 | fungi | and this is effectively a design flaw in nova because it assumes the call won't time out? or a bug in neutron for not treating it consistently using os-vif? | |
| 16:10:14 | sean-k-mooney | fungi: its legacy behavior form when nova woululd do firewalling for the port instead of neutron | |
| 16:10:54 | fungi | okay, so this will also eventually be solved when nova removes that deprecated behavior? | |
| 16:10:58 | sean-k-mooney | that said yes its partly a design flaw in nova. | |
| 16:11:29 | sean-k-mooney | yes if nova always used os-vif to plug the interface it would not happen | |
| 16:12:16 | fungi | and so https://review.openstack.org/602384 is basically a workaround to avoid having to switch nova to calling into os-vif for these? | |
| 16:12:33 | mriedem | umm...we use os-vif with nova-net too, so "yes if nova always used os-vif to plug the interface it would not happen" is kind of confusing | |
| 16:13:19 | sean-k-mooney | mriedem: the ovs plugin in os-vif was designed not to plug the interface in this case because libvirt does that | |
| 16:14:04 | sean-k-mooney | so we expressly do not create the ovs port and allow libvirt to do that currently. the change makes os-vif create the port which allows neutron to wire it up before we migrate the vm | |
| 16:15:16 | sean-k-mooney | mriedem: fungi so its not that nova does not call os-vif in this case. it does but os-vif was designed not to plug the interface in this case to maintain parity with how nova plugged interface before os-vif was split out | |
| 16:15:32 | sean-k-mooney | does that make sense? | |
| 16:16:00 | mriedem | shrug | |
| 16:16:03 | fungi | so the bug is in nova making assumptions about os-vif's behavior in this circumstance, or that os-vif doesn't fully implement the behavior nova expects? | |
| 16:16:20 | mriedem | sean-k-mooney: is this going to be a weird one off for ovs in os-vif only? | |
| 16:16:34 | mriedem | like will the behavior be different for all other vif types? | |
| 16:17:03 | mriedem | and when you say libvirt, do you mean the nova libvirt driver or libvirt the service? | |
| 16:17:11 | sean-k-mooney | that is a good question. i think there are a class of bugs related to this. | |
| 16:17:21 | sean-k-mooney | this bug predate os-vif | |
| 16:17:40 | sean-k-mooney | the original behavoir was incorrect | |
| 16:18:01 | sean-k-mooney | i say that becase anytime libvirt plugs the vif this can happen | |
| 16:18:48 | sean-k-mooney | this will not happen for vhost-user port as libvirt does not hanel plugin in that case. simplarly for siov libvirt set the vlan tag not neutron so that is safe | |
| 16:19:52 | sean-k-mooney | for now this is a one off but i need to look at other backends such as linux bridge to confirm this is a one off | |
| 16:19:56 | fungi | is this going to be backportable at least as far as stable/pike of os-vif? since we'll want a 1.7.1 tagged there for nova stable/pike to consume i guess | |
| 16:20:09 | sean-k-mooney | fungi: yes it should be | |
| 16:20:57 | sean-k-mooney | fungi: it requires not code change out side of os-vif and has no dependceis that i can tell to backport the change. | |
| 16:21:56 | fungi | i'm still a little iffy on how to go about describing this situation if we decide to publish an official advisory, particularly in that we consider it a nova bug but didn't patch nova to fix it. does the bug remain in nova even with newer os-vif? or is it simpler to explain it as a shortcoming of os-vif that we fixed to eliminate this behavior? | |
| 16:23:14 | jaypipes | mriedem: did the vmware live migration patch. | |
| 16:23:20 | sean-k-mooney | fungi: a newer os-vif will resolve the issue. my concern with calling this an os-vif only bug is if we go back to before we split out os-vif the bug i belive would still exist in the nova tree | |
| 16:24:24 | fungi | well, the advisory will only concern itself with the state of these repositories as of pike or later since we don't claim to provide security support to eol or em branches/releases | |
| 16:25:16 | sean-k-mooney | fungi: in that case its likely eaiser to discribe it as a os-vif bug given that pike uses os-vif | |
| 16:27:51 | fungi | and when you say "a class of bugs related to this" have any more been reported yet? | |
| 16:28:10 | sean-k-mooney | fungi: given the above if i add a release not to the os-vif change and propose backports. would it be inline with stable/vulnerablity policy to cut a release and bump the upper consttaint in the stable release | |
| 16:29:04 | fungi | mriedem would likely be able to better advise you on whether that particular change is suitable from a stable backport pilicy perspective | |
| 16:29:08 | sean-k-mooney | fungi: no but its posibly that thrid party plugins that manage ovs interfaces will need the same fix | |
| 16:30:02 | fungi | okay, so similar fixes may need to be applied to third-party ovs interface management plugins but not to any others officially managed by openstack as far as you're aware? | |
| 16:31:04 | sean-k-mooney | i just checked the linux bridge plugin and it does not need a similar fix as far as i can tell. so no none that i am aware of | |
| 16:31:41 | fungi | and what's the situation with https://review.openstack.org/602432 ? is that going to be abandoned as unneeded? | |
| 16:31:42 | sean-k-mooney | there are two out of tree plugins that i need to follow up on but i will check them and contact there maintianer if they need the same fix | |
| 16:32:24 | sean-k-mooney | fungi: yes i was going to that said i had planned on following up with the libvirt folks to see why it times out sometimes | |
| 16:34:05 | sean-k-mooney | we have had the issues using the ethernet iterface type in the past and i would like them to confirm why it does not work correctly in this case. that said its not relevent in to the bug disucssion | |
| 16:35:01 | fungi | okay, once you get a summary of the present state added to the bug and we get some confirmation that https://review.openstack.org/602384 is in line with stable policy, the vmt can write up an impact description, request a cve assignment and get the ball rolling on issuing an advisory | |
| 16:35:57 | fungi | and thanks for taking the time to explain this to me in such detail! | |
| 16:36:26 | mriedem | we'll want a release note on the os-vif change, we'll bump upper-constraints on stable but not lower-constraints, | |
| 16:36:31 | sean-k-mooney | fungi: no worries. i probably went into too much detail :) | |
| 16:36:43 | mriedem | it's unclear to me what, if any, side effects we could have on stable with different versions of ovs/libvirt being used | |
| 16:37:04 | mriedem | e.g. will libvirt complain if the port already exists because os-vif created it? | |
| 16:37:17 | fungi | yeah, some input on whether this is deemed safe enough to backport would also be most welcome | |
| 16:37:19 | mriedem | would libvirt create a duplicate? | |
| 16:37:20 | sean-k-mooney | this wont be effected by ovs. it may or may not be effected by libvirt version | |
| 16:37:55 | sean-k-mooney | mriedem: that is a good question and why i create https://review.openstack.org/#/c/602432/2 in the first place | |
| 16:38:18 | sean-k-mooney | i was expecting libvirt to be unhappy but the os-vif change passed tempest | |
| 16:39:33 | sean-k-mooney | mriedem: i think libivrt is doing th right thing here and recognising the port exits but it would be good to validate this espcially when backporting | |
| 16:40:56 | mriedem | you might want to start getting the backports lined up before we merge anything on master | |
| 16:41:29 | mriedem | i know we at least test different versions of libvirt in the gate between pike/queens and rocky/stein | |
| 16:42:23 | sean-k-mooney | mriedem: right ill -w the patch for now and respin with the release note then backport | |
| 16:43:41 | sean-k-mooney | i would be less comfrotable about backporting the nova change then the os-vif change to be hoenst as i dont really trust the libvirt ethernet type | |
| 16:57:34 | sean-k-mooney | moshele: you mentioned that macvtap livemigration was broken after the multiple port binings change. | |
| 16:58:41 | sean-k-mooney | moshele: i have fixed part of the issue locally but looking at https://github.com/openstack/nova/blame/fc58addab06134d7e6274a94d1ce456b0328723f/nova/network/neutronv2/api.py#L3054-L3062 live migration should have always been broken as the pci_mappings are only populated on cold migrate | |
| 16:58:48 | moshele | sean-k-mooney: broken if you don't need to update the pci address | |
| 16:59:52 | moshele | sean-k-mooney: it always broken if we need to change the pci_adress but if it the same on src and dest it should work (I think) | |
| 17:00:21 | sean-k-mooney | so i was able to migate the vm but nothing actully claimed the pci device on the new node | |
| 17:00:43 | sean-k-mooney | as a result we hit the exception in the else clause in post live migrate dest | |
| 17:00:45 | openstackgerrit | Jan Gutter proposed openstack/os-vif master: Add support for generic representors https://review.openstack.org/608693 | |
| 17:01:22 | sean-k-mooney | moshele: cold migrate was definetly broken but i dont think upstream nova ever wroked with livemigation due to that check | |
| 17:02:32 | sean-k-mooney | i.e. cold migrate used to work before multilple port binidngs | |
| 17:05:03 | moshele | sean-k-mooney: so live migration with macvtap never worked, but the multiple port binding a new bug to it | |
| 17:06:22 | sean-k-mooney | moshele: basically yes. so when i apply https://review.openstack.org/#/c/607365/ locally it correct the multiple port binding issue and live migrtion only fails because we dont claim the device on teh destination node | |
| 17:07:01 | sean-k-mooney | assumeing the pci address does not change. i.e. we also are missing the xml update code. | |
| 17:07:47 | sean-k-mooney | in anycase im going to lookin to how we do the claim in the cold migrate case and see if i can reuse that for live migration. | |
| 17:08:23 | moshele | sean-k-mooney: we can't be cause it use the migration context (move_claim) | |
| 17:09:18 | moshele | sean-k-mooney: we need just to call the pci resource tracker to claim it (and not call the move_claim) | |
| 17:09:53 | sean-k-mooney | ya i have scked that out also but i want to get migration without the claim working first then ill add the claim | |
| 17:11:04 | moshele | this is the cold migration and resize claim https://github.com/openstack/nova/blob/b4a3cdbe6e6a139dc11730c3046b728fb13d52e9/nova/compute/resource_tracker.py#L310-L354 | |
| 17:11:18 | sean-k-mooney | im just going to comment out the exception in the else block for now and add a todo/log then once i have migration back and fort working reliable with a singel vm ill add the claim logic and xml update code | |
| 17:11:24 | moshele | we should sub set of this just for pci | |
| 17:12:29 | sean-k-mooney | cool thanks yes what i really just wanted to grab out of that was how it was calling the resouce tracker to do the claim fo the devices as i was going to do it simlarly | |
| 17:13:13 | sean-k-mooney | anyway im going to go grab dinner but just wanted to checkin with you on the migration work. | |
| 17:15:01 | moshele | sean-k-mooney: let do a meeting about this thursday when adrianc will be back to sync on everything | |
| 18:06:08 | mriedem | guh, instance.launched_on, why | |
| 18:14:17 | mriedem | dansmith: true story, resize_claim doesn't handle volume-backed instance disk usage reporting properly yet... | |
| 18:20:21 | mdbooth | mriedem: Do we have an etherpad for BFV gaps? | |
| 18:20:33 | mriedem | not that i know of | |
| 18:21:22 | openstack | Launchpad bug 1796737 in OpenStack Compute (nova) "resize: hypervisor local_gb_used still reports usage even with volume-backed instances after fix for bug 1469179" [Undecided,New] | |
| 18:21:22 | mriedem | https://bugs.launchpad.net/nova/+bug/1796737 | |
| 18:21:43 | mriedem | i only hit this b/c of some functional tests in my cross-cell resize bafoonery | |
| 18:22:21 | mdbooth | You going to see how deep the rabbit hole goes? | |
| 18:22:25 | mdbooth | Or move on... | |
| 18:22:41 | mriedem | it's a relatively easy fix, | |
| 18:22:57 | mriedem | but i'll be commenting this out in my cross-cell resize test until fixed | |
| 18:24:16 | openstack | bug 1469179 in OpenStack Compute (nova) "instance.root_gb should be 0 for volume-backed instances" [Medium,Fix released] https://launchpad.net/bugs/1469179 - Assigned to Dan Smith (danms) | |
| 18:24:16 | melwitt | I wonder if the old func test from my NAKed bug 1469179 interim fix would be helpful. it did things like verify local_gb not reported after resize, shelve etc | |
| 18:25:09 | mriedem | it's a very easy recreate | |
| 18:25:14 | mriedem | resize a volume-backed instance and check disk usage | |
| 18:25:18 | mriedem | in the hypervisors API | |
| 18:25:28 | melwitt | I know, I'm saying I wrote a func test back then that does all of that | |
| 18:25:59 | melwitt | it was this one https://review.openstack.org/#/c/428505/19/nova/tests/functional/test_boot_from_volume.py | |
| 18:27:37 | melwitt | it does resize, shelve, unshelve, rebuild | |