| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-04-25 | |||
| 16:16:07 | elodilles | from my side | |
| 16:16:15 | elodilles | sorry :) | |
| 16:17:10 | bauzas | cool, moving on | |
| 16:17:19 | bauzas | #topic Open discussion | |
| 16:17:29 | bauzas | (ykarel) Allow to add tb-cache size libvirt option for qemu, context https://bugs.launchpad.net/nova/+bug/1949606 | |
| 16:17:33 | bauzas | ykarel: go for it | |
| 16:19:20 | ykarel | hi | |
| 16:19:41 | ykarel | so qemu-5.0.0(included in ubuntu jammy) update default tb cache size to 1 GiB(from 32 MiB) for system-emulated guest vms and with that each guest VM is using much more system memory(1 GB+ memory for guest), resulting into oom-kill issues when creating multiple guest vms concurrently in neutron scenario jobs using ubuntu guest vms | |
| 16:20:17 | ykarel | libvirt-8.0.0 added an option to configure it per guest vm | |
| 16:20:25 | ykarel | Currently testing WIP nova patch https://review.opendev.org/c/openstack/nova/+/868419 in https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/881391 | |
| 16:20:46 | ykarel | should this be a specless RFE or can continue as a bug? | |
| 16:20:54 | sean-k-mooney | ykarel: did you file the specless pluprint i asked for | |
| 16:20:56 | sean-k-mooney | ah | |
| 16:20:58 | sean-k-mooney | its not a bug | |
| 16:21:19 | ykarel | sean-k-mooney, i just added to meeting agenda to get clearity | |
| 16:21:29 | ykarel | but if it's a way to go then i will file it | |
| 16:21:30 | sean-k-mooney | we might conider it to be a small enough workaround that we might want to backport it upstream | |
| 16:21:33 | sean-k-mooney | to enable ci testing | |
| 16:22:27 | ykarel | hmm mainly would be needed for releases running on jammy | |
| 16:22:30 | sean-k-mooney | the issue is that i dont think 22.04 has libvirt 8.0.0 | |
| 16:22:37 | ykarel | it has | |
| 16:22:44 | ykarel | that's what we testing | |
| 16:22:54 | sean-k-mooney | ah ok | |
| 16:23:07 | bauzas | we damn need update of https://docs.openstack.org/nova/latest/reference/libvirt-distro-support-matrix.html | |
| 16:23:19 | sean-k-mooney | yes | |
| 16:23:30 | sean-k-mooney | so your patch is mising a min libvirt version check | |
| 16:23:33 | bauzas | we're still having libvirt 6.0.0 as a min, right? | |
| 16:23:35 | sean-k-mooney | so that will need to be added | |
| 16:23:40 | ykarel | from job logs libvirt-daemon 8.0.0-1ubuntu7.4 | |
| 16:23:55 | sean-k-mooney | bauzas: yes and im hoping to bump that to 7.0.0 this release | |
| 16:23:58 | ykarel | sean-k-mooney, yes will include that in next update | |
| 16:24:10 | sean-k-mooney | but we will sitll need to check 8.0.0 supprot | |
| 16:24:15 | sean-k-mooney | ack | |
| 16:24:16 | bauzas | sean-k-mooney: but we would still need to only set it if libvirt >=8 | |
| 16:24:32 | bauzas | do we want to schedule on it, or is it just a performance fix ? | |
| 16:24:34 | sean-k-mooney | ya either only set it or preferable check this in init-host | |
| 16:24:43 | sean-k-mooney | and raise an error if you set the config option on an old libvirt | |
| 16:24:58 | bauzas | will there be any config knob ? | |
| 16:25:07 | sean-k-mooney | yes there shoudl be | |
| 16:25:12 | ykarel | yes that's second question | |
| 16:25:14 | sean-k-mooney | to set the cache size | |
| 16:25:15 | ykarel | Default setting for this new option, unconfigured or set to defaults like 32MB or 128 MB etc? | |
| 16:26:07 | bauzas | ykarel: if that's a config option, that indeed needs to be an hard fail if the operators sets this config value | |
| 16:26:18 | bauzas | and libvirt isn't recent enough | |
| 16:26:23 | sean-k-mooney | its not somethign we should hardcode | |
| 16:26:28 | ykarel | bauzas, sure will take care that in the patch | |
| 16:26:29 | bauzas | for that reason, I'm not happy with a default value except none | |
| 16:26:47 | sean-k-mooney | so it shoudl eb a config option im ok with a low default or leaving it unset | |
| 16:27:02 | bauzas | I'd prefer it unset for upgrade reasons | |
| 16:27:04 | ykarel | okk Thanks will keep it like that no default and let user configure it | |
| 16:27:05 | sean-k-mooney | bauzas: to your poitn it woudl be nice to have a triat for this but not required | |
| 16:27:28 | bauzas | ykarel: s/user/operator but I think I get your point | |
| 16:27:50 | ykarel | so will propose the blueprint and update the patch with as per all the suggestions, Thanks | |
| 16:27:50 | sean-k-mooney | in this case really it will be the zuul job | |
| 16:28:01 | ykarel | yes | |
| 16:28:03 | sean-k-mooney | this is of interest to peopel usign qemu for emulation | |
| 16:28:06 | bauzas | sean-k-mooney: man, we could recommend the operators to provide custom traits for this, exactly like vgpu types | |
| 16:28:36 | bauzas | I mean, eventually all the computes will support that, right? | |
| 16:28:44 | sean-k-mooney | yes | |
| 16:28:53 | bauzas | after a couple of releases, once we cap libvirt to >=8 | |
| 16:29:10 | sean-k-mooney | so its not goign to break live migration | |
| 16:29:13 | bauzas | so I'm not a big fan of adding some scheduling thing for something that will eventually be supported mid-term | |
| 16:29:24 | sean-k-mooney | because we do not allwo live migration from a newer to older microversion | |
| 16:29:36 | sean-k-mooney | and for cold migration we will regenerate the xml on the approiate host | |
| 16:29:41 | sean-k-mooney | based on what it has avaiable | |
| 16:29:43 | bauzas | s/microversion/libvirt version but yeah | |
| 16:29:55 | sean-k-mooney | so i dont think we need anythign sepcial here | |
| 16:30:01 | sean-k-mooney | so ya custom trait woudl eb fine with me | |
| 16:30:13 | sean-k-mooney | they can use provider.yaml to set that if they want | |
| 16:30:17 | bauzas | cool, so that only seems a config knob to add, a check on init_host to fail if set and some magic in the driver to enable it | |
| 16:30:30 | bauzas | amirite ? | |
| 16:30:34 | sean-k-mooney | more or less | |
| 16:30:48 | bauzas | then, I'm OK for specless | |
| 16:30:56 | sean-k-mooney | +docs test ectra but its effectivly self contaied in the libvirt driver + the config tweak | |
| 16:30:58 | bauzas | we had precedents | |
| 16:31:11 | bauzas | + a relnote obviously | |
| 16:31:24 | bauzas | ykarel: do you agree with the direction ? | |
| 16:31:30 | ykarel | bauzas, yes | |
| 16:31:31 | sean-k-mooney | im ok with specless too assuming all of the above are done | |
| 16:31:41 | bauzas | anyone disagreeing ? | |
| 16:32:07 | bauzas | looks not | |
| 16:32:20 | ykarel | Thanks folks \o/ | |
| 16:32:41 | sean-k-mooney | the only other thin i would suggest is once this is done a devstack patch shoudl be added to set this by default to say 32mb if qemu is used instead of kvm | |
| 16:33:37 | sean-k-mooney | thats out of scope fo this meeting and coud be doen on a per job basis too | |
| 16:33:39 | bauzas | #agreed enabling tb cache seems a specless blueprint, provided it only adds a config knob defaulting to unset, init_host failing on an older libvirt and just libvirt config tweak | |
| 16:34:01 | bauzas | #action ykarel to ping bauzas once the blueprint is created so that he can approve it | |
| 16:34:34 | bauzas | ykarel: and yeah, the scope of this feature can include devstack change and testing, for sure | |
| 16:34:53 | bauzas | like we could enable it in nova-next | |
| 16:35:41 | sean-k-mooney | bauzas: it will reduce the memory pressue in all our jobs so ocne we know it does not have a negitive impact we will proably want to have it enable din all of them | |
| 16:35:42 | bauzas | anything else to add on this item ? | |
| 16:35:55 | sean-k-mooney | but we can take it slow like the mariadb reduced memroy | |
| 16:36:00 | bauzas | sean-k-mooney: oh yeah, but I'd be in favor of testing it first | |
| 16:36:08 | bauzas | yah | |
| 16:36:33 | bauzas | ok, fwiw, I have another item | |
| 16:36:53 | bauzas | (bauzas) Can https://blueprints.launchpad.net/nova/+spec/cold-migrate-to-host-policy be specless ? | |
| 16:37:05 | bauzas | tl;dr: we discussed this at the PTG | |
| 16:37:29 | sean-k-mooney | assuming there is no change in the default policy then yes i think so | |
| 16:37:32 | bauzas | operators want a better granularity and maybe change the cold-migrate action to be admin_or_owner | |
| 16:37:46 | bauzas | but, here, we're just adding a new policy which is admin-only | |
| 16:37:55 | bauzas | so no API change, and no policy changes | |