opendevreview | Merged openstack/python-novaclient stable/yoga: [stable-only] Pin tox <4 https://review.opendev.org/c/openstack/python-novaclient/+/869597 | 00:43 |
---|---|---|
gmann | dansmith: hi | 00:54 |
opendevreview | Merged openstack/python-novaclient stable/xena: [stable-only] Pin tox <4 https://review.opendev.org/c/openstack/python-novaclient/+/869598 | 02:47 |
opendevreview | Ghanshyam proposed openstack/nova master: Enable new defaults and scope checks by default https://review.opendev.org/c/openstack/nova/+/866218 | 05:37 |
*** bhagyashris|brb is now known as bhagyashris | 06:39 | |
*** bhagyashris is now known as bhagyashris|afk | 06:39 | |
opendevreview | Kirill proposed openstack/nova-specs master: new spec: support of vnc console for ironic https://review.opendev.org/c/openstack/nova-specs/+/863773 | 08:30 |
gibi | stephenfin: if you have time I fixed your comments in https://review.opendev.org/c/openstack/nova/+/854924 | 09:10 |
gibi | and also there are some other patches in the PCI series where I lost your +2 | 09:17 |
kashyap | Morning. Can anyone remind me again if this targetted 'recheck' works: "recheck tempest-integrated-compute" | 09:30 |
gibi | kashyap: npe | 09:42 |
gibi | nope | 09:42 |
gibi | there is now way to selectively recheck | 09:43 |
gibi | it is intentional | 09:43 |
kgube | gibi: Hi! Could you have another look at this: https://review.opendev.org/c/openstack/nova-specs/+/855490 | 09:50 |
kashyap | gibi: Aaah, I see. | 09:50 |
gibi | kgube: I will try | 09:51 |
kgube | thanks! | 09:51 |
gibi | kashyap: left feedback in https://review.opendev.org/c/openstack/nova/+/869587 | 10:01 |
gibi | kashyap, sean-k-mooney: I vaguely remember that you discussed removing the this check ^^ before. What was the outcome of that? | 10:02 |
auniyal | Hi gibi | 10:02 |
gibi | auniyal: hi! | 10:02 |
auniyal | can you please review these also | 10:02 |
auniyal | https://review.opendev.org/c/openstack/nova/+/864006 | 10:02 |
kashyap | gibi: Thanks! Will check in abit | 10:02 |
auniyal | https://review.opendev.org/c/openstack/nova/+/864006 | 10:03 |
auniyal | https://review.opendev.org/c/openinfra/openstack-map/+/866568 | 10:03 |
gibi | bauzas: 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 |
gibi | you have review prio +1 on it | 10:09 |
gibi | hence my question | 10:09 |
opendevreview | Lukas Piwowarski proposed openstack/nova stable/zed: DNM: Test change in run-tempest role https://review.opendev.org/c/openstack/nova/+/869804 | 10:11 |
bauzas | gibi: sorry, discussing with Uggla but sure I can do it | 10:14 |
gibi | bauzas: it is more like do you want to? we have 2 +2s on it | 10:15 |
kashyap | gibi: Responded. Thanks for the review. I hope I have answered at least 60% of your questions :) | 10:16 |
pslestang | Hello 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 |
gibi | kashyap: responeded | 10:42 |
opendevreview | Lukas Piwowarski proposed openstack/nova stable/zed: DNM: Test change in run-tempest role https://review.opendev.org/c/openstack/nova/+/869804 | 10:43 |
kashyap | gibi: Thank you. On the 2nd point, I'm wondering if there's a fairly uncomplicated way to return the flags | 10:52 |
kashyap | (And no your first point, will drop that dead variable) | 10:53 |
gibi | kashyap: 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 |
gibi | so I think the generation of that information can be removed too | 11:06 |
kashyap | gibi: Okay, let me play a little more with it. I only want to make sure people can still specify extra flags and we propagate that | 11:07 |
kashyap | gibi: Ah, we still consider the extra flags in _get_guest_cpu_model_config() method | 11:08 |
kashyap | So we should be good | 11:08 |
gibi | I believe this was not the place where we actually handled the extra flags | 11:08 |
gibi | yepp _get_guest_cpu_model_config seem to be the real place | 11:09 |
kashyap | Yep, indeed we handle it _get_guest_cpu_model_config(). I myself added that ... seeing the note <emabarassed emoji> | 11:09 |
kashyap | gibi: Thanks! Respinning. | 11:11 |
opendevreview | Aaron S proposed openstack/nova master: Add further workaround features for qemu_monitor_announce_self https://review.opendev.org/c/openstack/nova/+/867324 | 11:21 |
opendevreview | Kashyap Chamarthy proposed openstack/nova master: libvirt: Remove compareCPU() check in _check_cpu_compatibility() https://review.opendev.org/c/openstack/nova/+/869587 | 11:22 |
kashyap | gibi: (While you still have the context. Hope I got that right) --^ | 11:24 |
* kashyap --> bbiab | 11:24 | |
opendevreview | Aaron S proposed openstack/nova master: Add further workaround features for qemu_monitor_announce_self https://review.opendev.org/c/openstack/nova/+/867324 | 11:26 |
opendevreview | Merged openstack/nova stable/ussuri: Adds a repoducer for post live migration fail https://review.opendev.org/c/openstack/nova/+/864006 | 11:30 |
opendevreview | Merged openstack/nova stable/ussuri: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866196 | 11:31 |
opendevreview | Artom Lifshitz proposed openstack/nova master: [Broken WIP] 2.94: FQDN in hostname https://review.opendev.org/c/openstack/nova/+/869812 | 11:32 |
sean-k-mooney | gibi: 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 future | 11:32 |
*** bhagyashris|afk is now known as bhagyashris | 11:32 | |
*** artom_ is now known as artom | 11:33 | |
sean-k-mooney | currently we attempt to do that in pre-livemigrate using the old compareCPU api | 11:33 |
opendevreview | Balazs Gibizer proposed openstack/nova stable/yoga: Reproduce bug 1981813 in func env https://review.opendev.org/c/openstack/nova/+/859312 | 11:34 |
opendevreview | Balazs Gibizer proposed openstack/nova stable/yoga: Gracefully ERROR in _init_instance if vnic_type changed https://review.opendev.org/c/openstack/nova/+/859313 | 11:34 |
gibi | sean-k-mooney: the current patch is removing the compatibility check at the compute startup, it does not change the check at live migration | 11:35 |
sean-k-mooney | i have not looked at the current patch just the ones that were created before | 11:36 |
sean-k-mooney | why would we want to remove it at compute start up | 11:36 |
sean-k-mooney | looking at it now | 11:36 |
sean-k-mooney | as an unconditional change i think this is wrong | 11:38 |
gibi | sean-k-mooney: I think the renewed interest for this is coming from https://review.opendev.org/c/openstack/nova/+/869536/ | 11:41 |
kashyap | Please see the commit message. We have already talked about it in detail before, and the rationale explains it there | 11:41 |
* kashyap really needs to step out briefly | 11:41 | |
sean-k-mooney | i 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 workaround | 11:41 |
sean-k-mooney | kashyap: yep but you never convicend me it was right before | 11:42 |
sean-k-mooney | kashyap: i just agree to not block it because it was guarded behind a workaround and we still did the check by default | 11:42 |
sean-k-mooney | kashyap: really what i would liek to see is for you or someone else to complete swaping to the new api | 11:43 |
opendevreview | Balazs Gibizer proposed openstack/nova stable/xena: Reproduce bug 1981813 in func env https://review.opendev.org/c/openstack/nova/+/859314 | 11:43 |
opendevreview | Balazs Gibizer proposed openstack/nova stable/xena: Gracefully ERROR in _init_instance if vnic_type changed https://review.opendev.org/c/openstack/nova/+/859315 | 11:43 |
sean-k-mooney | gibi: yep proably however this was fixed in libvirt upstream with the intoduction fo new cpu models | 11:44 |
gibi | we tried the new API work that but is complicated enough that is stalls out | 11:44 |
sean-k-mooney | and i think we just have not backported that fix downsstream | 11:44 |
opendevreview | Manuel Bentele proposed openstack/nova master: libvirt: Add configuration options to set SPICE compression settings https://review.opendev.org/c/openstack/nova/+/828675 | 11:48 |
gibi | so 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 |
gibi | and we can only remove the WA flag after we resurrect https://review.opendev.org/c/openstack/nova/+/762330 and finish it? | 12:01 |
kashyap | sean-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 it | 12:02 |
kashyap | sean-k-mooney: Also, libvirt developers themselves are suggesting that management tools should let libvirt do the work here. | 12:03 |
kashyap | I have already mentioned that in the commit message. If that doesn't convince you; nothing else will :) | 12:03 |
sean-k-mooney | kashyap: they may be experts in libvirt but not the managemnet tools | 12:03 |
sean-k-mooney | failures in live_migrate are expensive | 12:03 |
sean-k-mooney | we have already plugged the networkign and done other operatiosn on the destination that need to be roled back | 12:04 |
kashyap | sean-k-mooney: Well, I totally realize that as someone who debugs a crap ton of live migration problems | 12:04 |
sean-k-mooney | the reason we have the check early in pre-livemeigrate is to prevent that setup | 12:04 |
kashyap | sean-k-mooney: Well. I would prefer if you don't block this on theoretical objections or "ideally X" scenario | 12:05 |
kashyap | Also, on the destination, removing the API check is still correct. I put the workaround as a compromise. | 12:05 |
sean-k-mooney | we can agree to disagree | 12:07 |
kashyap | Sure. FWIW, I heard no complaints from the folks using the workaround to skip the check on dest. (As I expected.) | 12:10 |
kashyap | gibi: 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/ecb85fe48cacb | 12:12 |
sean-k-mooney | kashyap: libvirt adressetd that by adding a new cpu model without mpx https://gitlab.com/lvoytek/libvirt/-/commit/39f5bcd48323e814c910a92bf8ef76dd0166680d | 12:13 |
kashyap | sean-k-mooney: It is rejected; it was just merely submitted as a patch. I (and libvirt devs) reviewed that upstream libvirt list | 12:14 |
kashyap | I even linked to that patch discussion in my URLs | 12:14 |
sean-k-mooney | i see | 12:15 |
sean-k-mooney | apparently the cpu_model_extra_flags is not working properly to work around that by the way | 12:16 |
sean-k-mooney | https://bugzilla.redhat.com/show_bug.cgi?id=2158181 | 12:16 |
kashyap | sean-k-mooney: Let me get the URL from the upstream libvirt folks (from last year). It was spread over months so the archives are split | 12:16 |
kashyap | sean-k-mooney: Indeed, that's because the model is evaluated _before_ the flags | 12:16 |
sean-k-mooney | i got that form https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1978064 | 12:16 |
sean-k-mooney | it was marked as fix released | 12:16 |
sean-k-mooney | kashyap: shoudl we not fix that then instead of removing the check | 12:17 |
sean-k-mooney | we should be validating the modifed one no? | 12:17 |
kashyap | sean-k-mooney: The problem here is entirely due the older API, hence the recommendetaion to remove that now-buggy check | 12:17 |
sean-k-mooney | that really feels like a regression/bug to remove it | 12:18 |
kashyap | Maybe they're carrying downstream Ubuntu-specific patch? | 12:18 |
* kashyap looks | 12:18 | |
sean-k-mooney | ya maybe im not sure | 12:18 |
sean-k-mooney | its confusing give i knwo libvirt uses bugzilla for trackign right | 12:19 |
sean-k-mooney | so this libvirt tracker is presumabel for the ubunu package | 12:19 |
* kashyap goes to check | 12:19 | |
kashyap | sean-k-mooney: Upstream libvirt uses gitlab now | 12: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-mooney | kashyap: so looking at https://gitlab.com/libvirt/libvirt/-/issues/304#note_1065798706 | 12:21 |
sean-k-mooney | do we use check=full or generate that check string today | 12:21 |
sean-k-mooney | this is not related to the start up check | 12:21 |
sean-k-mooney | but i think we leave that to libvirt to set in most if not all cases | 12:22 |
kashyap | sean-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 |
kashyap | Adding a new CPU model is not that serious, but it's not good either as | 12:23 |
kashyap | it causes unnecessary compatibility issues with older versions of | 12:23 |
kashyap | libvirt. Especially adding a new CPU model which does not exist in QEMU | 12:23 |
kashyap | does not make any sense, as libvirt would need to translate it to | 12:23 |
kashyap | something else when starting QEMU. | 12:23 |
kashyap | [/quote] | 12:23 |
kashyap | https://listman.redhat.com/archives/libvir-list/2022-August/233717.html | 12:23 |
sean-k-mooney | ya that makes sense why they would not want to do that | 12:24 |
kashyap | sean-k-mooney: Also almost since 4 years ago, QEMU and libvirt have stopped adding explicit named models like that "-noTSXAndThat_AndThisFeature" | 12:24 |
kashyap | Right. | 12:24 |
kashyap | sean-k-mooney: No, we don't use "check=full" | 12:24 |
sean-k-mooney | i 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-mooney | kashyap: ya i didnt think we did but wantted to confirm | 12:24 |
kashyap | sean-k-mooney: I'll let some actual tests with real CPU models be done by QE to see the existing patch's impact | 12:25 |
kashyap | That way we have clear evidence. | 12:25 |
* kashyap --> bbiab | 12:25 | |
sean-k-mooney | ok i need to go take my blood pressue medication and do a few bits. brb | 12:25 |
kashyap | Sure, take care! And thanks for the discussion | 12:26 |
zigo | Last night, we had a case of crash of nova when deleting a VM: | 12:54 |
zigo | https://paste.opendev.org/show/bRFOvaXxQxs5vVdwzVRV/ | 12:54 |
zigo | one colleague of mine says it's because the image was deleted first. Is he right? | 12:54 |
sean-k-mooney | no i dont think so | 12:55 |
sean-k-mooney | this is related to the nvram not the glance/vm disk image | 12:55 |
zigo | How come Nova can't "undefine domain with nvram" then? | 12:55 |
zigo | Was this fixed in a version higher than Victoria? | 12:56 |
sean-k-mooney | it might be related to https://bugs.launchpad.net/nova/+bug/1785123 | 12:56 |
zigo | Oh, thanks. | 12:57 |
sean-k-mooney | zigo: there have been some nvram/uefi issues fixed since then yes | 12:57 |
sean-k-mooney | https://review.opendev.org/q/project:openstack/nova+nvram | 12:57 |
sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/335512 that seams to be the most driectly related | 12:58 |
zigo | It's abandonned though ... :/ | 12:59 |
sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/guest.py#L298 | 12:59 |
sean-k-mooney | yes but the fix was condtionaly added on master | 12:59 |
sean-k-mooney | if uefi is supported | 12:59 |
sean-k-mooney | so we fixed it in a differnt patch | 12:59 |
sean-k-mooney | https://github.com/openstack/nova/commit/539d381434ccadcdc3f5d58c2705c35558a3a065 | 13:00 |
sean-k-mooney | hum apparently in ocata | 13:00 |
sean-k-mooney | zigo: what version of qemu are you using | 13:00 |
sean-k-mooney | sory libvirt | 13:00 |
zigo | 7.0.0 | 13:01 |
zigo | (the one from Bullseye) | 13:01 |
sean-k-mooney | i wonder what support_uefi is set too | 13:02 |
opendevreview | Merged openstack/nova master: Support unshelve with PCI in placement https://review.opendev.org/c/openstack/nova/+/854616 | 13:02 |
sean-k-mooney | zigo: so we do this https://github.com/openstack/nova/blob/70fd0cfc8ee2b541ffc1f9feb129314965d1670c/nova/virt/libvirt/driver.py#L1349-L1351 | 13:04 |
zigo | Victoria has the same code... | 13:05 |
zigo | So you see... instance.image_meta.properties.get | 13:06 |
zigo | Is this taken from the *image* ?!? | 13:06 |
zigo | Or is it a copy of the image meta? | 13:06 |
sean-k-mooney | a copy but i assume its uefi | 13:07 |
sean-k-mooney | you might be higing a kernel api change where some parmater changed for 1/0 to y/n | 13:09 |
sean-k-mooney | can you check the output of virsh domcapabilities | 13:09 |
zigo | https://paste.opendev.org/show/bEp66voE4chYd3lF3eme/ | 13:11 |
zigo | What am I looking for? | 13:11 |
zigo | The <os supported='yes'> bits? | 13:12 |
sean-k-mooney | <loader supported='yes'> | 13:12 |
sean-k-mooney | line 12 | 13:12 |
zigo | Is it supposed to be 0/1 instead ? | 13:12 |
sean-k-mooney | no i was thinkin of a change for secure boot | 13:13 |
sean-k-mooney | which is seperate | 13:13 |
opendevreview | Merged openstack/nova stable/train: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866201 | 13:16 |
zigo | sean-k-mooney: That bit of code is supposed to remove the disk containing the NVRAM values, right? | 13:18 |
sean-k-mooney | its not really a disk its a file that is uesd to store the uefi firmware data | 13:18 |
zigo | Right, that's what I had in mind. | 13:19 |
sean-k-mooney | but yes its to tell libvirt that its oke to delete the domain | 13:19 |
sean-k-mooney | inclucding undefining the nvram | 13:19 |
zigo | According 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 |
zigo | Is it still the case? | 13:39 |
sean-k-mooney | that i am not sure about. i know that was a proablem in the past | 13:41 |
sean-k-mooney | i think this bug is still a thing | 13:42 |
sean-k-mooney | honestly i proably should escalte this internally too becasue this keeps getting stalled out | 13:42 |
sean-k-mooney | the people that started fixing it have moved on form openstack | 13:43 |
opendevreview | Merged openstack/nova master: Add mock to avoid loading guestfs in unit test https://review.opendev.org/c/openstack/nova/+/862769 | 14:38 |
opendevreview | Jorge San Emeterio proposed openstack/nova-specs master: Review usage of oslo-privsep library on Nova https://review.opendev.org/c/openstack/nova-specs/+/865432 | 14:47 |
dansmith | 2023-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 |
dansmith | thousands of those in each run now.. I guess something in o.msg changed | 15:14 |
gibi | dansmith: 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 warning | 15:24 |
* dansmith nods | 15:24 | |
opendevreview | ribaudr 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/+/861881 | 15:25 |
opendevreview | ribaudr 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/+/861881 | 15:31 |
gmann | dansmith: replied and updated the rbac default switch change, please check when you have time https://review.opendev.org/c/openstack/nova/+/866218 | 15:47 |
gmann | I 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 defaults | 15:48 |
dansmith | cool | 15:50 |
*** d34dh0r5| is now known as d34dh0r53 | 16:56 | |
*** d34dh0r53 is now known as d34dh0r5- | 16:58 | |
opendevreview | sean mooney proposed openstack/placement master: support multiple config files with apache https://review.opendev.org/c/openstack/placement/+/869863 | 17:58 |
sean-k-mooney | ^ is the placement version fo https://review.opendev.org/c/openstack/nova/+/867162 | 17:58 |
opendevreview | Dan Smith proposed openstack/nova master: Add virt/node module for stable uuids https://review.opendev.org/c/openstack/nova/+/863915 | 20:12 |
opendevreview | Dan Smith proposed openstack/nova master: Pass service ref to init_host(), if exists https://review.opendev.org/c/openstack/nova/+/863916 | 20:12 |
opendevreview | Dan Smith proposed openstack/nova master: Add get_available_node_uuids() to virt driver https://review.opendev.org/c/openstack/nova/+/863917 | 20:12 |
opendevreview | Dan Smith proposed openstack/nova master: Make resource tracker use UUIDs instead of names https://review.opendev.org/c/openstack/nova/+/863919 | 20:12 |
opendevreview | Dan Smith proposed openstack/nova master: Persist existing node uuids locally https://review.opendev.org/c/openstack/nova/+/863918 | 20:12 |
opendevreview | Dan Smith proposed openstack/nova master: WIP: Detect host renames and abort startup https://review.opendev.org/c/openstack/nova/+/863920 | 20:12 |
opendevreview | Dan Smith proposed openstack/nova master: Make resource tracker use UUIDs instead of names https://review.opendev.org/c/openstack/nova/+/863919 | 20:38 |
opendevreview | Dan Smith proposed openstack/nova master: Persist existing node uuids locally https://review.opendev.org/c/openstack/nova/+/863918 | 20:38 |
opendevreview | Dan Smith proposed openstack/nova master: WIP: Detect host renames and abort startup https://review.opendev.org/c/openstack/nova/+/863920 | 20:38 |
opendevreview | Merged openstack/nova master: Support same host resize with PCI in placement https://review.opendev.org/c/openstack/nova/+/854441 | 21:24 |
*** dasm is now known as dasm|off | 23:20 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!