| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-08 | |||
| 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 | |
| 18:30:59 | mriedem | does it julienne cut? | |
| 18:32:54 | melwitt | well, I thought it might help if you're finding gaps with local_gb not being handled correctly after server actions. those were the other ones I tested | |
| 18:33:37 | mdbooth_bus | The shelve-o-matic | |
| 18:33:43 | mriedem | yeah, i know, thanks. :) i'm just joking about julienne cuts. | |
| 18:33:51 | melwitt | at the time that I was trying to fix the bug with the patch that's now part of starlingX | |
| 18:37:34 | mriedem | yeah i can restore that test locally and see what fails still with context on this resize_claim bug | |
| 19:01:48 | mdbooth_bus | b | |
| 19:02:15 | mdbooth_bus | mriedem: Do you happen to remember where the local ephemeral root bdm is defined? | |
| 19:02:42 | mdbooth_bus | I'm trying to re-grok the bdm code in compute api, and I'm running out of mental cache | |
| 19:02:58 | mdbooth_bus | Also, it's really late and I'm on a bus | |
| 19:03:49 | mdbooth_bus | There's a comment which suggests that the client does it, but I couldn't find it there, either | |
| 19:04:45 | mriedem | yeah the api creates a thing, sec | |
| 19:09:31 | mriedem | nova.block_device.create_image_bdm creates it, | |
| 19:09:40 | mriedem | called from nova.block_device.from_legacy_mapping, | |
| 19:09:44 | mdbooth_bus | mriedem: Thanks | |
| 19:09:58 | mriedem | called from the api | |
| 19:09:59 | mriedem | at some point | |
| 19:10:06 | mriedem | totally not obvious | |
| 19:10:51 | mdbooth_bus | Wow, I'd already found and dismissed the from_legacy_mapping call | |
| 19:11:00 | mdbooth_bus | Yeah, that's not obvious, thanks | |
| 19:11:31 | mriedem | right b/c even if you use bdm_v2 you end up down that legacy path which is confusin | |
| 19:11:39 | mriedem | since legacy should mean bdm v1 | |
| 19:12:21 | mriedem | legacy_image_defined = not image_properties.get('bdm_v2', False) | |
| 19:12:33 | mriedem | ^ is the source of the confusion | |
| 19:12:39 | mriedem | in _get_image_defined_bdms | |
| 19:12:44 | openstackgerrit | Merged openstack/nova master: VMware: Live migration of instances https://review.openstack.org/270116 | |
| 19:13:05 | mdbooth_bus | 'legacy' there presumably meant you specified --image <imageid> rather than --blockdevice ...image-fu... | |