)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dca886e01ef2a3c6342decc12a7b8796f7fb346a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"0dfb9677_0a6aad24","updated":"2023-05-22 13:57:19.000000000","message":"I know this is still WIP but a couple comments. Sounds like you\u0027re trying to debug why you don\u0027t see the exception in the reboot?","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"723a8415971bf165b7ca46688b14d343a2634477","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"5cc98317_a489fb71","updated":"2023-05-24 16:34:28.000000000","message":"Just went digging into the volume api to see what check_attached does and it\u0027s not specific enough for this use case. Also note that I just learned that apparently ironic is detaching and re-attaching volumes underneath nova in some cases, which will invalidate our BDMs. In reality, even with this too-weak check you have here, we could race with ironic, see the volume as not attached during a lifecycle event, and purge it from our BDMs. We\u0027ll have to figure out how to avoid ironic doing that, but we should go ahead and get the stronger check in place here.","commit_id":"10fbcfb23e56000a3babb00556cc753ebf718852"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0a2abd2620f227badb22bc95761c2b99e6290afb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"58a60e13_e0fa6aea","updated":"2023-05-24 03:46:47.000000000","message":"recheck tempest-integrated-compute ssh timout, in _get_ssh_connection\nerr:\ntempest.lib.exceptions.SSHTimeout: Connection to the 172.24.5.165 via SSH timed out.","commit_id":"10fbcfb23e56000a3babb00556cc753ebf718852"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9e9ad0b67bb90e3fb628b88d7277be163676be22","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"af9703da_e6d071ea","updated":"2023-07-11 15:44:17.000000000","message":"recheck tempest test test_tagged_boot_devices failed unrelated","commit_id":"cdeb01e75a0b1d7dc3d5540b05d1830c63621a13"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edcca28105a29fbd478c16f66fc3ac0a1beadb61","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"46c4555a_c650897b","updated":"2023-07-11 09:21:40.000000000","message":"tempest test test_tagged_boot_devices failed unrelated","commit_id":"cdeb01e75a0b1d7dc3d5540b05d1830c63621a13"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"7403b69ec021427fa14f0a8426063d081e8a3181","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":42,"id":"e323f1a5_71775b4a","updated":"2023-07-20 11:26:02.000000000","message":"recheck tempest test test_server_rescue_negative unrelated passed locally","commit_id":"da5b5b7be63e1df78d527340b807b93a42909721"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"3cd87fe77279d74d75f14787c50beb14bf2eac5d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":45,"id":"1d7950b0_3f8a0434","updated":"2023-08-22 05:17:41.000000000","message":"recheck resize req failed with 500 \n.OpenStackApiException: Unexpected status code: {\"computeFault\": {\"code\": 500,","commit_id":"0ae5b985d67b74356a2b6bf30002baed058b7ef5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4ee14373a14dfb2553c80457dc27bdf66a559900","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":47,"id":"5dde5e4c_6276ce29","updated":"2023-08-29 15:48:02.000000000","message":"A solid and easy to understand patch, thanks, but I have a few nits I\u0027d like to be addressed before giving my go.","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"98b5b5322205e95e1fd15d6e7a881c38684ee727","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":47,"id":"c7d859f5_6b32ea62","updated":"2023-08-26 05:00:56.000000000","message":"recheck grenade non-zero return code\nhttps://zuul.opendev.org/t/openstack/build/2364c6184ab14c948e029dad1df55185/log/job-output.txt#21336","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e7428fd89e844d0b4c006a92ed9394684e1d0558","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":48,"id":"27f7bbd2_bd785c27","updated":"2023-08-30 10:24:59.000000000","message":"Changing my vote to +1 to make it less blocker, but I\u0027d like to have others\u0027 opinion before either changing my vote to +2 or put it back to -1.","commit_id":"de5543ec333dbb3173ee048369c0df9d2d2c5be1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ce0f30bee2929e0bb57bfb78fec150ff62784d17","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":48,"id":"9868d2ba_6ca21012","updated":"2023-08-30 09:50:25.000000000","message":"I\u0027m still not convinced that we need to call twice the RPC API for getting a list of BDMs in the same reboot() method, but I leave other folks to bikeshed on it.","commit_id":"de5543ec333dbb3173ee048369c0df9d2d2c5be1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4439da8a50e64fc56e5057a66117016a47576c07","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":50,"id":"0e0b395b_71bee7a3","updated":"2023-08-30 14:32:26.000000000","message":"+1ing for now, discussing over IRC about the filtering pattern we may prefer.","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8bf650534ce65d21ea35d1f1f281f547f2c5328d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":50,"id":"81166061_e5cba844","updated":"2023-08-30 14:41:18.000000000","message":"Okay, we may bikeshed more on the best implementation pattern for filtering this list and a potential better approach would be to have explicit input/output on the method itself (instead of relying on inner mutable argument modification)\n```\neg. bdms \u003d _delete_dangling_bdms(bdms)\n```\n\nThat being said, no reason to put this series at risk due to the proximity of the FF. +2 and we can address this in a FUP.","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"081b3554e7b5167654b2c5b11f70a2629c85b251","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":50,"id":"f09838ca_359dae9c","updated":"2023-08-30 14:48:11.000000000","message":"some coverage gap I just found. You know what ? Just take the opportunity to stop doing in-place removal and rather return a new bdms list.","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6dd2f06c1201e73a9ac69c282269b815f55c96fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":50,"id":"7b31e9c4_358d8928","in_reply_to":"6d92da2e_96435f90","updated":"2023-10-04 08:11:21.000000000","message":"Done","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a0d1c121f31a05c90ed3d03db20c189685745014","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":50,"id":"6d92da2e_96435f90","in_reply_to":"f09838ca_359dae9c","updated":"2023-08-30 17:00:35.000000000","message":"Done","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f5690669b9e5795a70a3efb9311bdd100169af10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":53,"id":"1bd0765f_b2ef527a","updated":"2023-08-31 12:45:54.000000000","message":"Just a quick ask Amit, please.","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4dec5c0e7dde8a490d7d6239820dc85e84da6ed2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":53,"id":"775f70d3_d72dcc91","updated":"2023-08-31 10:45:50.000000000","message":"Okay, we could loop over this series for a while, but I think it\u0027s time to call it a day.","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"93bf0c6eeb19748e53091e02dc08d7def2f724da","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":55,"id":"e474dd0e_7d7be849","updated":"2023-08-31 14:11:11.000000000","message":"You also modified the UTs, so sounds good to me.\n\n(quickly compared PS53 and 55)","commit_id":"acbc5d5efb3d7ab1c3e329cbdc4cd08e7f16781b"},{"author":{"_account_id":35378,"name":"Sang Tran","email":"SangTQ8@fpt.com","username":"sang.tran"},"change_message_id":"c9c8efd65549265a91840960e1b0783a1c758ec4","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":56,"id":"77120cfc_180ec0ab","updated":"2024-01-04 19:00:18.000000000","message":"Hi contributor,\nIve got the error with this patch when rebooting vm with volume source from the image, the result is Error with VolumeDeviceNotFound (Tested with Netapp \u0026 PowerStrore iSCSI SAN driver)\nPlease take a look!","commit_id":"9d5935d007e5a50a4fd61ad033cd0135fdcaa2eb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9ed53e6324ca3461ddb57dbd4ced75ab5da5fc10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":56,"id":"ce4b8bf6_324c8630","updated":"2023-08-31 16:47:04.000000000","message":"recheck","commit_id":"9d5935d007e5a50a4fd61ad033cd0135fdcaa2eb"},{"author":{"_account_id":35378,"name":"Sang Tran","email":"SangTQ8@fpt.com","username":"sang.tran"},"change_message_id":"5a995431a2e197e71cd2af78b8af7478b46cc131","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":56,"id":"cf53c2f0_359870f2","in_reply_to":"0747dbf1_325150d7","updated":"2024-01-09 08:59:49.000000000","message":"Hi Amit,\nPlease check my reported bug\nhttps://bugs.launchpad.net/nova/+bug/2048154","commit_id":"9d5935d007e5a50a4fd61ad033cd0135fdcaa2eb"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a040510da4321c72353c1a9e3e534d3c58efa6cb","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":56,"id":"0747dbf1_325150d7","in_reply_to":"77120cfc_180ec0ab","updated":"2024-01-05 03:57:10.000000000","message":"Hi Sang,\n\ncan you please report a bug  https://bugs.launchpad.net/nova\nwith details and logs so we can understand the exact issue and fix it.\n\nthanks","commit_id":"9d5935d007e5a50a4fd61ad033cd0135fdcaa2eb"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a579abf91571db130d0d16b542a3e16950912213","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":56,"id":"df188fc3_6b7912e7","in_reply_to":"cf53c2f0_359870f2","updated":"2024-02-20 04:38:44.000000000","message":"this is fixed here https://review.opendev.org/c/openstack/nova/+/906089\nthanks for reporting bug","commit_id":"9d5935d007e5a50a4fd61ad033cd0135fdcaa2eb"}],"doc/source/admin/manage-volumes.rst":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"642e5611a1a58f12650c51c7a908c7f67392a999","unresolved":true,"context_lines":[{"line_number":84,"context_line":".. note::"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    If you encounter any dangling volume attachments in either the Nova or"},{"line_number":87,"context_line":"    Cinder databases, a simple restart of the affected instance can help"},{"line_number":88,"context_line":"    resolve the issue. During the instance reboot process, Nova performs"},{"line_number":89,"context_line":"    a synchronization mechanism that verifies the availability of volume"},{"line_number":90,"context_line":"    attachments in the Cinder database. Any missing or dangling/stale"}],"source_content_type":"text/x-rst","patch_set":53,"id":"b61679f8_f9c459b2","line":87,"range":{"start_line":87,"start_character":31,"end_line":87,"end_character":38},"updated":"2023-08-31 11:16:40.000000000","message":"nit: we shoudl say ``hard reboot`` not restart here to be clear.\nthis wont happen on a soft reboot.","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c2d4acdf8ee42b4d645e3c0f73841b2bba8ede40","unresolved":false,"context_lines":[{"line_number":84,"context_line":".. note::"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    If you encounter any dangling volume attachments in either the Nova or"},{"line_number":87,"context_line":"    Cinder databases, a simple restart of the affected instance can help"},{"line_number":88,"context_line":"    resolve the issue. During the instance reboot process, Nova performs"},{"line_number":89,"context_line":"    a synchronization mechanism that verifies the availability of volume"},{"line_number":90,"context_line":"    attachments in the Cinder database. Any missing or dangling/stale"}],"source_content_type":"text/x-rst","patch_set":53,"id":"b6495104_4c258135","line":87,"range":{"start_line":87,"start_character":31,"end_line":87,"end_character":38},"in_reply_to":"b61679f8_f9c459b2","updated":"2023-08-31 14:06:22.000000000","message":"Done","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"642e5611a1a58f12650c51c7a908c7f67392a999","unresolved":true,"context_lines":[{"line_number":88,"context_line":"    resolve the issue. During the instance reboot process, Nova performs"},{"line_number":89,"context_line":"    a synchronization mechanism that verifies the availability of volume"},{"line_number":90,"context_line":"    attachments in the Cinder database. Any missing or dangling/stale"},{"line_number":91,"context_line":"    attachments are automatically detected and deleted from both Nova and Cinder."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""}],"source_content_type":"text/x-rst","patch_set":53,"id":"167175a7_73dff36f","line":91,"updated":"2023-08-31 11:16:40.000000000","message":"again only on hard reboot.","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c2d4acdf8ee42b4d645e3c0f73841b2bba8ede40","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    resolve the issue. During the instance reboot process, Nova performs"},{"line_number":89,"context_line":"    a synchronization mechanism that verifies the availability of volume"},{"line_number":90,"context_line":"    attachments in the Cinder database. Any missing or dangling/stale"},{"line_number":91,"context_line":"    attachments are automatically detected and deleted from both Nova and Cinder."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""}],"source_content_type":"text/x-rst","patch_set":53,"id":"c4b49436_3b2a89ef","line":91,"in_reply_to":"167175a7_73dff36f","updated":"2023-08-31 14:06:22.000000000","message":"Done","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"}],"nova/compute/manager.py":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"fa9a5bcff20e4ea7bc1e37536effe467f9257af6","unresolved":true,"context_lines":[{"line_number":4164,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4165,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4166,"context_line":"                try:"},{"line_number":4167,"context_line":"                    volume \u003d self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.check_attached(admin_ctxt, volume)"},{"line_number":4169,"context_line":"                except exception.InvalidVolume as ex:"},{"line_number":4170,"context_line":"                    LOG.exception(ex)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fe144b9b_85f9e418","line":4167,"updated":"2023-05-22 13:42:53.000000000","message":"is it okay to call cinder api from compute.managers, on big deployment, can this be a performance issue ?","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"ac3d22e9061880feb53498e3a8a0a80fa0e52eeb","unresolved":false,"context_lines":[{"line_number":4164,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4165,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4166,"context_line":"                try:"},{"line_number":4167,"context_line":"                    volume \u003d self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.check_attached(admin_ctxt, volume)"},{"line_number":4169,"context_line":"                except exception.InvalidVolume as ex:"},{"line_number":4170,"context_line":"                    LOG.exception(ex)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9888db03_16617594","line":4167,"in_reply_to":"7017922b_0e5d10b6","updated":"2023-05-23 10:58:14.000000000","message":"Ack, thanks for confirming.","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdb3016ed05831e7dbb55cc78541089dfe1131a9","unresolved":true,"context_lines":[{"line_number":4164,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4165,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4166,"context_line":"                try:"},{"line_number":4167,"context_line":"                    volume \u003d self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.check_attached(admin_ctxt, volume)"},{"line_number":4169,"context_line":"                except exception.InvalidVolume as ex:"},{"line_number":4170,"context_line":"                    LOG.exception(ex)"}],"source_content_type":"text/x-python","patch_set":11,"id":"7017922b_0e5d10b6","line":4167,"in_reply_to":"fe144b9b_85f9e418","updated":"2023-05-22 15:56:56.000000000","message":"We already do elsewhere when needed, so yes.","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dca886e01ef2a3c6342decc12a7b8796f7fb346a","unresolved":true,"context_lines":[{"line_number":4167,"context_line":"                    volume \u003d self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.check_attached(admin_ctxt, volume)"},{"line_number":4169,"context_line":"                except exception.InvalidVolume as ex:"},{"line_number":4170,"context_line":"                    LOG.exception(ex)"},{"line_number":4171,"context_line":"                    LOG.info("},{"line_number":4172,"context_line":"                        f\"Deleting volume \u0027{bdm.volume_id}\u0027 \""},{"line_number":4173,"context_line":"                        \"from nova block device mapping.\")"}],"source_content_type":"text/x-python","patch_set":11,"id":"6fbc8162_8faeea0c","line":4170,"updated":"2023-05-22 13:57:19.000000000","message":"Why log the exception? This is the case you\u0027re trying to handle, right? Logging an exception (and a stack trace) right before an info at for the same thing is probably a sign of overkill. We should only be allowing stack traces in the logs for unhandled, unexpected, or extreme situations. The `LOG.info()` is plenty, IMHO.","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdb3016ed05831e7dbb55cc78541089dfe1131a9","unresolved":true,"context_lines":[{"line_number":4167,"context_line":"                    volume \u003d self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.check_attached(admin_ctxt, volume)"},{"line_number":4169,"context_line":"                except exception.InvalidVolume as ex:"},{"line_number":4170,"context_line":"                    LOG.exception(ex)"},{"line_number":4171,"context_line":"                    LOG.info("},{"line_number":4172,"context_line":"                        f\"Deleting volume \u0027{bdm.volume_id}\u0027 \""},{"line_number":4173,"context_line":"                        \"from nova block device mapping.\")"}],"source_content_type":"text/x-python","patch_set":11,"id":"6e353ce2_9fcd904f","line":4170,"in_reply_to":"3115c13f_956efa36","updated":"2023-05-22 15:56:56.000000000","message":"We should not be logging tracebacks in the logs for any situation other than unexpected fatal errors where *we* will need to debug the problem. This is nova healing itself from an expected/understood case and should be info or warning only, but *not* exception and *not* with a stack trace.","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"ac3d22e9061880feb53498e3a8a0a80fa0e52eeb","unresolved":false,"context_lines":[{"line_number":4167,"context_line":"                    volume \u003d self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.check_attached(admin_ctxt, volume)"},{"line_number":4169,"context_line":"                except exception.InvalidVolume as ex:"},{"line_number":4170,"context_line":"                    LOG.exception(ex)"},{"line_number":4171,"context_line":"                    LOG.info("},{"line_number":4172,"context_line":"                        f\"Deleting volume \u0027{bdm.volume_id}\u0027 \""},{"line_number":4173,"context_line":"                        \"from nova block device mapping.\")"}],"source_content_type":"text/x-python","patch_set":11,"id":"e0139f99_ea240604","line":4170,"in_reply_to":"6e353ce2_9fcd904f","updated":"2023-05-23 10:58:14.000000000","message":"Done","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"e1a945e1cf980c79bd3d8cc8e9af894315a376a5","unresolved":true,"context_lines":[{"line_number":4167,"context_line":"                    volume \u003d self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.check_attached(admin_ctxt, volume)"},{"line_number":4169,"context_line":"                except exception.InvalidVolume as ex:"},{"line_number":4170,"context_line":"                    LOG.exception(ex)"},{"line_number":4171,"context_line":"                    LOG.info("},{"line_number":4172,"context_line":"                        f\"Deleting volume \u0027{bdm.volume_id}\u0027 \""},{"line_number":4173,"context_line":"                        \"from nova block device mapping.\")"}],"source_content_type":"text/x-python","patch_set":11,"id":"3115c13f_956efa36","line":4170,"in_reply_to":"6fbc8162_8faeea0c","updated":"2023-05-22 15:14:50.000000000","message":"as this is not a standard scenario and nova will be doing DB-related automatic change, I wanted to show this to the operator, in logs, so they can easily find and understand what happend and how it gets resolved.\n```\n  ERROR nova.compute.manager Traceback (most recent call last):\n  ERROR nova.compute.manager   File \"/opt/stack/nova/nova/compute/manager.py\", line 4168, in _delete_dangling_bdms\n  ERROR nova.compute.manager     self.volume_api.check_attached(admin_ctxt, volume)\n  ERROR nova.compute.manager   File \"/opt/stack/nova/nova/volume/cinder.py\", line 524, in check_attached\n  ERROR nova.compute.manager     raise exception.InvalidVolume(reason\u003dmsg)\n  ERROR nova.compute.manager nova.exception.InvalidVolume: Invalid volume: volume \u0027VOLUME-ID\u0027 status must be \u0027in-use\u0027. Currently in \u0027available\u0027 status\n  ERROR nova.compute.manager\n  INFO nova.compute.manager [None \u0027REQ-ID\u0027 admin admin] Deleting volume \u0027VOLUME-ID\u0027 from nova block device mapping.\n```","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dca886e01ef2a3c6342decc12a7b8796f7fb346a","unresolved":true,"context_lines":[{"line_number":4170,"context_line":"                    LOG.exception(ex)"},{"line_number":4171,"context_line":"                    LOG.info("},{"line_number":4172,"context_line":"                        f\"Deleting volume \u0027{bdm.volume_id}\u0027 \""},{"line_number":4173,"context_line":"                        \"from nova block device mapping.\")"},{"line_number":4174,"context_line":"                    bdm.destroy()"},{"line_number":4175,"context_line":""},{"line_number":4176,"context_line":"    @wrap_exception()"}],"source_content_type":"text/x-python","patch_set":11,"id":"a039efdd_626a93ac","line":4173,"updated":"2023-05-22 13:57:19.000000000","message":"I think \"deleting volume\" is too scary a word. It\u0027s clear you\u0027re only deleting it from the BDMs, but BDMs are mostly an internal construct and not something they necessarily understand directly. If they don\u0027t, a reader might think you\u0027re deleting an *actual* volume. I\u0027d suggest something like \"Removing stale volume attachment from instance\".\n\nAlso, I think you should include `instance\u003dinstance` in the LOG.info call so that they know *which* instance is being affected.","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"e1a945e1cf980c79bd3d8cc8e9af894315a376a5","unresolved":true,"context_lines":[{"line_number":4170,"context_line":"                    LOG.exception(ex)"},{"line_number":4171,"context_line":"                    LOG.info("},{"line_number":4172,"context_line":"                        f\"Deleting volume \u0027{bdm.volume_id}\u0027 \""},{"line_number":4173,"context_line":"                        \"from nova block device mapping.\")"},{"line_number":4174,"context_line":"                    bdm.destroy()"},{"line_number":4175,"context_line":""},{"line_number":4176,"context_line":"    @wrap_exception()"}],"source_content_type":"text/x-python","patch_set":11,"id":"b6bd012b_a70311a6","line":4173,"in_reply_to":"a039efdd_626a93ac","updated":"2023-05-22 15:14:50.000000000","message":"Ack, will update as per comment.","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"58054c9d9ac5dd6f04c3496c5b1e5f7b1fcae76e","unresolved":false,"context_lines":[{"line_number":4170,"context_line":"                    LOG.exception(ex)"},{"line_number":4171,"context_line":"                    LOG.info("},{"line_number":4172,"context_line":"                        f\"Deleting volume \u0027{bdm.volume_id}\u0027 \""},{"line_number":4173,"context_line":"                        \"from nova block device mapping.\")"},{"line_number":4174,"context_line":"                    bdm.destroy()"},{"line_number":4175,"context_line":""},{"line_number":4176,"context_line":"    @wrap_exception()"}],"source_content_type":"text/x-python","patch_set":11,"id":"05f06324_a0dbc2d8","line":4173,"in_reply_to":"b6bd012b_a70311a6","updated":"2023-05-31 15:02:30.000000000","message":"Done","commit_id":"cb27ee8360339bc1b5ba4498f0c96c8cc9d2e9e9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"723a8415971bf165b7ca46688b14d343a2634477","unresolved":true,"context_lines":[{"line_number":4165,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4166,"context_line":"                try:"},{"line_number":4167,"context_line":"                    volume \u003d self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.check_attached(admin_ctxt, volume)"},{"line_number":4169,"context_line":"                except exception.InvalidVolume:"},{"line_number":4170,"context_line":"                    LOG.info("},{"line_number":4171,"context_line":"                        f\"Removing stale volume attachment from instance for \""}],"source_content_type":"text/x-python","patch_set":14,"id":"de166d5b_cf14cf42","line":4168,"updated":"2023-05-24 16:34:28.000000000","message":"So, this is not a strong enough check. This will return true if the volume is attached *anywhere*, not just to this instance. Especially given CVE-2023-2088, we need to check that our actual attachment id is still valid and purge if it is not. So that means using `attachment_get(attachment_id)` instead of just `check_attached()`.","commit_id":"10fbcfb23e56000a3babb00556cc753ebf718852"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"58054c9d9ac5dd6f04c3496c5b1e5f7b1fcae76e","unresolved":false,"context_lines":[{"line_number":4165,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4166,"context_line":"                try:"},{"line_number":4167,"context_line":"                    volume \u003d self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.check_attached(admin_ctxt, volume)"},{"line_number":4169,"context_line":"                except exception.InvalidVolume:"},{"line_number":4170,"context_line":"                    LOG.info("},{"line_number":4171,"context_line":"                        f\"Removing stale volume attachment from instance for \""}],"source_content_type":"text/x-python","patch_set":14,"id":"2dd37378_cc72b1d8","line":4168,"in_reply_to":"de166d5b_cf14cf42","updated":"2023-05-31 15:02:30.000000000","message":"thanks, attachment_get(attachment_id) worked better, as target attachment is not present at cinder side at all.","commit_id":"10fbcfb23e56000a3babb00556cc753ebf718852"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"18a042f73984ce0dea08562b2fdbe471c59fb85a","unresolved":true,"context_lines":[{"line_number":4164,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4165,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4166,"context_line":"                try:"},{"line_number":4167,"context_line":"                    self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.attachment_get(context, bdm.attachment_id)"},{"line_number":4169,"context_line":"                except exception.VolumeAttachmentNotFound:"},{"line_number":4170,"context_line":"                    LOG.info("}],"source_content_type":"text/x-python","patch_set":15,"id":"0d85d69c_d7aee5d6","line":4167,"updated":"2023-06-01 13:31:56.000000000","message":"Will `volume_get()` raise `VolumeAttachmentNotFound`? I would expect not... However they probably both inherit from a single `NotFound` class you can look for. If not, and assuming I\u0027m right, we should be expecting both types. That said, I\u0027m not sure we need to check both - if the attachment is gone, that\u0027s enough, and if the volume has been deleted, surely the `attachment_get` will also return `NotFound` right?","commit_id":"34d7b5e1e7c6b5ff93e8d6d02f081d34a0b64ba6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"5b7c90d827e8627962d11860458d77e35e1722e2","unresolved":false,"context_lines":[{"line_number":4164,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4165,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4166,"context_line":"                try:"},{"line_number":4167,"context_line":"                    self.volume_api.get(admin_ctxt, bdm.volume_id)"},{"line_number":4168,"context_line":"                    self.volume_api.attachment_get(context, bdm.attachment_id)"},{"line_number":4169,"context_line":"                except exception.VolumeAttachmentNotFound:"},{"line_number":4170,"context_line":"                    LOG.info("}],"source_content_type":"text/x-python","patch_set":15,"id":"18b7c052_200832cc","line":4167,"in_reply_to":"0d85d69c_d7aee5d6","updated":"2023-06-08 14:20:53.000000000","message":"yes. \nattachment_get is better, volume_get() failed for me on one TC, updated, thanks.","commit_id":"34d7b5e1e7c6b5ff93e8d6d02f081d34a0b64ba6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"eab8291ef9ff03474c468e35dc1c6e9306da5677","unresolved":true,"context_lines":[{"line_number":4187,"context_line":"        nova_attachments \u003d []"},{"line_number":4188,"context_line":"        for bdm in bdms.objects:"},{"line_number":4189,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4190,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4191,"context_line":"                try:"},{"line_number":4192,"context_line":"                    self.volume_api.attachment_get(context, bdm.attachment_id)"},{"line_number":4193,"context_line":"                except exception.VolumeAttachmentNotFound:"}],"source_content_type":"text/x-python","patch_set":28,"id":"e52cc955_8460dcad","line":4190,"updated":"2023-07-03 14:38:07.000000000","message":"test this, this is better:\n`if bdm.is_volume and bdm.attachment_id:`","commit_id":"0530449539ed2ad055e631ab2c7e2c834d8f93e5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6e41602fd3a93bfcf647ef121a5a0e997e3bf8c2","unresolved":false,"context_lines":[{"line_number":4187,"context_line":"        nova_attachments \u003d []"},{"line_number":4188,"context_line":"        for bdm in bdms.objects:"},{"line_number":4189,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4190,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4191,"context_line":"                try:"},{"line_number":4192,"context_line":"                    self.volume_api.attachment_get(context, bdm.attachment_id)"},{"line_number":4193,"context_line":"                except exception.VolumeAttachmentNotFound:"}],"source_content_type":"text/x-python","patch_set":28,"id":"de4089aa_6d9a70bc","line":4190,"in_reply_to":"e52cc955_8460dcad","updated":"2023-07-04 19:02:47.000000000","message":"detailed check is better.","commit_id":"0530449539ed2ad055e631ab2c7e2c834d8f93e5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4ee14373a14dfb2553c80457dc27bdf66a559900","unresolved":true,"context_lines":[{"line_number":4199,"context_line":"                except exception.VolumeAttachmentNotFound:"},{"line_number":4200,"context_line":"                    LOG.info("},{"line_number":4201,"context_line":"                        f\"Removing stale volume attachment from instance for \""},{"line_number":4202,"context_line":"                        f\"volume \u0027{bdm.volume_id}\u0027.\", instance\u003dinstance)"},{"line_number":4203,"context_line":"                    bdm.destroy()"},{"line_number":4204,"context_line":"                else:"},{"line_number":4205,"context_line":"                    nova_attachments.append(bdm.attachment_id)"}],"source_content_type":"text/x-python","patch_set":47,"id":"b29eb25d_8c268d77","line":4202,"updated":"2023-08-29 15:48:02.000000000","message":"here you should also give the attachment ID IMHO.","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"04f574b8437d259f8d29f741ec2771ea2403bbce","unresolved":false,"context_lines":[{"line_number":4199,"context_line":"                except exception.VolumeAttachmentNotFound:"},{"line_number":4200,"context_line":"                    LOG.info("},{"line_number":4201,"context_line":"                        f\"Removing stale volume attachment from instance for \""},{"line_number":4202,"context_line":"                        f\"volume \u0027{bdm.volume_id}\u0027.\", instance\u003dinstance)"},{"line_number":4203,"context_line":"                    bdm.destroy()"},{"line_number":4204,"context_line":"                else:"},{"line_number":4205,"context_line":"                    nova_attachments.append(bdm.attachment_id)"}],"source_content_type":"text/x-python","patch_set":47,"id":"c806d09a_adb375ab","line":4202,"in_reply_to":"b29eb25d_8c268d77","updated":"2023-08-30 07:54:45.000000000","message":"Done","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4ee14373a14dfb2553c80457dc27bdf66a559900","unresolved":true,"context_lines":[{"line_number":4211,"context_line":"        if len(set(cinder_attachments) - set(nova_attachments)):"},{"line_number":4212,"context_line":"            LOG.info("},{"line_number":4213,"context_line":"                \"Removing stale volume attachments of instance from \""},{"line_number":4214,"context_line":"                \"Cinder\", instance\u003dinstance)"},{"line_number":4215,"context_line":"        for each_attach in set(cinder_attachments) - set(nova_attachments):"},{"line_number":4216,"context_line":"            # delete only cinder known attachments, from cinder DB."},{"line_number":4217,"context_line":"            self.volume_api.attachment_delete(context, each_attach)"}],"source_content_type":"text/x-python","patch_set":47,"id":"a837704e_d6dba292","line":4214,"updated":"2023-08-29 15:48:02.000000000","message":"either you tell here which attachments are stale or...","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"04f574b8437d259f8d29f741ec2771ea2403bbce","unresolved":false,"context_lines":[{"line_number":4211,"context_line":"        if len(set(cinder_attachments) - set(nova_attachments)):"},{"line_number":4212,"context_line":"            LOG.info("},{"line_number":4213,"context_line":"                \"Removing stale volume attachments of instance from \""},{"line_number":4214,"context_line":"                \"Cinder\", instance\u003dinstance)"},{"line_number":4215,"context_line":"        for each_attach in set(cinder_attachments) - set(nova_attachments):"},{"line_number":4216,"context_line":"            # delete only cinder known attachments, from cinder DB."},{"line_number":4217,"context_line":"            self.volume_api.attachment_delete(context, each_attach)"}],"source_content_type":"text/x-python","patch_set":47,"id":"7e533eee_115acc1b","line":4214,"in_reply_to":"a837704e_d6dba292","updated":"2023-08-30 07:54:45.000000000","message":"Ack","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4ee14373a14dfb2553c80457dc27bdf66a559900","unresolved":true,"context_lines":[{"line_number":4214,"context_line":"                \"Cinder\", instance\u003dinstance)"},{"line_number":4215,"context_line":"        for each_attach in set(cinder_attachments) - set(nova_attachments):"},{"line_number":4216,"context_line":"            # delete only cinder known attachments, from cinder DB."},{"line_number":4217,"context_line":"            self.volume_api.attachment_delete(context, each_attach)"},{"line_number":4218,"context_line":""},{"line_number":4219,"context_line":"    @wrap_exception()"},{"line_number":4220,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":47,"id":"96f574f6_78473b75","line":4217,"updated":"2023-08-29 15:48:02.000000000","message":"... you just log something here, probably as DEBUG.","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"04f574b8437d259f8d29f741ec2771ea2403bbce","unresolved":false,"context_lines":[{"line_number":4214,"context_line":"                \"Cinder\", instance\u003dinstance)"},{"line_number":4215,"context_line":"        for each_attach in set(cinder_attachments) - set(nova_attachments):"},{"line_number":4216,"context_line":"            # delete only cinder known attachments, from cinder DB."},{"line_number":4217,"context_line":"            self.volume_api.attachment_delete(context, each_attach)"},{"line_number":4218,"context_line":""},{"line_number":4219,"context_line":"    @wrap_exception()"},{"line_number":4220,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":47,"id":"dbf3e309_7f054de3","line":4217,"in_reply_to":"96f574f6_78473b75","updated":"2023-08-30 07:54:45.000000000","message":"Done","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4ee14373a14dfb2553c80457dc27bdf66a559900","unresolved":true,"context_lines":[{"line_number":4246,"context_line":"        self._delete_dangling_bdms(context, instance)"},{"line_number":4247,"context_line":""},{"line_number":4248,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":4249,"context_line":"            context, instance.uuid)"},{"line_number":4250,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":4251,"context_line":"            context, instance, bdms\u003dbdms)"},{"line_number":4252,"context_line":""}],"source_content_type":"text/x-python","patch_set":47,"id":"c39bfd6a_ef91bf21","line":4249,"updated":"2023-08-29 15:48:02.000000000","message":"why aren\u0027t you taking the opportunity to get the BDM list here and pass it as an argument to the new method you create ?\n\nHere, we call twice the conductor by RPC for getting these objects from the DB.","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b90af6277a7932b62ffb06e5c13bcf162c2c8d70","unresolved":true,"context_lines":[{"line_number":4246,"context_line":"        self._delete_dangling_bdms(context, instance)"},{"line_number":4247,"context_line":""},{"line_number":4248,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":4249,"context_line":"            context, instance.uuid)"},{"line_number":4250,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":4251,"context_line":"            context, instance, bdms\u003dbdms)"},{"line_number":4252,"context_line":""}],"source_content_type":"text/x-python","patch_set":47,"id":"5917ac10_2c07f5cc","line":4249,"in_reply_to":"3b122c85_e8106ac0","updated":"2023-08-30 10:09:57.000000000","message":"To clarify, I\u0027m just proposing to get once the BDM list, and then passing this list to _delete_dangling_bdms as an argument.\n\nIf for some reason, we identify the BDM as stale, then just after we call bdm.destroy() we can *also* delete the BDM from the list.\n\nIn that case, the next call to _get_instance_block_device_info() will get the bdm list without the destroyed BDM, without requiring a second RPC call.\n\nhttps://paste.opendev.org/show/bBaeGvpA8Bx5sSF3wKgZ/ as a very high-level view of what I am saying.","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"65b4d32c1669c3dc7e065a0c5170889454a20682","unresolved":true,"context_lines":[{"line_number":4246,"context_line":"        self._delete_dangling_bdms(context, instance)"},{"line_number":4247,"context_line":""},{"line_number":4248,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":4249,"context_line":"            context, instance.uuid)"},{"line_number":4250,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":4251,"context_line":"            context, instance, bdms\u003dbdms)"},{"line_number":4252,"context_line":""}],"source_content_type":"text/x-python","patch_set":47,"id":"6d96c8f1_895db6a1","line":4249,"in_reply_to":"5917ac10_2c07f5cc","updated":"2023-08-30 11:40:34.000000000","message":"del bdms[bdm] and bdms.remove are not working\n```*** TypeError: \u0027BlockDeviceMappingList\u0027 object doesn\u0027t support item deletion```\n\nas bdms is not regular list.\nits inherited from ObjectListBase which is again from ovoo_base.ObjectListBase  whihc is oslo.\n\nBlockDeviceMappingList has these attributes and methods https://paste.opendev.org/show/bnDCZUcY00oiKOYIreMs/","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"f00b7ef7a26175b16c42672118cb68b57da99340","unresolved":true,"context_lines":[{"line_number":4246,"context_line":"        self._delete_dangling_bdms(context, instance)"},{"line_number":4247,"context_line":""},{"line_number":4248,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":4249,"context_line":"            context, instance.uuid)"},{"line_number":4250,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":4251,"context_line":"            context, instance, bdms\u003dbdms)"},{"line_number":4252,"context_line":""}],"source_content_type":"text/x-python","patch_set":47,"id":"97cfdc28_1ab39bef","line":4249,"in_reply_to":"6d96c8f1_895db6a1","updated":"2023-08-30 12:27:37.000000000","message":"deleting bdms.object works\n```BlockDeviceMappingList(objects\u003d[BlockDeviceMapping(UNKNOWN),BlockDeviceMapping(UNKNOWN)])```","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4dec5c0e7dde8a490d7d6239820dc85e84da6ed2","unresolved":false,"context_lines":[{"line_number":4246,"context_line":"        self._delete_dangling_bdms(context, instance)"},{"line_number":4247,"context_line":""},{"line_number":4248,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":4249,"context_line":"            context, instance.uuid)"},{"line_number":4250,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":4251,"context_line":"            context, instance, bdms\u003dbdms)"},{"line_number":4252,"context_line":""}],"source_content_type":"text/x-python","patch_set":47,"id":"af96c30a_ae9a01cb","line":4249,"in_reply_to":"97cfdc28_1ab39bef","updated":"2023-08-31 10:45:50.000000000","message":"Ack","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"04f574b8437d259f8d29f741ec2771ea2403bbce","unresolved":true,"context_lines":[{"line_number":4246,"context_line":"        self._delete_dangling_bdms(context, instance)"},{"line_number":4247,"context_line":""},{"line_number":4248,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":4249,"context_line":"            context, instance.uuid)"},{"line_number":4250,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":4251,"context_line":"            context, instance, bdms\u003dbdms)"},{"line_number":4252,"context_line":""}],"source_content_type":"text/x-python","patch_set":47,"id":"e3ba4138_27514c8f","line":4249,"in_reply_to":"c39bfd6a_ef91bf21","updated":"2023-08-30 07:54:45.000000000","message":"yes, intentionally to get a fresh copy of bdm from DB.","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ce0f30bee2929e0bb57bfb78fec150ff62784d17","unresolved":true,"context_lines":[{"line_number":4246,"context_line":"        self._delete_dangling_bdms(context, instance)"},{"line_number":4247,"context_line":""},{"line_number":4248,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":4249,"context_line":"            context, instance.uuid)"},{"line_number":4250,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":4251,"context_line":"            context, instance, bdms\u003dbdms)"},{"line_number":4252,"context_line":""}],"source_content_type":"text/x-python","patch_set":47,"id":"3b122c85_e8106ac0","line":4249,"in_reply_to":"e3ba4138_27514c8f","updated":"2023-08-30 09:50:25.000000000","message":"Hmpf, I don\u0027t think we race here with the list of BDMs, given we get it a microsecond after.\nHere, I rather see we call twice the RPC API for something that is 99.99% possibly identical and if it\u0027s really not, users can reboot a second time their instance.\n\nIs it really worth it ? I\u0027m not sure tbh.","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"5488697eb9565ca1163b04fd62e0c618478b7e60","unresolved":true,"context_lines":[{"line_number":4223,"context_line":"        for bdm in bdms_to_delete:"},{"line_number":4224,"context_line":"            index \u003d bdms.index(bdm)"},{"line_number":4225,"context_line":"            del bdms.objects[index]"},{"line_number":4226,"context_line":""},{"line_number":4227,"context_line":"    @wrap_exception()"},{"line_number":4228,"context_line":"    @reverts_task_state"},{"line_number":4229,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"}],"source_content_type":"text/x-python","patch_set":49,"id":"8d48ae00_cd980068","line":4226,"updated":"2023-08-30 12:40:23.000000000","message":"this could have \n```\nfor index, bdm in enumerate(bdms_to_delete):\n    del bdms.objects[index])```","commit_id":"00a55ff5dfe2f3cb8403ae1fc944777d1d728b9b"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a0d1c121f31a05c90ed3d03db20c189685745014","unresolved":false,"context_lines":[{"line_number":4223,"context_line":"        for bdm in bdms_to_delete:"},{"line_number":4224,"context_line":"            index \u003d bdms.index(bdm)"},{"line_number":4225,"context_line":"            del bdms.objects[index]"},{"line_number":4226,"context_line":""},{"line_number":4227,"context_line":"    @wrap_exception()"},{"line_number":4228,"context_line":"    @reverts_task_state"},{"line_number":4229,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"}],"source_content_type":"text/x-python","patch_set":49,"id":"1658520e_636b6553","line":4226,"in_reply_to":"8d48ae00_cd980068","updated":"2023-08-30 17:00:35.000000000","message":"Done","commit_id":"00a55ff5dfe2f3cb8403ae1fc944777d1d728b9b"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"75295a5de2c29fe37b1aed2f58201921068d348b","unresolved":false,"context_lines":[{"line_number":5689,"context_line":"        # in case _prep_resize fails."},{"line_number":5690,"context_line":"        instance_state \u003d instance.vm_state"},{"line_number":5691,"context_line":"        with self._error_out_instance_on_exception("},{"line_number":5692,"context_line":"                context, instance, instance_state\u003dinstance_state), \\"},{"line_number":5693,"context_line":"                errors_out_migration_ctxt(migration):"},{"line_number":5694,"context_line":""},{"line_number":5695,"context_line":"            self._send_prep_resize_notifications("}],"source_content_type":"text/x-python","patch_set":49,"id":"333d232f_88ae1dad","line":5692,"updated":"2023-08-30 12:31:47.000000000","message":"this change is not part of functionality, this was just a pep8 thing on rebase.","commit_id":"00a55ff5dfe2f3cb8403ae1fc944777d1d728b9b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"144f9e94196c4df88116b546a416c3987fb9b0c9","unresolved":true,"context_lines":[{"line_number":4190,"context_line":"        # attachments present in nova DB, ones nova knows about"},{"line_number":4191,"context_line":"        nova_attachments \u003d []"},{"line_number":4192,"context_line":"        bdms_to_delete \u003d []"},{"line_number":4193,"context_line":"        for bdm in bdms.objects:"},{"line_number":4194,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4195,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4196,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":50,"id":"77af66ec_ef0bbdcc","line":4193,"range":{"start_line":4193,"start_character":23,"end_line":4193,"end_character":31},"updated":"2023-08-30 14:20:33.000000000","message":"you don\u0027t need to get the field, you can just loop over bdms","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a0d1c121f31a05c90ed3d03db20c189685745014","unresolved":true,"context_lines":[{"line_number":4190,"context_line":"        # attachments present in nova DB, ones nova knows about"},{"line_number":4191,"context_line":"        nova_attachments \u003d []"},{"line_number":4192,"context_line":"        bdms_to_delete \u003d []"},{"line_number":4193,"context_line":"        for bdm in bdms.objects:"},{"line_number":4194,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4195,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4196,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":50,"id":"88ffbdda_56db65c3","line":4193,"range":{"start_line":4193,"start_character":23,"end_line":4193,"end_character":31},"in_reply_to":"57392288_0ead795d","updated":"2023-08-30 17:00:35.000000000","message":"earlier I did as `for bdm in bdms: work_on(bdm)` \nbut creating bdm for tests specifically this https://review.opendev.org/c/openstack/nova/+/882284/50/nova/tests/unit/compute/test_compute.py#1515\n\nI have use bdms.objects, and this worked forall scenarios.\ntest bdms object looks like:\n```\nBlockDeviceMappingList(objects\u003d[\n   BlockDeviceMapping(UNKNOWN),\n   BlockDeviceMapping(UNKNOWN)\n  ])```","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4439da8a50e64fc56e5057a66117016a47576c07","unresolved":true,"context_lines":[{"line_number":4190,"context_line":"        # attachments present in nova DB, ones nova knows about"},{"line_number":4191,"context_line":"        nova_attachments \u003d []"},{"line_number":4192,"context_line":"        bdms_to_delete \u003d []"},{"line_number":4193,"context_line":"        for bdm in bdms.objects:"},{"line_number":4194,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4195,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4196,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":50,"id":"57392288_0ead795d","line":4193,"range":{"start_line":4193,"start_character":23,"end_line":4193,"end_character":31},"in_reply_to":"77af66ec_ef0bbdcc","updated":"2023-08-30 14:32:26.000000000","message":"This is a nit, which could be addressed in a FUP.","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4dec5c0e7dde8a490d7d6239820dc85e84da6ed2","unresolved":false,"context_lines":[{"line_number":4190,"context_line":"        # attachments present in nova DB, ones nova knows about"},{"line_number":4191,"context_line":"        nova_attachments \u003d []"},{"line_number":4192,"context_line":"        bdms_to_delete \u003d []"},{"line_number":4193,"context_line":"        for bdm in bdms.objects:"},{"line_number":4194,"context_line":"            if bdm.volume_id and bdm.source_type \u003d\u003d \u0027volume\u0027 and \\"},{"line_number":4195,"context_line":"                bdm.destination_type \u003d\u003d \u0027volume\u0027:"},{"line_number":4196,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":50,"id":"7d764805_6a4f9f61","line":4193,"range":{"start_line":4193,"start_character":23,"end_line":4193,"end_character":31},"in_reply_to":"88ffbdda_56db65c3","updated":"2023-08-31 10:45:50.000000000","message":"Shall work with your tests, objects.BlockDeviceMappingList() is a python container and you should be able to iterate over it.\n\nAnyway, I don\u0027t wanna hold this series based on such nit.","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"144f9e94196c4df88116b546a416c3987fb9b0c9","unresolved":true,"context_lines":[{"line_number":4201,"context_line":"                        f\"\u0027{bdm.attachment_id}\u0027 from instance for \""},{"line_number":4202,"context_line":"                        f\"volume \u0027{bdm.volume_id}\u0027.\", instance\u003dinstance)"},{"line_number":4203,"context_line":"                    bdm.destroy()"},{"line_number":4204,"context_line":"                    bdms_to_delete.append(bdm)"},{"line_number":4205,"context_line":"                else:"},{"line_number":4206,"context_line":"                    nova_attachments.append(bdm.attachment_id)"},{"line_number":4207,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"ceba9f40_2c951458","line":4204,"updated":"2023-08-30 14:20:33.000000000","message":"this is an unnecessary extrastep, see above","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"144f9e94196c4df88116b546a416c3987fb9b0c9","unresolved":true,"context_lines":[{"line_number":4201,"context_line":"                        f\"\u0027{bdm.attachment_id}\u0027 from instance for \""},{"line_number":4202,"context_line":"                        f\"volume \u0027{bdm.volume_id}\u0027.\", instance\u003dinstance)"},{"line_number":4203,"context_line":"                    bdm.destroy()"},{"line_number":4204,"context_line":"                    bdms_to_delete.append(bdm)"},{"line_number":4205,"context_line":"                else:"},{"line_number":4206,"context_line":"                    nova_attachments.append(bdm.attachment_id)"},{"line_number":4207,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"8a1291a9_e0291cdc","line":4204,"updated":"2023-08-30 14:20:33.000000000","message":"this isn\u0027t needed, you can access the index directly, this way :\n\n```\nfor idx, bdm in enumerate(bdms):\n   \u003cdo the above\u003e\n   if \u003cbdm is a volume\u003e:\n      try:\n         \u003cabove\u003e\n      except \u003cvolumenotfound\u003e:\n         del bdms.objects[idx]\n```","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4439da8a50e64fc56e5057a66117016a47576c07","unresolved":false,"context_lines":[{"line_number":4201,"context_line":"                        f\"\u0027{bdm.attachment_id}\u0027 from instance for \""},{"line_number":4202,"context_line":"                        f\"volume \u0027{bdm.volume_id}\u0027.\", instance\u003dinstance)"},{"line_number":4203,"context_line":"                    bdm.destroy()"},{"line_number":4204,"context_line":"                    bdms_to_delete.append(bdm)"},{"line_number":4205,"context_line":"                else:"},{"line_number":4206,"context_line":"                    nova_attachments.append(bdm.attachment_id)"},{"line_number":4207,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"a1f6c374_c9f24cc2","line":4204,"in_reply_to":"8a1291a9_e0291cdc","updated":"2023-08-30 14:32:26.000000000","message":"Sorry, yeah, wrong pattern I just wrote (altering a list size while we loop), nevermind my comment","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4439da8a50e64fc56e5057a66117016a47576c07","unresolved":false,"context_lines":[{"line_number":4201,"context_line":"                        f\"\u0027{bdm.attachment_id}\u0027 from instance for \""},{"line_number":4202,"context_line":"                        f\"volume \u0027{bdm.volume_id}\u0027.\", instance\u003dinstance)"},{"line_number":4203,"context_line":"                    bdm.destroy()"},{"line_number":4204,"context_line":"                    bdms_to_delete.append(bdm)"},{"line_number":4205,"context_line":"                else:"},{"line_number":4206,"context_line":"                    nova_attachments.append(bdm.attachment_id)"},{"line_number":4207,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"d83c2e23_c9348430","line":4204,"in_reply_to":"ceba9f40_2c951458","updated":"2023-08-30 14:32:26.000000000","message":"keep it if you wish.","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4dec5c0e7dde8a490d7d6239820dc85e84da6ed2","unresolved":false,"context_lines":[{"line_number":4186,"context_line":"        :param context: The nova request context."},{"line_number":4187,"context_line":"        :param instance: instance object."},{"line_number":4188,"context_line":"        :param instance: BlockDeviceMappingList list object."},{"line_number":4189,"context_line":"        :returns: BlockDeviceMappingList list object."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":""},{"line_number":4192,"context_line":"        # attachments present in nova DB, ones nova knows about"}],"source_content_type":"text/x-python","patch_set":53,"id":"b8255e21_397d984a","line":4189,"updated":"2023-08-31 10:45:50.000000000","message":"++","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f5690669b9e5795a70a3efb9311bdd100169af10","unresolved":true,"context_lines":[{"line_number":4186,"context_line":"        :param context: The nova request context."},{"line_number":4187,"context_line":"        :param instance: instance object."},{"line_number":4188,"context_line":"        :param instance: BlockDeviceMappingList list object."},{"line_number":4189,"context_line":"        :returns: BlockDeviceMappingList list object."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":""},{"line_number":4192,"context_line":"        # attachments present in nova DB, ones nova knows about"}],"source_content_type":"text/x-python","patch_set":53,"id":"6da56f7d_aae094d5","line":4189,"in_reply_to":"289f9963_494fd7a9","updated":"2023-08-31 12:45:54.000000000","message":"Yeah, what I propose is to just copy the list before looping over it and then returning it.\n\nThis way, Amit wouldn\u0027t need to update all his tests since we\u0027re close to FF.","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b7a4ff8bee91b3d02d076e39e22da14da5d1d04c","unresolved":true,"context_lines":[{"line_number":4186,"context_line":"        :param context: The nova request context."},{"line_number":4187,"context_line":"        :param instance: instance object."},{"line_number":4188,"context_line":"        :param instance: BlockDeviceMappingList list object."},{"line_number":4189,"context_line":"        :returns: BlockDeviceMappingList list object."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":""},{"line_number":4192,"context_line":"        # attachments present in nova DB, ones nova knows about"}],"source_content_type":"text/x-python","patch_set":53,"id":"fc5ce69d_fb3ae1ac","line":4189,"in_reply_to":"6da56f7d_aae094d5","updated":"2023-08-31 14:07:11.000000000","message":"once \" :returns: BlockDeviceMappingList list object.\" is revmoved this is fine to merge i thihnk","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dbc19cff1919be5e598ec8a0909e7fc85b1f1ec8","unresolved":true,"context_lines":[{"line_number":4186,"context_line":"        :param context: The nova request context."},{"line_number":4187,"context_line":"        :param instance: instance object."},{"line_number":4188,"context_line":"        :param instance: BlockDeviceMappingList list object."},{"line_number":4189,"context_line":"        :returns: BlockDeviceMappingList list object."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":""},{"line_number":4192,"context_line":"        # attachments present in nova DB, ones nova knows about"}],"source_content_type":"text/x-python","patch_set":53,"id":"289f9963_494fd7a9","line":4189,"in_reply_to":"a1cf7727_0f5bafab","updated":"2023-08-31 12:06:37.000000000","message":"im fine with either a funciton reutnrung somehting or even multiple sumthigns\nor a function having no return value and modifying it inputs.\n\nthis patthern of doing both even if you are returnign the input that is modifed causes bug we have had to fix them in nova multipel times.\n\nfor now i would prefer if you just drop the return value and we can adress this later.","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"642e5611a1a58f12650c51c7a908c7f67392a999","unresolved":true,"context_lines":[{"line_number":4186,"context_line":"        :param context: The nova request context."},{"line_number":4187,"context_line":"        :param instance: instance object."},{"line_number":4188,"context_line":"        :param instance: BlockDeviceMappingList list object."},{"line_number":4189,"context_line":"        :returns: BlockDeviceMappingList list object."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":""},{"line_number":4192,"context_line":"        # attachments present in nova DB, ones nova knows about"}],"source_content_type":"text/x-python","patch_set":53,"id":"a1cf7727_0f5bafab","line":4189,"in_reply_to":"b8255e21_397d984a","updated":"2023-08-31 11:16:40.000000000","message":"--?\n\nrealasise this is now doing the one thing i said i really am not ok with.\n\nits both returning value and modifiying in place.\n\nif you are are going to modify it you need to make a copy or constuct a new bdm object","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6dd2f06c1201e73a9ac69c282269b815f55c96fc","unresolved":false,"context_lines":[{"line_number":4186,"context_line":"        :param context: The nova request context."},{"line_number":4187,"context_line":"        :param instance: instance object."},{"line_number":4188,"context_line":"        :param instance: BlockDeviceMappingList list object."},{"line_number":4189,"context_line":"        :returns: BlockDeviceMappingList list object."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":""},{"line_number":4192,"context_line":"        # attachments present in nova DB, ones nova knows about"}],"source_content_type":"text/x-python","patch_set":53,"id":"4504f07e_afbc64fb","line":4189,"in_reply_to":"fc5ce69d_fb3ae1ac","updated":"2023-10-04 08:11:21.000000000","message":"Done","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4dec5c0e7dde8a490d7d6239820dc85e84da6ed2","unresolved":false,"context_lines":[{"line_number":4223,"context_line":""},{"line_number":4224,"context_line":"        # refresh bdms object"},{"line_number":4225,"context_line":"        for bdm in bdms_to_delete:"},{"line_number":4226,"context_line":"            bdms.objects.remove(bdm)"},{"line_number":4227,"context_line":""},{"line_number":4228,"context_line":"        return bdms"},{"line_number":4229,"context_line":""}],"source_content_type":"text/x-python","patch_set":53,"id":"371e939b_8c01cd21","line":4226,"updated":"2023-08-31 10:45:50.000000000","message":"there would be many ways of filtering this list of BDMs, including (not exhaustively) :\n\n - a comprehension list\n - the use of python\u0027s filter()\n - scrubbing a copy of the BDMs\n \nWhile I think we could optimize this code, I don\u0027t feel it would be fair to hold this series based on those implementation details (and code complexity is not really a concern here, given the short list of BDMs)","commit_id":"c89ab3b9d1c3a02c116e183ed72c2d80d4739e18"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"2df2d80da51e383afc7b372d731562ef57bbf63f","unresolved":true,"context_lines":[{"line_number":4185,"context_line":""},{"line_number":4186,"context_line":"        :param context: The nova request context."},{"line_number":4187,"context_line":"        :param instance: instance object."},{"line_number":4188,"context_line":"        :param instance: BlockDeviceMappingList list object."},{"line_number":4189,"context_line":"        \"\"\""},{"line_number":4190,"context_line":""},{"line_number":4191,"context_line":"        # attachments present in nova DB, ones nova knows about"}],"source_content_type":"text/x-python","patch_set":56,"id":"b8ae62b1_94d8a927","line":4188,"range":{"start_line":4188,"start_character":15,"end_line":4188,"end_character":23},"updated":"2023-09-01 09:00:00.000000000","message":"you meant \"bdms\" here?","commit_id":"9d5935d007e5a50a4fd61ad033cd0135fdcaa2eb"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6dd2f06c1201e73a9ac69c282269b815f55c96fc","unresolved":false,"context_lines":[{"line_number":4185,"context_line":""},{"line_number":4186,"context_line":"        :param context: The nova request context."},{"line_number":4187,"context_line":"        :param instance: instance object."},{"line_number":4188,"context_line":"        :param instance: BlockDeviceMappingList list object."},{"line_number":4189,"context_line":"        \"\"\""},{"line_number":4190,"context_line":""},{"line_number":4191,"context_line":"        # attachments present in nova DB, ones nova knows about"}],"source_content_type":"text/x-python","patch_set":56,"id":"70845f1f_31a4fd6a","line":4188,"range":{"start_line":4188,"start_character":15,"end_line":4188,"end_character":23},"in_reply_to":"07b0b0f0_a23fdd7c","updated":"2023-10-04 08:11:21.000000000","message":"Done","commit_id":"9d5935d007e5a50a4fd61ad033cd0135fdcaa2eb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"318402352efb56cf3ebedf7da772a0f8121e1308","unresolved":true,"context_lines":[{"line_number":4185,"context_line":""},{"line_number":4186,"context_line":"        :param context: The nova request context."},{"line_number":4187,"context_line":"        :param instance: instance object."},{"line_number":4188,"context_line":"        :param instance: BlockDeviceMappingList list object."},{"line_number":4189,"context_line":"        \"\"\""},{"line_number":4190,"context_line":""},{"line_number":4191,"context_line":"        # attachments present in nova DB, ones nova knows about"}],"source_content_type":"text/x-python","patch_set":56,"id":"07b0b0f0_a23fdd7c","line":4188,"range":{"start_line":4188,"start_character":15,"end_line":4188,"end_character":23},"in_reply_to":"b8ae62b1_94d8a927","updated":"2023-09-01 17:00:39.000000000","message":"thats fixed in https://review.opendev.org/c/openstack/nova/+/893502 but yes","commit_id":"9d5935d007e5a50a4fd61ad033cd0135fdcaa2eb"}],"nova/tests/functional/regressions/test_bug_1835822.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4ee14373a14dfb2553c80457dc27bdf66a559900","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        self.useFixture(nova_fixtures.GlanceFixture(self))"},{"line_number":28,"context_line":"        self.useFixture(nova_fixtures.NeutronFixture(self))"},{"line_number":29,"context_line":"        self.useFixture(func_fixtures.PlacementFixture())"},{"line_number":30,"context_line":"        self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        self.api \u003d self.useFixture(nova_fixtures.OSAPIFixture("},{"line_number":33,"context_line":"            api_version\u003d\u0027v2.1\u0027)).api"}],"source_content_type":"text/x-python","patch_set":47,"id":"94a40db8_2d92903a","line":30,"updated":"2023-08-29 15:48:02.000000000","message":"note to reviewers : this is now necessary since we call Cinder every time we reboot.","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"eab8291ef9ff03474c468e35dc1c6e9306da5677","unresolved":true,"context_lines":[{"line_number":3272,"context_line":"                    \u0027instance_uuid\u0027: instance.uuid,"},{"line_number":3273,"context_line":"                    \u0027source_type\u0027: \u0027volume\u0027,"},{"line_number":3274,"context_line":"                    \u0027destination_type\u0027: \u0027volume\u0027})"},{"line_number":3275,"context_line":"                ])"},{"line_number":3276,"context_line":""},{"line_number":3277,"context_line":"        mock, _ \u003d self._test__delete_dangling_bdms(instance, bdms, [])"},{"line_number":3278,"context_line":"        self.assertTrue(mock.called)"}],"source_content_type":"text/x-python","patch_set":31,"id":"40adb368_3d305bd9","line":3275,"updated":"2023-07-03 14:38:07.000000000","message":"here both bdms will have same attachment-id!!\nthough, these test cases are not really that much dependent on it.\nstill this need fixing.","commit_id":"b9b3272a4a02ecd564e3fcf1a6194ff4a9c60431"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6e41602fd3a93bfcf647ef121a5a0e997e3bf8c2","unresolved":false,"context_lines":[{"line_number":3272,"context_line":"                    \u0027instance_uuid\u0027: instance.uuid,"},{"line_number":3273,"context_line":"                    \u0027source_type\u0027: \u0027volume\u0027,"},{"line_number":3274,"context_line":"                    \u0027destination_type\u0027: \u0027volume\u0027})"},{"line_number":3275,"context_line":"                ])"},{"line_number":3276,"context_line":""},{"line_number":3277,"context_line":"        mock, _ \u003d self._test__delete_dangling_bdms(instance, bdms, [])"},{"line_number":3278,"context_line":"        self.assertTrue(mock.called)"}],"source_content_type":"text/x-python","patch_set":31,"id":"02fa60ab_b8674c3b","line":3275,"in_reply_to":"40adb368_3d305bd9","updated":"2023-07-04 19:02:47.000000000","message":"fixed with AnonFakeDbBlockDeviceDict","commit_id":"b9b3272a4a02ecd564e3fcf1a6194ff4a9c60431"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"eab8291ef9ff03474c468e35dc1c6e9306da5677","unresolved":true,"context_lines":[{"line_number":3278,"context_line":"        self.assertTrue(mock.called)"},{"line_number":3279,"context_line":"        self.assertEqual(mock.call_count, 2)"},{"line_number":3280,"context_line":""},{"line_number":3281,"context_line":"    def test_dangling_bdms_4_cinder(self):"},{"line_number":3282,"context_line":"        instance \u003d self._create_fake_instance_obj()"},{"line_number":3283,"context_line":"        bdms \u003d block_device_obj.block_device_make_list(self.context, ["},{"line_number":3284,"context_line":"            fake_block_device.FakeDbBlockDeviceDict({"}],"source_content_type":"text/x-python","patch_set":31,"id":"d1a75161_ca623576","line":3281,"range":{"start_line":3281,"start_character":8,"end_line":3281,"end_character":35},"updated":"2023-07-03 14:38:07.000000000","message":"1, 2, 3, 4 in names.\ngive more appropriate names","commit_id":"b9b3272a4a02ecd564e3fcf1a6194ff4a9c60431"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6e41602fd3a93bfcf647ef121a5a0e997e3bf8c2","unresolved":false,"context_lines":[{"line_number":3278,"context_line":"        self.assertTrue(mock.called)"},{"line_number":3279,"context_line":"        self.assertEqual(mock.call_count, 2)"},{"line_number":3280,"context_line":""},{"line_number":3281,"context_line":"    def test_dangling_bdms_4_cinder(self):"},{"line_number":3282,"context_line":"        instance \u003d self._create_fake_instance_obj()"},{"line_number":3283,"context_line":"        bdms \u003d block_device_obj.block_device_make_list(self.context, ["},{"line_number":3284,"context_line":"            fake_block_device.FakeDbBlockDeviceDict({"}],"source_content_type":"text/x-python","patch_set":31,"id":"17e46c19_9f6b2fc2","line":3281,"range":{"start_line":3281,"start_character":8,"end_line":3281,"end_character":35},"in_reply_to":"d1a75161_ca623576","updated":"2023-07-04 19:02:47.000000000","message":"added description doc string in each test, will continue to have 1,2,3,4 for now, until suggested otherwise.","commit_id":"b9b3272a4a02ecd564e3fcf1a6194ff4a9c60431"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"eab8291ef9ff03474c468e35dc1c6e9306da5677","unresolved":true,"context_lines":[{"line_number":3299,"context_line":"        self.assertTrue(mock.call_count, 1)"},{"line_number":3300,"context_line":"        self.assertEqual(mock.call_args_list[0][0][1], 2)"},{"line_number":3301,"context_line":""},{"line_number":3302,"context_line":"    def test__delete_dangling_bdms(self):"},{"line_number":3303,"context_line":"        instance \u003d self._create_fake_instance_obj()"},{"line_number":3304,"context_line":"        bdms \u003d block_device_obj.block_device_make_list(self.context,"},{"line_number":3305,"context_line":"                [fake_block_device.FakeDbBlockDeviceDict({"},{"line_number":3306,"context_line":"                    \u0027id\u0027: 3,"},{"line_number":3307,"context_line":"                    \u0027volume_id\u0027: uuids.volume_id,"},{"line_number":3308,"context_line":"                    \u0027instance_uuid\u0027: instance.uuid,"},{"line_number":3309,"context_line":"                    \u0027source_type\u0027: \u0027volume\u0027,"},{"line_number":3310,"context_line":"                    \u0027destination_type\u0027: \u0027volume\u0027})"},{"line_number":3311,"context_line":"                ])"},{"line_number":3312,"context_line":""},{"line_number":3313,"context_line":"        with test.nested("},{"line_number":3314,"context_line":"            mock.patch.object(objects.BlockDeviceMappingList,"},{"line_number":3315,"context_line":"                              \u0027get_by_instance_uuid\u0027),"},{"line_number":3316,"context_line":"            mock.patch.object(cinder.API, \u0027attachment_get\u0027),"},{"line_number":3317,"context_line":"            mock.patch.object(objects.BlockDeviceMapping, \u0027destroy\u0027),"},{"line_number":3318,"context_line":"            mock.patch.object(cinder.API, \u0027attachment_get_all\u0027)"},{"line_number":3319,"context_line":"        ) as (mock_get_bdms, mock_attach_get,"},{"line_number":3320,"context_line":"              mock_destroy, mock_all_attachments):"},{"line_number":3321,"context_line":"            mock_get_bdms.return_value \u003d bdms"},{"line_number":3322,"context_line":"            mock_attach_get.side_effect \u003d exception.VolumeAttachmentNotFound("},{"line_number":3323,"context_line":"                attachment_id\u003dNone)"},{"line_number":3324,"context_line":"            mock_all_attachments.return_value \u003d []"},{"line_number":3325,"context_line":"            self.compute._delete_dangling_bdms("},{"line_number":3326,"context_line":"                self.context, instance)"},{"line_number":3327,"context_line":"            self.assertTrue(mock_destroy.called)"},{"line_number":3328,"context_line":""},{"line_number":3329,"context_line":"    def test_get_instance_block_device_info_source_image(self):"},{"line_number":3330,"context_line":"        bdms \u003d block_device_obj.block_device_make_list(self.context,"}],"source_content_type":"text/x-python","patch_set":31,"id":"7d30d026_fd224fde","line":3327,"range":{"start_line":3302,"start_character":4,"end_line":3327,"end_character":48},"updated":"2023-07-03 14:38:07.000000000","message":"this will be removed on next update","commit_id":"b9b3272a4a02ecd564e3fcf1a6194ff4a9c60431"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6e41602fd3a93bfcf647ef121a5a0e997e3bf8c2","unresolved":false,"context_lines":[{"line_number":3299,"context_line":"        self.assertTrue(mock.call_count, 1)"},{"line_number":3300,"context_line":"        self.assertEqual(mock.call_args_list[0][0][1], 2)"},{"line_number":3301,"context_line":""},{"line_number":3302,"context_line":"    def test__delete_dangling_bdms(self):"},{"line_number":3303,"context_line":"        instance \u003d self._create_fake_instance_obj()"},{"line_number":3304,"context_line":"        bdms \u003d block_device_obj.block_device_make_list(self.context,"},{"line_number":3305,"context_line":"                [fake_block_device.FakeDbBlockDeviceDict({"},{"line_number":3306,"context_line":"                    \u0027id\u0027: 3,"},{"line_number":3307,"context_line":"                    \u0027volume_id\u0027: uuids.volume_id,"},{"line_number":3308,"context_line":"                    \u0027instance_uuid\u0027: instance.uuid,"},{"line_number":3309,"context_line":"                    \u0027source_type\u0027: \u0027volume\u0027,"},{"line_number":3310,"context_line":"                    \u0027destination_type\u0027: \u0027volume\u0027})"},{"line_number":3311,"context_line":"                ])"},{"line_number":3312,"context_line":""},{"line_number":3313,"context_line":"        with test.nested("},{"line_number":3314,"context_line":"            mock.patch.object(objects.BlockDeviceMappingList,"},{"line_number":3315,"context_line":"                              \u0027get_by_instance_uuid\u0027),"},{"line_number":3316,"context_line":"            mock.patch.object(cinder.API, \u0027attachment_get\u0027),"},{"line_number":3317,"context_line":"            mock.patch.object(objects.BlockDeviceMapping, \u0027destroy\u0027),"},{"line_number":3318,"context_line":"            mock.patch.object(cinder.API, \u0027attachment_get_all\u0027)"},{"line_number":3319,"context_line":"        ) as (mock_get_bdms, mock_attach_get,"},{"line_number":3320,"context_line":"              mock_destroy, mock_all_attachments):"},{"line_number":3321,"context_line":"            mock_get_bdms.return_value \u003d bdms"},{"line_number":3322,"context_line":"            mock_attach_get.side_effect \u003d exception.VolumeAttachmentNotFound("},{"line_number":3323,"context_line":"                attachment_id\u003dNone)"},{"line_number":3324,"context_line":"            mock_all_attachments.return_value \u003d []"},{"line_number":3325,"context_line":"            self.compute._delete_dangling_bdms("},{"line_number":3326,"context_line":"                self.context, instance)"},{"line_number":3327,"context_line":"            self.assertTrue(mock_destroy.called)"},{"line_number":3328,"context_line":""},{"line_number":3329,"context_line":"    def test_get_instance_block_device_info_source_image(self):"},{"line_number":3330,"context_line":"        bdms \u003d block_device_obj.block_device_make_list(self.context,"}],"source_content_type":"text/x-python","patch_set":31,"id":"2836c5cb_8607aed8","line":3327,"range":{"start_line":3302,"start_character":4,"end_line":3327,"end_character":48},"in_reply_to":"7d30d026_fd224fde","updated":"2023-07-04 19:02:47.000000000","message":"Done","commit_id":"b9b3272a4a02ecd564e3fcf1a6194ff4a9c60431"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"081b3554e7b5167654b2c5b11f70a2629c85b251","unresolved":true,"context_lines":[{"line_number":1564,"context_line":"        mock_destroy, _ \u003d self._test__delete_dangling_bdms(instance, bdms, [])"},{"line_number":1565,"context_line":"        self.assertTrue(mock_destroy.called)"},{"line_number":1566,"context_line":"        self.assertEqual(mock_destroy.call_count, 2)"},{"line_number":1567,"context_line":""},{"line_number":1568,"context_line":"    def test_dangling_bdms_delete_cinder_attachments(self):"},{"line_number":1569,"context_line":"        \"\"\"out of 2 one cinder attachment is present in nova side\"\"\""},{"line_number":1570,"context_line":"        instance \u003d self._create_fake_instance_obj()"}],"source_content_type":"text/x-python","patch_set":50,"id":"bce00fcc_60a7bb92","line":1567,"updated":"2023-08-30 14:48:11.000000000","message":"actually you should doublecheck that you remove the bdm from the bdms","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c2d4acdf8ee42b4d645e3c0f73841b2bba8ede40","unresolved":false,"context_lines":[{"line_number":1564,"context_line":"        mock_destroy, _ \u003d self._test__delete_dangling_bdms(instance, bdms, [])"},{"line_number":1565,"context_line":"        self.assertTrue(mock_destroy.called)"},{"line_number":1566,"context_line":"        self.assertEqual(mock_destroy.call_count, 2)"},{"line_number":1567,"context_line":""},{"line_number":1568,"context_line":"    def test_dangling_bdms_delete_cinder_attachments(self):"},{"line_number":1569,"context_line":"        \"\"\"out of 2 one cinder attachment is present in nova side\"\"\""},{"line_number":1570,"context_line":"        instance \u003d self._create_fake_instance_obj()"}],"source_content_type":"text/x-python","patch_set":50,"id":"33d6b2f5_fe4cdf1d","line":1567,"in_reply_to":"9f1336a4_08574fcb","updated":"2023-08-31 14:06:22.000000000","message":"Done","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a0d1c121f31a05c90ed3d03db20c189685745014","unresolved":true,"context_lines":[{"line_number":1564,"context_line":"        mock_destroy, _ \u003d self._test__delete_dangling_bdms(instance, bdms, [])"},{"line_number":1565,"context_line":"        self.assertTrue(mock_destroy.called)"},{"line_number":1566,"context_line":"        self.assertEqual(mock_destroy.call_count, 2)"},{"line_number":1567,"context_line":""},{"line_number":1568,"context_line":"    def test_dangling_bdms_delete_cinder_attachments(self):"},{"line_number":1569,"context_line":"        \"\"\"out of 2 one cinder attachment is present in nova side\"\"\""},{"line_number":1570,"context_line":"        instance \u003d self._create_fake_instance_obj()"}],"source_content_type":"text/x-python","patch_set":50,"id":"9f1336a4_08574fcb","line":1567,"in_reply_to":"bce00fcc_60a7bb92","updated":"2023-08-30 17:00:35.000000000","message":"Done","commit_id":"54d7c1f8e568cb61d2d5282b945e052c771116bc"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2724b95e51b87508cd0e786991b180421cf01bf3","unresolved":true,"context_lines":[{"line_number":1559,"context_line":"                destroy_disks\u003dTrue, destroy_secrets\u003dTrue):"},{"line_number":1560,"context_line":"        self._destroy(instance)"},{"line_number":1561,"context_line":"        # NOTE(auniyal): delete dangling volumes  before cleanup"},{"line_number":1562,"context_line":"        self._delete_dangling_volumes(context, instance, block_device_info)"},{"line_number":1563,"context_line":"        # NOTE(gibi): if there was device detach in progress then we need to"},{"line_number":1564,"context_line":"        # unblock the waiting threads and clean up."},{"line_number":1565,"context_line":"        self._device_event_handler.cleanup_waiters(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7d87b70d_575b7743","line":1562,"updated":"2023-05-04 12:55:17.000000000","message":"after deleting, refreshing block_device_mapping object too does not solve issue.","commit_id":"2c7f5fb2b4fa2665daf8c2df82430a4bb132ae54"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2724b95e51b87508cd0e786991b180421cf01bf3","unresolved":true,"context_lines":[{"line_number":1559,"context_line":"                destroy_disks\u003dTrue, destroy_secrets\u003dTrue):"},{"line_number":1560,"context_line":"        self._destroy(instance)"},{"line_number":1561,"context_line":"        # NOTE(auniyal): delete dangling volumes  before cleanup"},{"line_number":1562,"context_line":"        self._delete_dangling_volumes(context, instance, block_device_info)"},{"line_number":1563,"context_line":"        # NOTE(gibi): if there was device detach in progress then we need to"},{"line_number":1564,"context_line":"        # unblock the waiting threads and clean up."},{"line_number":1565,"context_line":"        self._device_event_handler.cleanup_waiters(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ed818ddf_b8b7e768","line":1562,"updated":"2023-05-04 12:55:17.000000000","message":"right now, this deletes the volumes from DB, and we can not see volume attached to server via openstack cmds. \nBut server do not get rebooted and stay at ERROR state, rebooting again makes it ACTIVE and usable.","commit_id":"2c7f5fb2b4fa2665daf8c2df82430a4bb132ae54"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"58054c9d9ac5dd6f04c3496c5b1e5f7b1fcae76e","unresolved":false,"context_lines":[{"line_number":1559,"context_line":"                destroy_disks\u003dTrue, destroy_secrets\u003dTrue):"},{"line_number":1560,"context_line":"        self._destroy(instance)"},{"line_number":1561,"context_line":"        # NOTE(auniyal): delete dangling volumes  before cleanup"},{"line_number":1562,"context_line":"        self._delete_dangling_volumes(context, instance, block_device_info)"},{"line_number":1563,"context_line":"        # NOTE(gibi): if there was device detach in progress then we need to"},{"line_number":1564,"context_line":"        # unblock the waiting threads and clean up."},{"line_number":1565,"context_line":"        self._device_event_handler.cleanup_waiters(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f3db1ae7_23414016","line":1562,"in_reply_to":"76daf7df_2e32214f","updated":"2023-05-31 15:02:30.000000000","message":"Done","commit_id":"2c7f5fb2b4fa2665daf8c2df82430a4bb132ae54"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"ac3d22e9061880feb53498e3a8a0a80fa0e52eeb","unresolved":false,"context_lines":[{"line_number":1559,"context_line":"                destroy_disks\u003dTrue, destroy_secrets\u003dTrue):"},{"line_number":1560,"context_line":"        self._destroy(instance)"},{"line_number":1561,"context_line":"        # NOTE(auniyal): delete dangling volumes  before cleanup"},{"line_number":1562,"context_line":"        self._delete_dangling_volumes(context, instance, block_device_info)"},{"line_number":1563,"context_line":"        # NOTE(gibi): if there was device detach in progress then we need to"},{"line_number":1564,"context_line":"        # unblock the waiting threads and clean up."},{"line_number":1565,"context_line":"        self._device_event_handler.cleanup_waiters(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"921b289d_5eb7989e","line":1562,"in_reply_to":"7d87b70d_575b7743","updated":"2023-05-23 10:58:14.000000000","message":"Done","commit_id":"2c7f5fb2b4fa2665daf8c2df82430a4bb132ae54"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"ac3d22e9061880feb53498e3a8a0a80fa0e52eeb","unresolved":true,"context_lines":[{"line_number":1559,"context_line":"                destroy_disks\u003dTrue, destroy_secrets\u003dTrue):"},{"line_number":1560,"context_line":"        self._destroy(instance)"},{"line_number":1561,"context_line":"        # NOTE(auniyal): delete dangling volumes  before cleanup"},{"line_number":1562,"context_line":"        self._delete_dangling_volumes(context, instance, block_device_info)"},{"line_number":1563,"context_line":"        # NOTE(gibi): if there was device detach in progress then we need to"},{"line_number":1564,"context_line":"        # unblock the waiting threads and clean up."},{"line_number":1565,"context_line":"        self._device_event_handler.cleanup_waiters(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"76daf7df_2e32214f","line":1562,"in_reply_to":"ed818ddf_b8b7e768","updated":"2023-05-23 10:58:14.000000000","message":"from Sean, the reason here was:\n- first reboot used old bdm to generate xml and then updated BDM db\n- second reboot took bdm from updated DB.\n\nmoving fix to compute manager.reboot:\n- updated bdm\n- driver invoked from updated BDM\n- now this fix works for all drivers and not only for libvirt driver.\n\nalso, updating the db has no effect on the running instance, the change takes effect on the instance after it is stopped and the domain is redefied in the driver","commit_id":"2c7f5fb2b4fa2665daf8c2df82430a4bb132ae54"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e7e14a5eeec41bcde5b080151095aabcff2fcf81","unresolved":true,"context_lines":[{"line_number":1423,"context_line":"                  instance\u003dinstance)"},{"line_number":1424,"context_line":"        disk_api.teardown_container(container_dir, rootfs_dev)"},{"line_number":1425,"context_line":""},{"line_number":1426,"context_line":"    def _delete_dangling_volumes(self, context, instance, block_device_info):"},{"line_number":1427,"context_line":"        LOG.info(\"##\" * 100)"},{"line_number":1428,"context_line":"        LOG.debug(dict(instance))"},{"line_number":1429,"context_line":"        LOG.info(\"\u003d\u003d\" * 100)"}],"source_content_type":"text/x-python","patch_set":2,"id":"80e9ac35_b9e1df44","line":1426,"updated":"2023-05-04 13:29:17.000000000","message":"This should really be \"dangling BDMs\" right? It also seems like maybe this is not the best place to do this, but I guess need to know more. Is there a bug that describes the problem?","commit_id":"88eadc2bfc80c1b002c5305b98dd64d70934fdfb"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"f606425f80140f74d30ee66f2ad50d30642f77a8","unresolved":true,"context_lines":[{"line_number":1423,"context_line":"                  instance\u003dinstance)"},{"line_number":1424,"context_line":"        disk_api.teardown_container(container_dir, rootfs_dev)"},{"line_number":1425,"context_line":""},{"line_number":1426,"context_line":"    def _delete_dangling_volumes(self, context, instance, block_device_info):"},{"line_number":1427,"context_line":"        LOG.info(\"##\" * 100)"},{"line_number":1428,"context_line":"        LOG.debug(dict(instance))"},{"line_number":1429,"context_line":"        LOG.info(\"\u003d\u003d\" * 100)"}],"source_content_type":"text/x-python","patch_set":2,"id":"a35b616e_4c4067e1","line":1426,"in_reply_to":"80e9ac35_b9e1df44","updated":"2023-05-04 14:01:53.000000000","message":"ack.\n\nI am trying to implement this spec\nhttps://review.opendev.org/c/openstack/nova-specs/+/878757","commit_id":"88eadc2bfc80c1b002c5305b98dd64d70934fdfb"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0a2abd2620f227badb22bc95761c2b99e6290afb","unresolved":false,"context_lines":[{"line_number":1423,"context_line":"                  instance\u003dinstance)"},{"line_number":1424,"context_line":"        disk_api.teardown_container(container_dir, rootfs_dev)"},{"line_number":1425,"context_line":""},{"line_number":1426,"context_line":"    def _delete_dangling_volumes(self, context, instance, block_device_info):"},{"line_number":1427,"context_line":"        LOG.info(\"##\" * 100)"},{"line_number":1428,"context_line":"        LOG.debug(dict(instance))"},{"line_number":1429,"context_line":"        LOG.info(\"\u003d\u003d\" * 100)"}],"source_content_type":"text/x-python","patch_set":2,"id":"81ac0a6d_d55d8c4e","line":1426,"in_reply_to":"a35b616e_4c4067e1","updated":"2023-05-24 03:46:47.000000000","message":"Done","commit_id":"88eadc2bfc80c1b002c5305b98dd64d70934fdfb"}],"releasenotes/notes/delete-dangling-volumes-2615100187fe29fb.yaml":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4ee14373a14dfb2553c80457dc27bdf66a559900","unresolved":true,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    This change ensures the synchronization of volume attachments"},{"line_number":5,"context_line":"    between Nova and Cinder, by deletng any dangling volume attachments"},{"line_number":6,"context_line":"    and maintaining consistency between two databases."},{"line_number":7,"context_line":""},{"line_number":8,"context_line":"    Block device mapping (BDM) table in the Nova database, stores"}],"source_content_type":"text/x-yaml","patch_set":47,"id":"efef500d_6c5c8220","line":5,"range":{"start_line":5,"start_character":32,"end_line":5,"end_character":39},"updated":"2023-08-29 15:48:02.000000000","message":"deleting","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"04f574b8437d259f8d29f741ec2771ea2403bbce","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    This change ensures the synchronization of volume attachments"},{"line_number":5,"context_line":"    between Nova and Cinder, by deletng any dangling volume attachments"},{"line_number":6,"context_line":"    and maintaining consistency between two databases."},{"line_number":7,"context_line":""},{"line_number":8,"context_line":"    Block device mapping (BDM) table in the Nova database, stores"}],"source_content_type":"text/x-yaml","patch_set":47,"id":"3616c4bc_e778809c","line":5,"range":{"start_line":5,"start_character":32,"end_line":5,"end_character":39},"in_reply_to":"efef500d_6c5c8220","updated":"2023-08-30 07:54:45.000000000","message":"Done","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4ee14373a14dfb2553c80457dc27bdf66a559900","unresolved":true,"context_lines":[{"line_number":13,"context_line":"    With this change, on instance reboot, Nova will checks for all volume"},{"line_number":14,"context_line":"    attachments associated with the instance and verifies their availability"},{"line_number":15,"context_line":"    in the Cinder database. If attachments are not found they will get deleted"},{"line_number":16,"context_line":"    from Nova database too. "},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    Afeter Nova database cleanup, similarly Cinder database is checked for"},{"line_number":19,"context_line":"    attachments related to instance. If attachments found in Cinder DB that"}],"source_content_type":"text/x-yaml","patch_set":47,"id":"ad6aa2f8_f46b8729","line":16,"range":{"start_line":16,"start_character":26,"end_line":16,"end_character":28},"updated":"2023-08-29 15:48:02.000000000","message":"unnecessary space","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"04f574b8437d259f8d29f741ec2771ea2403bbce","unresolved":false,"context_lines":[{"line_number":13,"context_line":"    With this change, on instance reboot, Nova will checks for all volume"},{"line_number":14,"context_line":"    attachments associated with the instance and verifies their availability"},{"line_number":15,"context_line":"    in the Cinder database. If attachments are not found they will get deleted"},{"line_number":16,"context_line":"    from Nova database too. "},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    Afeter Nova database cleanup, similarly Cinder database is checked for"},{"line_number":19,"context_line":"    attachments related to instance. If attachments found in Cinder DB that"}],"source_content_type":"text/x-yaml","patch_set":47,"id":"fcf36b80_e4ffa07a","line":16,"range":{"start_line":16,"start_character":26,"end_line":16,"end_character":28},"in_reply_to":"ad6aa2f8_f46b8729","updated":"2023-08-30 07:54:45.000000000","message":"Done","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4ee14373a14dfb2553c80457dc27bdf66a559900","unresolved":true,"context_lines":[{"line_number":15,"context_line":"    in the Cinder database. If attachments are not found they will get deleted"},{"line_number":16,"context_line":"    from Nova database too. "},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    Afeter Nova database cleanup, similarly Cinder database is checked for"},{"line_number":19,"context_line":"    attachments related to instance. If attachments found in Cinder DB that"},{"line_number":20,"context_line":"    are not present in Nova DB, they will get deleted from Cinder databse."},{"line_number":21,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":47,"id":"c97670ee_67baa37f","line":18,"range":{"start_line":18,"start_character":4,"end_line":18,"end_character":10},"updated":"2023-08-29 15:48:02.000000000","message":"after","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"04f574b8437d259f8d29f741ec2771ea2403bbce","unresolved":false,"context_lines":[{"line_number":15,"context_line":"    in the Cinder database. If attachments are not found they will get deleted"},{"line_number":16,"context_line":"    from Nova database too. "},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    Afeter Nova database cleanup, similarly Cinder database is checked for"},{"line_number":19,"context_line":"    attachments related to instance. If attachments found in Cinder DB that"},{"line_number":20,"context_line":"    are not present in Nova DB, they will get deleted from Cinder databse."},{"line_number":21,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":47,"id":"fc09bea1_a8e8547f","line":18,"range":{"start_line":18,"start_character":4,"end_line":18,"end_character":10},"in_reply_to":"c97670ee_67baa37f","updated":"2023-08-30 07:54:45.000000000","message":"Done","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4ee14373a14dfb2553c80457dc27bdf66a559900","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    See `blueprint`__ for more details."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    __ https://blueprints.launchpad.net/nova/+spec/cleanup-dangling-volume-attachments"}],"source_content_type":"text/x-yaml","patch_set":47,"id":"8ead2163_1ea9b316","line":24,"updated":"2023-08-29 15:48:02.000000000","message":"please don\u0027t refer to the blueprint, its description is old and no longer accurate to the agreed design.\n\nYou should either mention the admin guide note you just added, or you could point to the generated spec file, which is easier to read :\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/2023.2/approved/cleanup-dangling-volume-attachments.html","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"04f574b8437d259f8d29f741ec2771ea2403bbce","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    See `blueprint`__ for more details."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    __ https://blueprints.launchpad.net/nova/+spec/cleanup-dangling-volume-attachments"}],"source_content_type":"text/x-yaml","patch_set":47,"id":"945a4a64_e257fc3f","line":24,"in_reply_to":"8ead2163_1ea9b316","updated":"2023-08-30 07:54:45.000000000","message":"Updated with spec link\nthanks","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ce0f30bee2929e0bb57bfb78fec150ff62784d17","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    See `blueprint`__ for more details."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    __ https://blueprints.launchpad.net/nova/+spec/cleanup-dangling-volume-attachments"}],"source_content_type":"text/x-yaml","patch_set":47,"id":"64e65ae7_b76fc834","line":24,"in_reply_to":"945a4a64_e257fc3f","updated":"2023-08-30 09:50:25.000000000","message":"Ack","commit_id":"d488ee936aede6e23fdd3028a1042d87cf65bf6e"}]}