Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-09
20:23:18 dansmith efried: around?
20:23:23 efried dansmith: yah
20:23:47 dansmith efried: gibi was +2 on this before I found one more thing that needed fixing in the object: https://review.opendev.org/#/c/671072/17..18/nova/objects/request_spec.py
20:23:56 efried yah
20:23:59 dansmith efried: permission to fast-approve since she made the change
20:24:02 efried please do
20:24:38 efried she had to fix some stuff up the pile for the new non-defaulting-to-set() as well.
20:25:31 dansmith I saw
20:25:55 dansmith efried: this one had a couple test tweaks from me which she also did, so I shan't fast-approve it, but maybe you can skim and get it underway: https://review.opendev.org/#/c/671075/20
20:26:06 efried on it now
20:26:52 dansmith I don't think I've looked at the doc patch since it was megasplit so I'll leave that to you and gibi which have looked recently
20:29:47 efried ack. Thanks for your work on this dansmith
20:30:14 shilpasd dansmith: Gibi: Eric:Stephan: thank you for review
20:30:48 dansmith aye
20:52:05 openstackgerrit Shilpa Devharakar proposed openstack/nova master: Docs for isolated aggregates request filter https://review.opendev.org/667952
20:52:06 openstackgerrit Eric Fried proposed openstack/nova master: DNM: uncover func tests with mismatched allocations https://review.opendev.org/681122
21:12:13 melwitt mriedem: downstream this bug (fill_virtual_interface_list fails on second attempt) was hit and they proposed a patch with a different approach from the workaround you had worked on the past. I'm curious what you think about it https://review.opendev.org/678776
21:20:03 mriedem ack, just getting caught up with scrollback
21:22:51 mriedem dansmith: so tl;dr on artom's series is we're not sure about all of the non-numa vpu pinning stuff that was recently added?
21:23:13 mriedem yeah this "now we have the infra in place to be able to fix this long-standing bug"
21:23:28 artom mriedem, sean-k-mooney tested it, there's an edge case that we're missing, but otherwise we're pretty confident
21:23:29 mriedem that's what i was thinking about that cpu pinning stuff being munged in
21:23:44 artom As I said to dansmith, at this point, it'd be more work to yank it out than to fix it
21:24:04 artom But I can go either way, if we want to be safe
21:24:30 artom It is indeed technically out of scope, for a long-standing latent thing
21:24:45 mriedem "also more churn doesn't make re-review easier" yup
21:24:53 mriedem i feel like this is mostly a re-review each time for me
21:25:20 mriedem it's not terrible though, but it definitely made me not able to feel comfortable with +2ing https://review.opendev.org/#/c/635229/
21:26:04 artom mriedem, I get you. I keep finding more stuff to fix. Maybe the non-NUMA instance thing I jumped the gun on
21:26:17 artom Should have left it as is
21:40:00 sean-k-mooney mriedem: in the one case that is not covered by atroms code for the non numa instances it reverts to the master behavior
21:40:13 sean-k-mooney mriedem: in the other cases the behaivor appears to be correct
21:40:54 sean-k-mooney that said we could drop that change or add support for the last edgecase
21:41:20 sean-k-mooney both of which are tangental to the main content of the series
21:43:42 mriedem melwitt: i left comments, that's a bit hard to follow
21:43:48 mriedem obviously needs tests and such
21:44:05 mriedem it's been a long time since i remembered what triggered that bug
21:44:35 mriedem sean-k-mooney: mo cases mo problems
21:45:24 mriedem also, if that non-numa stuff is tracked in the LibvirtLiveMigrateNUMAInfo object, isn't that confusing given the name of the object?
21:46:06 sean-k-mooney mriedem: well as i said the behvior is either identical to the behavior on master or fixed so its stitcly better. yes that did occure to me but i didnt think of a better name yet
21:46:37 melwitt mriedem: ack, thanks
21:47:02 mriedem sean-k-mooney: how about just not putting non-numa things a numa object for migrate_data?
21:47:04 sean-k-mooney we could uses LibvirtLiveMigrateTopologyInfo
21:47:10 sean-k-mooney to mirror the toplogy api?
21:47:23 sean-k-mooney ya we could do that too
21:47:26 mriedem that or just put a different field on LibvirtLiveMigrateData
21:47:53 mriedem or otherwise yeah you need to make LibvirtLiveMigrateNUMAInfo more generic
21:47:56 artom mriedem, in any case, it seems we have enough misgivings about this first whack at it to not go ahead
21:48:04 artom And yeah, it's tangential to the NUMA case
21:48:19 artom I lumped it in because it's triggered by the same code
21:48:27 artom (driver._get_guest_numa_config)
21:49:11 mriedem sure, scope creep
21:49:24 mriedem killer of dreams and introducerer of regression bugs
21:49:33 artom Ack
21:49:36 artom Supper time, back after
21:50:17 sean-k-mooney this fell like a thing we would fix if we had an extra week but since we dont if ye prefer to remove it or move it to a patch at the end i guess that makes sense
21:51:04 sean-k-mooney i would perfer to keep it in in its half fixed state personllay but i also know how to work around it so ill leave that to ye to decided
21:51:38 sean-k-mooney this is a behavior i have been ware of for years so i kind of just assume this was going to be fixed as part of either artoms work or stephens
21:54:47 mriedem i felt pretty happy with the progress we had by thursday last week, we had a working CI run, you had done manual testing of upgrade paths, etc. dan and i did solid reviews, so i was thinking just fixing up those comments and going over again would do it, but the non-numa scope creep stuff threw a wrench into the mix adding what appears to be unnecessary risk
21:55:33 mriedem and this isn't the only thing i'm trying to review this week to get in so i'm not 100% focused on this, so frequently changing is costly
21:55:35 sean-k-mooney mriedem: well we can always jsut revert to that version that had been tested
21:55:45 sean-k-mooney ack
21:55:53 mriedem he'll still have to make changes, but yeah i know
21:56:12 mriedem it's going to need re-test either way i think since there were non-trivial changes outside of the non-numa pinning stuff since last week
21:56:44 mriedem melwitt: maybe we can get imacdonn to re-test, but he doesn't appear to be around,
21:56:55 mriedem but fwiw i was able to reproduce in devstack - i think i left notes in the bug report on how to do that
21:57:17 mriedem so i'm also not against just recreating in devstack w/o the fix, then applying the fix and assert the issue is gone
21:57:32 mriedem this security groups crap all gets burned out with nova-net at some point anyway
21:57:45 mriedem the fact we create a default security group when using neutron is pointless anyway since it's not used
22:01:15 sean-k-mooney artom: dansmith mriedem. if ye come to a decision on a path forward i will retest everything tommorow with whatever ye choose.
22:10:55 mriedem idk, i hate to make artom undo a bunch of work, but also don't like munging a bunch of other stuff in here just b/c it's convenient though out of scope
22:11:24 mriedem i'd prefer to separate the non-numa pinning stuff out
22:11:27 mriedem and deal with that later
22:11:46 sean-k-mooney i.e. make it a patch at the end
22:11:54 sean-k-mooney that we can merge when we get to it
22:12:02 mriedem yes
22:12:38 mriedem think of it like someone pushing a bug fix that also refactors a bunch of code unnecessarily, we're generally not cool with that
22:13:25 sean-k-mooney ya its an unrelated or tangentally related change in a non trivial feature
22:20:45 openstackgerrit Brin Zhang proposed openstack/python-novaclient master: Microversion 2.80: Add user_id/project_id to migration-list API https://review.opendev.org/675023
22:25:47 openstackgerrit Merged openstack/nova master: Don't duplicate PlacementFixture in libvirt func tests https://review.opendev.org/681059
22:40:27 melwitt mriedem: ack, gonna add more comments to the review as far as why this patch gets around the problem
22:40:36 melwitt (based on my read of the bug and code)
22:42:06 mriedem gibi: as for your series (when you're back) you've got a real failure in the cold migration revert with a mixed compute grenade scenario https://review.opendev.org/#/c/676140/18
22:42:15 mriedem good thing you added that test coverage though :)
22:42:47 efried dansmith: would you please have another (hopefully last) look at the bottom two patches in the vpmem series https://review.opendev.org/#/c/678447/
22:45:03 efried TIA.
22:45:03 efried (I would feel better if it was you doing that than me)
22:45:03 efried stephenfin: If you can hit the currently-bottom SEV patch, the rest is +A https://review.opendev.org/#/c/644565/
23:47:48 openstackgerrit Takashi NATSUME proposed openstack/nova stable/stein: doc: Fix a broken reference link https://review.opendev.org/681141
23:48:38 openstackgerrit Takashi NATSUME proposed openstack/nova stable/rocky: doc: Fix a broken reference link https://review.opendev.org/681142
23:49:36 openstackgerrit Takashi NATSUME proposed openstack/nova stable/queens: doc: Fix a broken reference link https://review.opendev.org/681143
23:49:40 alex_xu efried: yes, I don't found design problem and upgrade plan makes sense for me also. What I'm just doing is the manual test of the upgrade process, ensure thing works as expected now
#openstack-nova - 2019-09-10
01:59:55 openstackgerrit Merged openstack/python-novaclient master: Microversion 2.79: Add delete_on_termination to volume-attach API https://review.opendev.org/673485
02:27:30 openstackgerrit Akira KAMIO proposed openstack/nova master: VMware: disk_io_limits settings are not reflected when resize https://review.opendev.org/680296
03:01:08 openstackgerrit Artom Lifshitz proposed openstack/nova master: LM: Use Claims to update numa-related XML on the source https://review.opendev.org/635229
03:01:08 openstackgerrit Artom Lifshitz proposed openstack/nova master: New objects for NUMA live migration https://review.opendev.org/634827
03:01:09 openstackgerrit Artom Lifshitz proposed openstack/nova master: Deprecate CONF.workarounds.enable_numa_live_migration https://review.opendev.org/640021
03:01:09 openstackgerrit Artom Lifshitz proposed openstack/nova master: NUMA live migration support https://review.opendev.org/634606
03:01:10 openstackgerrit Artom Lifshitz proposed openstack/nova master: Functional tests for NUMA live migration https://review.opendev.org/672595
03:38:34 Izza_ hello...good day, i'm doing tempest testing on openstack helm environment but i encountered error "Got Server Fault"

Earlier   Later