| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-11 | |||
| 13:57:50 | mriedem | dansmith: i think a couple of questions for you to opine on in there as well ^ | |
| 13:58:49 | efried | o/ dtroyer | |
| 13:58:49 | efried | sean-k-mooney, would you please explain to dtroyer what needs to happen in that CI repo to make it right? | |
| 13:58:49 | dtroyer | o/ | |
| 13:59:02 | sean-k-mooney | can you try one last thing can you sett <maxMemory slots="16" unit='GiB'>8</maxMemory> to inclnde the pmem but set <memory unit='KiB'>524288</memory> | |
| 13:59:04 | sean-k-mooney | <currentMemory unit='KiB'>524288</currentMemory> | |
| 13:59:11 | sean-k-mooney | to a lower value | |
| 14:00:02 | sean-k-mooney | if that does not work then in the sake of time i think we have to live with this for Train | |
| 14:00:09 | artom | mriedem, ack, looking | |
| 14:00:23 | sean-k-mooney | and in Ussuri we need to find a way to converge this somehow | |
| 14:02:06 | stephenfin | luyao: I'm assuming that's because you're setting <maxMemory> and not <memory> | |
| 14:02:19 | sean-k-mooney | yep | |
| 14:02:21 | stephenfin | You were able to omit that before because "In case NUMA is configured for the guest the memory element can be omitted" | |
| 14:02:25 | stephenfin | from https://libvirt.org/formatdomain.html#elementsMemoryAllocation | |
| 14:02:35 | stephenfin | sean-k-mooney: Cool. Thought that's what you were saying but I wasn't sure :) | |
| 14:02:38 | efried | dtroyer: the simple solution is removing this thing https://github.com/intel/Intel-OpenStack-CI-jobs/pull/1/commits/5d4a998a890fef3a2b0a46f73d26aa1deeb3cd8b | |
| 14:02:51 | artom | mriedem, about the qemu/kvm stuff - it's not "lower" that's the problem, is that the code used to check for "kvm", but then we realized that it should be checking for "qemu", and not all of the tests were changed | |
| 14:03:13 | sean-k-mooney | the max memory is the maium memory the geust can boot with or have hotplugged as addtional memeory | |
| 14:03:25 | artom | mriedem, the first one is harmless, but second one actually needs to change, because it was triggering a false positive by passing the hypervisor check before the thing we were actually testing | |
| 14:03:39 | sean-k-mooney | so we shoudl be setting that to guest ram + nvdim size | |
| 14:03:52 | sean-k-mooney | and the guest memory shoudl be set to the ram size in the flavor | |
| 14:04:08 | dtroyer | efried: would getting rid of the hard-coded ps13 work? | |
| 14:04:10 | efried | dtroyer: I think that's what we want long term. For now it would be nice if it only ran the pmem tests against patches that have at least https://review.opendev.org/#/c/679640/ involved -- otherwise it's running on everything and will therefore fail handily. | |
| 14:04:31 | efried | dtroyer: idk, does refs/changes/$gerritnum work without /$psnum ? | |
| 14:04:32 | sean-k-mooney | it would fail until its merged but it would work after that | |
| 14:04:36 | mriedem | artom: ok i guess i can buy that second one | |
| 14:04:41 | mriedem | artom: so might as well leave them | |
| 14:04:41 | dtroyer | yeah, the filter needs cleaning up, its pretty open now | |
| 14:05:10 | sean-k-mooney | dtroyer: i dont know if the ci is using the git driver or github dirver | |
| 14:05:13 | efried | dtroyer: I always use git review -d, never git fetch | |
| 14:05:28 | sean-k-mooney | dtroyer: if its using the github dirver then https://review.opendev.org/#/c/681474/2 should work | |
| 14:05:43 | sean-k-mooney | efried: same | |
| 14:06:03 | efried | sean-k-mooney: yeah, unless the CI env doesn't have git-review installed :P | |
| 14:06:16 | dtroyer | mee too… I approved that PR, we can iterate from there | |
| 14:06:18 | sean-k-mooney | efried: no it will work | |
| 14:06:41 | sean-k-mooney | this shoudl all be done via the zuul cloner | |
| 14:06:59 | sean-k-mooney | e.g. zuul should be preparing all the repos before the job starts | |
| 14:07:12 | sean-k-mooney | and then the jobs should use those prepreapred repos | |
| 14:07:27 | sean-k-mooney | so we shoudl not use git fetach or git review in the scripts | |
| 14:08:14 | sean-k-mooney | dtroyer: oh i actully didnt mean for it to be aproved since i think there was an internal one coming but i guess thats fine too | |
| 14:08:19 | luyao | sean-k-mooney: http://paste.openstack.org/show/775129/ | |
| 14:09:01 | dansmith | mriedem: just got off a call so I'll look at that deprecation patch in a minute | |
| 14:09:26 | dtroyer | sean-k-mooney: I think Rui is looking at this in the internal env, yes, I'm not sure where he is on that... | |
| 14:09:33 | dansmith | mriedem: considering how long things are taking to get through the gate, are you comfortable going ahead and letting the bottom three patches there go in or do you want to wait? | |
| 14:10:36 | alex_xu | dtroyer: the mroning, Rui and Dolpher talk with me we can remove the hardcode patch checkout and add a filter. Dolpher will do that when he wakeup I think. Dolpher in the US now | |
| 14:11:03 | stephenfin | luyao, sean-k-mooney: so that's triggering this https://github.com/libvirt/libvirt/blob/master/src/qemu/qemu_domain.c#L11604-L11615 | |
| 14:11:08 | dtroyer | alex_xu: ok, thanks, I'll check with him | |
| 14:11:15 | alex_xu | dtroyer: thanks | |
| 14:12:43 | luyao | sean-k-mooney: that's it! | |
| 14:13:17 | alex_xu | yea...that's it | |
| 14:13:19 | sean-k-mooney | this really isnt a hotplug but i guess they build pmem on that backend | |
| 14:13:43 | luyao | stephenfin: thanks | |
| 14:13:49 | stephenfin | so it looks like we need a guest NUMA topology, meaning we're back to "do we have to treat VPMEM different from hugepages/other things that imply a NUMA topology in nova" | |
| 14:14:04 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Follow up for Ib50b6b02208f5bd2972de8a6f8f685c19745514c https://review.opendev.org/681490 | |
| 14:14:12 | mriedem | dansmith: i've already started +2ing | |
| 14:14:24 | mriedem | just cross referencing logs with the latest passing CI job run | |
| 14:15:04 | stephenfin | dansmith: Morning. Can I get you to weigh in on https://review.opendev.org/#/c/681383 before I merge it back? | |
| 14:15:08 | dansmith | mriedem: but the bottom one? /me looks | |
| 14:15:27 | stephenfin | (this is the "try for PCPUs, and if that fails try for VCPUs" change) | |
| 14:15:33 | dansmith | mriedem: oh I see okay | |
| 14:15:42 | dansmith | stephenfin: okay, in a bit | |
| 14:15:45 | luyao | stephenfin: yes, we create one guest numa if no numa specified in Nova, if numa is set, just put vpmems on numa 0 | |
| 14:15:48 | stephenfin | ta | |
| 14:16:12 | stephenfin | luyao: Okay, but why don't we do the pinning to a host NUMA node like we do normally? | |
| 14:16:27 | sean-k-mooney | stephenfin: it did in a previous version | |
| 14:16:42 | sean-k-mooney | it was removed to allow vms that are bigger then a singel numa node to work | |
| 14:17:06 | stephenfin | can we not do that? | |
| 14:17:09 | alex_xu | stephenfin: just because in PTG someone said we should support the floating VM with vpmem | |
| 14:17:13 | stephenfin | and fix it later, for huge pages too | |
| 14:17:42 | alex_xu | then that use-case write into the spec, then that is why we support that | |
| 14:17:58 | sean-k-mooney | alex_xu: i can pretty much guranette that this wont be supported with floating instance downstream. | |
| 14:18:27 | sean-k-mooney | it might but we have no customer askign for that as far as im aware | |
| 14:18:37 | stephenfin | I don't think that's a valid request, and I don't think we should be catering to it this cycle | |
| 14:18:53 | stephenfin | If we do want to cater to it, we should do the same for hugepages too | |
| 14:19:00 | sean-k-mooney | and cpu pinning | |
| 14:19:05 | stephenfin | and CPU pinning, yeah | |
| 14:19:24 | sean-k-mooney | there was a propal for hyperv to allow this more or less years ago | |
| 14:19:34 | alex_xu | sean-k-mooney: yea, I have to say the initiial proposal doesn't support that also, it added due to the ptg discussion. | |
| 14:19:47 | alex_xu | emm.... | |
| 14:19:56 | alex_xu | I think how much effort we can remove it | |
| 14:19:59 | stephenfin | (apologies, btw, for brining this up so late in the day /o\ This week was the first time I made my way that far up the patch list) | |
| 14:20:06 | mriedem | artom: a couple more nits in https://review.opendev.org/#/c/634606/84 if you're respinning or add them in a follow up | |
| 14:20:09 | alex_xu | do we need to change hw:pmem parse too much | |
| 14:20:22 | artom | http://giphygifs.s3.amazonaws.com/media/rl0FOxdz7CcxO/giphy.gif | |
| 14:20:27 | bauzas | stephenfin: do you want to continue reviewing your series ? | |
| 14:20:35 | sean-k-mooney | alex_xu: i think we dont need to chagne that at all | |
| 14:20:36 | artom | mriedem, yeah, respinning at this point | |
| 14:20:41 | stephenfin | bauzas: Yup, go for it. There's only two changes to be aware of | |
| 14:20:42 | bauzas | or maybe I should look at gibi_fly's one | |
| 14:21:02 | alex_xu | sean-k-mooney: stephenfin how about just like cpu_policy, when the cpu_policy specific, then we create numa_topology for the instance in the api layer | |
| 14:21:15 | sean-k-mooney | alex_xu: the only thing that would change is we create a numa toplogy in the schduler and we ingore pmem devices in the numa toplogy filter | |
| 14:21:40 | sean-k-mooney | alex_xu: that is what we want to do | |
| 14:21:59 | alex_xu | luyao: ^ sorry, is that easy for your | |
| 14:22:02 | sean-k-mooney | we want to add anothger condtional to get_instance_numa_constiratis ? | |
| 14:22:15 | stephenfin | bauzas: Your call :) I would ask that you hit the bottom three though since you hit them already and all I've done if merge back the follow-ups | |
| 14:22:17 | stephenfin | They start here https://review.opendev.org/#/c/671793/ | |
| 14:22:26 | sean-k-mooney | stephenfin hand the sepcific funtion that need 1 more if | |
| 14:22:33 | bauzas | stephenfin: cool | |
| 14:22:47 | stephenfin | sean-k-mooney: um, what? :) | |
| 14:22:57 | alex_xu | sean-k-mooney: stephenfin luyao I guess we just fall into this branch https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1740 | |