| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-09-02 | |||
| 11:29:58 | sean-k-mooney | stephenfin: if you want to go for it | |
| 11:30:10 | sean-k-mooney | i havent fully pasrced what you wrote | |
| 11:30:22 | sean-k-mooney | are you concerned about the other places you have used that | |
| 11:30:35 | sean-k-mooney | i noticed you did not add that excption in the first patch | |
| 11:30:54 | sean-k-mooney | unless i missed it | |
| 11:31:15 | stephenfin | I'm concerned about the other places _others_ have used it | |
| 11:31:41 | sean-k-mooney | ah ok ya i tought about that but didnt have a chance to look | |
| 11:32:45 | sean-k-mooney | stephenfin: you could also just make that a follow up change that change the base code | |
| 11:48:06 | opendevreview | Stephen Finucane proposed openstack/nova master: Add 'hw:vif_multiqueue_enabled' flavor extra spec https://review.opendev.org/c/openstack/nova/+/792356 | |
| 11:48:07 | opendevreview | Stephen Finucane proposed openstack/nova master: Move 'hw:pmu', 'hw_pmu' parsing to nova.virt.hardware https://review.opendev.org/c/openstack/nova/+/792364 | |
| 11:48:07 | opendevreview | Stephen Finucane proposed openstack/nova master: docs: Document virtio-net multiqueue https://review.opendev.org/c/openstack/nova/+/792362 | |
| 11:48:29 | stephenfin | sean-k-mooney: Okay, new comments left. I've respin it to drop that code setting on the back of said comments. Let me know if I missed anything obvious | |
| 11:48:39 | sean-k-mooney | stephenfin: so https://github.com/openstack/nova/blob/75f719e0/nova/api/openstack/compute/servers.py#L854-L855 will still change the code form a 403 to a 400 right | |
| 11:49:04 | sean-k-mooney | so that is still not correct? | |
| 11:49:12 | stephenfin | It will result in a HTTP 400. Where are you getting HTTP 403 from? | |
| 11:49:27 | sean-k-mooney | the orginial expciton inherited form forbiden | |
| 11:49:32 | sean-k-mooney | which returned a 403 | |
| 11:49:49 | stephenfin | which original exception? | |
| 11:51:02 | sean-k-mooney | this one | |
| 11:51:04 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/792364/8/nova/exception.py | |
| 11:51:06 | sean-k-mooney | in the followup | |
| 11:51:20 | stephenfin | sean-k-mooney: that's a different review | |
| 11:51:29 | sean-k-mooney | yes | |
| 11:51:45 | sean-k-mooney | hum | |
| 11:51:52 | stephenfin | ohhhhh | |
| 11:51:54 | sean-k-mooney | ok its also in INVALID_FLAVOR_IMAGE_EXCEPTIONS | |
| 11:52:01 | sean-k-mooney | so it was getting converted too | |
| 11:52:08 | sean-k-mooney | ok it should be good | |
| 11:52:17 | stephenfin | so because we were introducing this new helper in that change, you suggested changing it there | |
| 11:52:26 | stephenfin | that was not at all obvious :-D context, lad | |
| 11:52:45 | stephenfin | anyway, yeah, it was all getting translated to HTTP 400 in the end | |
| 11:52:54 | stephenfin | and even if it wasn't, this would be an okay change IMO | |
| 11:53:06 | sean-k-mooney | to a 400 | |
| 11:53:13 | sean-k-mooney | yes to a 500 no | |
| 11:53:26 | stephenfin | yup, fair | |
| 11:53:28 | sean-k-mooney | anyway ok it should be good again | |
| 11:53:34 | stephenfin | yup, should be | |
| 11:54:50 | sean-k-mooney | and no i was chaning the code in hte bottom change becasue i tought it was not being caught there | |
| 11:55:08 | stephenfin | gotcha | |
| 11:55:53 | sean-k-mooney | we have a top level excption handeler above that that catuche anything that inherits form NovaException | |
| 11:56:11 | sean-k-mooney | that is not caught else whwere i tought we were going all the way to that | |
| 11:56:31 | stephenfin | The whole nova.exception module needs a rework to either double down on exception inheritance or remove it entirely. It's all very confusing. That rework would be seriously tedious though and I doubt anyone sane will want to do it :) | |
| 11:57:07 | sean-k-mooney | ya espcially since we marshal between them in a few places too | |
| 11:57:16 | stephenfin | yup | |
| 11:57:36 | opendevreview | Balazs Gibizer proposed openstack/nova master: Add --sleep option for archive_deleted_rows --until-complete https://review.opendev.org/c/openstack/nova/+/771749 | |
| 12:10:07 | kashyap | sean-k-mooney: Heya, are you able to tell what's off with this negative test (only remaining failure from the CI for me) - It passed when I posted the patch locally; https://cpaste.org/?e49aa4ae738ca284#5xdtN3VCgvzp3P5fFPXDmYJ8gYxVRQvuGaKyfxsRjYfb | |
| 12:11:02 | sean-k-mooney | do you have a link to the review | |
| 12:12:00 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/794796? | |
| 12:12:04 | kashyap | Yep | |
| 12:12:11 | kashyap | Sorry, was just about to fetch it | |
| 12:12:30 | sean-k-mooney | simple | |
| 12:12:45 | sean-k-mooney | 1.30 is not less then 1.30 https://review.opendev.org/c/openstack/nova/+/794796/3/nova/objects/image_meta.py#190 | |
| 12:12:46 | kashyap | (I fixed the trivial notification object part; so ignore that test, pls) | |
| 12:12:59 | sean-k-mooney | so self.assertRaises(exception.ObjectActionError, | |
| 12:13:01 | sean-k-mooney | obj.obj_to_primitive, '1.30') | |
| 12:13:05 | sean-k-mooney | shoudl ne '1.29' | |
| 12:13:47 | kashyap | sean-k-mooney: Urgh, <embarassed> :D | |
| 12:14:55 | kashyap | sean-k-mooney: Thanks! | |
| 12:15:03 | sean-k-mooney | There are two hard things in computer science: cache invalidation, naming things, and off-by-one errors. | |
| 12:15:19 | kashyap | sean-k-mooney: LOL | |
| 12:16:55 | opendevreview | Merged openstack/nova master: workarounds: Remove rbd_volume_local_attach https://review.opendev.org/c/openstack/nova/+/805648 | |
| 12:18:33 | opendevreview | Federico Ressi proposed openstack/nova master: [WIP] Debug APIs call failures https://review.opendev.org/c/openstack/nova/+/806683 | |
| 12:18:57 | opendevreview | Federico Ressi proposed openstack/nova master: Debug Nova APIs call failures https://review.opendev.org/c/openstack/nova/+/806683 | |
| 12:27:06 | opendevreview | Federico Ressi proposed openstack/nova master: [WIP] Check Nova project changes with tobiko scenario test cases https://review.opendev.org/c/openstack/nova/+/806853 | |
| 12:31:28 | opendevreview | Federico Ressi proposed openstack/nova master: Check Nova project changes with Tobiko scenario test cases https://review.opendev.org/c/openstack/nova/+/806853 | |
| 12:40:55 | opendevreview | Kashyap Chamarthy proposed openstack/nova master: Allow 'bochs' as a display device option https://review.opendev.org/c/openstack/nova/+/794796 | |
| 12:51:54 | gibi | sean-k-mooney: :D | |
| 13:13:06 | opendevreview | Takashi Natsume proposed openstack/nova master: Update compute rpc version alias for xena https://review.opendev.org/c/openstack/nova/+/807137 | |
| 13:20:54 | kashyap | sean-k-mooney: Oh, duh. In my revision I missed the extra test that stephenfin asked above. Adding that too | |
| 13:50:13 | opendevreview | Kevin Zhao proposed openstack/nova master: Support Cpu Compararion on Aarch64 Platform https://review.opendev.org/c/openstack/nova/+/763928 | |
| 14:00:17 | opendevreview | Kashyap Chamarthy proposed openstack/nova master: Allow 'bochs' as a display device option https://review.opendev.org/c/openstack/nova/+/794796 | |
| 14:00:59 | kashyap | stephenfin: Fixed --^ That should address all you asked for. Let me know if I missed anything. And thanks for the review. | |
| 14:04:14 | opendevreview | Balazs Gibizer proposed openstack/placement master: Reproduce 404 when allocation queried with 1.38 https://review.opendev.org/c/openstack/placement/+/807154 | |
| 14:04:21 | opendevreview | Balazs Gibizer proposed openstack/placement master: Fix adding 'unknown' to the ConsumerTypeCache https://review.opendev.org/c/openstack/placement/+/807155 | |
| 14:14:57 | gibi | lyarwood, stephenfin, bauzas: can I get a second set of eyes on https://review.opendev.org/c/openstack/osc-placement/+/804458 (consumer_types osc-placement client support) | |
| 14:15:55 | bauzas | gibi: doing it now | |
| 14:17:06 | gibi | thanks you | |
| 14:24:54 | kashyap | gibi: WHen you get 20 minutes, mind have a look at this? - https://review.opendev.org/c/openstack/nova/+/763928 | |
| 14:31:58 | opendevreview | Merged openstack/osc-placement master: Add support for microversion 1.38 consumer types https://review.opendev.org/c/openstack/osc-placement/+/804458 | |
| 14:49:52 | gibi | kashyap: are you OK with that patch? | |
| 15:01:56 | kashyap | gibi: 80%; I just want to do a real migration test w/ that patch - that gives me more peace of mind. (I wanted to split it up for better reviewability but just couldn't get around to it.) | |
| 15:02:33 | kashyap | gibi: How about this: | |
| 15:03:02 | gibi | kashyap: I have no problem with the patch, as far as see it is specialized to arm64 so we cannot broke our main deployers with it :) | |
| 15:03:04 | kashyap | gibi: I'll setup a local env to do a bunch more tests on KVM (our CI tests only QEMU) | |
| 15:03:11 | kashyap | gibi: Oh, wait | |
| 15:03:24 | kashyap | gibi: I gave you the wrong patch; damn | |
| 15:03:25 | gibi | or you linked me something else :D | |
| 15:03:35 | kashyap | gibi: Yeah, grrrr me. | |
| 15:03:39 | gibi | no worries | |
| 15:03:47 | kashyap | I meant this chunky one (hence the "20 minutes") - https://review.opendev.org/c/openstack/nova/+/763928 | |
| 15:04:10 | kashyap | Feel free to say: "no, that is too much review at this point". | |
| 15:04:27 | kashyap | But I can answer any questions about that patch. | |
| 15:04:56 | kashyap | gibi: Incidentally, the commit message from the patch author is a verbatim copy/paste of what I wrote in one of the reviews :D | |
| 15:05:15 | gibi | :) | |
| 15:05:54 | gibi | you linked the wrong patch again https://review.opendev.org/c/openstack/nova/+/763928 | |
| 15:05:57 | kashyap | gibi: The short summary is - this patch will be replacing the two older libvirt CPU APIs that are now causing more problems in the new world of hardware CVE flaws | |
| 15:06:11 | kashyap | gibi: Gah, sorry, this one - https://review.opendev.org/c/openstack/nova/+/762330/ | |
| 15:06:19 | kashyap | "libvirt: Use improved guest CPU config APIs" | |
| 15:06:47 | gibi | ok that is chunky indeed :) | |
| 15:06:49 | kashyap | gibi: It's a chunky one. Actually, this hour is not good to review it. And will only make you grumpy, I'm afraid | |