| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-08-29 | |||
| 16:34:29 | bauzas | so, anything anyone ? | |
| 16:37:03 | bauzas | looks not | |
| 16:37:30 | bauzas | thanks all, and tbh I'm happy to see all of you back :) | |
| 16:37:36 | bauzas | #endmeetinh | |
| 16:37:38 | bauzas | doh | |
| 16:37:43 | bauzas | #endmeeting | |
| 16:37:43 | opendevmeet | Meeting ended Tue Aug 29 16:37:43 2023 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | |
| 16:37:43 | opendevmeet | Minutes: https://meetings.opendev.org/meetings/nova/2023/nova.2023-08-29-16.00.html | |
| 16:37:43 | opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/nova/2023/nova.2023-08-29-16.00.txt | |
| 16:37:43 | opendevmeet | Log: https://meetings.opendev.org/meetings/nova/2023/nova.2023-08-29-16.00.log.html | |
| 16:37:46 | elodilles | thanks o/ | |
| 16:38:31 | gibi | o/ | |
| 17:38:47 | opendevreview | Merged openstack/os-vif master: Don't break traffic if port already exists https://review.opendev.org/c/openstack/os-vif/+/885127 | |
| 18:11:00 | opendevreview | Raul Moldovan proposed openstack/nova master: Update resources post live migration https://review.opendev.org/c/openstack/nova/+/892614 | |
| 20:28:41 | opendevreview | Raul Moldovan proposed openstack/nova master: Update resources post live migration https://review.opendev.org/c/openstack/nova/+/892614 | |
| #openstack-nova - 2023-08-30 | |||
| 00:01:08 | opendevreview | melanie witt proposed openstack/nova master: nova-manage: Add 'limits migrate_to_unified_limits' https://review.opendev.org/c/openstack/nova/+/891646 | |
| 01:26:45 | opendevreview | melanie witt proposed openstack/nova master: nova-manage: Add 'limits migrate_to_unified_limits' https://review.opendev.org/c/openstack/nova/+/891646 | |
| 04:33:18 | opendevreview | melanie witt proposed openstack/nova master: Add documentation for unified limits https://review.opendev.org/c/openstack/nova/+/893127 | |
| 04:42:28 | melwitt | bauzas: I looked at the blueprint status etherpad after reading the nova meeting notes, just fyi it's no longer WIP https://review.opendev.org/q/topic:bp%252Funified-limits-nova-tool-and-docs | |
| 06:44:22 | opendevreview | melanie witt proposed openstack/nova master: Add documentation for unified limits https://review.opendev.org/c/openstack/nova/+/893127 | |
| 06:50:49 | bauzas | melwitt: ack, will then look today :) | |
| 07:51:28 | opendevreview | Amit Uniyal proposed openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/889947 | |
| 07:51:28 | opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | |
| 08:43:04 | auniyal | bauzas, I have updated patches, can you please have another look | |
| 08:45:29 | bauzas | auniyal: sure | |
| 08:45:58 | auniyal | thanks | |
| 08:46:19 | auniyal | there are very less zuul jobs ran for reproducer patch -https://review.opendev.org/c/openstack/nova/+/889947/6?tab=change-view-tab-header-zuul-results-summary | |
| 08:46:26 | auniyal | like no nova-next | |
| 08:47:45 | auniyal | bauzas, is it because the only file which added/modified is in nova/tests directory ? | |
| 08:55:41 | bauzas | auniyal: indeed | |
| 08:55:57 | bauzas | hmmm, looks like we have a venv issue | |
| 08:56:07 | bauzas | https://zuul.opendev.org/t/openstack/build/b915c9886c2a420fbc686b2ca4ccece0 | |
| 08:56:43 | bauzas | most of the jobs are failing when creating the venv due to a NoneType exception | |
| 08:56:52 | bauzas | AttributeError I mean | |
| 09:01:46 | bauzas | nvm, this looks a transient issue https://zuul.opendev.org/t/openstack/builds?job_name=openstack-tox-py39&project=openstack%2Fnova&skip=0 | |
| 09:21:45 | opendevreview | melanie witt proposed openstack/nova master: Add documentation for unified limits https://review.opendev.org/c/openstack/nova/+/893127 | |
| 09:40:52 | bauzas | auniyal: I'm eventually +2 on the repro patch but please read my comment for understanding the difference between annotations and defaulting arguments, and why you should *never* set default args with mutables https://review.opendev.org/c/openstack/nova/+/889947/comment/34e10b03_a5d53174/ | |
| 09:51:55 | auniyal | bauzas, ack thanks | |
| 09:52:21 | auniyal | I knew about setting list() insignature but missed, thanks for the comments | |
| 09:53:20 | auniyal | can we +W this https://review.opendev.org/c/openstack/nova/+/881457 | |
| 09:55:32 | auniyal | bauzas, regading this https://review.opendev.org/c/openstack/nova/+/882284/47/nova/compute/manager.py#4246 | |
| 09:55:55 | auniyal | s on bdm.destroy(), bdm object wont get updated correct ? | |
| 09:56:17 | auniyal | from DB bdm will will deleted | |
| 09:56:33 | auniyal | but locally bdm object, will stay as same | |
| 09:59:52 | bauzas | auniyal: not if you play with mutables | |
| 10:00:01 | bauzas | bdms is a list, right? | |
| 10:00:09 | auniyal | yes | |
| 10:04:05 | bauzas | auniyal: https://paste.opendev.org/show/bBaeGvpA8Bx5sSF3wKgZ/ | |
| 10:05:02 | bauzas | in my example, tamper_bdms is altering a mutable object, which is a list | |
| 10:05:20 | bauzas | that's semantically identical to your _delete_dangling_bdms() method | |
| 10:05:30 | bauzas | (provided you alter it) | |
| 10:19:42 | auniyal | bauzas, I might be wrong but this is what I tested - https://paste.opendev.org/show/bnMHd0uJuFYaSWSuP3Gu/ | |
| 10:20:44 | auniyal | the first obj was bdms, then I destroyed one of bdm from it, there is no change in bdms, then later when I retrieved new bdms list, it did not have deleted bdm | |
| 10:20:55 | bauzas | auniyal: because you haven't deleted the local copy | |
| 10:21:05 | auniyal | yes, | |
| 10:21:08 | bauzas | bdm.destroy() doesn't remove the object from the list | |
| 10:21:30 | bauzas | you somehow need to either pop it from the list, or just delete it by hand | |
| 10:21:37 | auniyal | yes, so the same I was trying to say, so instead of deleted their, we ca have fresh copy | |
| 10:22:12 | auniyal | will it affect performance | |
| 10:22:35 | bauzas | auniyal: sure, I don't disagree, but it's just doing an extra RPC roundtrip for some calculation we already made | |
| 10:23:12 | bauzas | auniyal: if you compare your new bdm list after the cleanup with the old one, you should in theory see that only the destroyed BDMs are left | |
| 10:23:33 | bauzas | auniyal: well, it's optimization | |
| 10:23:53 | bauzas | you know what? I'll switch my vote to +1 and I'll leave others chime in | |
| 10:28:44 | auniyal | if it may affect, after destroy I can remove bdm from local copy and return same one. | |
| 10:28:53 | auniyal | I think only one line change | |
| 10:39:18 | bauzas | you don't need to return anything, bdms is a mutable | |
| 10:39:46 | bauzas | take my pastebin as a reference | |
| 10:39:47 | auniyal | yes, just pass bdm, update the list after bdm destroy | |
| 11:14:06 | auniyal | bauzas, TypeError: 'BlockDeviceMappingList' object does not support item deletion, neither remove | |
| 11:14:38 | auniyal | it has these - https://paste.opendev.org/show/bnDCZUcY00oiKOYIreMs/ | |
| 11:16:34 | auniyal | its inherited from ObjectListBase which is again from ovoo_base.ObjectListBase | |
| 11:17:03 | auniyal | this is the oslo one, is there a way to del bdms obj with oslo ? | |
| 12:23:51 | opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | |
| 12:32:13 | auniyal | bauzas, I have updated the patch can you please have another look | |
| 12:46:22 | opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | |
| 13:43:00 | bauzas | auniyal: sec, finding a quick alternative | |
| 14:04:43 | bauzas | sorry, I needed to leave for a sec, back | |
| 14:20:12 | bauzas | dansmith: I'm just discussing with auniyal | |
| 14:20:53 | dansmith | about removing items from the BDM list? I think he resolved it no? | |
| 14:20:59 | bauzas | dansmith: I wonder why oslo.versionedobjects doesn't support directly deleting a list attribute | |
| 14:21:13 | bauzas | is it by design or just a miss ? | |
| 14:21:24 | dansmith | but I definitely agree with you that fetching again to refresh is not good, and I thought I made that point already, but that might have been *another* patch | |
| 14:21:28 | bauzas | we have many builtins but not this one | |
| 14:21:37 | dansmith | bauzas: I think it works by index, didn't he say? | |
| 14:21:57 | bauzas | dansmith: not exactly, see my comment : https://review.opendev.org/c/openstack/nova/+/882284 | |
| 14:21:59 | bauzas | shit | |
| 14:22:02 | bauzas | wrong url | |
| 14:22:08 | bauzas | dansmith: https://review.opendev.org/c/openstack/nova/+/882284/comment/8a1291a9_e0291cdc/ | |
| 14:22:42 | bauzas | I just simplified the loop | |
| 14:22:47 | sean-k-mooney | generally its preferable to not use del by the way | |
| 14:22:59 | bauzas | but, the fact is, we can access the index, but we can't directly del it | |
| 14:23:00 | sean-k-mooney | i know it works but it raises if its not present | |
| 14:23:24 | bauzas | so we need to access the somehow hidden objects internal field of the CoercedList objet | |
| 14:23:57 | bauzas | sean-k-mooney: I don't disagree on the general case, but in this case, I want to prevent an extra RPC roundtrip for something easy to cleanup | |
| 14:24:08 | sean-k-mooney | the bdms is a list of objects filed right | |
| 14:24:09 | sean-k-mooney | https://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/fields.py#L1179-L1183 | |
| 14:24:10 | dansmith | bauzas: your suggestion is to modify the list while iterating right? that's not good. | |
| 14:24:44 | bauzas | dansmith: well, yeah you're right, it's a bad pattern | |
| 14:24:46 | sean-k-mooney | we should not need to fetch again | |
| 14:24:59 | sean-k-mooney | just recreate the bdms object wihtout the relevent bdms | |