| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-01-11 | |||
| 11:31:35 | opendevreview | Merged openstack/nova stable/ussuri: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866196 | |
| 11:32:17 | opendevreview | Artom Lifshitz proposed openstack/nova master: [Broken WIP] 2.94: FQDN in hostname https://review.opendev.org/c/openstack/nova/+/869812 | |
| 11:32:29 | 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:33:19 | sean-k-mooney | currently we attempt to do that in pre-livemigrate using the old compareCPU api | |
| 11:34:28 | opendevreview | Balazs Gibizer proposed openstack/nova stable/yoga: Reproduce bug 1981813 in func env https://review.opendev.org/c/openstack/nova/+/859312 | |
| 11:34:29 | 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:35:13 | 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:36:05 | sean-k-mooney | i have not looked at the current patch just the ones that were created before | |
| 11:36:13 | sean-k-mooney | why would we want to remove it at compute start up | |
| 11:36:17 | sean-k-mooney | looking at it now | |
| 11:38:16 | sean-k-mooney | as an unconditional change i think this is wrong | |
| 11:41:32 | gibi | sean-k-mooney: I think the renewed interest for this is coming from https://review.opendev.org/c/openstack/nova/+/869536/ | |
| 11:41:36 | kashyap | Please see the commit message. We have already talked about it in detail before, and the rationale explains it there | |
| 11:41:47 | 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:42:07 | sean-k-mooney | kashyap: yep but you never convicend me it was right before | |
| 11:42:31 | 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:43:10 | 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:47 | opendevreview | Balazs Gibizer proposed openstack/nova stable/xena: Reproduce bug 1981813 in func env https://review.opendev.org/c/openstack/nova/+/859314 | |
| 11:43:48 | 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:44:06 | sean-k-mooney | gibi: yep proably however this was fixed in libvirt upstream with the intoduction fo new cpu models | |
| 11:44:19 | gibi | we tried the new API work that but is complicated enough that is stalls out | |
| 11:44:20 | sean-k-mooney | and i think we just have not backported that fix downsstream | |
| 11:48:18 | opendevreview | Manuel Bentele proposed openstack/nova master: libvirt: Add configuration options to set SPICE compression settings https://review.opendev.org/c/openstack/nova/+/828675 | |
| 12:00:50 | 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:01:33 | gibi | and we can only remove the WA flag after we resurrect https://review.opendev.org/c/openstack/nova/+/762330 and finish it? | |
| 12:02:31 | 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:03:23 | kashyap | sean-k-mooney: Also, libvirt developers themselves are suggesting that management tools should let libvirt do the work here. | |
| 12:03:33 | kashyap | I have already mentioned that in the commit message. If that doesn't convince you; nothing else will :) | |
| 12:03:44 | sean-k-mooney | kashyap: they may be experts in libvirt but not the managemnet tools | |
| 12:03:57 | sean-k-mooney | failures in live_migrate are expensive | |
| 12:04:25 | sean-k-mooney | we have already plugged the networkign and done other operatiosn on the destination that need to be roled back | |
| 12:04:32 | kashyap | sean-k-mooney: Well, I totally realize that as someone who debugs a crap ton of live migration problems | |
| 12:04:41 | sean-k-mooney | the reason we have the check early in pre-livemeigrate is to prevent that setup | |
| 12:05:07 | kashyap | sean-k-mooney: Well. I would prefer if you don't block this on theoretical objections or "ideally X" scenario | |
| 12:05:40 | kashyap | Also, on the destination, removing the API check is still correct. I put the workaround as a compromise. | |
| 12:07:15 | sean-k-mooney | we can agree to disagree | |
| 12:10:44 | kashyap | Sure. FWIW, I heard no complaints from the folks using the workaround to skip the check on dest. (As I expected.) | |
| 12:12:46 | 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:13:50 | sean-k-mooney | kashyap: libvirt adressetd that by adding a new cpu model without mpx https://gitlab.com/lvoytek/libvirt/-/commit/39f5bcd48323e814c910a92bf8ef76dd0166680d | |
| 12:14:42 | 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:59 | kashyap | I even linked to that patch discussion in my URLs | |
| 12:15:20 | sean-k-mooney | i see | |
| 12:16:01 | sean-k-mooney | apparently the cpu_model_extra_flags is not working properly to work around that by the way | |
| 12:16:03 | sean-k-mooney | https://bugzilla.redhat.com/show_bug.cgi?id=2158181 | |
| 12:16:11 | 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:22 | kashyap | sean-k-mooney: Indeed, that's because the model is evaluated _before_ the flags | |
| 12:16:46 | sean-k-mooney | i got that form https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1978064 | |
| 12:16:57 | sean-k-mooney | it was marked as fix released | |
| 12:17:15 | sean-k-mooney | kashyap: shoudl we not fix that then instead of removing the check | |
| 12:17:31 | sean-k-mooney | we should be validating the modifed one no? | |
| 12:17:45 | kashyap | sean-k-mooney: The problem here is entirely due the older API, hence the recommendetaion to remove that now-buggy check | |
| 12:18:08 | sean-k-mooney | that really feels like a regression/bug to remove it | |
| 12:18:30 | kashyap | Maybe they're carrying downstream Ubuntu-specific patch? | |
| 12:18:44 | sean-k-mooney | ya maybe im not sure | |
| 12:19:05 | sean-k-mooney | its confusing give i knwo libvirt uses bugzilla for trackign right | |
| 12:19:22 | sean-k-mooney | so this libvirt tracker is presumabel for the ubunu package | |
| 12:19:58 | kashyap | sean-k-mooney: Upstream libvirt uses gitlab now | |
| 12:20:24 | kashyap | (E.g. the "mpx" issue was discussed here, also filed by the Ubuntu person: https://gitlab.com/libvirt/libvirt/-/issues/304) | |
| 12:21:09 | kashyap | (Also notice that is a private libvirt branch that you linked to) | |
| 12:21:12 | sean-k-mooney | kashyap: so looking at https://gitlab.com/libvirt/libvirt/-/issues/304#note_1065798706 | |
| 12:21:24 | sean-k-mooney | do we use check=full or generate that check string today | |
| 12:21:45 | sean-k-mooney | this is not related to the start up check | |
| 12:22:02 | sean-k-mooney | but i think we leave that to libvirt to set in most if not all cases | |
| 12:23:03 | kashyap | sean-k-mooney: Here was the rationale that libvirt rejected that adding extra named model. Which I fully agree with: | |
| 12:23:09 | kashyap | [quote] | |
| 12:23:09 | kashyap | Adding a new CPU model is not that serious, but it's not good either as | |
| 12:23:10 | kashyap | it causes unnecessary compatibility issues with older versions of | |
| 12:23:10 | kashyap | libvirt. Especially adding a new CPU model which does not exist in QEMU | |
| 12:23:11 | kashyap | does not make any sense, as libvirt would need to translate it to | |
| 12:23:11 | kashyap | something else when starting QEMU. | |
| 12:23:14 | kashyap | [/quote] | |
| 12:23:16 | kashyap | https://listman.redhat.com/archives/libvir-list/2022-August/233717.html | |
| 12:24:01 | sean-k-mooney | ya that makes sense why they would not want to do that | |
| 12:24:23 | 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:26 | kashyap | Right. | |
| 12:24:38 | kashyap | sean-k-mooney: No, we don't use "check=full" | |
| 12:24:39 | sean-k-mooney | i think i would still prefer to move the cpu check to include the extra flags | |
| 12:24:50 | kashyap | (Answering the earlier question) | |
| 12:24:53 | sean-k-mooney | kashyap: ya i didnt think we did but wantted to confirm | |
| 12:25:11 | 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:16 | kashyap | That way we have clear evidence. | |
| 12:25:55 | sean-k-mooney | ok i need to go take my blood pressue medication and do a few bits. brb | |
| 12:26:08 | kashyap | Sure, take care! And thanks for the discussion | |
| 12:54:51 | zigo | Last night, we had a case of crash of nova when deleting a VM: | |
| 12:54:51 | zigo | https://paste.opendev.org/show/bRFOvaXxQxs5vVdwzVRV/ | |
| 12:54:51 | zigo | one colleague of mine says it's because the image was deleted first. Is he right? | |
| 12:55:20 | sean-k-mooney | no i dont think so | |
| 12:55:39 | sean-k-mooney | this is related to the nvram not the glance/vm disk image | |
| 12:55:43 | zigo | How come Nova can't "undefine domain with nvram" then? | |
| 12:56:39 | zigo | Was this fixed in a version higher than Victoria? | |
| 12:56:41 | sean-k-mooney | it might be related to https://bugs.launchpad.net/nova/+bug/1785123 | |
| 12:57:06 | zigo | Oh, thanks. | |
| 12:57:18 | sean-k-mooney | zigo: there have been some nvram/uefi issues fixed since then yes | |
| 12:57:52 | sean-k-mooney | https://review.opendev.org/q/project:openstack/nova+nvram | |
| 12:58:29 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/335512 that seams to be the most driectly related | |
| 12:59:29 | zigo | It's abandonned though ... :/ | |
| 12:59:35 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/guest.py#L298 | |
| 12:59:44 | sean-k-mooney | yes but the fix was condtionaly added on master | |
| 12:59:48 | sean-k-mooney | if uefi is supported | |
| 12:59:56 | sean-k-mooney | so we fixed it in a differnt patch | |