Wednesday, 2023-01-11

opendevreviewMerged openstack/python-novaclient stable/yoga: [stable-only] Pin tox <4  https://review.opendev.org/c/openstack/python-novaclient/+/86959700:43
gmanndansmith: hi00:54
opendevreviewMerged openstack/python-novaclient stable/xena: [stable-only] Pin tox <4  https://review.opendev.org/c/openstack/python-novaclient/+/86959802:47
opendevreviewGhanshyam proposed openstack/nova master: Enable new defaults and scope checks by default  https://review.opendev.org/c/openstack/nova/+/86621805:37
*** bhagyashris|brb is now known as bhagyashris06:39
*** bhagyashris is now known as bhagyashris|afk06:39
opendevreviewKirill proposed openstack/nova-specs master: new spec: support of vnc console for ironic  https://review.opendev.org/c/openstack/nova-specs/+/86377308:30
gibistephenfin: if you have time I fixed your comments in https://review.opendev.org/c/openstack/nova/+/85492409:10
gibiand also there are some other patches in the PCI series where I lost your +209:17
kashyapMorning.  Can anyone remind me again if this targetted 'recheck' works: "recheck tempest-integrated-compute"09:30
gibikashyap: npe09:42
gibinope09:42
gibithere is now way to selectively recheck09:43
gibiit is intentional 09:43
kgubegibi: Hi! Could you have another look at this: https://review.opendev.org/c/openstack/nova-specs/+/85549009:50
kashyapgibi: Aaah, I see.09:50
gibikgube: I will try09:51
kgubethanks!09:51
gibikashyap: left feedback in https://review.opendev.org/c/openstack/nova/+/86958710:01
gibikashyap, sean-k-mooney: I vaguely remember that you discussed removing the this check ^^ before. What was the outcome of that?10:02
auniyalHi gibi 10:02
gibiauniyal: hi!10:02
auniyalcan you please review these also10:02
auniyalhttps://review.opendev.org/c/openstack/nova/+/86400610:02
kashyapgibi: Thanks!  Will check in abit10:02
auniyalhttps://review.opendev.org/c/openstack/nova/+/86400610:03
auniyalhttps://review.opendev.org/c/openinfra/openstack-map/+/86656810:03
gibibauzas: do you want to check https://review.opendev.org/c/openstack/nova-specs/+/855490 (Use extend volume completion action) or I can +A it?10:09
gibiyou have review prio +1 on it10:09
gibihence my question10:09
opendevreviewLukas Piwowarski proposed openstack/nova stable/zed: DNM: Test change in run-tempest role  https://review.opendev.org/c/openstack/nova/+/86980410:11
bauzasgibi: sorry, discussing with Uggla but sure I can do it10:14
gibibauzas: it is more like do you want to? we have 2 +2s on it10:15
kashyapgibi: Responded.  Thanks for the review.  I hope I have answered at least 60% of your questions :)10:16
pslestangHello all, dansmith will you have some time to review https://review.opendev.org/c/openstack/nova/+/867832, I pushed an other patchset after your +2 review 10:40
gibikashyap: responeded 10:42
opendevreviewLukas Piwowarski proposed openstack/nova stable/zed: DNM: Test change in run-tempest role  https://review.opendev.org/c/openstack/nova/+/86980410:43
kashyapgibi: Thank you.  On the 2nd point, I'm wondering if there's a fairly uncomplicated way to return the flags10:52
kashyap(And no your first point, will drop that dead variable)10:53
gibikashyap: my point is that the original function did not return that information, so that information was only use locally there, but the usage of it is removed by you 11:05
gibiso I think the generation of that information can be removed too11:06
kashyapgibi: Okay, let me play a little more with it.  I only want to make sure people can still specify extra flags and we propagate that11:07
kashyapgibi: Ah, we still consider the extra flags in _get_guest_cpu_model_config() method11:08
kashyapSo we should be good11:08
gibiI believe this was not the place where we actually handled the extra flags 11:08
gibiyepp _get_guest_cpu_model_config seem to be the real place11:09
kashyapYep, indeed we handle it _get_guest_cpu_model_config().  I myself added that ... seeing the note <emabarassed emoji>11:09
kashyapgibi: Thanks!  Respinning.11:11
opendevreviewAaron S proposed openstack/nova master: Add further workaround features for qemu_monitor_announce_self  https://review.opendev.org/c/openstack/nova/+/86732411:21
opendevreviewKashyap Chamarthy proposed openstack/nova master: libvirt: Remove compareCPU() check in _check_cpu_compatibility()  https://review.opendev.org/c/openstack/nova/+/86958711:22
kashyapgibi: (While you still have the context.  Hope I got that right) --^11:24
* kashyap --> bbiab11:24
opendevreviewAaron S proposed openstack/nova master: Add further workaround features for qemu_monitor_announce_self  https://review.opendev.org/c/openstack/nova/+/86732411:26
opendevreviewMerged openstack/nova stable/ussuri: Adds a repoducer for post live migration fail  https://review.opendev.org/c/openstack/nova/+/86400611:30
opendevreviewMerged openstack/nova stable/ussuri: Adapt websocketproxy tests for SimpleHTTPServer fix  https://review.opendev.org/c/openstack/nova/+/86619611:31
opendevreviewArtom Lifshitz proposed openstack/nova master: [Broken WIP] 2.94: FQDN in hostname  https://review.opendev.org/c/openstack/nova/+/86981211:32
sean-k-mooneygibi: basically i thing its wrong for nova not to enfore the cpu compatiablity itself ideally at teh schduleing point and failures at the live migrate call to livert are too late. however apprently the libvirt folks are advising that we delegate this check to libvirt alone. so if we remove this i want us to add something to our backloag to go replace it in the future11:32
*** bhagyashris|afk is now known as bhagyashris11:32
*** artom_ is now known as artom11:33
sean-k-mooneycurrently we attempt to do that in pre-livemigrate using the old compareCPU api11:33
opendevreviewBalazs Gibizer proposed openstack/nova stable/yoga: Reproduce bug 1981813 in func env  https://review.opendev.org/c/openstack/nova/+/85931211:34
opendevreviewBalazs Gibizer proposed openstack/nova stable/yoga: Gracefully ERROR in _init_instance if vnic_type changed  https://review.opendev.org/c/openstack/nova/+/85931311:34
gibisean-k-mooney: the current patch is removing the compatibility check at the compute startup, it does not change the check at live migration11:35
sean-k-mooneyi have not looked at the current patch just the ones that were created before11:36
sean-k-mooneywhy would we want to remove it at compute start up11:36
sean-k-mooneylooking at it now11:36
sean-k-mooneyas an unconditional change i think this is wrong11:38
gibisean-k-mooney: I think the renewed interest for this is coming from https://review.opendev.org/c/openstack/nova/+/869536/ 11:41
kashyapPlease see the commit message.  We have already talked about it in detail before, and the rationale explains it there11:41
* kashyap really needs to step out briefly11:41
sean-k-mooneyi objected to the live migraiton workaround as i think that was also wrong fundementaly at a nova level so if we want to disabel this is think we shoudl have it behaind a workaround11:41
sean-k-mooneykashyap: yep but you never convicend me it was right before11:42
sean-k-mooneykashyap: i just agree to not block it because it was guarded behind a workaround and we still did the check by default11:42
sean-k-mooneykashyap: really what i would liek to see is for you or someone else to complete swaping to the new api11:43
opendevreviewBalazs Gibizer proposed openstack/nova stable/xena: Reproduce bug 1981813 in func env  https://review.opendev.org/c/openstack/nova/+/85931411:43
opendevreviewBalazs Gibizer proposed openstack/nova stable/xena: Gracefully ERROR in _init_instance if vnic_type changed  https://review.opendev.org/c/openstack/nova/+/85931511:43
sean-k-mooneygibi: yep proably however this was fixed in libvirt upstream with the intoduction fo new cpu models 11:44
gibiwe tried the new API work that but is complicated enough that is stalls out 11:44
sean-k-mooneyand i think we just have not backported that fix downsstream11:44
opendevreviewManuel Bentele proposed openstack/nova master: libvirt: Add configuration options to set SPICE compression settings  https://review.opendev.org/c/openstack/nova/+/82867511:48
gibiso moving this under a WA flag; would that mean that people with certain hw should always set the WA flag, i.e. in case of mpx https://review.opendev.org/c/openstack/nova/+/869536/12:00
gibiand we can only remove the WA flag after we resurrect https://review.opendev.org/c/openstack/nova/+/762330 and finish it?12:01
kashyapsean-k-mooney: Hi, just reading back.  As I noted in the commit, swapping out the APIs is not worth it at this point - and I don't have bandwidth for it12:02
kashyapsean-k-mooney: Also, libvirt developers themselves are suggesting that management tools should let libvirt do the work here.12:03
kashyapI have already mentioned that in the commit message.  If that doesn't convince you; nothing else will :)12:03
sean-k-mooneykashyap: they may be experts in libvirt but not the managemnet tools12:03
sean-k-mooneyfailures in live_migrate are expensive12:03
sean-k-mooneywe have already plugged the networkign and done other operatiosn on the destination that need to be roled back12:04
kashyapsean-k-mooney: Well, I totally realize that as someone who debugs a crap ton of live migration problems12:04
sean-k-mooneythe reason we have the check early in pre-livemeigrate is to prevent that setup12:04
kashyapsean-k-mooney: Well.  I would prefer if you don't block this on theoretical objections or "ideally X" scenario12:05
kashyapAlso, on the destination, removing the API check is still correct.  I put the workaround as a compromise.12:05
sean-k-mooneywe can agree to disagree12:07
kashyapSure.  FWIW, I heard no complaints from the folks using the workaround to skip the check on dest.  (As I expected.)12:10
kashyapgibi: Also on that Intel "mpx" saga: the issue is due to two reasons: (a) Intel is phasing out MPX; and consequently (b) QEMU removed that feature from all CPU models (Skylake, Icelake, Cascadelake) starting from QEMU 4.0 - https://gitlab.com/qemu-project/qemu/-/commit/ecb85fe48cacb12:12
sean-k-mooneykashyap: libvirt adressetd that by adding a new cpu model without mpx https://gitlab.com/lvoytek/libvirt/-/commit/39f5bcd48323e814c910a92bf8ef76dd0166680d12:13
kashyapsean-k-mooney: It is rejected; it was just merely submitted as a patch.  I (and libvirt devs) reviewed that upstream libvirt list 12:14
kashyapI even linked to that patch discussion in my URLs12:14
sean-k-mooneyi see12:15
sean-k-mooneyapparently the cpu_model_extra_flags is not working properly to work around that by the way12:16
sean-k-mooneyhttps://bugzilla.redhat.com/show_bug.cgi?id=215818112:16
kashyapsean-k-mooney: Let me get the URL from the upstream libvirt folks (from last year).  It was spread over months so the archives are split12:16
kashyapsean-k-mooney: Indeed, that's because the model is evaluated _before_ the flags12:16
sean-k-mooneyi got that form https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/197806412:16
sean-k-mooneyit was marked as fix released12:16
sean-k-mooneykashyap: shoudl we not fix that then instead of removing the check12:17
sean-k-mooneywe should be validating the modifed one no?12:17
kashyapsean-k-mooney: The problem here is entirely due the older API, hence the recommendetaion to remove that now-buggy check12:17
sean-k-mooneythat really feels like a regression/bug to remove it12:18
kashyapMaybe they're carrying downstream Ubuntu-specific patch?12:18
* kashyap looks12:18
sean-k-mooneyya maybe im not sure12:18
sean-k-mooneyits confusing give i knwo libvirt uses bugzilla for trackign right12:19
sean-k-mooneyso this libvirt tracker is presumabel for the ubunu package12:19
* kashyap goes to check12:19
kashyapsean-k-mooney: Upstream libvirt uses gitlab now12:19
kashyap(E.g. the "mpx" issue was discussed here, also filed by the Ubuntu person: https://gitlab.com/libvirt/libvirt/-/issues/304)12:20
kashyap(Also notice that is a private libvirt branch that you linked to)12:21
sean-k-mooneykashyap: so looking at https://gitlab.com/libvirt/libvirt/-/issues/304#note_106579870612:21
sean-k-mooneydo we use check=full or generate that check string today12:21
sean-k-mooneythis is not related to the start up check12:21
sean-k-mooneybut i think we leave that to libvirt to set in most if not all cases12:22
kashyapsean-k-mooney: Here was the rationale that libvirt rejected that adding extra named model.  Which I fully agree with:12:23
kashyap[quote]12:23
kashyapAdding a new CPU model is not that serious, but it's not good either as12:23
kashyapit causes unnecessary compatibility issues with older versions of12:23
kashyaplibvirt. Especially adding a new CPU model which does not exist in QEMU12:23
kashyapdoes not make any sense, as libvirt would need to translate it to12:23
kashyapsomething else when starting QEMU.12:23
kashyap[/quote]12:23
kashyaphttps://listman.redhat.com/archives/libvir-list/2022-August/233717.html12:23
sean-k-mooneyya that makes sense why they would not want to do that12:24
kashyapsean-k-mooney: Also almost since 4 years ago, QEMU and libvirt have stopped adding explicit named models like that "-noTSXAndThat_AndThisFeature"12:24
kashyapRight.12:24
kashyapsean-k-mooney: No, we don't use "check=full"12:24
sean-k-mooneyi think i would still prefer to move the cpu check to include the extra flags 12:24
kashyap(Answering the earlier question)12:24
sean-k-mooneykashyap: ya i didnt think we did but wantted to confirm12:24
kashyapsean-k-mooney: I'll let some actual tests with real CPU models be done by QE to see the existing patch's impact12:25
kashyapThat way we have clear evidence.12:25
* kashyap --> bbiab12:25
sean-k-mooneyok i need to go take my blood pressue medication and do a few bits. brb12:25
kashyapSure, take care!  And thanks for the discussion12:26
zigoLast night, we had a case of crash of nova when deleting a VM:12:54
zigohttps://paste.opendev.org/show/bRFOvaXxQxs5vVdwzVRV/12:54
zigoone colleague of mine says it's because the image was deleted first. Is he right?12:54
sean-k-mooneyno i dont think so12:55
sean-k-mooneythis is related to the nvram not the glance/vm disk image12:55
zigoHow come Nova can't "undefine domain with nvram" then?12:55
zigoWas this fixed in a version higher than Victoria?12:56
sean-k-mooneyit might be related to https://bugs.launchpad.net/nova/+bug/178512312:56
zigoOh, thanks.12:57
sean-k-mooneyzigo:  there have been some nvram/uefi issues fixed since then yes12:57
sean-k-mooneyhttps://review.opendev.org/q/project:openstack/nova+nvram12:57
sean-k-mooneyhttps://review.opendev.org/c/openstack/nova/+/335512 that seams to be the most driectly related 12:58
zigoIt's abandonned though ... :/12:59
sean-k-mooneyhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/guest.py#L29812:59
sean-k-mooneyyes but the fix was condtionaly added on master12:59
sean-k-mooneyif uefi is supported12:59
sean-k-mooneyso we fixed it in a differnt patch12:59
sean-k-mooneyhttps://github.com/openstack/nova/commit/539d381434ccadcdc3f5d58c2705c35558a3a06513:00
sean-k-mooneyhum apparently in ocata13:00
sean-k-mooneyzigo: what version of qemu are you using13:00
sean-k-mooneysory libvirt13:00
zigo7.0.013:01
zigo(the one from Bullseye)13:01
sean-k-mooneyi wonder what support_uefi is set too13:02
opendevreviewMerged openstack/nova master: Support unshelve with PCI in placement  https://review.opendev.org/c/openstack/nova/+/85461613:02
sean-k-mooneyzigo: so we do this https://github.com/openstack/nova/blob/70fd0cfc8ee2b541ffc1f9feb129314965d1670c/nova/virt/libvirt/driver.py#L1349-L135113:04
zigoVictoria has the same code...13:05
zigoSo you see... instance.image_meta.properties.get 13:06
zigoIs this taken from the *image* ?!?13:06
zigoOr is it a copy of the image meta?13:06
sean-k-mooneya copy but i assume its uefi13:07
sean-k-mooneyyou might be higing a kernel api change where some parmater changed for 1/0 to y/n13:09
sean-k-mooneycan you check the output of virsh domcapabilities13:09
zigohttps://paste.opendev.org/show/bEp66voE4chYd3lF3eme/13:11
zigoWhat am I looking for?13:11
zigoThe  <os supported='yes'> bits?13:12
sean-k-mooney<loader supported='yes'> 13:12
sean-k-mooneyline 1213:12
zigoIs it supposed to be 0/1 instead ?13:12
sean-k-mooneyno i was thinkin of a change for secure boot13:13
sean-k-mooneywhich is seperate13:13
opendevreviewMerged openstack/nova stable/train: Adapt websocketproxy tests for SimpleHTTPServer fix  https://review.opendev.org/c/openstack/nova/+/86620113:16
zigosean-k-mooney: That bit of code is supposed to remove the disk containing the NVRAM values, right?13:18
sean-k-mooneyits not really a disk its a file that is uesd to store the uefi firmware data13:18
zigoRight, that's what I had in mind.13:19
sean-k-mooneybut yes its to tell libvirt that its oke to delete the domain13:19
sean-k-mooneyinclucding undefining the nvram 13:19
zigoAccording to this patch's patch header https://review.opendev.org/c/openstack/nova/+/621646/ the UEFI NVRAM variable store isn't preserved on stop/start, hard reboot, cold migration, resize and live migration...13:38
zigoIs it still the case?13:39
sean-k-mooneythat i am not sure about. i know that was a proablem in the past13:41
sean-k-mooneyi think this bug is still a thing13:42
sean-k-mooneyhonestly i proably should escalte this internally too becasue this keeps getting stalled out13:42
sean-k-mooneythe people that started fixing it have moved on form openstack13:43
opendevreviewMerged openstack/nova master: Add mock to avoid loading guestfs in unit test  https://review.opendev.org/c/openstack/nova/+/86276914:38
opendevreviewJorge San Emeterio proposed openstack/nova-specs master: Review usage of oslo-privsep library on Nova  https://review.opendev.org/c/openstack/nova-specs/+/86543214:47
dansmith2023-01-11 07:00:50,878 WARNING [oslo_messaging.rpc.client] Using RPCClient manually to instantiate client. Please use get_rpc_client to obtain an RPC client instance.15:14
dansmiththousands of those in each run now.. I guess something in o.msg changed15:14
gibidansmith: I think https://review.opendev.org/c/openstack/requirements/+/869340 pulled in https://review.opendev.org/c/openstack/oslo.messaging/+/862419 that has the new warning15:24
* dansmith nods15:24
opendevreviewribaudr proposed openstack/nova-specs master: Allow local scaphandre directory to be mapped to an instance using virtiofs  https://review.opendev.org/c/openstack/nova-specs/+/86188115:25
opendevreviewribaudr proposed openstack/nova-specs master: Allow local scaphandre directory to be mapped to an instance using virtiofs  https://review.opendev.org/c/openstack/nova-specs/+/86188115:31
gmanndansmith: replied and updated the rbac default switch change, please check when you have time  https://review.opendev.org/c/openstack/nova/+/86621815:47
gmannI have pushed devstack changes (depends-on) to keep running all existing jobs on old default and new jobs run with new defaults. After we do 2023.1 release we can switch it to run all existing jobs to new defaults and one job to run on old defaults15:48
dansmithcool15:50
*** d34dh0r5| is now known as d34dh0r5316:56
*** d34dh0r53 is now known as d34dh0r5-16:58
opendevreviewsean mooney proposed openstack/placement master: support multiple config files with apache  https://review.opendev.org/c/openstack/placement/+/86986317:58
sean-k-mooney^ is the placement version fo https://review.opendev.org/c/openstack/nova/+/86716217:58
opendevreviewDan Smith proposed openstack/nova master: Add virt/node module for stable uuids  https://review.opendev.org/c/openstack/nova/+/86391520:12
opendevreviewDan Smith proposed openstack/nova master: Pass service ref to init_host(), if exists  https://review.opendev.org/c/openstack/nova/+/86391620:12
opendevreviewDan Smith proposed openstack/nova master: Add get_available_node_uuids() to virt driver  https://review.opendev.org/c/openstack/nova/+/86391720:12
opendevreviewDan Smith proposed openstack/nova master: Make resource tracker use UUIDs instead of names  https://review.opendev.org/c/openstack/nova/+/86391920:12
opendevreviewDan Smith proposed openstack/nova master: Persist existing node uuids locally  https://review.opendev.org/c/openstack/nova/+/86391820:12
opendevreviewDan Smith proposed openstack/nova master: WIP: Detect host renames and abort startup  https://review.opendev.org/c/openstack/nova/+/86392020:12
opendevreviewDan Smith proposed openstack/nova master: Make resource tracker use UUIDs instead of names  https://review.opendev.org/c/openstack/nova/+/86391920:38
opendevreviewDan Smith proposed openstack/nova master: Persist existing node uuids locally  https://review.opendev.org/c/openstack/nova/+/86391820:38
opendevreviewDan Smith proposed openstack/nova master: WIP: Detect host renames and abort startup  https://review.opendev.org/c/openstack/nova/+/86392020:38
opendevreviewMerged openstack/nova master: Support same host resize with PCI in placement  https://review.opendev.org/c/openstack/nova/+/85444121:24
*** dasm is now known as dasm|off23:20

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