Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-12-12
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
17:20:37 mriedem and throw the lock remove at the end
17:20:52 mriedem i will knife someone if i have to redo this again and then wait
17:20:53 dansmith I was about to +2, so I'd definitely +2 two patches instead yeah

Earlier   Later