| *** mhen_ is now known as mhen | 03:00 | |
| rm_work | sean-k-mooney: So the issue I guess was that A) it doesn't get everything, only things set to be "cloud-init", and then also it doesn't accept JSON, it has to be a pure string, which is kinda dumb but it "works", I just `json.dumps` my json before I send it and have to load it up on the other side T_T | 03:03 |
|---|---|---|
| rm_work | but did get it working finally. like... an hour ago lol | 03:03 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Update api-ref for server create https://review.opendev.org/c/openstack/nova/+/937534 | 05:41 |
| rm_work | It’s weird that “DynamicJSON” type vendor data endpoint can’t return JSON 😅 | 06:15 |
| rm_work | But will cloud-init run code if I return a code block the same way as user data? | 06:16 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Update api-ref for server create https://review.opendev.org/c/openstack/nova/+/937534 | 06:25 |
| jkulik | sean-k-mooney: if you have some time left, https://review.opendev.org/c/openstack/nova/+/699176 got updated again (faults from cell DB) to address your comments. if not, happy holidays :) | 09:39 |
| *** Jeff is now known as Guest33849 | 09:51 | |
| sean-k-mooney | jkulik: +2 from me but im not sure if melwitt or gibi will have time to review before the go on pto for the year | 11:30 |
| jkulik | thank you. that's fine, I'll keep on bugging people next year then. but if it looks finished to you, that's already very helpful feedback :) | 11:31 |
| sean-k-mooney | we can always ad dmore testing liek fucnitonal test ectra but i think what you have proesed is good enough and i think it sovles the orginal reported porblem without changing things unessisarlly | 11:33 |
| sean-k-mooney | so to me it looks good as a single atomic commit that is self contained and backportable | 11:33 |
| opendevreview | huanhongda proposed openstack/nova master: WIP: NUMA fit instance to host with cpu/ram_allocation_ratio https://review.opendev.org/c/openstack/nova/+/971409 | 12:55 |
| opendevreview | huanhongda proposed openstack/nova master: WIP: NUMA fit instance to host with cpu/ram_allocation_ratio https://review.opendev.org/c/openstack/nova/+/971409 | 12:59 |
| opendevreview | Dmitriy Rabotyagov proposed openstack/nova-specs master: Add 2026.2 spec structure https://review.opendev.org/c/openstack/nova-specs/+/971413 | 13:46 |
| gibi | Uggla: gmaan: FYI I filed the bug for the py312-threading intermittent failure I mentioned yesterday https://bugs.launchpad.net/nova/+bug/2136815 I have a good idea now why it is happening so I will probably able to fix it. | 14:04 |
| Uggla | gibi 👍 | 14:05 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: [DNM]Show issue with loopingcall in threading mode https://review.opendev.org/c/openstack/nova/+/971431 | 14:41 |
| stephenfin | gibi: random question on https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L2680-L2685 | 14:59 |
| stephenfin | that assumes the device has an alias. Will devices always have an alias? | 14:59 |
| stephenfin | I ask because we have tests validating behavior of functions with devices with and without the alias attribute, e.g. https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_driver.py#L10725 | 15:01 |
| stephenfin | I don't know if those are valid tests or not | 15:01 |
| gibi | stephenfin: I assuem that libvirt always assign alias as that is what coming back from libvirt in the event. Nova might or might not explicitly set the alias | 15:02 |
| stephenfin | Yes, that's my thinking also (and my experience from brief local testing) | 15:03 |
| gibi | i think that test covers the case when nova request the detach by using an alias or by not using the alias | 15:03 |
| gibi | that is independent from the device having an alias in libvirt | 15:04 |
| stephenfin | right, but will be ever attempt to detach without using an alias, given an alias is always present? | 15:04 |
| stephenfin | and if not, does that mean that code is dead code? | 15:04 |
| stephenfin | fwiw, the context of this a rework of the mypy configuration that I'm working on: mypy is complaining because create_waiter expects the dev.alias arg to be a string (which I think is correct) but our tests explicitly set it to null is some places | 15:06 |
| gibi | this is where the two codepath is coming from https://github.com/openstack/nova/blob/3e5a2dbad73ff61d27db8750480e2c0b52133676/nova/virt/libvirt/driver.py#L2818-L2825 | 15:13 |
| gibi | so when the detach checks that the device is really gone it uses a callable to query the device from libvirt, and that callable has two modes for disks | 15:14 |
| gibi | so the question is do we ever create a partiall callable from _get_guest_disk_device in a way that volume_uuid is not provided | 15:15 |
| gibi | and passing that to detach | 15:15 |
| gibi | https://github.com/openstack/nova/blob/3e5a2dbad73ff61d27db8750480e2c0b52133676/nova/virt/libvirt/driver.py#L2856-L2864 | 15:16 |
| gibi | if volume_uuid is not None here then we will go with the alias path for sure during detach | 15:16 |
| gibi | unfortunately the code might return non for that volume id above | 15:17 |
| gibi | https://github.com/openstack/nova/blob/3e5a2dbad73ff61d27db8750480e2c0b52133676/nova/virt/block_device.py#L1031 | 15:17 |
| gibi | it feels strange that we don't have a connectin_info for a disk we want to detach, but I have no proof that we always have | 15:19 |
| gibi | so meh | 15:19 |
| stephenfin | sorry, got distracted by CI woes elsewhere /o\ | 15:43 |
| stephenfin | I suspect libvirt might auto-generate aliases (without the ua- prefix) if a user doesn't provide one. That would explain why we need the ua- prefix in the first place. Just trying to find the smoking gun for that | 15:46 |
| stephenfin | If so, I think that instead of not setting an alias, tests like that should be setting a non-ua prefixed alias | 15:47 |
| gibi | the problem in my eyes that the code might use a non-alias based device lookup that depends on if the volume id is available for the disk being detached or not | 15:50 |
| gibi | so to get rid of the extra codepath you would need to ensure that the code always uses an alias and not a device path for lookup | 15:51 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Do not mock threading.Event.wait https://review.opendev.org/c/openstack/nova/+/971448 | 15:51 |
| gibi | Uggla: gmaan: ^^ that is the fix for the py312-threading instability | 15:51 |
| gibi | I will push some followups as well to prevent it to happen again | 15:52 |
| gmaan | gibi: ack. will check | 16:23 |
| stephenfin | gibi: https://github.com/libvirt/libvirt/blob/dad8b0fc527b37e072d8baaace45ae6a29394f27/src/qemu/qemu_alias.c#L692-L786 | 16:25 |
| stephenfin | devices will *always* have an alias | 16:25 |
| stephenfin | but that may or may not be user-defined | 16:25 |
| stephenfin | ...and has done so for a very long time (I went back as far as 2010 and decided that was good enough :)) | 16:28 |
| opendevreview | Stephen Finucane proposed openstack/nova master: trivial: Use functools.wraps https://review.opendev.org/c/openstack/nova/+/971450 | 16:42 |
| opendevreview | Stephen Finucane proposed openstack/nova master: tests: Set disk alias in libvirt fixture https://review.opendev.org/c/openstack/nova/+/971451 | 16:42 |
| opendevreview | Stephen Finucane proposed openstack/nova master: tests: Always set device aliases https://review.opendev.org/c/openstack/nova/+/971452 | 16:42 |
| stephenfin | and there are fixes for tests and a runtime check so we fail early | 16:43 |
| gibi | stephenfin: if the device's alias is not set by nova, can nova still know that alias and can use it to look up the device? | 16:50 |
| stephenfin | that I don't know, so I have touched the detach code otherwise | 16:52 |
| stephenfin | I just know that we need *an* alias for the device event handler, and that we will always *an* alias | 16:53 |
| gibi | OK so you are not removing the fallback code path. Then I think it is OK | 16:53 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: [hacking]Do not mock threading.Event.wait https://review.opendev.org/c/openstack/nova/+/971454 | 16:53 |
| opendevreview | Merged openstack/nova master: Do not mock threading.Event.wait https://review.opendev.org/c/openstack/nova/+/971448 | 17:23 |
| gmaan | gibi: +1 on hacking rule, but it seems it needs to be fixed in other places also and we should see how much time it increase unless you add some timeout like you did for detach | 17:33 |
| gibi | gmaan: yepp I'm on it | 17:34 |
| gmaan | k | 17:34 |
| gibi | fixed most of it locally there is one that needs more attention tomorrow | 17:34 |
| gmaan | ++ | 17:35 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: [hacking]Do not mock threading.Event.wait https://review.opendev.org/c/openstack/nova/+/971454 | 17:40 |
| gibi | ^^ I pushed it but it still has one functional test that needs a fix now | 17:40 |
| gibi | anyhow that is all from me today. o/ | 17:40 |
| gmaan | k, o/ | 17:41 |
| sean-k-mooney | gibi: is that the cause fo the race? | 17:45 |
| sean-k-mooney | by the way i have not mention my ai code review ci job much lately | 17:45 |
| sean-k-mooney | but one of the thing im plannign to impelment next year is making it read and follow the guidnce in HACKING.rst if that file exsits in a repo | 17:46 |
| opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Ensure device alias is present https://review.opendev.org/c/openstack/nova/+/971452 | 17:52 |
| opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Remove import hacks https://review.opendev.org/c/openstack/nova/+/971460 | 17:59 |
| JayF | If folks have time, https://review.opendev.org/c/openstack/nova/+/969321 could still use a review. Follow-up for a previous bugfix. | 18:10 |
| *** haleyb is now known as haleyb|out | 21:34 | |
| opendevreview | Merged openstack/nova master: Remove tpool_execute as it is unused https://review.opendev.org/c/openstack/nova/+/966552 | 22:39 |
| sean-k-mooney | gmaan: stephenfin i finsihed the openapi serice. i think they are mostly all readytto merge. | 23:37 |
| sean-k-mooney | im finsihing for the year now | 23:38 |
| sean-k-mooney | JayF: the use of ddt looks fine to me too | 23:39 |
| JayF | sean-k-mooney: honestly I care more about unifying those states files, I have an ironic-side commit waiting with a comment to keep 'em synced | 23:39 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!