| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-12-12 | |||
| 15:36:20 | sean-k-mooney | adrianc: im just getting around to setting up that test env now but i will try a few different things and let you know | |
| 15:37:20 | sean-k-mooney | should i put this in the spec by the way. im inclined to say no as this is too technical/hardware specific | |
| 15:37:31 | adrianc | sean-k-mooney: ack, let me know if you can find a nicer WA | |
| 15:38:01 | adrianc | well i dont think its related to the spec, its a long existing problem IMO and not related to live-migration | |
| 15:38:19 | sean-k-mooney | we need to adress this in the code obviously but i think code commetns/dev docs | |
| 15:38:47 | sean-k-mooney | adrianc: i guess you could consider it a cold migration bug | |
| 15:38:48 | adrianc | there are additional gaps with macVtap like multiple port binding so | |
| 15:40:38 | adrianc | sean-k-mooney: it can happen also without LM being involved, easiest is to boot a VM on node A delete and boot it on node B | |
| 15:41:02 | adrianc | the mac will be reused and you hit the issue | |
| 15:41:14 | sean-k-mooney | adrianc: reusing the same neutorn port so that the mac and vlan remain the same | |
| 15:41:49 | adrianc | adrianc: yes, using the same neutron port | |
| 15:42:13 | adrianc | sean-k-mooney: ^ | |
| 15:42:22 | sean-k-mooney | ya ok that is proably ground enought to just treat this as a seperate bug and maybe backport the fix | |
| 15:43:53 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Add workaround to cleanup instance dir during evac with rbd https://review.openstack.org/618478 | |
| 15:45:41 | lyarwood | mriedem / mdbooth / melwitt ; ^ thanks again for the reviews, I appreciate just how ugly this is but hopefully this is btw but with downstream customers asking for this to be fixed back in Newton I'm being forced into a corner here tbh. | |
| 15:46:02 | lyarwood | thanks weechat-- | |
| 15:46:53 | sean-k-mooney | lyarwood: what wrong with weechat++ | |
| 15:47:53 | lyarwood | ^ the above mess, I was trying to edit the line and it decided to send it out, fun times. | |
| 16:02:27 | mriedem | melwitt: did you see my question in your spec for configurable max volumes? https://review.openstack.org/#/c/597306/1/specs/stein/approved/conf-max-attach-volumes.rst@44 | |
| 16:05:01 | melwitt | mriedem: I did, sorry. the immediate answer was "not 100% sure, need to check my notes again". it was "fun" tracing it all and I can't remember whether bfv vs attach additional end up using the same code path that leads to the limit of 26. I need to double check it in order to reply and haven't gotten to it yet | |
| 16:09:09 | mriedem | ok, but i'm assuming even if it's not the same code path, bfv would need a check somewhere for the config option to limit the number of volumes attached to an instance during spawn | |
| 16:10:21 | mriedem | it might have to be here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1598 | |
| 16:10:34 | mriedem | or a new driver method called from _prep_block_devices | |
| 16:10:53 | mriedem | because you don't want to create or attach more than the limit before you get to driver.spawn just to have it fail and have to roll those all back | |
| 16:20:00 | melwitt | mriedem: yeah, I guess the context is in my WIP patch for the implementation I've put the check at a lower level where all of the code paths end up https://review.openstack.org/616777 there's a default impl in compute/utils that all drivers other than libvirt use, and libvirt has its own override | |
| 16:20:36 | melwitt | I don't have any func test coverage yet to verify it all works right, so I'm in process of writing those | |
| 16:20:53 | melwitt | but if you want to take a quick look to point out anything bad that jumps out, I'd appreciate that | |
| 16:27:24 | mriedem | yeah that won't cut it for bfv | |
| 16:27:27 | mriedem | i'll leave a comment | |
| 16:28:03 | melwitt | thanks | |
| 16:38:56 | openstackgerrit | Merged openstack/nova master: Drop pre-cellsv2 compat in compute API.get() https://review.openstack.org/615347 | |
| 16:53:46 | openstackgerrit | Chris Dent proposed openstack/os-resource-classes master: Sync STANDARDS with placement/nova rc_fields https://review.openstack.org/624370 | |
| 16:53:47 | openstackgerrit | Chris Dent proposed openstack/os-resource-classes master: Tune up documentation to be more useful https://review.openstack.org/624384 | |
| 16:56:47 | moshele | jangutter: hi | |
| 16:57:09 | jangutter | moshele: hi, long time no chat! | |
| 16:57:58 | moshele | jangutter: yes indeed | |
| 16:58:30 | moshele | jangutter: I am not sure I understand you comments | |
| 16:58:44 | jangutter | moshele: sorry, it's a bit late in the year.... | |
| 16:58:45 | moshele | jangutter: https://review.openstack.org/#/c/619920/ | |
| 16:58:47 | dansmith | mriedem: was your lazy-load patch trying to get rid of the billion "Lock placement_client acquired" messages on startup? | |
| 17:00:02 | jangutter | moshele: the first question is possibly the easiest: on the NIC you have two ports, B and C. I would have thought that it makes sense specifying "connect B and C" to the bridge? | |
| 17:02:52 | mriedem | dansmith: yes | |
| 17:02:59 | mriedem | https://review.openstack.org/#/c/623246/ | |
| 17:03:01 | dansmith | mriedem: yeah that's pretty bad | |
| 17:03:11 | mriedem | it now removes the lock as well on jay's request | |
| 17:06:08 | dansmith | mriedem: just noticed while reviewing the logs forthis: https://review.openstack.org/#/c/623284/4 | |
| 17:06:19 | dansmith | mriedem: which does now select the right rpc version once computes get started | |
| 17:06:23 | dansmith | which was not happening before | |
| 17:07:22 | sean-k-mooney | adrianc: so yes if i just do "sudo ifconfig enp1s17f5 0" i does not fix the issue either | |
| 17:07:39 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Ensure that allocated PF matches the used PF https://review.openstack.org/623543 | |
| 17:08:01 | mriedem | dansmith: which job was that message from? | |
| 17:08:10 | mriedem | because on tempest-full nova-api starts up before there are any computes | |
| 17:08:12 | dansmith | mriedem: tempest-full | |
| 17:08:17 | sean-k-mooney | adrianc: sorry "sudo ifconfig enp1s17f5 down" | |
| 17:08:37 | dansmith | mriedem: right you see it fail once first, and then once computes are started, it finds the version | |
| 17:08:58 | dansmith | mriedem: before that "all cells" patch, you saw the failure on start and it never finds them after that | |
| 17:09:03 | mriedem | ah yeah http://logs.openstack.org/84/623284/4/check/tempest-full/23568c2/controller/logs/screen-n-api.txt.gz#_Dec_11_20_56_12_002867 | |
| 17:09:15 | mriedem | http://logs.openstack.org/84/623284/4/check/tempest-full/23568c2/controller/logs/screen-n-api.txt.gz#_Dec_11_21_00_06_274322 | |
| 17:09:25 | dansmith | yep | |
| 17:09:59 | mriedem | huh wonder if i made devstack switch upgrade_levels=auto in stein or rocky... | |
| 17:10:16 | dansmith | it's been a while I thought, so I'm surprised this didn't break us | |
| 17:10:23 | dansmith | but we also haven't had many rpc versions recently | |
| 17:10:25 | mriedem | https://review.openstack.org/#/c/550108/ | |
| 17:10:34 | mriedem | we went to auto upgrade levels in rocky | |
| 17:11:01 | dansmith | well, we'll definitely want to backport this | |
| 17:11:04 | mriedem | how far back do you think we need this to go? | |
| 17:11:07 | mriedem | pike? | |
| 17:11:28 | dansmith | pike was when you could actually have multiple cells right? so yeah, there | |
| 17:11:38 | mriedem | i guess no one is multicell in pike right now, that we know of | |
| 17:11:46 | mriedem | but yeah | |
| 17:11:59 | mriedem | cern would be the only one to have hit this | |
| 17:12:07 | dansmith | that we know of | |
| 17:12:10 | mriedem | right | |
| 17:12:27 | mriedem | could be chinese telecoms that have backported this to havana already | |
| 17:12:28 | dansmith | melwitt: ready to approve this? https://review.openstack.org/#/c/623284/4 | |
| 17:12:33 | sean-k-mooney | adrianc: actully this time the vf still had the vm mac for some reason if i changed it with "sudo ip link set enp1s0f1 vf 6 mac 1e:d5:87:7c:6f:b1" i can ping | |
| 17:13:06 | melwitt | dansmith: yeah, hang on | |
| 17:15:34 | dansmith | mriedem: why was that thing @synchronized in the first place? | |
| 17:15:49 | dansmith | changing that makes me more nervous than the mechanical lazy-loading | |
| 17:16:37 | mriedem | dansmith: (1) yes i said the same, i.e. i didn't really want to munge that into the lazy-load patch i had, and (2) looking at the change that added the lock, it was added as a precaution - i linked to that from the commit message | |
| 17:16:52 | dansmith | ah sorry | |
| 17:17:08 | dansmith | so you agree it's concerning but changed it anyway? | |
| 17:17:09 | mriedem | jay, chris and eric seemed to think it was fine to remove the lock | |
| 17:17:24 | mriedem | i didn't really want to change it within the same change that does the lazy-load, | |
| 17:17:27 | melwitt | I wondered the same. last efried had mentioned about it, I thought he had said the lock was needed. but didn't get into details | |
| 17:17:28 | mriedem | but jay -1ed me over it | |
| 17:17:45 | melwitt | but maybe I missed convo after that | |
| 17:17:50 | mriedem | i don't care if we remove the lock or not really | |
| 17:18:04 | mriedem | i didn't really want to do squash both things into the same patch, but again, the -1 | |
| 17:18:15 | mriedem | honestly i just wanted to move forward with it, | |
| 17:18:18 | mriedem | but now jay and eric are gone | |
| 17:18:49 | dansmith | I guess I'd just rather remove it but in a separate patch to avoid co-mingling the effects of these two changes, if we have to revert one, etc | |
| 17:18:57 | dansmith | but I'm probably over-thinking it | |
| 17:19:22 | mriedem | i thought the same | |
| 17:19:34 | dansmith | heh, your logs are flooded with the rpc thing, while looking for flooding with placement messages | |
| 17:19:43 | dansmith | getting all these committed would be, like, good | |
| 17:19:58 | mriedem | yes, i'm spinning several gate plates right now | |
| 17:20:03 | mriedem | thought about sending a summary to the ML | |
| 17:20:05 | melwitt | I'd want them separate for the potential revert reason too but since you've already been through the wringer on it, so we could just go ahead and do a partial revert if we have to for the lock thing *shrug* | |
| 17:20:27 | mriedem | i can also just remove the lock part of this if dansmith and melwitt agree to approve the lazy-load today | |