Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-10
15:09:38 mriedem i'm not sure what you mean by 'new confirm resize tests'
15:10:08 mriedem like the new bw provider migration tests?
15:10:17 gibi mriedem: the current patch adds the instance action event waiting for every confirm resize tests in test_servers
15:10:38 gibi mriedem: not just to the bw related confirm resize tests
15:10:55 mriedem yeah i didn't expect to see all of those other changes in tests that aren't doing exotic allocatoins
15:11:05 mriedem i mean, it's fine, and we could just put the helper in ServerMovingTests
15:11:47 gibi this race does not depend on any exotic allocation.
15:12:15 gibi true we only saw the issue in the bw related tests
15:12:51 mriedem putting it in ServerMovingTests is probably not good enough then, since the new tests don't use that
15:12:54 mriedem in PortResourceRequestBasedSchedulingTestBase
15:13:02 mriedem could go in ProviderUsageBaseTestCase though...
15:13:36 mriedem up to you - i just don't love the copy/paste so we should use a helper at some point, fup or whatever - up to you
15:13:48 gibi mriedem: OK. I will respin the patch
15:14:28 openstackgerrit Artom Lifshitz proposed openstack/nova master: NUMA live migration support https://review.opendev.org/634606
15:14:29 openstackgerrit Artom Lifshitz proposed openstack/nova master: Deprecate CONF.workarounds.enable_numa_live_migration https://review.opendev.org/640021
15:14:29 openstackgerrit Artom Lifshitz proposed openstack/nova master: Functional tests for NUMA live migration https://review.opendev.org/672595
15:14:50 bauzas gibi: could you please give me again the race fix change ?
15:14:58 bauzas gibi: can't easily find it
15:15:03 gibi bauzas: https://review.opendev.org/#/c/681238
15:15:08 bauzas gibi: thanks
15:15:34 bauzas ah, that's because the topic name changed :)
15:16:15 sean-k-mooney alex_xu: luyao have you tested the vpmem code with instace that request hugepage or pinning but dont specify a numa toplogy
15:17:23 artom dansmith, mriedem, ^^
15:18:00 bauzas gibi: so you'll respin https://review.opendev.org/#/c/681238 ?
15:18:06 alex_xu sean-k-mooney: I guess luyao have test on that. But that will be a case like normal instance with numa. Since when we specify pinning or hugepage, we will create one node numa for the instance, right?
15:19:01 sean-k-mooney alex_xu: we should but i need to make sure https://review.opendev.org/#/c/678455/25/nova/virt/libvirt/driver.py@4664 does nto break that
15:19:44 sean-k-mooney e.g. and instance with hw:cpu_policy=dedicated + vpmem needs to be pinned
15:19:59 sean-k-mooney and i need to ensure adding not need_pin wont break that
15:20:19 alex_xu sean-k-mooney: https://review.opendev.org/#/c/678455/25/nova/virt/libvirt/driver.py@5454, for that case, the need_pin should be True, since the instance has numa_topology
15:20:31 sean-k-mooney instance.numa_topology shoudl be set in the calim but i need to triple check
15:20:41 alex_xu yea
15:20:53 gibi bauzas: yeah, I will respin soon.
15:21:09 bauzas cool
15:21:22 alex_xu sean-k-mooney: actually we set instance.numa_topology in the API layer I think
15:21:30 sean-k-mooney alex_xu: i think the code is correct but im not sure im happy with this be in a diffreent location then the other places we create a numa toplogy implcitly
15:22:15 alex_xu sean-k-mooney: yea, that is agreement on the PTG, people said they want the vpmem on no numa instance, but I totally understand your point
15:23:01 alex_xu I can't remember who is asking, but it should be redhat people
15:23:46 sean-k-mooney well as a redhat person i did not but i think maybe dansmith prefered supporting this. i kown we said we woudl do that at the PTG however
15:24:16 alex_xu yea, actually I'm on your side in the beginning
15:24:17 sean-k-mooney alex_xu: do you have an xml i can inspect
15:24:42 dansmith sean-k-mooney: um, what?
15:24:44 sean-k-mooney e.g. an exmaple xml that was generated using this code
15:25:11 alex_xu sean-k-mooney: luyao has, but she is on the way home~
15:25:14 sean-k-mooney dansmith: you prefered to add pmem suport without requireing numa in the inital version corect?
15:25:29 sean-k-mooney dansmith: i could be miss rememebring if so my applogies
15:26:13 sean-k-mooney dansmith: i just rememebr you and i were the most active redhatters in that discuession in the ptg.
15:26:43 alex_xu sean-k-mooney: here is one http://52.27.155.124/95/674895/35/check/pmem-tempest-plugin-filtered/af5fdef/controller/logs/screen-n-cpu.txt.gz
15:27:06 alex_xu sean-k-mooney: search the 'pmem', you will see the xml output by nova-compute log
15:27:34 dansmith sean-k-mooney: I seriously doubt I said that
15:29:22 sean-k-mooney dansmith: in that case my applogies for invoking your name :)
15:29:34 sean-k-mooney alex_xu: so this is the generated domain
15:29:36 sean-k-mooney http://paste.openstack.org/show/774840/
15:30:06 openstackgerrit Balazs Gibizer proposed openstack/nova master: Fix the race in confirm resize func test https://review.opendev.org/681238
15:30:13 gibi mriedem, bauzas: ^^
15:30:58 sean-k-mooney alex_xu: it is both pinning ram of the and the core of the guest to float over host numa node 0
15:31:23 alex_xu sean-k-mooney: yes, but I think the is based on old patchset, since that ci job always pull a old version of luyao's patch
15:31:34 alex_xu in new version, it shouldn't have the pinning
15:31:35 sean-k-mooney alex_xu: but it is not specificy a constrait on pmem device as far as i can see
15:33:05 alex_xu sean-k-mooney: yes, that is the output for patchset13, sorry
15:33:24 sean-k-mooney alex_xu: im not sure that is a good thing. but ok. do you have an updated run
15:33:29 sean-k-mooney ill check the ci logs i guess
15:33:48 alex_xu sean-k-mooney: luyao is trying to get one for you
15:34:04 sean-k-mooney http://52.27.155.124/93/671793/23/check/pmem-tempest-plugin-filtered/23b32fa/ shoudl have them right
15:34:28 sean-k-mooney that from ps 23
15:34:49 sean-k-mooney i found one
15:35:03 alex_xu sean-k-mooney: no, it won't, the CI always try to apply the PS13, Rui is trying to remove that. That is why you can see other patch also can pass the pmem ci test
15:37:04 sean-k-mooney in the new code will both the numatune and cputune element be removed if only a pmem device is requested in the flavor
15:39:19 kashyap sean-k-mooney: To my eyes this whole NUMA and PMEM interaction looks sutble enough that some more functional testing is required...
15:41:48 sean-k-mooney kashyap: well there are functional tests in https://review.opendev.org/#/c/678470/27
15:41:53 sean-k-mooney but i have not looked at them yet
15:42:50 kashyap sean-k-mooney: Ah, missed it; thx
15:43:59 sean-k-mooney ... its locking up my firefox windwo trying to open it for some reason
15:44:16 bauzas gibi: https://review.opendev.org/#/c/681238/3
15:44:22 bauzas gibi: can I update the commit msg ?
15:44:45 sean-k-mooney kashyap: the functional test will not assert anythin about the xml generation
15:44:46 gibi bauzas: sure go ahead
15:45:55 kashyap sean-k-mooney: Sure, I see those are already in the main change.
15:46:41 sean-k-mooney there are no assertion made about numa element in the main change as far as i can tell
15:46:54 sean-k-mooney rather numatune and cputune
15:48:07 bauzas gibi: cool, will do and +2
15:48:12 gibi bauzas: thanks a lot
15:48:41 mriedem gibi: nits in https://review.opendev.org/#/c/676140/19 for a follow up
15:48:52 openstackgerrit Sylvain Bauza proposed openstack/nova master: Fix the race in confirm resize func test https://review.opendev.org/681238
15:49:25 sean-k-mooney efried_afk: alex_xu ill try to come back to the pmem stuff in an hour or so. i need to review stephens patch then sync with artom
15:49:52 gibi mriedem: ack. thanks
15:50:07 sean-k-mooney stephenfin: you wanted me to review https://review.opendev.org/#/c/671793/ specifcally right. should i also review the rest
15:50:27 stephenfin sean-k-mooney: would be helpful, aye
15:50:35 alex_xu sean-k-mooney: i got one http://paste.openstack.org/show/774842/
15:50:37 stephenfin I'm marching through the pmem stuff
15:51:05 bauzas but I'll look at your series tomorrow morning
15:51:13 sean-k-mooney alex_xu: looking
15:51:16 artom sean-k-mooney, for once, I think I'm good, and do not require your excellent services :)
15:52:30 sean-k-mooney alex_xu: i think that that is correct. it is creatin a virtual numa toplogy but it is not tiying it to the host in any way
15:53:06 sean-k-mooney artom: the pmem device is also assocaitated with the virtual guest numa node 0
15:53:27 sean-k-mooney artom: are you working on fixing the persitence issue
15:54:05 sean-k-mooney artom: i jsut wanted to circle back and see if you had anything form me to test or if i should start looking into where to fix the issue
15:54:22 artom sean-k-mooney, I added instance.refresh() back it, so that's settled
15:54:31 alex_xu sean-k-mooney: but I think this is wrong https://review.opendev.org/#/c/678455/25/nova/virt/libvirt/driver.py@5458
15:54:42 sean-k-mooney artom: ok did you push that?
15:54:51 artom The func tests weren't hitting it because driver.cleanup() is called conditionally, and the func test env isn't meeting those conditions

Earlier   Later