)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"17488a227626af82fb4757ba1d24a83bf9abf1b2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fbe00d76_aead4d75","updated":"2026-02-05 17:30:45.000000000","message":"This patch looks great. I’m adding a small -1 for now, based on the same concern I raised on the previous patch.","commit_id":"ef511fdca5b23fd90607e73b70123f9cd4ce2340"},{"author":{"_account_id":35676,"name":"Max","email":"max.lamprecht@digits.schwarz","username":"lamprechtm"},"change_message_id":"536e23c75eca106d64b4427abad164d0d6ce5ee3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"28da58d1_08ccd3a0","updated":"2026-01-03 10:16:40.000000000","message":"recheck - nova-ceph-multistore - CI failure","commit_id":"ef511fdca5b23fd90607e73b70123f9cd4ce2340"},{"author":{"_account_id":35676,"name":"Max","email":"max.lamprecht@digits.schwarz","username":"lamprechtm"},"change_message_id":"b3a8965881021a0b42a7de85c18a479275060455","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"819d7647_c2d87623","in_reply_to":"fbe00d76_aead4d75","updated":"2026-03-02 08:42:26.000000000","message":"I reworked it and implemented polling","commit_id":"ef511fdca5b23fd90607e73b70123f9cd4ce2340"},{"author":{"_account_id":35676,"name":"Max","email":"max.lamprecht@digits.schwarz","username":"lamprechtm"},"change_message_id":"4a8d161b01765c479c020d397f9b4b8920cee6f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"02b90b12_0bab8dfa","updated":"2026-02-16 11:04:24.000000000","message":"rechcheck - No module named \u0027pkg_resources","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"},{"author":{"_account_id":35676,"name":"Max","email":"max.lamprecht@digits.schwarz","username":"lamprechtm"},"change_message_id":"495ed9bab11d90ebe880845c9243b52f4a34cb32","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8c34fac4_676a95ba","updated":"2026-02-25 16:36:49.000000000","message":"recheck - CI failed due to pkg_resources","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"},{"author":{"_account_id":35676,"name":"Max","email":"max.lamprecht@digits.schwarz","username":"lamprechtm"},"change_message_id":"c9ab4d00615073a0fcef26309e265cafe797a100","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"86397315_fa284958","updated":"2026-02-16 17:07:26.000000000","message":"recheck - No module named \u0027pkg_resources","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"},{"author":{"_account_id":35676,"name":"Max","email":"max.lamprecht@digits.schwarz","username":"lamprechtm"},"change_message_id":"d9158c88cdab34e9615daf2830d06b4d887f459f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1d96798b_58e18d59","updated":"2026-02-26 16:58:09.000000000","message":"recheck - nova-multi-cell - SSH timed out","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"}],"nova/compute/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8b492578eb6479132ffe253852bad3e84d58705e","unresolved":true,"context_lines":[{"line_number":9465,"context_line":"                        # 1. The current bdm.attachment_id(can be the dest)"},{"line_number":9466,"context_line":"                        # 2. The old attachment from old_vol_attachment_ids"},{"line_number":9467,"context_line":"                        # This ensures that no leftover attachments remain"},{"line_number":9468,"context_line":"                        # in cinder."},{"line_number":9469,"context_line":"                        try:"},{"line_number":9470,"context_line":"                            bdm.save()"},{"line_number":9471,"context_line":"                        except exception.BDMNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9d6ba811_15a0ad75","line":9468,"updated":"2026-03-09 17:31:53.000000000","message":"This makes sense, but seems fragile to me. Meaning, seems like some code refactoring in the other service could cause this complex chain of events to not mean the same thing it has meant before and either this could fail to clean up or do something more destructive.\n\nIs there some reason we can\u0027t fix the compute code to clean up properly?","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5f696ec45bd7eb5db73e2b36e03f1bda0f75f346","unresolved":true,"context_lines":[{"line_number":9465,"context_line":"                        # 1. The current bdm.attachment_id(can be the dest)"},{"line_number":9466,"context_line":"                        # 2. The old attachment from old_vol_attachment_ids"},{"line_number":9467,"context_line":"                        # This ensures that no leftover attachments remain"},{"line_number":9468,"context_line":"                        # in cinder."},{"line_number":9469,"context_line":"                        try:"},{"line_number":9470,"context_line":"                            bdm.save()"},{"line_number":9471,"context_line":"                        except exception.BDMNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"60775929_2b72453a","line":9468,"in_reply_to":"403934c9_5e374311","updated":"2026-03-11 13:34:11.000000000","message":"Okay, I\u0027m trying to ask how the in-migration cancel/fail could do the right thing if the `old_attachments` list is lost when we return from this function and also it\u0027s the only place that has them. If there\u0027s code to handle it elsewhere I want to consider why it\u0027s not cleaning up this situation as well and _if_ we could make it do so to avoid separate code paths to do the same. If you have a pointer, that\u0027d be appreciated, else I\u0027ll go looking.","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"},{"author":{"_account_id":35676,"name":"Max","email":"max.lamprecht@digits.schwarz","username":"lamprechtm"},"change_message_id":"2a205b422c4ef16f5dacd36dab7e35a807d0d83a","unresolved":true,"context_lines":[{"line_number":9465,"context_line":"                        # 1. The current bdm.attachment_id(can be the dest)"},{"line_number":9466,"context_line":"                        # 2. The old attachment from old_vol_attachment_ids"},{"line_number":9467,"context_line":"                        # This ensures that no leftover attachments remain"},{"line_number":9468,"context_line":"                        # in cinder."},{"line_number":9469,"context_line":"                        try:"},{"line_number":9470,"context_line":"                            bdm.save()"},{"line_number":9471,"context_line":"                        except exception.BDMNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"d9ff92ac_491c382d","line":9468,"in_reply_to":"5b9dee72_11b10b42","updated":"2026-03-10 16:10:00.000000000","message":"\u003e Doesn\u0027t this basically only clean up in the case where we fail to setup_networks_on_host()? \n\nExactly - It can also happen that we spend here much time if we enable live_migration_wait_for_vif_plug in the nova.conf and wait for the the vif plug in a big environment(with SDN change delay or so)\n\n\u003e After we drop out of here, we\u0027ve lost old_attachments, so what happens if we fail the next step?\n\nSo then nova deleted the instance + BDM and also the destination attachment on cinder side. The source attachment will remain on cinder side. Due to that we have an attachment on cinder side to an \"non existent/deleted\" server and the volume will remain in state in-use/reserved.\nWith default permissions an customer can\u0027t delete the wrong attachment and the volume can\u0027t be attached to any other instances due to the \"in-use/reserved\" state. So the attachment needs to be manually deleted by the cloud operator/provider.\n\n\u003eSeems like \"while preparing for a live migration\" would be more appropriate.\n\nYou\u0027re right. Should I change the wording to \"while preparing for a live migration\"?","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"},{"author":{"_account_id":35676,"name":"Max","email":"max.lamprecht@digits.schwarz","username":"lamprechtm"},"change_message_id":"29b8b3a4490a45d88e1706ce83a345c7855b87ca","unresolved":true,"context_lines":[{"line_number":9465,"context_line":"                        # 1. The current bdm.attachment_id(can be the dest)"},{"line_number":9466,"context_line":"                        # 2. The old attachment from old_vol_attachment_ids"},{"line_number":9467,"context_line":"                        # This ensures that no leftover attachments remain"},{"line_number":9468,"context_line":"                        # in cinder."},{"line_number":9469,"context_line":"                        try:"},{"line_number":9470,"context_line":"                            bdm.save()"},{"line_number":9471,"context_line":"                        except exception.BDMNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"74657273_4a326ea9","line":9468,"in_reply_to":"60775929_2b72453a","updated":"2026-04-01 16:33:18.000000000","message":"I\u0027m not sure how we can clean that up if we have lost the old_attachment list. I don\u0027t think that we are able to query and parse the attachments nicely from cinder side - in order to map it to a VM/hypervisor. But I also haven\u0027t looked deeply into it as this proposed solution solved that problem perfectly in our environment","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"95eeaf0c266cd7ff0e09a4abf3b13215d9a71fbd","unresolved":true,"context_lines":[{"line_number":9465,"context_line":"                        # 1. The current bdm.attachment_id(can be the dest)"},{"line_number":9466,"context_line":"                        # 2. The old attachment from old_vol_attachment_ids"},{"line_number":9467,"context_line":"                        # This ensures that no leftover attachments remain"},{"line_number":9468,"context_line":"                        # in cinder."},{"line_number":9469,"context_line":"                        try:"},{"line_number":9470,"context_line":"                            bdm.save()"},{"line_number":9471,"context_line":"                        except exception.BDMNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5b9dee72_11b10b42","line":9468,"in_reply_to":"7028fbd1_a7467958","updated":"2026-03-10 14:37:38.000000000","message":"Okay I see, yeah we\u0027re the only one with old_attachments and only ephemerally.\n\n\u003eI mean we can do a complete rework and change the database structure to store multiple attachments per BDM which will be a really complex change.\n\nGiven this ridiculously deeply-nested structure of this code here, a total refactor surely seems warranted (but not for you to do before this fix).\n\n\u003e For me this patch helped a lot (we have running it since 6 months in production) and we haven\u0027t observed such cases again.\n\nYeah, as I said: I\u0027m not arguing it doesn\u0027t fix this _now_.\n\nLooking at this more since yesterday it surely seems like there\u0027s really no harm in deleting those attachments in cinder if we fail to look up a BDM. I\u0027m not sure that will cover _all_ the cases of this, but at least this one. Doesn\u0027t this basically only clean up in the case where we fail to `setup_networks_on_host()`? That\u0027s the only long-running thing I see here that gives us a reasonable window to be deleted. The commit message says \"while a live migration is in progress\" but we haven\u0027t even started it here. Seems like \"while preparing for a live migration\" would be more appropriate. After we drop out of here, we\u0027ve lost `old_attachments`, so what happens if we fail the _next_ step?\n\nI guess it seems like it would be more robust, when we go to delete the instance and the _current_ BDMs, to also delete and log any cinder attachments we find for that instance instead of relying on the other compute to do it.","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"},{"author":{"_account_id":35676,"name":"Max","email":"max.lamprecht@digits.schwarz","username":"lamprechtm"},"change_message_id":"061499f668670c89404b2c36b4dbb7dc0864c0bf","unresolved":true,"context_lines":[{"line_number":9465,"context_line":"                        # 1. The current bdm.attachment_id(can be the dest)"},{"line_number":9466,"context_line":"                        # 2. The old attachment from old_vol_attachment_ids"},{"line_number":9467,"context_line":"                        # This ensures that no leftover attachments remain"},{"line_number":9468,"context_line":"                        # in cinder."},{"line_number":9469,"context_line":"                        try:"},{"line_number":9470,"context_line":"                            bdm.save()"},{"line_number":9471,"context_line":"                        except exception.BDMNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7028fbd1_a7467958","line":9468,"in_reply_to":"9d6ba811_15a0ad75","updated":"2026-03-10 08:20:04.000000000","message":"Hi Dan,\n\nmy problem here is, that 1 nova BDM will have 2 attachments IDs on cinder side during an live-migration(for source and dest hypervisor).\n\nSadly nova doesn\u0027t track both attachments on the server side. In the nova db, 1 BDM can only have 1 attachment_id. Therefore the destination hypervisor will overwrite the source attachment in the db and track the old ID locally in \"old_attachments\" [1]. Therefore the destination compute is the only component that has the source attachment ID information in memory.\n\nI mean we can do a complete rework and change the database structure to store multiple attachments per BDM which will be a really complex change.\n\nFor me this patch helped a lot (we have running it since 6 months in production) and we haven\u0027t observed such cases again.\n\n[1] https://review.opendev.org/c/openstack/nova/+/972045/2/nova/compute/manager.py#9443","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9c69f8ee170f6943b00781cf8fbb8e4a3c8f0f29","unresolved":true,"context_lines":[{"line_number":9465,"context_line":"                        # 1. The current bdm.attachment_id(can be the dest)"},{"line_number":9466,"context_line":"                        # 2. The old attachment from old_vol_attachment_ids"},{"line_number":9467,"context_line":"                        # This ensures that no leftover attachments remain"},{"line_number":9468,"context_line":"                        # in cinder."},{"line_number":9469,"context_line":"                        try:"},{"line_number":9470,"context_line":"                            bdm.save()"},{"line_number":9471,"context_line":"                        except exception.BDMNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fea26824_8577414f","line":9468,"in_reply_to":"d9ff92ac_491c382d","updated":"2026-03-10 18:12:58.000000000","message":"\u003e \u003e After we drop out of here, we\u0027ve lost old_attachments, so what happens if we fail the next step?\n\u003e \n\u003e So then nova deleted the instance + BDM and also the destination attachment on cinder side. The source attachment will remain on cinder side. Due to that we have an attachment on cinder side to an \"non existent/deleted\" server and the volume will remain in state in-use/reserved.\n\u003e With default permissions an customer can\u0027t delete the wrong attachment and the volume can\u0027t be attached to any other instances due to the \"in-use/reserved\" state. So the attachment needs to be manually deleted by the cloud operator/provider.\n\nRight, so my point is - this fixes the problem for a small window of time (basically while waiting for network setup) but does not fix it for the bulk (I know, depending on the environment) of the process of migrating...correct?\n\n\u003e You\u0027re right. Should I change the wording to \"while preparing for a live migration\"?\n\nI\u0027d prefer it, yeah 😊\n\nI guess I\u0027m not opposed to closing this small window in this way now, but it still feels like something that needs a better/bigger fix.","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"},{"author":{"_account_id":35676,"name":"Max","email":"max.lamprecht@digits.schwarz","username":"lamprechtm"},"change_message_id":"7eae542d736435cae5d90215b415811c1273eee5","unresolved":true,"context_lines":[{"line_number":9465,"context_line":"                        # 1. The current bdm.attachment_id(can be the dest)"},{"line_number":9466,"context_line":"                        # 2. The old attachment from old_vol_attachment_ids"},{"line_number":9467,"context_line":"                        # This ensures that no leftover attachments remain"},{"line_number":9468,"context_line":"                        # in cinder."},{"line_number":9469,"context_line":"                        try:"},{"line_number":9470,"context_line":"                            bdm.save()"},{"line_number":9471,"context_line":"                        except exception.BDMNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"403934c9_5e374311","line":9468,"in_reply_to":"fea26824_8577414f","updated":"2026-03-11 09:44:20.000000000","message":"\u003e this fixes the problem for a small window of time (basically while waiting for network setup) but does not fix it for the bulk (I know, depending on the environment) of the process of migrating\n\nI never observed that problem outside of the \"pre migration phase\". From my point of view the attachment handling in the \"actual migration\" works and also handels well the deletion of the server. I also tested it right now -\u003e works 😊\n\nChanged the workding of the commit msg :)","commit_id":"6388a5a1ea59749c25149c1fb9a951489ba88480"}]}