)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"9ba50f4b2e0f9e07945073365dae81c19d0ffcdb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f5b9dc20_4e9da852","updated":"2022-02-18 13:32:56.000000000","message":"As the commit message says, it\u0027s only a partial fix, and there\u0027s some cleanup left to do, but it\u0027s self-contained and a step in the right direction, so +1 from me.","commit_id":"4acae1cae20e9033c33e3ff1ec01b600333cc971"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"7e31e1aa08bd2e440f3a10d27b955e3d5fb5ce52","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2d03324d_8283f420","updated":"2022-02-18 13:52:20.000000000","message":"So thinking about this some more, I\u0027m not convinced that the proposed partial fix is OK.\n\nPreviously, the admin at least had an indication that something was wrong because the migration had been aborted, but the status was never updated.\n\nWith this patch, everything *appears* to be smooth, *but* there are still potentially port bindings to cleanup. On balance, I\u0027d rather have the first situation then the second, as at least the failure and need for manual cleanup is obvious.\n\nSo while this patch in itself is fine, I don\u0027t think we should merge it until the follow-up fix is present as well.","commit_id":"4acae1cae20e9033c33e3ff1ec01b600333cc971"},{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"f3965eae486ab60099f1e2a88782878510e0c110","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6246b65e_aa5ab58e","in_reply_to":"2d03324d_8283f420","updated":"2022-02-23 15:44:21.000000000","message":"Resolved.","commit_id":"4acae1cae20e9033c33e3ff1ec01b600333cc971"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"58e293ca8acb013807061813134b5ce02e403257","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c5e7ce15_04992b9a","updated":"2022-03-04 21:42:11.000000000","message":"Nice! Same suggestion inline as the previous one, other than that it looks good.","commit_id":"d21f832e917a7c4715714ac9cfe6c9ff48620afa"},{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"b886d9c7ba55a5082933a81e3d415e7f27c13811","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"aaf92492_06ed6053","in_reply_to":"c5e7ce15_04992b9a","updated":"2022-03-07 15:59:41.000000000","message":"Thank you for your review!","commit_id":"d21f832e917a7c4715714ac9cfe6c9ff48620afa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d1515a55e2849b261992bfea9c850bbdbab29b10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"64cd0a6b_03930057","updated":"2022-03-08 19:34:17.000000000","message":"ill try and review this again tomorrow.","commit_id":"d97afdfbebd050c4ed565c2c7c91e0f7b9242042"},{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"b6e3ef62ef64b6cf84f9f95178cea1f3f9b28929","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"31a9e321_5502a29c","updated":"2022-03-08 15:18:53.000000000","message":"recheck","commit_id":"d97afdfbebd050c4ed565c2c7c91e0f7b9242042"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"11a7008a0e914164d5f1b0ae0c1a5296895ef4e4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ab874ed8_e696372f","updated":"2022-03-08 13:53:53.000000000","message":"recheck While it is a live migration test that failed, it seems unrelated to the current patch as no rollback was involved.","commit_id":"d97afdfbebd050c4ed565c2c7c91e0f7b9242042"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed3e5c893f03d5efa3a6230fa325b8c394b1da96","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"e9fe07c5_3558cdc2","updated":"2022-03-10 12:32:45.000000000","message":"Looks good to me","commit_id":"219520d9cec6a204e0d0f75881d75c8db48e7f56"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8c35eb4bfedb6bd7e6c8f133a6a514ee9872f0f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"dbf97c47_0f638b74","updated":"2022-03-10 12:20:52.000000000","message":"recheck bug 1964472","commit_id":"219520d9cec6a204e0d0f75881d75c8db48e7f56"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4fba59f7f86e6f940f8f27b62cc4dd436b6083fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"c6296614_72d59489","updated":"2022-03-10 13:18:51.000000000","message":"recheck bug: #1964472","commit_id":"219520d9cec6a204e0d0f75881d75c8db48e7f56"}],"nova/compute/manager.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d1515a55e2849b261992bfea9c850bbdbab29b10","unresolved":true,"context_lines":[{"line_number":8717,"context_line":"                    self.network_api.setup_networks_on_host("},{"line_number":8718,"context_line":"                        context, instance, host\u003dmigration.dest_compute,"},{"line_number":8719,"context_line":"                        teardown\u003dTrue)"},{"line_number":8720,"context_line":"                except exception.PortBindingDeletionFailed as e:"},{"line_number":8721,"context_line":"                    # Removing the inactive port bindings from the destination"},{"line_number":8722,"context_line":"                    # host is not critical so just log an error but don\u0027t fail."},{"line_number":8723,"context_line":"                    LOG.error("},{"line_number":8724,"context_line":"                        \u0027Network cleanup failed for destination host %s \u0027"},{"line_number":8725,"context_line":"                        \u0027during live migration rollback. You may need to \u0027"},{"line_number":8726,"context_line":"                        \u0027manually clean up resources in the network service. \u0027"},{"line_number":8727,"context_line":"                        \u0027Error: %s\u0027, migration.dest_compute, str(e))"},{"line_number":8728,"context_line":"                except Exception:"},{"line_number":8729,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":8730,"context_line":"                        LOG.exception("}],"source_content_type":"text/x-python","patch_set":7,"id":"4f885a93_2163badc","line":8727,"range":{"start_line":8720,"start_character":14,"end_line":8727,"end_character":68},"updated":"2022-03-08 19:34:17.000000000","message":"one thing we need to keep in mind is that multiple port bindings are still optional in neutron so we have to also support the case where only a single port binding is used.\n\nI would have to double-check since it\u0027s been a while but I believe that in the non-multiple port bindings case the port will still be pointing at the source node provided we are not in post live migration.\n\nso when aborting a live migration before that point we should not have to update the port but we should confirm that.","commit_id":"d97afdfbebd050c4ed565c2c7c91e0f7b9242042"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed3e5c893f03d5efa3a6230fa325b8c394b1da96","unresolved":true,"context_lines":[{"line_number":8717,"context_line":"                    self.network_api.setup_networks_on_host("},{"line_number":8718,"context_line":"                        context, instance, host\u003dmigration.dest_compute,"},{"line_number":8719,"context_line":"                        teardown\u003dTrue)"},{"line_number":8720,"context_line":"                except exception.PortBindingDeletionFailed as e:"},{"line_number":8721,"context_line":"                    # Removing the inactive port bindings from the destination"},{"line_number":8722,"context_line":"                    # host is not critical so just log an error but don\u0027t fail."},{"line_number":8723,"context_line":"                    LOG.error("},{"line_number":8724,"context_line":"                        \u0027Network cleanup failed for destination host %s \u0027"},{"line_number":8725,"context_line":"                        \u0027during live migration rollback. You may need to \u0027"},{"line_number":8726,"context_line":"                        \u0027manually clean up resources in the network service. \u0027"},{"line_number":8727,"context_line":"                        \u0027Error: %s\u0027, migration.dest_compute, str(e))"},{"line_number":8728,"context_line":"                except Exception:"},{"line_number":8729,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":8730,"context_line":"                        LOG.exception("}],"source_content_type":"text/x-python","patch_set":7,"id":"69c8405f_4ab7687d","line":8727,"range":{"start_line":8720,"start_character":14,"end_line":8727,"end_character":68},"in_reply_to":"21836ee1_70e6a921","updated":"2022-03-10 12:32:45.000000000","message":"As far as I see setup_networks_on_host already handle the port binding deletion conditionally based on the neutron extension availability. I also agree with Sean that the non-multiple port binding case the port hasn\u0027t been bound to the target host yet at this point so nothing to do here.","commit_id":"d97afdfbebd050c4ed565c2c7c91e0f7b9242042"},{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"98f974fab1a811e00ff760b255c3d3eef1215eb6","unresolved":true,"context_lines":[{"line_number":8717,"context_line":"                    self.network_api.setup_networks_on_host("},{"line_number":8718,"context_line":"                        context, instance, host\u003dmigration.dest_compute,"},{"line_number":8719,"context_line":"                        teardown\u003dTrue)"},{"line_number":8720,"context_line":"                except exception.PortBindingDeletionFailed as e:"},{"line_number":8721,"context_line":"                    # Removing the inactive port bindings from the destination"},{"line_number":8722,"context_line":"                    # host is not critical so just log an error but don\u0027t fail."},{"line_number":8723,"context_line":"                    LOG.error("},{"line_number":8724,"context_line":"                        \u0027Network cleanup failed for destination host %s \u0027"},{"line_number":8725,"context_line":"                        \u0027during live migration rollback. You may need to \u0027"},{"line_number":8726,"context_line":"                        \u0027manually clean up resources in the network service. \u0027"},{"line_number":8727,"context_line":"                        \u0027Error: %s\u0027, migration.dest_compute, str(e))"},{"line_number":8728,"context_line":"                except Exception:"},{"line_number":8729,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":8730,"context_line":"                        LOG.exception("}],"source_content_type":"text/x-python","patch_set":7,"id":"21836ee1_70e6a921","line":8727,"range":{"start_line":8720,"start_character":14,"end_line":8727,"end_character":68},"in_reply_to":"4f885a93_2163badc","updated":"2022-03-09 14:16:36.000000000","message":"Thank you Sean. I may have missed something and can add extra functional test to address this situation if I am missing something... Maybe we can also introduce check for network_api.has_port_binding_extension capability to make this calls optional. Please let me know.","commit_id":"d97afdfbebd050c4ed565c2c7c91e0f7b9242042"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4fba59f7f86e6f940f8f27b62cc4dd436b6083fb","unresolved":true,"context_lines":[{"line_number":8717,"context_line":"                    self.network_api.setup_networks_on_host("},{"line_number":8718,"context_line":"                        context, instance, host\u003dmigration.dest_compute,"},{"line_number":8719,"context_line":"                        teardown\u003dTrue)"},{"line_number":8720,"context_line":"                except exception.PortBindingDeletionFailed as e:"},{"line_number":8721,"context_line":"                    # Removing the inactive port bindings from the destination"},{"line_number":8722,"context_line":"                    # host is not critical so just log an error but don\u0027t fail."},{"line_number":8723,"context_line":"                    LOG.error("},{"line_number":8724,"context_line":"                        \u0027Network cleanup failed for destination host %s \u0027"},{"line_number":8725,"context_line":"                        \u0027during live migration rollback. You may need to \u0027"},{"line_number":8726,"context_line":"                        \u0027manually clean up resources in the network service. \u0027"},{"line_number":8727,"context_line":"                        \u0027Error: %s\u0027, migration.dest_compute, str(e))"},{"line_number":8728,"context_line":"                except Exception:"},{"line_number":8729,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":8730,"context_line":"                        LOG.exception("}],"source_content_type":"text/x-python","patch_set":7,"id":"36f49904_457bb82d","line":8727,"range":{"start_line":8720,"start_character":14,"end_line":8727,"end_character":68},"in_reply_to":"69c8405f_4ab7687d","updated":"2022-03-10 13:18:51.000000000","message":"as gibi said setup_networks_on_host already has a check for the extension so it\u0027s safe to call it unconditionally.\n\nin the single port binding case it won\u0027t try to delete the port binding so we won\u0027t get this exception and this log won\u0027t fire so I think this is good as it is.\n\nit\u0027s just the fact that both paths exit is often missed so I wanted to double-check this was correct.\n\nI think this change is correct as is.\nthanks gibi confirming my assertion","commit_id":"d97afdfbebd050c4ed565c2c7c91e0f7b9242042"}],"nova/tests/functional/libvirt/test_live_migration.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"58e293ca8acb013807061813134b5ce02e403257","unresolved":true,"context_lines":[{"line_number":217,"context_line":"            self.neutron._port_bindings[self.neutron.port_2[\u0027id\u0027]]"},{"line_number":218,"context_line":"        )"},{"line_number":219,"context_line":"        self.assertEqual(1, len(serverb_pb))"},{"line_number":220,"context_line":"        self.assertRaises(KeyError, lambda: serverb_pb[\u0027dest\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"a2e258cc_83f708ba","line":220,"updated":"2022-03-04 21:42:11.000000000","message":"Same suggestions as the previous patch, assertNotIn reads much better.","commit_id":"d21f832e917a7c4715714ac9cfe6c9ff48620afa"},{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"b886d9c7ba55a5082933a81e3d415e7f27c13811","unresolved":false,"context_lines":[{"line_number":217,"context_line":"            self.neutron._port_bindings[self.neutron.port_2[\u0027id\u0027]]"},{"line_number":218,"context_line":"        )"},{"line_number":219,"context_line":"        self.assertEqual(1, len(serverb_pb))"},{"line_number":220,"context_line":"        self.assertRaises(KeyError, lambda: serverb_pb[\u0027dest\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"093b3c9b_37717d80","line":220,"in_reply_to":"a2e258cc_83f708ba","updated":"2022-03-07 15:59:41.000000000","message":"Done","commit_id":"d21f832e917a7c4715714ac9cfe6c9ff48620afa"}]}