| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-07-29 | |||
| 13:08:22 | sean-k-mooney | we dont have result anymore but it did actully fail | |
| 13:08:24 | sean-k-mooney | https://zuul.opendev.org/t/openstack/build/42a251fe1d174a2abef7016b96fe0dc4 | |
| 13:10:46 | kashyap | Ohh, so the vGPU + Cinder volume is the combo | |
| 13:14:28 | opendevreview | Merged openstack/nova stable/2024.1: Remove AMI snapshot format special case https://review.opendev.org/c/openstack/nova/+/924982 | |
| 13:21:43 | sean-k-mooney | kashyap: i think so but again they didnt provide a clear repoducer in the bug so its hard to tell exactly | |
| 13:21:50 | sean-k-mooney | but i think that has to be the case looking at the code | |
| 13:22:09 | sean-k-mooney | we are looping over the devices in the guest domain and the error is in the mdev class | |
| 13:22:21 | sean-k-mooney | so it must only be an issue when comibining both features | |
| 13:22:34 | sean-k-mooney | which would expliain why we dont see this today | |
| 13:30:59 | dansmith | sean-k-mooney: gibi: I thought we were into "written by a core, backported by a core, can be single-vote-landed by a core" on stable now | |
| 13:31:44 | sean-k-mooney | dansmith: i guess, it does not come up often | |
| 13:32:12 | sean-k-mooney | but in either case i was going to proxy your/my +2 by end of day if noone else said go for it | |
| 13:34:22 | dansmith | ack | |
| 13:45:09 | opendevreview | Rajesh Tailor proposed openstack/nova master: Fix KeyError on assisted snapshot call https://review.opendev.org/c/openstack/nova/+/900783 | |
| 13:57:06 | opendevreview | Rajesh Tailor proposed openstack/nova master: Handle InstanceExists exception for duplicate instance https://review.opendev.org/c/openstack/nova/+/860938 | |
| 14:00:39 | elodilles | sean-k-mooney: i'm OK to propose stable releases anytime & i don't have any important backport on my radar | |
| 14:04:37 | sean-k-mooney | elodilles: bauzas asked me to include https://review.opendev.org/c/openstack/nova/+/924514 and its backports | |
| 14:05:06 | sean-k-mooney | i think we can include https://review.opendev.org/c/openstack/nova/+/923689 also on 2024.1 | |
| 14:06:03 | elodilles | ACK, I'll wait for them then o/ | |
| 14:06:10 | bauzas | cool | |
| 14:07:45 | sean-k-mooney | elodilles: i have not realy looked at the older branch contet much | |
| 14:07:58 | sean-k-mooney | i do see some nice to haves so i may do some stable review today | |
| 14:08:17 | sean-k-mooney | but i dont see anythign super urgent | |
| 14:09:08 | elodilles | ACK, i'll check them too | |
| 14:34:37 | bauzas | sean-k-mooney: can you summarize the mdev issue you said ? | |
| 14:34:51 | bauzas | -ETOOMUCHCONTENTTODIGEST | |
| 14:35:35 | bauzas | fwiw, I need to resume my mtty series now that devstack supports 24.04 | |
| 14:36:58 | sean-k-mooney | bauzas: it appears that if a gues has an mdev and a cinder volume we cant detach t he cinder volume because the get_device_by_alis code cant hanel the fact that we dont have an aias field in the class that modeles the mdev device in the guest xml | |
| 14:37:10 | sean-k-mooney | i.e. you cant detach a cinder volume for a guest with a vgpu | |
| 14:37:34 | sean-k-mooney | we can fix that in the way propsoed in the patch but it missed the underly ing bug and did not include a repoducer | |
| 14:37:53 | bauzas | i see | |
| 14:38:06 | sean-k-mooney | the underlying bug is the get_device_by_alias code assumes all geust devices have an alias filed | |
| 14:38:18 | bauzas | so people need to rebuild the instance with a flavor without VGPU first before detaching | |
| 14:38:19 | sean-k-mooney | so we shoudl fix that | |
| 14:38:29 | bauzas | agreed | |
| 14:38:47 | bauzas | is this https://review.opendev.org/c/openstack/nova/+/925037 ? | |
| 14:38:49 | sean-k-mooney | bauzas: they might just need to stop the instnace | |
| 14:39:01 | sean-k-mooney | yep | |
| 14:39:13 | sean-k-mooney | that patch will work but it does not have enouch test coverage | |
| 14:39:30 | sean-k-mooney | and as i said the bug is reallly here https://github.com/openstack/nova/blob/7a7427691e0bd4818bb7a2c5f5371e0244addbbb/nova/virt/libvirt/guest.py#L419 | |
| 14:40:03 | bauzas | the real problem is that we didn't had any functest for that | |
| 14:40:06 | sean-k-mooney | that if assumes all devices in get_all_devices will have an alias field | |
| 14:40:19 | bauzas | because mdev support predates alias | |
| 14:40:24 | sean-k-mooney | yep | |
| 14:40:33 | sean-k-mooney | so ideally we woudl create a fucntional repoducer | |
| 14:40:35 | bauzas | but when we merged aliases series, we created a regression | |
| 14:40:48 | bauzas | yup | |
| 14:40:52 | bauzas | we need a functtest | |
| 14:41:01 | sean-k-mooney | and then fix both the underlying bug and optionall add supprot ofr alias ot mdevs even if not used today | |
| 14:41:22 | sean-k-mooney | we can fix the actual bug in 1 of 2 ways | |
| 14:41:34 | sean-k-mooney | either check if dev has an alais field before usign it | |
| 14:42:02 | sean-k-mooney | or make sure that all device will have that eihter by extending the base class or checking all child class have it | |
| 14:43:10 | sean-k-mooney | so either add it here https://github.com/openstack/nova/blob/7a7427691e0bd4818bb7a2c5f5371e0244addbbb/nova/virt/libvirt/config.py#L56 or add a hasattr check | |
| 14:46:12 | bauzas | I'm fine with the first option | |
| 14:46:42 | bauzas | the second option is better in case of *another* regression | |
| 14:46:54 | bauzas | want me to help with the reproducer ? | |
| 14:47:20 | bauzas | I just have 3.5 days to work before my PTO and I really want my mtty series to be reviewable before I leavred | |
| 15:23:57 | opendevreview | Pavlo Shchelokovskyy proposed openstack/nova master: Replace -- as 3-4 char in hostname with - https://review.opendev.org/c/openstack/nova/+/893072 | |
| 15:33:08 | opendevreview | Merged openstack/nova stable/2023.2: Remove AMI snapshot format special case https://review.opendev.org/c/openstack/nova/+/924999 | |
| 16:09:14 | opendevreview | Merged openstack/nova stable/2023.2: Fixed an error when caching multiple images in aggregate https://review.opendev.org/c/openstack/nova/+/904017 | |
| 16:20:49 | sean-k-mooney | bauzas: i rehcecked the last patch in your mtty seriese and it didnt repoduce the cinder issue but it did fail with https://paste.opendev.org/show/bexn6H6KYRrAlhNONtVn/ | |
| 16:22:49 | sean-k-mooney | there is a bug somewehre but im not sure that https://review.opendev.org/c/openstack/nova/+/918420/ is even current | |
| 16:28:30 | opendevreview | sean mooney proposed openstack/nova stable/2023.1: Fixed an error when caching multiple images in aggregate https://review.opendev.org/c/openstack/nova/+/904018 | |
| 16:39:52 | opendevreview | Merged openstack/nova stable/2023.2: Added context manager for instance lock https://review.opendev.org/c/openstack/nova/+/911394 | |
| 16:44:24 | opendevreview | Merged openstack/nova stable/2023.2: Separate OSError with ValueError https://review.opendev.org/c/openstack/nova/+/911395 | |
| 17:25:47 | sean-k-mooney | bauzas: if you have time to work on it do | |
| 17:26:21 | sean-k-mooney | perhaps just try that usecase "mdev + cinder volume in the same vm" when working on the mtty stuff | |
| 17:26:27 | sean-k-mooney | and see if you see the same issue | |
| 17:26:35 | sean-k-mooney | if you do then we can confirm the bug | |
| 17:26:39 | sean-k-mooney | and create a repoducer | |
| 17:26:46 | sean-k-mooney | in the fucntional tests | |
| 19:51:34 | opendevreview | Merged openstack/nova stable/2023.1: Fixed an error when caching multiple images in aggregate https://review.opendev.org/c/openstack/nova/+/904018 | |
| 21:08:16 | opendevreview | Pavlo Shchelokovskyy proposed openstack/nova master: Replace -- as 3-4 char in hostname with - https://review.opendev.org/c/openstack/nova/+/893072 | |
| #openstack-nova - 2024-07-30 | |||
| 06:20:04 | opendevreview | Masahito Muroi proposed openstack/nova master: Change instance_system_metadata value to Text type https://review.opendev.org/c/openstack/nova/+/925163 | |
| 09:50:21 | opendevreview | Julien Le Jeune proposed openstack/nova stable/2024.1: Refactor vf profile for PCI device https://review.opendev.org/c/openstack/nova/+/925181 | |
| 10:08:07 | opendevreview | Rajesh Tailor proposed openstack/nova master: Fix KeyError on assisted snapshot call https://review.opendev.org/c/openstack/nova/+/900783 | |
| 10:14:13 | opendevreview | Rajesh Tailor proposed openstack/nova master: Restore original AZ when unshelve fails https://review.opendev.org/c/openstack/nova/+/911108 | |
| 10:28:42 | opendevreview | Michael Still proposed openstack/nova master: libvirt: Add config option to require secure SPICE. https://review.opendev.org/c/openstack/nova/+/922544 | |
| 10:28:43 | opendevreview | Michael Still proposed openstack/nova master: libvirt: allow direct SPICE connections to qemu https://review.opendev.org/c/openstack/nova/+/924844 | |
| 10:32:05 | opendevreview | Michael Still proposed openstack/nova master: libvirt: Add config option to require secure SPICE. https://review.opendev.org/c/openstack/nova/+/922544 | |
| 10:32:06 | opendevreview | Michael Still proposed openstack/nova master: libvirt: allow direct SPICE connections to qemu https://review.opendev.org/c/openstack/nova/+/924844 | |
| 10:33:57 | opendevreview | Michael Still proposed openstack/nova master: libvirt: Add config option to require secure SPICE. https://review.opendev.org/c/openstack/nova/+/922544 | |
| 10:33:57 | opendevreview | Michael Still proposed openstack/nova master: libvirt: allow direct SPICE connections to qemu https://review.opendev.org/c/openstack/nova/+/924844 | |
| 10:34:15 | mikal | Sorry about that, I have no idea how that hook ended up in my rebase. | |
| 10:35:07 | mikal | sean-k-mooney: I've dropped the concurrency patch, and the first two patches in the series should be good to review now. I'll do the sound and usb device extra specs next but that's not done yet. | |
| 11:54:22 | opendevreview | Merged openstack/nova stable/2023.1: Remove AMI snapshot format special case https://review.opendev.org/c/openstack/nova/+/925000 | |
| 12:46:29 | jlejeune | sean-k-mooney: hello, since the last commit "Remove AMI snapshot format special case" has been merged, the test_snapshot_running test is now running but it depends on qemu-img binary which is not necessary installed, so it fails | |
| 12:55:02 | jlejeune | why not putting qemu-utils package in bindep.txt ? | |
| 13:08:20 | sean-k-mooney | jlejeune: because our unit test are not ment to depend on it | |
| 13:08:34 | sean-k-mooney | jlejeune: its an optiona depency | |
| 13:08:52 | sean-k-mooney | and we will be removing those tests when we factor this out of nova | |
| 13:09:36 | sean-k-mooney | we could add it as a test only dep in bindep | |
| 13:09:50 | sean-k-mooney | but we dont want to require peopel to install qemu image just to run unit test in general | |
| 13:17:32 | jlejeune | agree, so in the meantime waiting for the removal of that code in oslo instead of nova, we need to skip that test if qemu-img binary is missing | |
| 13:21:18 | sean-k-mooney | yep like you did for the ohter one | |
| 13:21:30 | jlejeune | yep :) | |
| 13:21:52 | sean-k-mooney | its not a blocker just an anoyance currently | |
| 13:22:18 | sean-k-mooney | i was hopign to do a new release today/tomorrow | |
| 13:22:33 | sean-k-mooney | we could include it if the patch is ready quickly | |
| 13:22:41 | sean-k-mooney | but i dont really want to hold the release just for this | |