| opendevreview | Masanori Kuroha proposed openstack/nova-specs master: Allow only one custom trait with the same prefix in provider.yaml https://review.opendev.org/c/openstack/nova-specs/+/940225 | 03:08 |
|---|---|---|
| opendevreview | Masanori Kuroha proposed openstack/nova-specs master: Allow only one custom trait with the same prefix in provider.yaml https://review.opendev.org/c/openstack/nova-specs/+/937587 | 03:10 |
| opendevreview | Masanori Kuroha proposed openstack/nova-specs master: Allow only one custom trait with the same prefix in provider.yaml https://review.opendev.org/c/openstack/nova-specs/+/937587 | 03:26 |
| priteau | Hello Nova team. Could another core approve this backport? https://review.opendev.org/c/openstack/nova/+/939665 | 10:57 |
| gibi | priteau: done, thanks for backporting | 11:01 |
| opendevreview | Biser Milanov proposed openstack/nova master: StorPool: Pass the instance UUID and device_name to os-brick https://review.opendev.org/c/openstack/nova/+/930297 | 12:20 |
| opendevreview | Merged openstack/nova stable/2024.2: libvirt: Wrap un-proxied listDevices() and listAllDevices() https://review.opendev.org/c/openstack/nova/+/939665 | 13:14 |
| opendevreview | Pierre Riteau proposed openstack/nova stable/2024.1: libvirt: Wrap un-proxied listDevices() and listAllDevices() https://review.opendev.org/c/openstack/nova/+/939695 | 13:19 |
| priteau | Thank you gibi. The Caracal backport is also ready to review ^ | 13:21 |
| opendevreview | Biser Milanov proposed openstack/nova master: StorPool: Pass the instance UUID and device_name to os-brick https://review.opendev.org/c/openstack/nova/+/930297 | 13:29 |
| bauzas | sean-k-mooney: hrmmm, looks like system_metadata needs to be lazy-loaded https://paste.opendev.org/show/boOPqY0RzgGM3SYO8k5M/ | 14:44 |
| bauzas | sean-k-mooney: context being https://meetings.opendev.org/meetings/nova/2025/nova.2025-01-07-16.01.log.html#l-86 | 14:45 |
| sean-k-mooney | bauzas: it depend where. its in the default filed so it should be loaded by default unless you change that | 14:45 |
| bauzas | sean-k-mooney: I tested on the HostState.instances value | 14:45 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L69-L71 | 14:46 |
| sean-k-mooney | maybe we are not using that for what i tought we were | 14:47 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L875-L896 | 14:50 |
| sean-k-mooney | so it looks like we are building fake instance object wiht only the uuid set | 14:50 |
| sean-k-mooney | bauzas: do you have track_instance_changes enabled | 14:52 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Add support for showing image properties in server show response https://review.opendev.org/c/openstack/nova/+/939649 | 14:53 |
| bauzas | sean-k-mooney: yes indeed, but I'll doublecheck | 14:54 |
| sean-k-mooney | bauzas: what we likely need to do is repalce the call to just get the uuid and expiclty get the uuid and system metadata as teh only requested fieds | 14:54 |
| sean-k-mooney | by calling https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L1441-L1446 | 14:54 |
| sean-k-mooney | although i migh tbe looking at the wrong place | 14:57 |
| sean-k-mooney | i was looking at teh host manager. its fine for it to have just the id | 14:57 |
| sean-k-mooney | its https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L144 that we really care about | 14:57 |
| sean-k-mooney | i suspect its also jsut a dict with the minimal object | 14:58 |
| sean-k-mooney | they are consutcted here https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L790-L827 | 15:00 |
| sean-k-mooney | so the instnace info is populated by https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L880 | 15:01 |
| sean-k-mooney | so unfortunetly it will have a minor db impact to lookup taht data instead of just the uuid | 15:02 |
| artom | Uggla, since you mentioned doing the os-traits release, the TPM traits patch is pending: https://review.opendev.org/c/openstack/os-traits/+/940191 | 15:04 |
| bauzas | sean-k-mooney: you know what ? we could just say that it could have some db performance if we use the weigher, but fwiw, my change for moment doesn't default to use this weigher | 15:08 |
| bauzas | so let's discuss this on the review then | 15:08 |
| sean-k-mooney | sure, we have 2 options as far as i see, 1 lazy load if you enable the weigher, 2 perload the filed either based on the weigher multipler or a new config | 15:12 |
| sean-k-mooney | *feild | 15:13 |
| sean-k-mooney | in either case you are opting in to the schdulelr memory/db overhead | 15:13 |
| sean-k-mooney | so there is no impact for existign deployment which wont use it by default | 15:13 |
| sean-k-mooney | im happy to disscuss that more in the review, as long as we document the impact im not stongly pushed one way or another althgouh i would tend to prefer not lazyloading | 15:14 |
| sean-k-mooney | bauzas: what you could do is have a folloup patch POCing the preloading in the host_manager | 15:15 |
| sean-k-mooney | at the end fo the day that is an indepented patch form my perspective as it does nto actully change the implemtion of the weigher | 15:15 |
| bauzas | sean-k-mooney: we also call the scheduler RPC API when creating an instance | 15:15 |
| bauzas | sean-k-mooney: yes indeed a follow-up patch for that | 15:16 |
| bauzas | or a dependend one, whether other people have some performance concerns | 15:16 |
| bauzas | anyway, thanks | 15:16 |
| bauzas | sean-k-mooney: fwiw, I'm wrapping my head around on how to compare ImageMetaProps fields against system_metadata dict | 18:08 |
| bauzas | this is like comparing apples vs. oranges since the keys are fucking different | 18:09 |
| sean-k-mooney | ya the image meta is sotred in system_metadata with a img_ prefix | 18:11 |
| sean-k-mooney | btu we look at the request spec | 18:11 |
| sean-k-mooney | for the image properties | 18:11 |
| sean-k-mooney | not the host state | 18:11 |
| bauzas | https://paste.opendev.org/show/bX5inHhgIOOmOgSwVxyH/ | 18:11 |
| bauzas | so I need to find a way to compare both | 18:12 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/scheduler/filters/image_props_filter.py#L122 | 18:12 |
| sean-k-mooney | bauzas: we have a helper function to get the image props form the system_metadta table i belive | 18:13 |
| sean-k-mooney | let me see if i can find it | 18:13 |
| sean-k-mooney | i think its defiend on the instnace object | 18:13 |
| bauzas | we have a from_dict() helper | 18:13 |
| bauzas | on the ImageMetaProps object | 18:13 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L256-L258 | 18:13 |
| bauzas | but some of the keys are not having an 'image_' prefix | 18:14 |
| sean-k-mooney | so instnace.image_meta | 18:14 |
| bauzas | that would work but that's still a lazy-load IIRC | 18:15 |
| bauzas | anyway, thnaks | 18:15 |
| priteau | Could Nova cores please approve this backport? https://review.opendev.org/c/openstack/nova/+/939695 | 18:16 |
| sean-k-mooney | ah the 2024.2 version is merged | 18:17 |
| bauzas | priteau: done | 18:17 |
| sean-k-mooney | sure | 18:17 |
| sean-k-mooney | priteau: +2w | 18:18 |
| priteau | Thank you both! | 18:18 |
| sean-k-mooney | priteau: once thats merged feel free to ping us for the 2023.2 version | 18:18 |
| priteau | I deployed a downstream backport on 2023.1 today, for a customer that has been suffering from this issue (mostly on a couple of hypervisors that were hosting around 50 VMs each), it appears to be helping so far. | 18:20 |
| mikal | sean-k-mooney: if you're still around, are you ok with the plan to just drop everything except the two xhci USB controllers from that final SPICE patch? We can always add the others back if someone genuinely does need an ancient controller, although it would be a bit like someone asking for IDE support. | 18:34 |
| sean-k-mooney | im ok with that yes. it just means limiting the allowed values of the image property | 18:38 |
| sean-k-mooney | so we can alwyas extend the enuma if there is a reason to do so | 18:38 |
| sean-k-mooney | mikal: as your aware nova is nto a virtualisation plathform its ment to be a cloud computing serivice so we dont have to, and generally shoudl not expose every possibel value supproted by any given hypervior, we shoudl make an informed choice | 18:40 |
| sean-k-mooney | mikal: the usb over ip spec https://review.opendev.org/c/openstack/nova-specs/+/920687/7/specs/2024.2/approved/usb-over-ip.rst#132 from shadow play also wanted to sue qemu-xhci | 18:42 |
| mikal | sean-k-mooney: yeah that's pretty much my take too -- adding enum entries is cheap, and the other controllers are inefficient and don't support USB3. I think we _should_ keep qemu-xhci and nec-xhci, just in case we encounter someone who can't install the qemu drivers (bootstrapping them on windows is sometimes fiddly). I'll make that change, limit the port count, and leave a comment in the enum explaining ho | 18:42 |
| mikal | w we got here. | 18:42 |
| mikal | sean-k-mooney: hmmm, that spec doesn't cover the maximum port count thing though. I guess it was only thought of later. | 18:48 |
| mikal | sean-k-mooney: also, the relevant qemu maintainer agrees with us: https://www.kraxel.org/blog/2018/08/qemu-usb-tips/ | 18:49 |
| opendevreview | Merged openstack/nova stable/2024.1: libvirt: Wrap un-proxied listDevices() and listAllDevices() https://review.opendev.org/c/openstack/nova/+/939695 | 21:13 |
| opendevreview | Pierre Riteau proposed openstack/nova stable/2023.2: libvirt: Wrap un-proxied listDevices() and listAllDevices() https://review.opendev.org/c/openstack/nova/+/940272 | 21:17 |
| *** bbezak is now known as Guest7191 | 23:32 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!