Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-06-07
14:49:11 efried artom: "our" meaning RH?
14:49:18 mriedem the upstream argument would be, "oot? f'em"
14:49:23 mriedem efried: yes
14:50:01 efried I don't know what qualifies as a RH "customer", but the products that are based on the oot PowerVM driver are packaged as some form of RH distribution.
14:50:15 efried so whoever's using them is a RH "customer" in some capacity.
14:50:15 bauzas mriedem: okay, I was thinking we could be good citizens and notify OOT maintainers, like asked in https://review.opendev.org/#/c/589085/
14:50:40 bauzas anyway, I need to leave
14:51:30 stephenfin Who's a good person to review resize flow bugfixes? https://review.opendev.org/#/c/662522/
14:52:10 mriedem stephenfin: i want to say alex_xu already had a bug / patch for that
14:52:15 mriedem but you'd have to ask him
14:52:36 mriedem stephenfin: gibi: i changed those notification sample resize reschedule tests to move the patching out of the side effect method, the diff is in here https://review.opendev.org/#/c/660774/4/nova/tests/functional/notification_sample_tests/test_instance.py
14:52:38 artom mriedem, efried, yeah, I meant our = RH, but... I'm saying I'll do the nice things for oot drivers in my patch, as just replying to sean-k-mooney that RH customers are most likely *not* among those affected
14:52:47 mriedem it's not a ton better, but it's different so see what you think
14:54:27 mriedem https://review.opendev.org/#/c/620874/
14:55:27 mriedem stephenfin: -1 on your fix for the same reason i -1'ed alex_xu's change
14:56:00 mriedem sean-k-mooney: heh you even said, "this is a provisional +1 as i think this is correct assumeing we confirm the request_spec is not saved in its modifed form."
14:56:15 mriedem dangerous to +1 before you know that to be true
14:57:35 sean-k-mooney mriedem: i suspected it was true because we had whitebox test that were passing that implied the request spec was updated correctly on compute node but yes you are right i should have checked it end ot end first before +1
14:57:52 stephenfin mriedem: Yeah, we were trying to figure out if it was saved anywhere but I haven't been able to trace a save on the request_spec anywhere
14:58:05 stephenfin Is there any reason we need to persist the request_spec on the object, tbh?
14:58:07 mriedem it's very clearly saved in the conductor
14:58:09 stephenfin *numa_topology
14:58:46 mriedem https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L367
14:58:56 mriedem ^ is where i spend a good deal of my time fixing regressions
14:59:16 mriedem https://review.opendev.org/#/q/topic:bug/1669054+(status:open+OR+status:merged)
14:59:49 mriedem https://review.opendev.org/#/q/topic:bug/1830747+(status:open+OR+status:merged)
14:59:54 stephenfin I saw that, but that's only for cold migrations?
15:00:04 mriedem resize and cold migrate are the same flow stephen
15:00:20 stephenfin oh yeah
15:00:22 sean-k-mooney mriedem: so my original asstetion that we should use the temporay mutation context manager or somthing similar is proably correct?
15:01:06 mriedem sean-k-mooney: correct you're right on the money here https://review.opendev.org/#/c/662522/1/nova/compute/api.py@3562
15:01:59 mriedem right on the MOONEY
15:02:00 mriedem get it?!
15:02:28 sean-k-mooney hehe i should frame that and put it on my wall
15:02:44 mriedem get a bad ass trucker hate made out of it
15:02:49 sean-k-mooney "mriedem: said i was right about somthing" :)
15:02:59 mriedem you're right about lots of things
15:04:01 sean-k-mooney :) ok so it should be a fairly simple fix so are you ok to proceed with stephens version or alex's
15:04:27 mriedem they are both broken
15:04:29 mriedem equally
15:04:43 mriedem i've duplicated stephen's bug against alex's since alex's came first,
15:04:48 mriedem but alex seems to have abandoned his change effectively
15:05:05 mriedem by "simple fix" you mean shove the new numa topology into the migration contexT?
15:05:48 mriedem i'm not sure we'd need to do that if we can, on revert, build the old numa topology from the old flavor that is in the migration context before we drop the migration context
15:08:43 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove 'ComputeManager._reschedule' https://review.opendev.org/660774
15:08:49 mriedem gibi: stephenfin: ^ updated i'll +W
15:09:08 sean-k-mooney by simpel fix i ment we jsut need to reset the change state on the field so that it does not get saved when the condoctor calls save.
15:09:39 gibi mriedem: +2, thanks
15:10:24 mriedem sean-k-mooney: but we actually do need to persist the new numa topology on the request spec for any subsequent move operations
15:10:58 sean-k-mooney mriedem: yes but we update it on the compute host
15:10:59 mriedem otherwise i can (1) create my server with flavor1 and numa1, (2) resize to flavor2/numa2, confirm, (3) cold migrate and now i'm scheduling with flavor1's numa1
15:11:06 mriedem sean-k-mooney: the request spec?
15:11:15 mriedem that would be an up-call...
15:11:25 mriedem that also doesn't solve your revert problem
15:11:58 sean-k-mooney ya we i think recalualate the numa toplogy in the driver and store it in the ... instance not he request spec never mind
15:12:24 sean-k-mooney so ya we need to actuly save it and then revert it
15:12:29 stephenfin sean-k-mooney: Definitely? I know we used to mess with it in the filters https://github.com/openstack/nova/blob/master/nova/scheduler/filters/numa_topology_filter.py#L61-L68
15:15:06 sean-k-mooney well see the fact the numa_fit_instance_to_host fucntion modifes the request sepc is what made me suspect this would get updated by the compute node if we booted sucessfuly
15:15:24 sean-k-mooney but honestly we need 1 trace teh code and two test it locally to confirm
15:15:27 stephenfin We probably don't want to persist that change though, in any case
15:16:32 sean-k-mooney proably. the compute node really should not need to modify the request_spec
15:16:50 sean-k-mooney well in this instance
15:16:51 mriedem the compute service should never be saving changes to the RequestSpec, ever, since it's an up-call to the API DB
15:16:57 mriedem dansmith: agree with me here? ^
15:17:24 mriedem stephenfin: as i explained above, the RequestSpec.numa_topology is used in the scheduler so yes it needs to be persisted for subsequent move operations,
15:17:36 mriedem however, on revert we have to make sure we revert the numa_topology in the request spec to the old flavor's values
15:17:37 dansmith mriedem: of course
15:17:39 sean-k-mooney we only added the request spec to the api db around rocky correct
15:17:40 mriedem which is my -1
15:17:46 mriedem jesus gd
15:17:52 stephenfin Any changes from the compute node, I mean
15:17:54 sean-k-mooney buy you are right
15:18:00 mriedem i would like to phone a friend
15:19:14 mriedem see this code on revert https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3410
15:19:19 mriedem that's the thing you need to concern yourself with
15:19:29 mriedem and why i -1'ed alex_xu's change
15:19:30 mriedem and yours
15:21:42 mriedem sean-k-mooney: request spec has been around since mitaka or newton at least, which is why when i have to fix a regression bug b/c of the request spec, i have to backport it through 20 releases
15:21:48 dansmith sean-k-mooney: reqspec has been around a lot longer than rocky
15:21:50 mriedem which makes me a sad panda
15:22:39 sean-k-mooney oh i know the reqest spec has been
15:23:00 sean-k-mooney i just tought we only strated storing it in the api db more recently
15:23:03 openstackgerrit Balazs Gibizer proposed openstack/nova master: allow getting resource request of every bound ports of an instance https://review.opendev.org/655110
15:23:33 sean-k-mooney i tought melwitt added it to the api db like 2-3 release ago but i could be wrong about that
15:24:09 dansmith we've had it in the api db since we've had an api db AFAIK
15:24:29 mriedem 4th api db migration
15:24:35 dansmith yeah
15:24:51 sean-k-mooney ah ok
15:24:54 mriedem sweet laski
15:26:45 mriedem apparently we aren't keeping track of runways anymore https://etherpad.openstack.org/p/nova-runways-train
15:28:42 sean-k-mooney mriedem: i mentioned on irc that my one can be dropped form the runway
15:29:08 panda uh ?
15:29:15 panda oh, the sad me.
15:29:17 panda ok.
15:29:37 stephenfin panda: Damn it man, look sadder!
15:29:46 sean-k-mooney it still need a second +2 but its more or less done at this point
15:32:45 mriedem i've got it in a tab
15:33:07 sean-k-mooney mriedem: oh incase you missed it i set https://blueprints.launchpad.net/nova/+spec/libvirt-neutron-sriov-livemigration to implemented since the neutron docs change has merged too and updated the spec link to the renderd version instead of gerrit
15:33:46 mriedem ack thanks
15:34:35 sean-k-mooney im going to go grab lunch ill be around later for an hour or so o/

Earlier   Later