Tuesday, 2023-08-22

opendevreviewmelanie witt proposed openstack/nova master: DNM test ephemeral encryption + resize: qcow2, raw, rbd  https://review.opendev.org/c/openstack/nova/+/86241602:01
opendevreviewOpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata  https://review.opendev.org/c/openstack/nova/+/89164803:28
*** efried1 is now known as efried05:22
dvo-plvHello Nova folks. maybe you will have a chance to review https://review.opendev.org/c/openstack/nova/+/87607508:27
opendevreviewDanylo Vodopianov proposed openstack/nova master: Napatech SmartNIC support  https://review.opendev.org/c/openstack/nova/+/85957709:29
opendevreviewDanylo Vodopianov proposed openstack/nova master: Napatech SmartNIC support  https://review.opendev.org/c/openstack/nova/+/85957709:34
auniyalhi dvo-plv, I am a newbie in Nova  and have tried to review your patch some times, but it's seems complex. Will it be possible (if you consider) to break or segregate it into different relation patches? It might be helpful for others too.09:39
opendevreviewDanylo Vodopianov proposed openstack/os-vif master: Openvswitch driver was extended  https://review.opendev.org/c/openstack/os-vif/+/85957409:40
sean-k-mooneyauniyal: i dont really sea how you would split up dvo-plv's patches there are pretty small as is11:24
dvo-plvI can move tests coverage into another patch and maybe some other features of it will be useful11:27
dvo-plvif it will be useful*11:28
sean-k-mooneytest coverage should not be in diffent patches11:39
auniyalsean-k-mooney ack11:45
opendevreviewPierre Riteau proposed openstack/nova master: Fix a couple of typos  https://review.opendev.org/c/openstack/nova/+/89230011:54
opendevreviewsean mooney proposed openstack/nova master: introduce global greenpool  https://review.opendev.org/c/openstack/nova/+/87306112:28
opendevreviewsean mooney proposed openstack/nova master: introduce global greenpool  https://review.opendev.org/c/openstack/nova/+/87306112:51
*** d34dh0r5- is now known as d34dh0r5313:09
dansmithgibi: can you re-+2 this? I forgot to update the log level in the test since you last saw: https://review.opendev.org/c/openstack/nova/+/89134013:34
gibidansmith: done13:43
dansmiththanks13:43
sean-k-mooneyhum interesting https://review.opendev.org/c/openstack/nova/+/873061?tab=change-view-tab-header-zuul-results-summary15:12
sean-k-mooneyso the devstack jobs all passed15:12
sean-k-mooneyall the leaked greentrheads are in 3 moudles nova.tests.unit.virt.libvirt nova.tests.unit.test_service.TestWSGIService and nova.tests.unit.test_wsgi.TestWSGIServer15:14
sean-k-mooneythe functionaltests are similar so its proably that the relevent code that is leakign the greenthreads is shared.15:15
gibinova weekly meeting will start here in 10 minutes. I will be your host today and I try to do a quick meeting.15:50
gibi#startmeeting nova16:00
opendevmeetMeeting started Tue Aug 22 16:00:59 2023 UTC and is due to finish in 60 minutes.  The chair is gibi. Information about MeetBot at http://wiki.debian.org/MeetBot.16:00
opendevmeetUseful Commands: #action #agreed #help #info #idea #link #topic #startvote.16:00
opendevmeetThe meeting name has been set to 'nova'16:00
auniyal\o16:01
gibio/16:01
sean-k-mooneyo/16:01
dansmitho/16:01
gmanno/16:01
dansmithwhen does bauzas get back? 2024?16:01
sean-k-mooneyi think monday16:01
sean-k-mooneybut i didnt check whcih year16:01
dansmith"Monday, 2024"16:01
gibiyepp 28th of Aug he is back16:02
gibiso you will get rid of me being the chair after this meeting :)16:02
gibilets start16:03
gibi#topic Bugs (stuck/critical)16:03
gibiI don't see any critical bug16:03
gibi#link https://bugs.launchpad.net/nova/+bugs?search=Search&field.status=New 43 new untriaged bugs (+3 since the last meeting)16:03
gibi#info bug baton is on PTO (bauzas please pass it forward after the summer period)16:03
gibiis there any bugs we need to discuss?16:04
andrewbonneyIf anyone could assess https://bugs.launchpad.net/nova/+bug/2023035 it would be appreciated as it's blocking us upgrading to 2023.1 without using old config workarounds16:04
opendevreviewMerged openstack/nova master: Log excessive lazy-loading behavior  https://review.opendev.org/c/openstack/nova/+/89134016:05
dansmith\o/16:05
gibiandrewbonney: I see you know about the workaround16:06
andrewbonneyYeah, just doesn't feel ideal if it's easily patchable16:07
gibiandrewbonney: I did not look deeper but it seems the one of the recent comment shows what is the difference between the two host cpus16:07
dansmithisn't this a libvirt thing anyway?16:07
gibicould be 16:08
dansmithI guess it says libvirt version is the same on both ends, so hmm16:08
gibi`cpu_mode = host-model` so I guess it is difference between the model16:08
dansmithmaybe we could get kashyap to look at this16:09
andrewbonneyWe'd be happy to provide any other debug if it's useful16:09
sean-k-mooneythere are many factor that can cause it to fail16:09
sean-k-mooneydo you have diffent kernel version on the diffent hosts16:10
sean-k-mooneythis kid of sound like perhaps some of the mitigations that are enabeld are not the same on both16:10
andrewbonneyThe testing we did found that the running XML for a guest failed in the virsh hypervisor-cpu-compare on the same host it was running on16:10
andrewbonneyWhere cpu-compare worked16:11
sean-k-mooneyand do  you have the patchs that we mreged recently to update how we did the cpu compatiblity checks16:11
andrewbonneyThis was using b9089ac from 2023.116:12
sean-k-mooneynamely https://review.opendev.org/c/openstack/nova/+/86995016:12
andrewbonneyYes, we think that caused it16:12
sean-k-mooneyok that should be in that then16:12
gibiOK. I will ask kashyap to look at it when he is back16:13
sean-k-mooneywell that is usign the new api16:13
sean-k-mooneythat the libvirt folks told use to use16:13
dansmithandrewbonney: is it simple to just revert that patch from what you have to see?16:13
andrewbonneyYes I could do that tomorrow and update the bug, but given what we tested with virsh command line I think it proves it already16:14
dansmithbut yeah, we probably need to dig deeper if this is the new api we're supposed to be using and it's not doing what we want.. not sure if that means we need to do something different or not16:14
andrewbonneyIt felt like the guest XML that was being put into the new API was wrong, but I don't know libvirt enough to be sure16:14
dansmithbut kashyap would probably be a good person to chase that down16:14
andrewbonneyThanks. Don't want to take up all your meeting time :)16:14
gibiOK, moving on16:15
gibiany other bug to discuss?16:15
gibi#topic Gate status16:16
dansmithdefinitely improving16:16
gibiglad to hear that16:16
dansmithone of my patches merged today with a single +W and no rechecks16:16
dansmithwhich was pretty shocking :)16:16
gibiwe also merged one functional improvement https://review.opendev.org/c/openstack/nova/+/89212616:17
gibihopefully that will help too16:17
dansmithyeah, which sounds like may have more impact than it appears, which is cool16:17
dansmithI'll be looking for functional failures in the gate queue once that has soaked for a while16:18
gibiI will check the functional results later this week to see if the common issues are disappeared or not16:18
gibiyeah16:18
gmannyeah, hopefully it does not go worst during release time, current gate stability is much better16:18
dansmithyep, and if we can get some +Ws on these, they'll further reduce some of our runtime loading: https://review.opendev.org/q/topic:reduce-lazy-loads16:18
dansmithduring periods of high load, it can take a bit to execute a lazy load, and it's all unnecessary16:19
dansmithbut yeah, summary is we're in a much better place than we were.. not enough to consider it done, but enough to not be very fearful of release time I think :)16:20
gibiyepp, those are easy wins16:20
gibi#info Please look at the gate failures and file a bug report with the gate-failure tag.16:20
sean-k-mooneyi can try and take a look at them16:20
gibimoving on16:21
gibi#topic Release Planning16:21
gibiwe have 2 weeks before Feature Freeze16:21
gibiI expect bauzas will start a feature tracking pad as soon as he is back16:22
gibiany features we need to discuss here?16:22
gibithen moving on16:24
gibi#topic Stable Branches16:24
gibielodilles_pto is off16:24
gibiis there any stable topic to discuss?16:25
gibi#topic Open discussion16:26
gibinothing on the agenda16:26
opendevreviewSpencer Harmon proposed openstack/nova master: libvirt tsc_frequency  https://review.opendev.org/c/openstack/nova/+/89192416:26
gibiany other topics to bring up?16:26
kgubeYes16:27
gibigo ahead16:27
kgubeI sould have mentioned this with the features maybe16:27
gibino worries16:27
kgubeI'm still working on the NFS online resize feature16:27
sean-k-mooneydidnt we say no to exposing the tsc_frequency in the past...16:28
spencerh`Sorry for noise; didn't mean to interrupt. Trying to get tests working. Maybe a topic for next week? 16:28
kgubeAnd there is a change for nova that has open dependencies in Cinder: https://review.opendev.org/c/openstack/nova/+/87356016:29
kgubeSo bauzas listed it as "Action from owner required" in the feature tracking pad16:30
sean-k-mooneykgube: so that likely will have to be next cycle16:30
dansmithkgube: is tere a nova spec for that side?16:30
kgubeyes16:30
sean-k-mooneyit was not approved this cycle i think16:30
kgubethe spec has been merged16:30
sean-k-mooneymaybe last cyle16:30
dansmithokay I can't find it16:30
sean-k-mooneyoh it was16:31
kgubehttps://review.opendev.org/c/openstack/nova-specs/+/87723316:31
sean-k-mooneyhttps://specs.openstack.org/openstack/nova-specs/specs/2023.2/approved/use-extend-volume-completion-action.html16:31
gibihttps://review.opendev.org/c/openstack/nova-specs/+/87723316:31
dansmithgot it, wrong topic16:31
kgubeAnyway, the Cinder team has been slow with reviews 16:32
gibidoes the nova side of this breaks anything without the cinder dependency? 16:32
sean-k-mooneyok well we can review what is the status of the cinder side16:32
sean-k-mooneyi dont belive so16:32
kgubeThe is a +1, but not from the core16:32
dansmithif the cinder side isn't going to be merged ASAP, I think we probably need to punt16:32
dansmithand not waste time reviewing the nova side in the last two weeks16:32
kgubehm, okay16:32
gibiI agree16:33
sean-k-mooneyso if we merged the nova code the cinder volume would never be in extending so it would not break anything but i also dont like ahvign dead code espeicly when it has an api microverion16:34
sean-k-mooneyso ya i think we would likely want to merge both in the same release16:35
sean-k-mooneystrictly speaking old nova and new cinder or old cinder and new nova shoudl work with each other16:35
dansmithyeah, but since it requires an api change,16:36
dansmithI think we should not presumptively merge this because cinder might change their definition of what the event means, when it comes, etc and then we end up with a thing we have to technically support for no reason16:36
sean-k-mooney+ compute service change and min version check16:36
kgubeIt's a bit frustrating because the Cinder change has been ready for review for weeks, and even on top of their feature tracking etherpad: https://etherpad.opendev.org/p/cinder-2023.2-bobcat-features16:36
dansmithI think we've always expected these to merge in the dependent service first16:36
kgubebut I guess they have been very busy16:37
dansmithsean-k-mooney: right, there are REST API changes, implicit RPC ones, service versions, etc16:37
sean-k-mooneycertenly with neutron we have16:37
dansmithkgube: yeah, I definitely understand your frustration16:37
gibikgube: yes, this is frustrating16:37
sean-k-mooneykgube: do you have depends on or tempest/devestack covergae to show this working by any chance16:37
sean-k-mooneythis is feeling more like an early pre milestone 1 type of change then the next two weeks16:38
sean-k-mooneybut if  you have already staged the patches with depends on and can show this working in ci then that more compleing16:39
kgubesean-k-mooney: It is covered by the existing online volume extend tests if you run them on an affected driver16:39
dansmithno devstack change that I see16:39
sean-k-mooneywe do not have a cinder nfs job in novas gate16:40
dansmithkgube: right, but we need to see them running against the proper driver and this change16:40
sean-k-mooneythe depend on relation ship si not quite right althoguh its close16:40
dansmithI guess the nfs job on this patch maybe? https://review.opendev.org/c/openstack/cinder/+/873889/7?tab=change-view-tab-header-zuul-results-summary16:40
kgubeThere is a devstack-nfs job16:40
sean-k-mooneyhttps://review.opendev.org/c/openstack/cinder/+/873686/9 woudl be the patch to make the nfs driver work16:40
dansmiththat patch has depends-on relationships to cinder itself, which is weird16:41
sean-k-mooneyya so that patch should depend on either the nova patch16:41
sean-k-mooneyro the nova patch should ideally depend on it16:41
dansmithwell, not reall,y16:41
dansmithbecause you need to run a cinder patch to see that job16:42
sean-k-mooneythe nova patch depens on the cinder client patch which depends on https://review.opendev.org/c/openstack/cinder/+/873557/816:42
dansmithit looks like every patch in the stack depends on the thing below it or something?16:42
dansmithconfusing16:42
dansmiththis one depends on the nova patch: https://review.opendev.org/c/openstack/nova/+/87356016:42
sean-k-mooneyya that will work but its not how its ment to be used16:42
dansmithand the top cinder one depends on that one16:42
dansmithso it's a bit convoluted16:42
sean-k-mooneyoh the netapp one16:42
dansmithanyway kgube disconnected16:42
sean-k-mooneyya so the issue is that the api change patch is first16:42
dansmithso we can probably end it here and discuss later16:43
sean-k-mooneyand then the driver patches ar after that16:43
sean-k-mooneysure16:43
gibiOK16:43
gibiis the any other topic before we close?16:43
kgubesorry, I got disconnected16:44
dansmithkgube: we were just deciphering the patch dependency network to figure out how to see a job running the whole stack16:44
dansmithwhich I think is there, but it's convoluted16:44
dansmitheither way I think the answer is the same :/16:44
kgubeyeah, I was afraid of that16:45
kgubebut I can understand it16:45
sean-k-mooneyi dont know if cinder has the same policy as nova16:45
sean-k-mooneybut for nova we merge the api change last in a series16:46
sean-k-mooneyso the fact that is first in the cidner one is also a little odd16:46
gibiI don't see any other topic being raised. So I will close the meeting and you can continue untangling the dep tree on the channel16:47
kgubeWell the nova change needs the new volume action16:47
gibi#endmeeting16:47
opendevmeetMeeting ended Tue Aug 22 16:47:15 2023 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)16:47
opendevmeetMinutes:        https://meetings.opendev.org/meetings/nova/2023/nova.2023-08-22-16.00.html16:47
opendevmeetMinutes (text): https://meetings.opendev.org/meetings/nova/2023/nova.2023-08-22-16.00.txt16:47
opendevmeetLog:            https://meetings.opendev.org/meetings/nova/2023/nova.2023-08-22-16.00.log.html16:47
kgubeand everything else is dependent on the support in nova16:47
sean-k-mooneyalso an fyi every time you use depends on it makes zuul clone an addtional copy of the relevent repo then it does a merge of all repos with the same name16:47
sean-k-mooneyso while you can use a depend on in the same repo in some cases its generally not a good idea as it causes a much more complex git merge to be done16:48
kgubeoh, okay, so I don't need them if the changes are already ordered in a branch16:49
kgube?16:49
dansmithkgube: right, a single stack of patches are necessarily dependent and ordered16:49
dansmithkgube: you do need them cross-repo (i.e. cinder->nova) but not within a single branch in cinder16:50
kgubedansmith: alright, thanks!16:50
dansmithalso, the newer way to depends-on is to use a URL to the actual change, which also makes it easier for us to see "this is the nova dependency"16:50
dansmithI thought zuul actually stopped supporting the change-id way, but I guess it still works (or at least the tagging does)16:50
kgubethat is also good to know. I kinda just copied it from other changes that I saw16:51
sean-k-mooneythe reason i mentioned the depend on mechanics is i have seen it cause merge conflict in the ci jobs that done repoduce locally16:52
sean-k-mooneywhich can be very hard to debug16:52
dansmithkgube: here's an example of how it should work these days: https://review.opendev.org/c/openstack/nova/+/65017216:52
sean-k-mooneyi.e. using depend on in the same repo16:52
kgubedansmith: thanks!16:53
spencerh`sean-k-mooney: I am curious about the history with nova and libvirt tsc frequency. Is there a link to a discussion or is it possible to summarize concerns? Not expecting to get this in this cycle, but this patch has been critical for us.16:59
sean-k-mooneykgube: the VolumesExtendAttachedTest test in tempest in the nfs job is skiped by hte way16:59
sean-k-mooneykgube: so its not actully being tested by that job16:59
sean-k-mooneyspencerh`: bascially it comes done to the fact it will break live migration17:00
sean-k-mooneywe do not have the ablity to schdule based on the clock rate fo the hsot cpus17:00
sean-k-mooneyand if we allwo you to set the tsc_frequency via a host config option then we need rpc change to check comaptiablity17:01
spencerh`The tsc frequency doesn't have to match the host capabilities on our cpus. Maybe that's vendor specific?17:01
sean-k-mooneythis is also not somethign that should chagne over the life time of the vm so it should not be a host level opation but rather an image property17:01
spencerh`In my experience, it's the opposite: when invtsc is enabled, not setting tsc frequency will break live migration.17:01
sean-k-mooneyas far as ia am aware it cant exceed the host frequency on intel17:02
sean-k-mooneya requicemnet if invtsc is that live migration is only supproted between hosts with the same frequesncy yes17:02
sean-k-mooneyand seting it in the xml is the workaroudn for aht17:02
sean-k-mooneyhttps://blueprints.launchpad.net/nova/+spec/config-tsc-freq17:02
sean-k-mooneythat is the blueprint that was propsed for adding invTSC orginally17:03
dansmithspencerh`: I imagine anything like this will need a spec and discussion where we can make sure the experts in a given area have time to opine about such a change17:03
dansmithspencerh`: so that's a good place to document the how and why, the impacts and other considerations17:04
spencerh`Sounds good! Sorry, I'm not familiar with the spec process, but I'll be happy to talk about our environment and why we need this. Thanks for the background!17:04
kgubesean-k-mooney, yeah, I will try to make a change that enables it in the job and make this dependent on the NFS driver support change.17:05
dansmithspencerh`: https://specs.openstack.org/openstack/nova-specs/17:05
spencerh`dansmith: ty!17:06
sean-k-mooneyspencerh`: i started reviewing your code change by the way. you have a lot of random formatingin changes in it that should be remvoed17:06
spencerh`Oh no. Probably because I enabled autopep8. I will fix17:06
sean-k-mooneyautopep8 shoudl not make these changes17:07
sean-k-mooneywe have it enabeld in ci17:07
spencerh`Strange. Maybe my local instance is using different settings. There were fewer lines of diff before I messed it up this morning. I'll get that fixed.17:08
opendevreviewSpencer Harmon proposed openstack/nova master: libvirt tsc_frequency  https://review.opendev.org/c/openstack/nova/+/89192417:21
spencerh`Looks better now. Sorry about that. (But no rush to review)17:23
sean-k-mooneyspencerh`: what is your usecase for enabeling the invtsc by the way18:35
sean-k-mooneyim just wonderign if the hpet provided by https://review.opendev.org/c/openstack/nova/+/605902 is enouch for your usecase of if you really need the tsc time source18:36
spencerh`Haha it is very similar. I work at blizzard and we're running game servers on that have invtsc as a requirement (for lower latency/better performance). It's not windows specific, though. We're running various flavors of linux for the guests.18:38
spencerh`Oh sorry. I conflated what you said with the spec.18:39
spencerh`No, I don't think this will provide what's required for libvirt to live migrate with invtsc enabled.18:40
sean-k-mooneyyes we added invtsc and htpe for blizzard specicifcally in the past18:40
sean-k-mooneyright it does not i was asking can invtsc be disabled and use the hpet instead18:40
sean-k-mooneythe invtsc flag tells the guest tha tthe TSC is stable and wont change frequency18:41
spencerh`I'll have to benchmark that to see if it solves the same issues that invtsc does. AFAIK, these are not fungible as far as the workload is concerned.18:41
sean-k-mooneythey gerneally shoudl be if the workload is not direfctly calling the TSC18:42
spencerh`The workload does directly call the tsc.18:42
sean-k-mooneythe tsc is often used because its really fast to read18:42
sean-k-mooneybut its also generally not the most stable time source espcially in a vm18:42
sean-k-mooneyack18:42
sean-k-mooneydansmith: the lazy load serise all looks good overal. i had one of two nits inlien but they are all approved now19:34
sean-k-mooneydo you know off the top of your head what "trusted_certs" is used for in the instance. is that related to trusted boot?19:35
melwittsean-k-mooney: it's for this https://specs.openstack.org/openstack/nova-specs/specs/rocky/implemented/nova-validate-certificates.html19:51
sean-k-mooney possibly but why would nova have the certs19:52
sean-k-mooneyi would expect that to be a propry of the image and there for in the instnace_system_metadata rather then the instance table19:53
sean-k-mooneyoh its form a join19:54
sean-k-mooneyya its that 19:54
sean-k-mooney"""This change will update the InstanceExtra schema to support a trusted_certs column, """19:54
opendevreviewsean mooney proposed openstack/nova master: introduce global greenpool  https://review.opendev.org/c/openstack/nova/+/87306120:04
sean-k-mooneyi have 7 failing funcitonal tests i need to update but i think that is passing everythign else20:05
dansmithsean-k-mooney: ack thanks20:11
opendevreviewSpencer Harmon proposed openstack/nova master: libvirt tsc_frequency  https://review.opendev.org/c/openstack/nova/+/89192422:45
opendevreviewMerged openstack/nova master: Avoid lazy-loading in resize and rebuild/evacuate  https://review.opendev.org/c/openstack/nova/+/89133622:54
opendevreviewmelanie witt proposed openstack/nova master: Enable use of native threads in scatter_gather_cells  https://review.opendev.org/c/openstack/nova/+/65017223:53

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!