| opendevreview | Callum Dickinson proposed openstack/nova master: Fix image ID in libvirt metadata when unshelving https://review.opendev.org/c/openstack/nova/+/942973 | 00:09 |
|---|---|---|
| opendevreview | Callum Dickinson proposed openstack/nova master: Add image meta to libvirt XML metadata https://review.opendev.org/c/openstack/nova/+/942766 | 03:00 |
| opendevreview | Callum Dickinson proposed openstack/nova master: Add more flavor metadata to libvirt guest XML https://review.opendev.org/c/openstack/nova/+/942974 | 05:03 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Drop quota options from the DEFAULT section https://review.opendev.org/c/openstack/nova/+/943131 | 08:36 |
| opendevreview | Tobias Urdin proposed openstack/nova master: wip: Reserve volumes when doing instance resize https://review.opendev.org/c/openstack/nova/+/942985 | 09:04 |
| tobias-urdin | when testing above^ patch i just realized that you cannot reserve a cinder volume when status is in-use, based on the issue described there i guess the only two alternatives to solve the problem in that patch is either 1) add a simple check for volume status and live with the potential race condition between check of volume status and actual _terminate_volume_connections call | 09:08 |
| tobias-urdin | or 2) catch InvalidVolume exceptions directly in _terminate_volume_connections and try to rollback volume attachments and then raise a InstanceFaultRollback exception, anybody else has any better ideas? | 09:08 |
| opendevreview | ribaudr proposed openstack/nova master: Update driver to map the targeted address for SR-IOV PCI devices https://review.opendev.org/c/openstack/nova/+/942147 | 09:37 |
| opendevreview | ribaudr proposed openstack/nova master: FUP improve comment accuracy and variable naming for tag removal https://review.opendev.org/c/openstack/nova/+/943124 | 09:37 |
| sean-k-mooney | tobias-urdin: i have not looked into your patches yet but waht was the problem you were tryign to resolve? | 09:44 |
| sean-k-mooney | tobias-urdin: i vagely saw them coming in last week but didnt have time to read them | 09:44 |
| sean-k-mooney | tobias-urdin: oh ok... so thisi is kind of a cinder bug | 09:46 |
| sean-k-mooney | tobias-urdin: so the problem is that cinder does not knwo that nova is in the middle of doing a resize and nova does not know that cidner wants to do a backup | 09:46 |
| tobias-urdin | sean-k-mooney: if i try to resize and instance it goes to ERROR and needs operator to restore it (shutdown has already been called) because attachment_create() volume API raises InvalidVolume when an attached volume has backing-up status | 09:46 |
| sean-k-mooney | so we can get in a broken state | 09:46 |
| sean-k-mooney | tobias-urdin: well you can resize instnace with cinder volumes | 09:47 |
| sean-k-mooney | that not the problem its because you also happen to have a cinder backup at the same time | 09:47 |
| sean-k-mooney | tobias-urdin: both nova and cidner backup exepct to be the only thing modifying the volume state when doign a resize or backup | 09:48 |
| tobias-urdin | yea, i could just loop volumes and check status in the context and raise InstanceFaultRollback i guess for resize to not hit it, but there would still be a race between me checking the status and attachment code being called in resize | 09:49 |
| sean-k-mooney | that not really the fix | 09:49 |
| tobias-urdin | i was hoping i could reserve the volumes before starting the operation, but my research failed because it didn't accommodate for in-use not being possible on in-use volume :( | 09:49 |
| sean-k-mooney | we either need nova to annotate the volume with somethjing to indicate that we are migrating | 09:50 |
| sean-k-mooney | or we need to have cidner check the status of attached instnace before doing a backup | 09:50 |
| sean-k-mooney | tobias-urdin: is cidner backup changing the state of the volume to something other then inuse | 09:51 |
| tobias-urdin | yes "backing-up" is set | 09:51 |
| sean-k-mooney | right os that also a bug | 09:51 |
| sean-k-mooney | once a volume is attached to a nova instnace we do not exect the voluem status to change unless its via a call to the nova api | 09:52 |
| sean-k-mooney | so form a nova perspecitve it should remain in use | 09:52 |
| sean-k-mooney | it would be fine to have a seperate task_state that backing-up | 09:53 |
| sean-k-mooney | but if cidner want to modify it they need to make the attchment api treat backing-up the same as in-use | 09:54 |
| tobias-urdin | that actually makes more sense | 09:54 |
| sean-k-mooney | we have a bunch of task_state that we translate to "active" because semanticly external service should treat them the same | 09:55 |
| sean-k-mooney | i dont know how to express that in cidner api but i think if they have internal states liek backing-up they may need a similar mechanisum | 09:56 |
| sean-k-mooney | like this https://github.com/openstack/nova/blob/master/nova/api/openstack/common.py#L46-L63 | 09:59 |
| tobias-urdin | sean-k-mooney: thanks for the directions! i will investigate further and see if i can find a better path forward | 10:03 |
| sean-k-mooney | ill try an comment on the bug and maybe a good dicusion for the ptg in a nova/cinder cross project if we dont have a path forward by then | 10:04 |
| tobias-urdin | sean-k-mooney: ack, looks like they account for some migration logic in _attachment_reserve() https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L2409 | 10:08 |
| tobias-urdin | so imo handling backing-up (if "previous status", that it sets back when backup is complete, is in-use) as if it were in-use makes sense | 10:09 |
| sean-k-mooney | tobias-urdin: https://bugs.launchpad.net/nova/+bug/2077512/comments/5 i tried to summeriese my toughts and i added cinder to the bug | 10:44 |
| tobias-urdin | sean-k-mooney: this is the real path forward https://review.opendev.org/c/openstack/cinder-specs/+/915973 instead of my new hack approach https://review.opendev.org/c/openstack/cinder/+/943176 | 13:10 |
| opendevreview | Merged openstack/nova master: Update driver to deal with managed flag https://review.opendev.org/c/openstack/nova/+/938405 | 13:54 |
| sean-k-mooney | tobias-urdin: ah yes that does seam to be adressign this edgecase | 14:04 |
| *** priteau is now known as Guest10549 | 14:46 | |
| *** priteau2 is now known as priteau | 14:46 | |
| cardoe | hello. hoping to get https://review.opendev.org/c/openstack/nova/+/942019 into 2025.1. This fixes an issue in 2024.1 and 2024.2 when you use Ironic and Nova is unable to deploy the error message is just the name of the OpenStack SDK object and the memory address instead of the actual error message. | 15:41 |
| sean-k-mooney | cardoe: ah i skimmed that. im not sure if we want to merge that in the RC period but i think its mostly a vailid change | 15:49 |
| sean-k-mooney | cardoe: my main question is did it report "No Error" | 15:49 |
| sean-k-mooney | before when usign ironic client? | 15:50 |
| cardoe | It printed an empty string | 15:50 |
| cardoe | It felt awkward so I added that. | 15:50 |
| sean-k-mooney | ok so im not sure if we would want to maintain the old behivor or not | 15:51 |
| cardoe | It said "(deploy: power: some message storage: )" | 15:51 |
| sean-k-mooney | i think no error is ok | 15:51 |
| sean-k-mooney | oh | 15:51 |
| sean-k-mooney | they ya its better with teh current version | 15:51 |
| sean-k-mooney | wha ti was going to suggest is only inluciding the one that had an error | 15:52 |
| sean-k-mooney | btu that could break people if they depeneded on all 3 being there | 15:52 |
| cardoe | Now it says "(deploy: <openstack.response 0xDEADBEEF> power: <openstack.response 0xBEEFDEAD> storage: <openstack.response 0xFFFFFFFF)" | 15:52 |
| cardoe | My patch makes it say "(deploy: No Error power: some message storage: No Error)" | 15:52 |
| sean-k-mooney | ya which is an improvmenet over the old and current behavior | 15:53 |
| cardoe | I was originally gonna drop the "deploy:" and "storage:" section if there wasn't a problem. But like you said, if people are expecting it I didn't wanna break them. | 15:53 |
| sean-k-mooney | i have not looked at this in deaile but the other question i was wondering about is is this only looged or can this be returned to an end user | 15:54 |
| cardoe | It's only logged today. I wanna return it to the user. | 15:55 |
| sean-k-mooney | ok so the warning i was oging ot give is these mesagges come from ironic | 15:55 |
| cardoe | I'm refactoring a few parts of that because today the user gets "Failed to prepare block device" as the error for nearly everything. | 15:55 |
| sean-k-mooney | it many not be ok to show that to a user unless they are admin | 15:55 |
| cardoe | Like if the VIF cannot be attached or created in Neutron, the error is still "Failed to prepare block device" | 15:55 |
| sean-k-mooney | fun | 15:56 |
| sean-k-mooney | we can give them a better message but we cant including things like ipmi details of the node ectra | 15:56 |
| cardoe | But I figured that wasn't a backportable change so I tried to split this up into 2 changes. One to just fix the log message and then one that'll be a behavior change and would go in a future release. | 15:56 |
| sean-k-mooney | ya it should be two diffent patches at any rate | 15:57 |
| sean-k-mooney | depending on what it looks like it may or may not be backportable but we just have to be carful to not let them see info that you can only see as an admin | 15:57 |
| cardoe | right | 15:58 |
| sean-k-mooney | cardoe: so my main ask on this patch would be to add a unit test | 15:58 |
| cardoe | okay will do | 15:59 |
| sean-k-mooney | basiclly just assert the log message is printed with the expect values | 15:59 |
| sean-k-mooney | otherwise i think its fine | 15:59 |
| cardoe | makes sense. I'll add it. | 15:59 |
| sean-k-mooney | cool i just captured that in a comment on the patch | 16:03 |
| sean-k-mooney | cardoe: tiraged https://bugs.launchpad.net/nova/+bug/2100009 we dont have an offical rc lable yet but it will be called epoxy-rc-potential so i added that premtively | 16:07 |
| cardoe | Thanks | 16:11 |
| sean-k-mooney | bauzas: jsut an fyi we still need to merge the linux bridge deprecation patch in nova https://review.opendev.org/c/openstack/nova/+/942291 | 18:02 |
| melwitt | fyi the fix for the tpool proxy listDevices() regression needs a second +2 for stable/2024.2 https://review.opendev.org/c/openstack/nova/+/942652 and the func test below it, if anyone could take a look. it will need to be reviewed for stable/2024.1, stable/2023.2, and unmaintained/2023.1 also 😕 | 19:10 |
| sean-k-mooney | i can take a look shortly | 19:10 |
| melwitt | thanks sean-k-mooney | 19:11 |
| sean-k-mooney | +2w on 2024.2 ill try to remember to loop back to 2024.1 tomorrow | 19:15 |
| sean-k-mooney | if not feel free to ping | 19:15 |
| melwitt | ok, thank you sean-k-mooney | 19:16 |
| opendevreview | Merged openstack/nova stable/2024.2: Reproducer for bug 2098892 https://review.opendev.org/c/openstack/nova/+/942651 | 20:09 |
| opendevreview | ribaudr proposed openstack/nova master: Update driver to map the targeted address for SR-IOV PCI devices https://review.opendev.org/c/openstack/nova/+/942147 | 20:23 |
| opendevreview | ribaudr proposed openstack/nova master: FUP improve comment accuracy and variable naming for tag removal https://review.opendev.org/c/openstack/nova/+/943124 | 20:23 |
| opendevreview | ribaudr proposed openstack/nova master: Update libvirt fixtures to support hostdevs https://review.opendev.org/c/openstack/nova/+/943207 | 20:23 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!