| opendevreview | Ghanshyam Maan proposed openstack/nova master: Add doc and release notes for the graceful shutdown task tracking https://review.opendev.org/c/openstack/nova/+/997141 | 00:42 |
|---|---|---|
| gmaan | dansmith: melwitt: graceful shutdown changes are ready for review https://review.opendev.org/q/topic:%22bp/nova-services-graceful-shutdown-part2-task-tracking%22 | 00:42 |
| opendevreview | yaguang proposed openstack/nova master: libvirt: enable Hyper-V evmcs on Intel VMX hosts https://review.opendev.org/c/openstack/nova/+/995620 | 02:50 |
| opendevreview | Masahito Muroi proposed openstack/nova master: Add bug 2106085 reproducer test case https://review.opendev.org/c/openstack/nova/+/971153 | 04:20 |
| opendevreview | Masahito Muroi proposed openstack/nova master: Add check_instance_state validation to the four action API https://review.opendev.org/c/openstack/nova/+/946223 | 04:20 |
| opendevreview | Bhavana proposed openstack/nova master: Add custom GPU PCI weigher for scheduler optimization Add Minimal Displacement Solver for MIG GPU Repartitioning https://review.opendev.org/c/openstack/nova/+/996058 | 04:37 |
| *** ralonsoh_ is now known as ralonsoh | 06:22 | |
| ralonsoh | hi folks, if you have a few minutes, please check https://review.opendev.org/c/openstack/os-vif/+/995933 | 07:26 |
| ralonsoh | dansmith, ^ thanks in advance | 07:27 |
| gibi | gmaan: thanks I will check your feedback | 07:58 |
| gibi | sean-k-mooney: elodilles: the oslo.concurrency 7.6.0 cross test failure turns out to be a real nova bug we now see but was hidden before https://bugs.launchpad.net/nova/+bug/2160596 Two VMs on the host using the same multi attach volume being deleted in parallel can result in danging volume attachments | 09:03 |
| gibi | as we are not serializing the deletion by volume id just by VM id | 09:03 |
| gibi | as the attachment delete (in the manager) is far from the volume disconnect check (in the driver) it is not easy to just add a new lock. | 09:04 |
| gibi | not dangling volume attachements, but dangling volume host mounts | 09:17 |
| elodilles | gibi: ah, ACK, thanks for the information | 09:36 |
| sean-k-mooney | gibi: hum i really tought i had properly fixed that | 09:46 |
| sean-k-mooney | gibi: did we regress it | 09:46 |
| sean-k-mooney | gibi: that what i was fixing in https://bugs.launchpad.net/nova/+bug/2048837 | 09:47 |
| sean-k-mooney | gibi: if this only happens with the new oslo.concurrency then its a but in oslo | 09:48 |
| sean-k-mooney | gibi: i added per volume locking in https://opendev.org/openstack/nova/commit/72132f89ee4ca7367a6a5639f763f9b20d2c441a | 09:49 |
| sean-k-mooney | sorry in https://opendev.org/openstack/nova/commit/22012360c40045ac1bf6dd0dc95aea3e15fed0cd | 09:50 |
| gibi | I have to look deeper as my simple reproduction does not work without the oslo change so something important changed in oslo | 09:52 |
| gibi | the only real change is oslo.concurrency is eventlet related | 10:20 |
| gibi | reverting https://review.opendev.org/c/openstack/oslo.concurrency/+/988652 stops my reproduction to recreate the issue | 10:23 |
| sean-k-mooney | hum i looked at that and discounted it | 10:24 |
| sean-k-mooney | i kwno that eventlet has import order dependent behvior | 10:25 |
| sean-k-mooney | i dont know why that would break locking | 10:25 |
| gibi | yeah me neither but I will dig further | 10:26 |
| sean-k-mooney | espically when we are not chanign the patching | 10:26 |
| sean-k-mooney | gibi: does the cross job run wiht our without monkey patching? | 10:28 |
| sean-k-mooney | i assume with eventlet for now | 10:28 |
| sean-k-mooney | since that stil technially our default | 10:28 |
| sean-k-mooney | i think this this early out https://review.opendev.org/c/openstack/oslo.concurrency/+/988652/1/oslo_concurrency/lockutils.py#111 | 10:30 |
| sean-k-mooney | that is shared across all locks | 10:30 |
| sean-k-mooney | but https://review.opendev.org/c/openstack/oslo.concurrency/+/988652/1/oslo_concurrency/lockutils.py#131 | 10:30 |
| sean-k-mooney | need to run for all rwlocks when we are patched | 10:30 |
| sean-k-mooney | withihs chagne it will hapen for the first lock but not the rest | 10:31 |
| sean-k-mooney | so _EVENTLET_CHECKED should just be removed | 10:31 |
| gibi | it is eventlet for now | 10:31 |
| sean-k-mooney | the refactor i think is fine it the _EVENTLET_CHECKED caching that i think broke things | 10:32 |
| gibi | you mean the current thread thing breaks our multi threahdead testing | 10:32 |
| gibi | *do you mean? | 10:32 |
| sean-k-mooney | the _EVENTLET_CHECKED early out reintoduced https://github.com/eventlet/eventlet/issues/731 | 10:33 |
| gibi | ack, testing it... | 10:33 |
| gibi | yepp commenting out https://review.opendev.org/c/openstack/oslo.concurrency/+/988652/1/oslo_concurrency/lockutils.py#135 makes it not possible the reproduce the problem | 10:38 |
| gibi | switching the bug to oslo.concurrency... | 10:38 |
| sean-k-mooney | ya so the early out i thinkwas an attempt to not import it over and over | 10:39 |
| sean-k-mooney | but python caches import so its cheap | 10:39 |
| sean-k-mooney | if we want to cache somethin we coudl cache the result of eventlet_patcher.is_monkey_patched('thread') | 10:39 |
| sean-k-mooney | but i would just delete it | 10:39 |
| sean-k-mooney | we dont need the extra complications of _EVENTLET_CHECKED | 10:40 |
| sean-k-mooney | brb coffee | 10:40 |
| gibi | I need to craft a oslo.concurrency level of test for this to avoid future regressions | 10:44 |
| sean-k-mooney | in this case just create 2 rwlocks when eventlet is monky patched and assert both have the same method fo self._current_thread | 11:05 |
| sean-k-mooney | a better test might be to test the side effect | 11:07 |
| sean-k-mooney | i.e. that they both agree on the current thread id | 11:08 |
| gibi | yeah I want the latter as that does not depend on the implementation | 11:15 |
| gibi | hm | 11:15 |
| gibi | not event that | 11:15 |
| gibi | I want a test that shows the mutual exlusiveness of a write lock | 11:15 |
| sean-k-mooney | hum ya that is higher level and less coupled | 11:16 |
| sean-k-mooney | well | 11:16 |
| sean-k-mooney | 2 locks are indpemendent | 11:17 |
| sean-k-mooney | so its not as simple as aquire lock a and lock b and try to access somethign shared | 11:17 |
| sean-k-mooney | you need to create 2 locks | 11:18 |
| sean-k-mooney | and aquire both in 2 diffent eventsletlest or somthing | 11:18 |
| sean-k-mooney | ideally with less infram then nova test case | 11:18 |
| gibi | hm. isn't it the second lock I create will not work if used from two different greenlet? | 11:19 |
| sean-k-mooney | its not really that you need two locks | 11:20 |
| sean-k-mooney | in this case it that the second time any RW lock was inited the relevent code was skipped | 11:20 |
| gibi | yepp | 11:20 |
| gibi | so the second lock is broken when shared across greenlets | 11:20 |
| gibi | that is my theory | 11:20 |
| sean-k-mooney | so for the workers you can just use 1 lock and a thread.event | 11:21 |
| sean-k-mooney | gibi: yes thre is a rentrency issue | 11:21 |
| sean-k-mooney | in that when shared the second lock seas the real treadid | 11:21 |
| sean-k-mooney | not the evnetlet greenthread id | 11:21 |
| sean-k-mooney | so it goes one im on the same trhead where i aquired the lock and it races | 11:22 |
| sean-k-mooney | i think its breaking https://github.com/openstack/nova/blob/master/nova/utils.py#L1233 | 11:23 |
| sean-k-mooney | hum although maybe not | 11:24 |
| sean-k-mooney | sinc ei have test for that and those pass | 11:24 |
| sean-k-mooney | it was only one test that actully failed right | 11:24 |
| sean-k-mooney | so its not any fo the simple test case i am already testing | 11:24 |
| sean-k-mooney | im going to stop guessing if you come up with a test that works let me know and i can take a look | 11:26 |
| gibi | sure I'm working on it | 11:31 |
| gibi | it is a bit more complicated due to https://github.com/openstack/oslo.concurrency/blob/9aff32494fb97b7fc7508269224353aceb9ceef6/oslo_concurrency/lockutils.py#L101-L103 | 11:31 |
| gibi | so they are also passing threading.current_thread to the parent | 11:32 |
| gibi | sorry I ment https://github.com/openstack/oslo.concurrency/blob/9aff32494fb97b7fc7508269224353aceb9ceef6/oslo_concurrency/lockutils.py#L101-L108 | 11:32 |
| sean-k-mooney | ya i was looking at that functor | 11:33 |
| sean-k-mooney | threading.current_thread should be monkeypatch to eventlet.getcurrent in general before we hit this code | 11:34 |
| sean-k-mooney | if we are usign eventlet | 11:34 |
| sean-k-mooney | https://github.com/openstack/oslo.concurrency/commit/02dee958453513bb8a5aaae905312d306c2d9083#diff-60d0bce7c34c2c70cf4e3ac63060779235ede2a247bd39d1188f8fedd6306b38R109 | 11:35 |
| sean-k-mooney | so that defaulting was added in the typeing patch | 11:35 |
| gibi | ahh I see it, I need to use spawn_n not spawn to reproduce it | 11:36 |
| sean-k-mooney | and it proably should not be | 11:36 |
| sean-k-mooney | oh | 11:36 |
| gibi | the original eventlet issue is only affecting spawn_n | 11:36 |
| gibi | not spawn | 11:36 |
| gibi | thanks melwitt for the good bug report | 11:36 |
| sean-k-mooney | i know that changes between a raw greenlet and a greenthread | 11:37 |
| sean-k-mooney | is that really the level where it broke | 11:37 |
| gibi | yepp | 11:38 |
| sean-k-mooney | i tought i had a patch to remove all useage of spawn_n at one point | 11:38 |
| gibi | that is a good point ^^ we need to look | 11:38 |
| sean-k-mooney | i am :) | 11:38 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/892722 | 11:39 |
| sean-k-mooney | so yes i did but we went with https://review.opendev.org/c/openstack/nova/+/873061 | 11:40 |
| sean-k-mooney | instead | 11:40 |
| sean-k-mooney | it would still be an oslo bug | 11:40 |
| sean-k-mooney | but ya we can likely simplify now | 11:41 |
| sean-k-mooney | oh we already have | 11:42 |
| sean-k-mooney | spwan is now a call to spawn on with the default executor | 11:42 |
| sean-k-mooney | but i bet the futurist executor might be using spwan_n somewhere along the way | 11:43 |
| sean-k-mooney | we have a hacking check to prevent raw usage of spawn and spwan_n https://github.com/openstack/nova/commit/6179854ae77960d4ae883349626069023b407cb0 | 11:44 |
| sean-k-mooney | well and eventlet in genreal so any usage is likely not in nova | 11:45 |
| sean-k-mooney | gibi: yep https://github.com/openstack/futurist/blob/6f870896819197ca323b3dd8af8f3b0389aff69c/futurist/_futures.py#L718 | 11:46 |
| sean-k-mooney | gibi: so the GreenThreadPoolExecotor uses spwan_n whne it spins up new workers | 11:46 |
| gibi | I will be so happy when we finally delete eventlet :) | 11:47 |
| sean-k-mooney | hehe so that litrally made me lol | 11:47 |
| sean-k-mooney | but i agree | 11:47 |
| sean-k-mooney | gibi: that can help you create a test case but i assume you already have one now | 11:48 |
| opendevreview | sean mooney proposed openstack/nova stable/2025.2: Make QEMU_IMG_LIMITS process limits configurable https://review.opendev.org/c/openstack/nova/+/997028 | 11:53 |
| opendevreview | sean mooney proposed openstack/nova stable/2025.1: Make QEMU_IMG_LIMITS process limits configurable https://review.opendev.org/c/openstack/nova/+/997029 | 11:54 |
| gibi | sean-k-mooney: here is the repro and the fix https://review.opendev.org/c/openstack/oslo.concurrency/+/997200 | 12:02 |
| sean-k-mooney | oh thats the fix :) | 12:05 |
| sean-k-mooney | i was like wait they alrady have a bug for this | 12:05 |
| sean-k-mooney | i.e. reading "# This is bug https://bugs.launchpad.net/oslo.concurrency/+bug/2160596" | 12:05 |
| gibi | :) | 12:06 |
| gibi | sean-k-mooney: thanks for the help root causing this issue | 12:07 |
| sean-k-mooney | no worries i wrote the test that was beign broken so i was interested in the resolution | 12:07 |
| sean-k-mooney | i tried really hard to make the FairLockGuard robust | 12:08 |
| sean-k-mooney | so i was concerned i missed somthing | 12:08 |
| gibi | sean-k-mooney: yeah all that effort is in vain if eventlet breaks you | 12:08 |
| sean-k-mooney | minior nit in the commit messge of the second patch | 12:17 |
| sean-k-mooney | but the logic in both looks good to me | 12:18 |
| gibi | fixed | 12:18 |
| sean-k-mooney | :) | 12:20 |
| opendevreview | Joan Gilabert proposed openstack/nova master: Report OWNER_NOVA trait on resource providers https://review.opendev.org/c/openstack/nova/+/994299 | 12:59 |
| opendevreview | Joan Gilabert proposed openstack/nova master: Add libvirt driver MDEV ARQ support for Cyborg https://review.opendev.org/c/openstack/nova/+/995840 | 12:59 |
| opendevreview | Merged openstack/nova master: Reproducer for https://bugs.launchpad.net/nova/+bug/2160224 https://review.opendev.org/c/openstack/nova/+/996756 | 13:10 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Do not try to return empty attachment_id from API https://review.opendev.org/c/openstack/nova/+/996757 | 13:49 |
| gibi | gmaan: sean-k-mooney: ^^ this is now ready to land ;) | 13:52 |
| gibi | sean-k-mooney: also if you have a sec we have this small fix https://review.opendev.org/c/openstack/nova/+/996757/1 | 13:53 |
| sean-k-mooney | isnt that te same link | 13:54 |
| gibi | then when these are landed I will propose a patch to make the functional-py313-threading patch as voting | 13:54 |
| gibi | https://review.opendev.org/c/openstack/nova/+/996793 this is the correct link for the small fix | 13:54 |
| sean-k-mooney | :) | 13:54 |
| gibi | (copy paste is hard as you see) | 13:54 |
| sean-k-mooney | sure ill look at both now | 13:54 |
| gibi | thanks | 13:54 |
| sean-k-mooney | +2w on the fix the cyborg irc call is starting nowish so ill see if i can review the attachmet id one before then | 13:56 |
| gibi | OK | 13:56 |
| sean-k-mooney | ach cool you addeed the return code checks | 13:56 |
| sean-k-mooney | well you added them as new tests | 13:57 |
| sean-k-mooney | but ya i think thats ok | 13:57 |
| sean-k-mooney | gibi: +2 on that too i dont really have any feedback to add so if gmaan is happy to proceed then cool | 14:04 |
| sean-k-mooney | irc meting just started but are you plannning to follow up with the other bug to make repsonce validation return a 500 | 14:05 |
| gibi | I will take a look on that bug too yes | 14:06 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Make functional-py313-threading voting https://review.opendev.org/c/openstack/nova/+/997216 | 14:12 |
| gibi | sambork: am I correct that this is still valid as you only fixed something in vncproxy especially? https://bugs.launchpad.net/nova/+bug/2151537 | 14:13 |
| sambork | gibi I think so because my fix was only in proxies | 14:16 |
| sambork | but I can take a look and apply similar fix if it's applies also to this issue | 14:17 |
| gibi | sambork: so far I only know it is a pickling issue but not more | 14:17 |
| gibi | we don't need to rush, if you have time you can take it I'm not actively working on it. I just did a quick scrub of eventlet-removal bugs as some was outdated and I noticed this | 14:18 |
| sambork | ack! | 14:18 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Stabilize test_create_images_negative_invalid_state https://review.opendev.org/c/openstack/nova/+/997222 | 15:16 |
| opendevreview | Merged openstack/nova stable/2026.1: Make QEMU_IMG_LIMITS process limits configurable https://review.opendev.org/c/openstack/nova/+/997027 | 15:22 |
| opendevreview | Mike Lowe proposed openstack/nova stable/2025.1: libvirt: Fix false mem-encryption/UEFI conflict with cpu_mode=custom https://review.opendev.org/c/openstack/nova/+/997258 | 17:39 |
| opendevreview | Mike Lowe proposed openstack/nova master: libvirt: Fix false mem-encryption/UEFI conflict with cpu_mode=custom https://review.opendev.org/c/openstack/nova/+/997259 | 17:42 |
| gmaan | gibi: sean-k-mooney lgtm, +w | 18:14 |
| sean-k-mooney | gmaan: you may also be intrested in https://review.opendev.org/c/openstack/nova/+/971153/2 and the fix https://review.opendev.org/c/openstack/nova/+/946223/6 | 18:20 |
| sean-k-mooney | there is a window where invoking instnace actions can reulst in a 500 before the inital server create is complete | 18:20 |
| sean-k-mooney | those repoduce and adress that and call out that precondtion more explcitly | 18:21 |
| gmaan | sean-k-mooney: ack, will check sometime today otherwise tomorrow | 18:21 |
| sean-k-mooney | the repoducer need a littel work so no rush | 18:21 |
| sean-k-mooney | the fix is more or less correct but ya just good to add to yoru queue | 18:21 |
| gmaan | sure | 18:23 |
| opendevreview | Fabian Wiesel proposed openstack/nova master: vmware: optionally combine host+datastore inventory query https://review.opendev.org/c/openstack/nova/+/997283 | 18:28 |
| opendevreview | sean mooney proposed openstack/os-vif master: typing: Adapt to oslo.versionedobjects 3.11 https://review.opendev.org/c/openstack/os-vif/+/997289 | 18:34 |
| opendevreview | sean mooney proposed openstack/os-vif master: typing: Adapt to oslo.versionedobjects 3.11 https://review.opendev.org/c/openstack/os-vif/+/997289 | 19:51 |
| opendevreview | sean mooney proposed openstack/os-vif master: typing: Define supported OVS VIF types https://review.opendev.org/c/openstack/os-vif/+/997296 | 19:51 |
| opendevreview | sean mooney proposed openstack/os-vif master: ovs: Preserve versioned field compatibility https://review.opendev.org/c/openstack/os-vif/+/997297 | 19:51 |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!