)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0d86e046423d095001c8ec5d4943284e1a5ac61a","unresolved":false,"context_lines":[{"line_number":13,"context_line":"in that call were unhandled, and caused both the server and the"},{"line_number":14,"context_line":"migration to end up in ERROR. This patch handles the error, and"},{"line_number":15,"context_line":"modifies the functional test to reflect the newly fixed behavior."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Closes-bug: 1879787"},{"line_number":18,"context_line":"Change-Id: I4c89c8ba0153ec01ff57979b9bf1cfd5b2a9da89"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"9f560f44_e85a4881","line":17,"range":{"start_line":16,"start_character":0,"end_line":17,"end_character":19},"updated":"2020-08-13 00:18:27.000000000","message":"this should be partial bug.\n\nthis will clost it on master but its not a valid fix for queens/osp13 which is called out in the bug report.\n\nwe need a follow up path to handel the case where the vif info is not in the migrate_data otherwise we will not uplug the interfaces on the source host.\nthis can be a follow up i guess but we should still address it before starting to backport this.","commit_id":"9f205c620e57c9af760d3b52c93a4ff6d5c0e618"}],"nova/compute/manager.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"154f17462c670686ff1454aff4e6f664b843c7bc","unresolved":false,"context_lines":[{"line_number":8411,"context_line":""},{"line_number":8412,"context_line":"        network_info \u003d []"},{"line_number":8413,"context_line":"        try:"},{"line_number":8414,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(ctxt, instance)"},{"line_number":8415,"context_line":"        except Exception as e:"},{"line_number":8416,"context_line":"            LOG.warning(\u0027Unable to obtain network info: %s. Networks will not \u0027"},{"line_number":8417,"context_line":"                        \u0027be cleaned up correctly.\u0027, e, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_8b5db67a","line":8414,"updated":"2020-05-20 23:06:59.000000000","message":"pep8: E501 line too long (80 \u003e 79 characters)","commit_id":"95572842e9cb4ad733050d7eab2721858bbc478f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6543594b770409b119a5d88481cebad4f36efcea","unresolved":false,"context_lines":[{"line_number":8414,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(ctxt, instance)"},{"line_number":8415,"context_line":"        except Exception as e:"},{"line_number":8416,"context_line":"            LOG.warning(\u0027Unable to obtain network info: %s. Networks will not \u0027"},{"line_number":8417,"context_line":"                        \u0027be cleaned up correctly.\u0027, e, instance\u003dinstance)"},{"line_number":8418,"context_line":""},{"line_number":8419,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8420,"context_line":"                                          \"live_migration._post.start\","}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_987fce64","line":8417,"updated":"2020-05-20 21:27:40.000000000","message":"I wonder if we should log this at error level, similar to the volume-related logs on L8364 and L8369. Because this will need operator action, right? To clean up bindings on the source, IIUC. Also, this message should say \"cleaned up correctly on the source\" for clarity.","commit_id":"95572842e9cb4ad733050d7eab2721858bbc478f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0bda8bdd5a8f9eba2cfa930cc2ea5c307d93c35","unresolved":false,"context_lines":[{"line_number":8414,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(ctxt, instance)"},{"line_number":8415,"context_line":"        except Exception as e:"},{"line_number":8416,"context_line":"            LOG.warning(\u0027Unable to obtain network info: %s. Networks will not \u0027"},{"line_number":8417,"context_line":"                        \u0027be cleaned up correctly.\u0027, e, instance\u003dinstance)"},{"line_number":8418,"context_line":""},{"line_number":8419,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8420,"context_line":"                                          \"live_migration._post.start\","}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_05e227a0","line":8417,"in_reply_to":"ff570b3c_117530cd","updated":"2020-07-16 14:29:17.000000000","message":"Actually, given what Lee has said on L8334 and L8369 of PS2, I don\u0027t think this has any effect on cleanup. `if migrate_data and \u0027vifs\u0027 in migrate_data:` will always be true with train and later computes, so `unplug_nw_info` will always be set in that if block. I\u0027ve changed the error message, we\u0027ll have to put back the old one in the pre-train backports.","commit_id":"95572842e9cb4ad733050d7eab2721858bbc478f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"a107cfa6280632c79b0c981defd5b01cd7c2f535","unresolved":false,"context_lines":[{"line_number":8414,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(ctxt, instance)"},{"line_number":8415,"context_line":"        except Exception as e:"},{"line_number":8416,"context_line":"            LOG.warning(\u0027Unable to obtain network info: %s. Networks will not \u0027"},{"line_number":8417,"context_line":"                        \u0027be cleaned up correctly.\u0027, e, instance\u003dinstance)"},{"line_number":8418,"context_line":""},{"line_number":8419,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8420,"context_line":"                                          \"live_migration._post.start\","}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_117530cd","line":8417,"in_reply_to":"ff570b3c_987fce64","updated":"2020-07-16 14:03:50.000000000","message":"Yep, meant to do error here, thanks for catching it (ironically, that code on L8346 is my own)","commit_id":"95572842e9cb4ad733050d7eab2721858bbc478f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6543594b770409b119a5d88481cebad4f36efcea","unresolved":false,"context_lines":[{"line_number":8428,"context_line":"                     \u0027dest_compute\u0027: dest, }"},{"line_number":8429,"context_line":"        # For neutron, migrate_instance_start will activate the destination"},{"line_number":8430,"context_line":"        # host port bindings, if there are any created by conductor before live"},{"line_number":8431,"context_line":"        # migration started."},{"line_number":8432,"context_line":"        self.network_api.migrate_instance_start(ctxt,"},{"line_number":8433,"context_line":"                                                instance,"},{"line_number":8434,"context_line":"                                                migration)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_18e0de25","line":8431,"updated":"2020-05-20 21:27:40.000000000","message":"I do wonder if it would be actually safe to do this knowing that we\u0027d be unable to clean up the source. What are the implications of having bindings/plugged vifs on both the source and dest?","commit_id":"95572842e9cb4ad733050d7eab2721858bbc478f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"a107cfa6280632c79b0c981defd5b01cd7c2f535","unresolved":false,"context_lines":[{"line_number":8428,"context_line":"                     \u0027dest_compute\u0027: dest, }"},{"line_number":8429,"context_line":"        # For neutron, migrate_instance_start will activate the destination"},{"line_number":8430,"context_line":"        # host port bindings, if there are any created by conductor before live"},{"line_number":8431,"context_line":"        # migration started."},{"line_number":8432,"context_line":"        self.network_api.migrate_instance_start(ctxt,"},{"line_number":8433,"context_line":"                                                instance,"},{"line_number":8434,"context_line":"                                                migration)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_17188073","line":8431,"in_reply_to":"ff570b3c_18e0de25","updated":"2020-07-16 14:03:50.000000000","message":"Sean can keep me honest here, but I think activating the bindings on the destination and (un)plugging the VIFs are different things. Failing to unplug the VIFs on the source is a problem, but it\u0027s no worse than having _post_live_migration() fail entirely, and it shouldn\u0027t affect binding the port(s) on the destination and plugging the VIFs there.","commit_id":"95572842e9cb4ad733050d7eab2721858bbc478f"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3126799ce4dd91b2184fd32a8b425d047667d92b","unresolved":false,"context_lines":[{"line_number":8331,"context_line":"        self._post_live_migration_remove_source_vol_connections("},{"line_number":8332,"context_line":"            ctxt, instance, source_bdms)"},{"line_number":8333,"context_line":""},{"line_number":8334,"context_line":"        # Releasing vlan."},{"line_number":8335,"context_line":"        # (not necessary in current implementation?)"},{"line_number":8336,"context_line":""},{"line_number":8337,"context_line":"        network_info \u003d []"},{"line_number":8338,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_1600ad01","line":8335,"range":{"start_line":8334,"start_character":0,"end_line":8335,"end_character":52},"updated":"2020-07-15 19:43:22.000000000","message":"unrelated but should we remove this in a FUP?","commit_id":"07021fdbac131da55c83bddd37e76a6accb8aa7e"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"a107cfa6280632c79b0c981defd5b01cd7c2f535","unresolved":false,"context_lines":[{"line_number":8331,"context_line":"        self._post_live_migration_remove_source_vol_connections("},{"line_number":8332,"context_line":"            ctxt, instance, source_bdms)"},{"line_number":8333,"context_line":""},{"line_number":8334,"context_line":"        # Releasing vlan."},{"line_number":8335,"context_line":"        # (not necessary in current implementation?)"},{"line_number":8336,"context_line":""},{"line_number":8337,"context_line":"        network_info \u003d []"},{"line_number":8338,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_056187d3","line":8335,"range":{"start_line":8334,"start_character":0,"end_line":8335,"end_character":52},"in_reply_to":"bf51134e_1600ad01","updated":"2020-07-16 14:03:50.000000000","message":"Yeah, looks like the only point of getting network_info here is to pass is to _notify_about_instance_usage...","commit_id":"07021fdbac131da55c83bddd37e76a6accb8aa7e"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3126799ce4dd91b2184fd32a8b425d047667d92b","unresolved":false,"context_lines":[{"line_number":8342,"context_line":"            LOG.warning(\u0027Unable to obtain network info: %s. Networks will not \u0027"},{"line_number":8343,"context_line":"                        \u0027be cleaned up correctly.\u0027, e, instance\u003dinstance)"},{"line_number":8344,"context_line":""},{"line_number":8345,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8346,"context_line":"                                          \"live_migration._post.start\","},{"line_number":8347,"context_line":"                                          network_info\u003dnetwork_info)"},{"line_number":8348,"context_line":"        compute_utils.notify_about_instance_action("},{"line_number":8349,"context_line":"            ctxt, instance, self.host,"},{"line_number":8350,"context_line":"            action\u003dfields.NotificationAction.LIVE_MIGRATION_POST,"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_497a8a87","line":8347,"range":{"start_line":8345,"start_character":0,"end_line":8347,"end_character":68},"updated":"2020-07-15 19:43:22.000000000","message":"unrelated but this only calls compute_utils.notify_about_instance_usage?!","commit_id":"07021fdbac131da55c83bddd37e76a6accb8aa7e"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3126799ce4dd91b2184fd32a8b425d047667d92b","unresolved":false,"context_lines":[{"line_number":8366,"context_line":"            # stashed source vifs in migrate_data.vifs (if present) to unplug"},{"line_number":8367,"context_line":"            # on the source host."},{"line_number":8368,"context_line":"            unplug_nw_info \u003d network_info"},{"line_number":8369,"context_line":"            if migrate_data and \u0027vifs\u0027 in migrate_data:"},{"line_number":8370,"context_line":"                nw_info \u003d []"},{"line_number":8371,"context_line":"                for migrate_vif in migrate_data.vifs:"},{"line_number":8372,"context_line":"                    nw_info.append(migrate_vif.source_vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_29b6363f","line":8369,"range":{"start_line":8369,"start_character":0,"end_line":8369,"end_character":55},"updated":"2020-07-15 19:43:22.000000000","message":"This should always be True now right?","commit_id":"07021fdbac131da55c83bddd37e76a6accb8aa7e"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"a107cfa6280632c79b0c981defd5b01cd7c2f535","unresolved":false,"context_lines":[{"line_number":8366,"context_line":"            # stashed source vifs in migrate_data.vifs (if present) to unplug"},{"line_number":8367,"context_line":"            # on the source host."},{"line_number":8368,"context_line":"            unplug_nw_info \u003d network_info"},{"line_number":8369,"context_line":"            if migrate_data and \u0027vifs\u0027 in migrate_data:"},{"line_number":8370,"context_line":"                nw_info \u003d []"},{"line_number":8371,"context_line":"                for migrate_vif in migrate_data.vifs:"},{"line_number":8372,"context_line":"                    nw_info.append(migrate_vif.source_vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_c5982fc1","line":8369,"range":{"start_line":8369,"start_character":0,"end_line":8369,"end_character":55},"in_reply_to":"bf51134e_29b6363f","updated":"2020-07-16 14:03:50.000000000","message":"Yeah, this was added in Train [1] (without a TODO to remove it :( ), so should be safe to remove now.\n\n[1] https://review.opendev.org/#/c/620115/35/nova/compute/manager.py@6227","commit_id":"07021fdbac131da55c83bddd37e76a6accb8aa7e"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3126799ce4dd91b2184fd32a8b425d047667d92b","unresolved":false,"context_lines":[{"line_number":8422,"context_line":"        self.update_available_resource(ctxt)"},{"line_number":8423,"context_line":""},{"line_number":8424,"context_line":"        self._update_scheduler_instance_info(ctxt, instance)"},{"line_number":8425,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8426,"context_line":"                                          \"live_migration._post.end\","},{"line_number":8427,"context_line":"                                          network_info\u003dnetwork_info)"},{"line_number":8428,"context_line":"        compute_utils.notify_about_instance_action("}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_89a5c205","line":8425,"range":{"start_line":8425,"start_character":13,"end_line":8425,"end_character":41},"updated":"2020-07-15 19:43:22.000000000","message":"ditto","commit_id":"07021fdbac131da55c83bddd37e76a6accb8aa7e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9910c0d54fd3a47abf1e545d21e5fe48c306c47b","unresolved":false,"context_lines":[{"line_number":8334,"context_line":"        # Releasing vlan."},{"line_number":8335,"context_line":"        # (not necessary in current implementation?)"},{"line_number":8336,"context_line":""},{"line_number":8337,"context_line":"        network_info \u003d []"},{"line_number":8338,"context_line":"        try:"},{"line_number":8339,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(ctxt,"},{"line_number":8340,"context_line":"                                                                 instance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_1b01df05","line":8337,"range":{"start_line":8337,"start_character":23,"end_line":8337,"end_character":25},"updated":"2020-07-16 22:36:55.000000000","message":"Is this really right to be []? AFAICT network info is a nova.network.Model object. Shouldn\u0027t this just be None here?\n\nIf it needs to be iterable for some reason, it might be worth a code comment, because it\u0027s not obvious at least to me.","commit_id":"fca48fa2cd291a808a2a3b8d753bbd6cad48d78a"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"17d10d0c4e1c5dfe6234c73119093e470fb52270","unresolved":false,"context_lines":[{"line_number":8334,"context_line":"        # Releasing vlan."},{"line_number":8335,"context_line":"        # (not necessary in current implementation?)"},{"line_number":8336,"context_line":""},{"line_number":8337,"context_line":"        network_info \u003d []"},{"line_number":8338,"context_line":"        try:"},{"line_number":8339,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(ctxt,"},{"line_number":8340,"context_line":"                                                                 instance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_bb7a7322","line":8337,"range":{"start_line":8337,"start_character":23,"end_line":8337,"end_character":25},"in_reply_to":"bf51134e_1b01df05","updated":"2020-07-16 23:03:14.000000000","message":"Not sure why I had it as [] to be honest. The notification stuff looks like it should be able to handle None, as that\u0027s the default kwarg value, and the vif unplug stuff clearly doesn\u0027t care, since it uses unplug_nw_info which is populated independently.","commit_id":"fca48fa2cd291a808a2a3b8d753bbd6cad48d78a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9910c0d54fd3a47abf1e545d21e5fe48c306c47b","unresolved":false,"context_lines":[{"line_number":8340,"context_line":"                                                                 instance)"},{"line_number":8341,"context_line":"        except Exception as e:"},{"line_number":8342,"context_line":"            LOG.error(\u0027Unable to obtain network info: %s. Network info in \u0027"},{"line_number":8343,"context_line":"                      \u0027notifications will be incorrect.\u0027, e, instance\u003dinstance)"},{"line_number":8344,"context_line":""},{"line_number":8345,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8346,"context_line":"                                          \"live_migration._post.start\","}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_f804f11a","line":8343,"updated":"2020-07-16 22:36:55.000000000","message":"Now that the error message has changed, is this still ERROR log level worthy? Unless this failing is a symptom of something that should be fixed/addressed by the operator immediately, then ERROR makes sense I think.","commit_id":"fca48fa2cd291a808a2a3b8d753bbd6cad48d78a"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"17d10d0c4e1c5dfe6234c73119093e470fb52270","unresolved":false,"context_lines":[{"line_number":8340,"context_line":"                                                                 instance)"},{"line_number":8341,"context_line":"        except Exception as e:"},{"line_number":8342,"context_line":"            LOG.error(\u0027Unable to obtain network info: %s. Network info in \u0027"},{"line_number":8343,"context_line":"                      \u0027notifications will be incorrect.\u0027, e, instance\u003dinstance)"},{"line_number":8344,"context_line":""},{"line_number":8345,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8346,"context_line":"                                          \"live_migration._post.start\","}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_1ba39fb3","line":8343,"in_reply_to":"bf51134e_f804f11a","updated":"2020-07-16 23:03:14.000000000","message":"Good point, info would work better.","commit_id":"fca48fa2cd291a808a2a3b8d753bbd6cad48d78a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4a361f175cfe21d394e56bd69d3a66a48d1d8d7d","unresolved":false,"context_lines":[{"line_number":8340,"context_line":"                                                                 instance)"},{"line_number":8341,"context_line":"        except Exception as e:"},{"line_number":8342,"context_line":"            LOG.info(\u0027Unable to obtain network info: %s. Network info in \u0027"},{"line_number":8343,"context_line":"                     \u0027notifications will be incorrect.\u0027, e, instance\u003dinstance)"},{"line_number":8344,"context_line":""},{"line_number":8345,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8346,"context_line":"                                          \"live_migration._post.start\","}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_dcd54849","line":8343,"range":{"start_line":8343,"start_character":44,"end_line":8343,"end_character":53},"updated":"2020-07-22 18:40:12.000000000","message":"Since looking at the notifications code:\n\nhttps://github.com/openstack/nova/blob/97b0d251f7a5cc90d5a99a70499e57b1ae816b14/nova/notifications/base.py#L453\n\nIIUC the network info will be \"omitted\" rather than \"incorrect\" if it is None. Am I parsing that right?","commit_id":"af8ae54aa5db2245845cc2152ed931db7b49ecf1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1a72110c6cbd273b54215c59dfb98255ed854e5c","unresolved":false,"context_lines":[{"line_number":8365,"context_line":"            # host and is already bound and active, so we need to use the"},{"line_number":8366,"context_line":"            # stashed source vifs in migrate_data.vifs (if present) to unplug"},{"line_number":8367,"context_line":"            # on the source host."},{"line_number":8368,"context_line":"            unplug_nw_info \u003d network_info"},{"line_number":8369,"context_line":"            if migrate_data and \u0027vifs\u0027 in migrate_data:"},{"line_number":8370,"context_line":"                nw_info \u003d []"},{"line_number":8371,"context_line":"                for migrate_vif in migrate_data.vifs:"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_5c6e7496","line":8368,"range":{"start_line":8368,"start_character":29,"end_line":8368,"end_character":41},"updated":"2020-07-21 19:03:33.000000000","message":"Note to self: on the surface, it looks like this being None (or formerly []) will cause a problem with cleanup. However...","commit_id":"af8ae54aa5db2245845cc2152ed931db7b49ecf1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1a72110c6cbd273b54215c59dfb98255ed854e5c","unresolved":false,"context_lines":[{"line_number":8366,"context_line":"            # stashed source vifs in migrate_data.vifs (if present) to unplug"},{"line_number":8367,"context_line":"            # on the source host."},{"line_number":8368,"context_line":"            unplug_nw_info \u003d network_info"},{"line_number":8369,"context_line":"            if migrate_data and \u0027vifs\u0027 in migrate_data:"},{"line_number":8370,"context_line":"                nw_info \u003d []"},{"line_number":8371,"context_line":"                for migrate_vif in migrate_data.vifs:"},{"line_number":8372,"context_line":"                    nw_info.append(migrate_vif.source_vif)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_7cf6d8da","line":8369,"updated":"2020-07-21 19:03:33.000000000","message":"From \u003e\u003d Train, we are saying this will be True...","commit_id":"af8ae54aa5db2245845cc2152ed931db7b49ecf1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1a72110c6cbd273b54215c59dfb98255ed854e5c","unresolved":false,"context_lines":[{"line_number":8370,"context_line":"                nw_info \u003d []"},{"line_number":8371,"context_line":"                for migrate_vif in migrate_data.vifs:"},{"line_number":8372,"context_line":"                    nw_info.append(migrate_vif.source_vif)"},{"line_number":8373,"context_line":"                unplug_nw_info \u003d network_model.NetworkInfo.hydrate(nw_info)"},{"line_number":8374,"context_line":"                LOG.debug(\u0027Calling driver.post_live_migration_at_source \u0027"},{"line_number":8375,"context_line":"                          \u0027with original source VIFs from migrate_data: %s\u0027,"},{"line_number":8376,"context_line":"                          unplug_nw_info, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_9cfdacb3","line":8373,"range":{"start_line":8373,"start_character":16,"end_line":8373,"end_character":30},"updated":"2020-07-21 19:03:33.000000000","message":"And here we re-assign the unplug nw info with something from the migrate_data. That\u0027s why there\u0027s not an issue with cleanup \u003e\u003d Train.\n\nAnd then for \u003c Train, we\u0027ll have to use the other error message about cleanup being impacted. And we recognize that the impacted cleanup is no worse than the previous behavior of bailing out and not cleaning up at all.\n\nSo our goal is to get to post_live_migration on the source so we get the database updated to reflect the state of the instance better.\n\nFor \u003c Train, where we will have unplug_nw_info \u003d None, looking at the code in self.driver.post_live_migration_at_source, the only thing it does it unplug vifs, but I now see why you originally initialized network_info \u003d [], the network_info needs to be iterable here for the unplug to be a no-op (linking the current stable/stein code here though it hasn\u0027t changed):\n\nhttps://github.com/openstack/nova/blob/77225c9e83b38517696a3da206afcd609393b12c/nova/virt/libvirt/driver.py#L884\n\notherwise we will raise and not proceed to the rest of post_live_migration on the destination.\n\nSo we *do* need network_info \u003d [] on L8337 (with a code comment please, explaining why it needs to be []) and it would be nice if we cover that in a test somewhere... Ideally we\u0027d want a func test that simulates this network_info fetch failure and asserts the expected instance.host afterward. Do we not have the ability to do that in a func test?","commit_id":"af8ae54aa5db2245845cc2152ed931db7b49ecf1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f2b7f510733afec13011d4edfae5e7a1b0b041f9","unresolved":false,"context_lines":[{"line_number":8370,"context_line":"                nw_info \u003d []"},{"line_number":8371,"context_line":"                for migrate_vif in migrate_data.vifs:"},{"line_number":8372,"context_line":"                    nw_info.append(migrate_vif.source_vif)"},{"line_number":8373,"context_line":"                unplug_nw_info \u003d network_model.NetworkInfo.hydrate(nw_info)"},{"line_number":8374,"context_line":"                LOG.debug(\u0027Calling driver.post_live_migration_at_source \u0027"},{"line_number":8375,"context_line":"                          \u0027with original source VIFs from migrate_data: %s\u0027,"},{"line_number":8376,"context_line":"                          unplug_nw_info, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_59bdcad5","line":8373,"range":{"start_line":8373,"start_character":16,"end_line":8373,"end_character":30},"in_reply_to":"bf51134e_18907395","updated":"2020-07-22 18:31:43.000000000","message":"In a way it would be weird, yes, but my thinking was that if we have a code path on master, we could have test coverage for that path on master. What\u0027s the alternative? Adding stable-only test coverage for backports?\n\nHm, yeah I see now that unplug_vifs is just a pass in the fake driver, so that\u0027s not helpful. Maybe some sort of unit test then? Maybe a unit test that calls _post_live_migration and has get_instance_nw_info raise ConnectionError, then asserts that the network_info kwarg passed to _notify_about_instance_usage is []?\n\nMy overall thought on this is it seems more bad to not have any test coverage for something that would have not worked at all to fix the bug [in the \u003c Train backports]. But I do see your point.\n\nWhat do you think? Are you thinking to leave this change as-is and then when we get to the Stein/Queens backports, we change network_info \u003d [] and then add a test for that, stable-only?\n\nI note that the notifications code also expects network_info to be None if it\u0027s not to be used:\n\nhttps://github.com/openstack/nova/blob/97b0d251f7a5cc90d5a99a70499e57b1ae816b14/nova/notifications/base.py#L453\n\nso on master, I agree that network_info \u003d None is the best initial value.","commit_id":"af8ae54aa5db2245845cc2152ed931db7b49ecf1"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"541672676d553b251fadac64f6523750aed8239f","unresolved":false,"context_lines":[{"line_number":8370,"context_line":"                nw_info \u003d []"},{"line_number":8371,"context_line":"                for migrate_vif in migrate_data.vifs:"},{"line_number":8372,"context_line":"                    nw_info.append(migrate_vif.source_vif)"},{"line_number":8373,"context_line":"                unplug_nw_info \u003d network_model.NetworkInfo.hydrate(nw_info)"},{"line_number":8374,"context_line":"                LOG.debug(\u0027Calling driver.post_live_migration_at_source \u0027"},{"line_number":8375,"context_line":"                          \u0027with original source VIFs from migrate_data: %s\u0027,"},{"line_number":8376,"context_line":"                          unplug_nw_info, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_9f4efe6b","line":8373,"range":{"start_line":8373,"start_character":16,"end_line":8373,"end_character":30},"in_reply_to":"bf51134e_59bdcad5","updated":"2020-07-24 12:03:48.000000000","message":"fwiw, I see not reason to add the test when we get back to stable/queens","commit_id":"af8ae54aa5db2245845cc2152ed931db7b49ecf1"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"e3c9aebccce5bd6399d230b33304e5ad1473b23c","unresolved":false,"context_lines":[{"line_number":8370,"context_line":"                nw_info \u003d []"},{"line_number":8371,"context_line":"                for migrate_vif in migrate_data.vifs:"},{"line_number":8372,"context_line":"                    nw_info.append(migrate_vif.source_vif)"},{"line_number":8373,"context_line":"                unplug_nw_info \u003d network_model.NetworkInfo.hydrate(nw_info)"},{"line_number":8374,"context_line":"                LOG.debug(\u0027Calling driver.post_live_migration_at_source \u0027"},{"line_number":8375,"context_line":"                          \u0027with original source VIFs from migrate_data: %s\u0027,"},{"line_number":8376,"context_line":"                          unplug_nw_info, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_18907395","line":8373,"range":{"start_line":8373,"start_character":16,"end_line":8373,"end_character":30},"in_reply_to":"bf51134e_9c1c8cc2","updated":"2020-07-22 17:53:05.000000000","message":"It\u0027d be weird to have a functional test for something that can\u0027t happen on master, no? Also, we would have to do it with the real libvirt driver and fakelibvirt, and I\u0027m not sure how well those support VIFs (live migration mostly works, it\u0027s how I did the NUMA live migration functional tests).","commit_id":"af8ae54aa5db2245845cc2152ed931db7b49ecf1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"342eb4659400d2221183a8155565044331ca101f","unresolved":false,"context_lines":[{"line_number":8370,"context_line":"                nw_info \u003d []"},{"line_number":8371,"context_line":"                for migrate_vif in migrate_data.vifs:"},{"line_number":8372,"context_line":"                    nw_info.append(migrate_vif.source_vif)"},{"line_number":8373,"context_line":"                unplug_nw_info \u003d network_model.NetworkInfo.hydrate(nw_info)"},{"line_number":8374,"context_line":"                LOG.debug(\u0027Calling driver.post_live_migration_at_source \u0027"},{"line_number":8375,"context_line":"                          \u0027with original source VIFs from migrate_data: %s\u0027,"},{"line_number":8376,"context_line":"                          unplug_nw_info, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_9c1c8cc2","line":8373,"range":{"start_line":8373,"start_character":16,"end_line":8373,"end_character":30},"in_reply_to":"bf51134e_9cfdacb3","updated":"2020-07-21 19:07:42.000000000","message":"\u003e we cover that in a test somewhere... Ideally we\u0027d want a func test\n \u003e that simulates this network_info fetch failure and asserts the\n \u003e expected instance.host afterward. Do we not have the ability to do\n \u003e that in a func test?\n\nNote: I realize the func test would only apply to \u003c Train though, so it wouldn\u0027t make real-life sense on the master change. But I don\u0027t think we could or should have a stable-only func test, it would be better IMHO to have the func test on this change if not \u0027vifs\u0027 in migrate data and then remove it at the same time the \u0027if migrate_data and \u0027vifs\u0027 in migrate_data:\u0027 condition is removed.","commit_id":"af8ae54aa5db2245845cc2152ed931db7b49ecf1"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"43f646d429b2ebd2b17eb498cc1043be2b5e4579","unresolved":false,"context_lines":[{"line_number":8370,"context_line":"                nw_info \u003d []"},{"line_number":8371,"context_line":"                for migrate_vif in migrate_data.vifs:"},{"line_number":8372,"context_line":"                    nw_info.append(migrate_vif.source_vif)"},{"line_number":8373,"context_line":"                unplug_nw_info \u003d network_model.NetworkInfo.hydrate(nw_info)"},{"line_number":8374,"context_line":"                LOG.debug(\u0027Calling driver.post_live_migration_at_source \u0027"},{"line_number":8375,"context_line":"                          \u0027with original source VIFs from migrate_data: %s\u0027,"},{"line_number":8376,"context_line":"                          unplug_nw_info, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_3adb807d","line":8373,"range":{"start_line":8373,"start_character":16,"end_line":8373,"end_character":30},"in_reply_to":"bf51134e_9f4efe6b","updated":"2020-07-24 13:15:27.000000000","message":"@stephen\n \u003e fwiw, I see not reason to add the test when we get back to\n \u003e stable/queens\n\nYou mean you\u0027re OK with a stable-only test?\n\n@melwitt\n \u003e so on master, I agree that network_info \u003d None is the best initial\n \u003e value.\n\nSo is there consensus to do it with None on master, without any addition tests for code paths that can\u0027t get hit, and then move to [] in the backports, and add the test there?","commit_id":"af8ae54aa5db2245845cc2152ed931db7b49ecf1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"dff8f66fff49ec69201367f15492e79188883677","unresolved":false,"context_lines":[{"line_number":8340,"context_line":"                                                                 instance)"},{"line_number":8341,"context_line":"        except Exception as e:"},{"line_number":8342,"context_line":"            LOG.info(\u0027Unable to obtain network info: %s. Network info in \u0027"},{"line_number":8343,"context_line":"                     \u0027notifications will be incorrect.\u0027, e, instance\u003dinstance)"},{"line_number":8344,"context_line":""},{"line_number":8345,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8346,"context_line":"                                          \"live_migration._post.start\","}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_173534f8","line":8343,"range":{"start_line":8343,"start_character":44,"end_line":8343,"end_character":53},"updated":"2020-07-24 16:14:42.000000000","message":"I still have a comment from PS4 on this:\n\nhttps://review.opendev.org/#/c/729763/4/nova/compute/manager.py@8343\n\nAFAICT the network info in the notification will be completely omitted if we pass network_info\u003dNone, so it seems like this should say something else like \"omitted\" for the \u003e\u003d Train changes right? And then for \u003c Train it should says \"incorrect\".","commit_id":"1788b19ab150add1b1dc1fc102e1eb4e327880ed"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1d0c886ad948203a2246a016232c5083e0017a3e","unresolved":false,"context_lines":[{"line_number":8340,"context_line":"                                                                 instance)"},{"line_number":8341,"context_line":"        except Exception as e:"},{"line_number":8342,"context_line":"            LOG.info(\u0027Unable to obtain network info: %s. Network info in \u0027"},{"line_number":8343,"context_line":"                     \u0027notifications will be incorrect.\u0027, e, instance\u003dinstance)"},{"line_number":8344,"context_line":""},{"line_number":8345,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8346,"context_line":"                                          \"live_migration._post.start\","}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_daf9214e","line":8343,"range":{"start_line":8343,"start_character":44,"end_line":8343,"end_character":53},"in_reply_to":"9f560f44_173534f8","updated":"2020-07-24 17:04:27.000000000","message":"Yep, sorry, was hard to untangle discussion and \"-1 comments\". Done now.","commit_id":"1788b19ab150add1b1dc1fc102e1eb4e327880ed"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"dd500b95e4724e7e4a66e36fbebcd7b462a7525b","unresolved":false,"context_lines":[{"line_number":8362,"context_line":"        # Releasing vlan."},{"line_number":8363,"context_line":"        # (not necessary in current implementation?)"},{"line_number":8364,"context_line":""},{"line_number":8365,"context_line":"        network_info \u003d None"},{"line_number":8366,"context_line":"        try:"},{"line_number":8367,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(ctxt,"},{"line_number":8368,"context_line":"                                                                 instance)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_762520c2","line":8365,"range":{"start_line":8365,"start_character":8,"end_line":8365,"end_character":27},"updated":"2020-07-31 13:46:03.000000000","message":"Assuming you\u0027re planning on backporting this can you capture some of the discussion in the change about this and just note that \u003e\u003d Train this omits network_info from the notification but before that it\u0027s just incorrect?","commit_id":"148c3574c373b97c204623b35ea64bb40bfefbc3"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4a2b50150b39c052cb7d9973653f0fe9d04ef0b5","unresolved":false,"context_lines":[{"line_number":8362,"context_line":"        # Releasing vlan."},{"line_number":8363,"context_line":"        # (not necessary in current implementation?)"},{"line_number":8364,"context_line":""},{"line_number":8365,"context_line":"        network_info \u003d None"},{"line_number":8366,"context_line":"        try:"},{"line_number":8367,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(ctxt,"},{"line_number":8368,"context_line":"                                                                 instance)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_01d25b00","line":8365,"range":{"start_line":8365,"start_character":8,"end_line":8365,"end_character":27},"in_reply_to":"9f560f44_762520c2","updated":"2020-08-05 20:11:48.000000000","message":"I\u0027d rather not \"pollute\" master with talk of backports, but I\u0027ll definitely make a note of this in whatever release first hits the issue when I backport.","commit_id":"148c3574c373b97c204623b35ea64bb40bfefbc3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4c24743dd0f975ea8df87c4de1661ae4a06f3012","unresolved":false,"context_lines":[{"line_number":8364,"context_line":""},{"line_number":8365,"context_line":"        network_info \u003d None"},{"line_number":8366,"context_line":"        try:"},{"line_number":8367,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(ctxt,"},{"line_number":8368,"context_line":"                                                                 instance)"},{"line_number":8369,"context_line":"        except Exception as e:"},{"line_number":8370,"context_line":"            LOG.info(\u0027Unable to obtain network info: %s. Network info in \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_a14bf456","line":8367,"range":{"start_line":8367,"start_character":65,"end_line":8367,"end_character":70},"updated":"2020-07-31 13:55:22.000000000","message":"style nit: any chance you could drop this down to the next line? These severely indented hanging indents make my eyes bleed","commit_id":"148c3574c373b97c204623b35ea64bb40bfefbc3"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4a2b50150b39c052cb7d9973653f0fe9d04ef0b5","unresolved":false,"context_lines":[{"line_number":8364,"context_line":""},{"line_number":8365,"context_line":"        network_info \u003d None"},{"line_number":8366,"context_line":"        try:"},{"line_number":8367,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(ctxt,"},{"line_number":8368,"context_line":"                                                                 instance)"},{"line_number":8369,"context_line":"        except Exception as e:"},{"line_number":8370,"context_line":"            LOG.info(\u0027Unable to obtain network info: %s. Network info in \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_41d8d31c","line":8367,"range":{"start_line":8367,"start_character":65,"end_line":8367,"end_character":70},"in_reply_to":"9f560f44_a14bf456","updated":"2020-08-05 20:11:48.000000000","message":"Done","commit_id":"148c3574c373b97c204623b35ea64bb40bfefbc3"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"dd500b95e4724e7e4a66e36fbebcd7b462a7525b","unresolved":false,"context_lines":[{"line_number":8371,"context_line":"                     \u0027live.migration._post.start notification will be \u0027"},{"line_number":8372,"context_line":"                     \u0027omitted.\u0027, e, instance\u003dinstance)"},{"line_number":8373,"context_line":""},{"line_number":8374,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8375,"context_line":"                                          \"live_migration._post.start\","},{"line_number":8376,"context_line":"                                          network_info\u003dnetwork_info)"},{"line_number":8377,"context_line":"        compute_utils.notify_about_instance_action("},{"line_number":8378,"context_line":"            ctxt, instance, self.host,"},{"line_number":8379,"context_line":"            action\u003dfields.NotificationAction.LIVE_MIGRATION_POST,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_d61d0c74","line":8376,"range":{"start_line":8374,"start_character":0,"end_line":8376,"end_character":68},"updated":"2020-07-31 13:46:03.000000000","message":"hmm I wonder if we want to assert that network_info is omitted in the func test?","commit_id":"148c3574c373b97c204623b35ea64bb40bfefbc3"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4a2b50150b39c052cb7d9973653f0fe9d04ef0b5","unresolved":false,"context_lines":[{"line_number":8371,"context_line":"                     \u0027live.migration._post.start notification will be \u0027"},{"line_number":8372,"context_line":"                     \u0027omitted.\u0027, e, instance\u003dinstance)"},{"line_number":8373,"context_line":""},{"line_number":8374,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8375,"context_line":"                                          \"live_migration._post.start\","},{"line_number":8376,"context_line":"                                          network_info\u003dnetwork_info)"},{"line_number":8377,"context_line":"        compute_utils.notify_about_instance_action("},{"line_number":8378,"context_line":"            ctxt, instance, self.host,"},{"line_number":8379,"context_line":"            action\u003dfields.NotificationAction.LIVE_MIGRATION_POST,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_c1fd036f","line":8376,"range":{"start_line":8374,"start_character":0,"end_line":8376,"end_character":68},"in_reply_to":"9f560f44_a192d49c","updated":"2020-08-05 20:11:48.000000000","message":"Yeah, it was definitely mentioned before - calling Neutron over HTTP just for some (old school unversioned!!) notifications is pretty dumb, but I think fixing that should be part of a larger refactor. This isn\u0027t the first time we\u0027ve had to do try/except to make sure we saved the instance correctly in case a REST call failed, it\u0027s a sign that we probably need to rejigger a bunch of stuff to be safer/smarter.","commit_id":"148c3574c373b97c204623b35ea64bb40bfefbc3"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4a2b50150b39c052cb7d9973653f0fe9d04ef0b5","unresolved":false,"context_lines":[{"line_number":8371,"context_line":"                     \u0027live.migration._post.start notification will be \u0027"},{"line_number":8372,"context_line":"                     \u0027omitted.\u0027, e, instance\u003dinstance)"},{"line_number":8373,"context_line":""},{"line_number":8374,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8375,"context_line":"                                          \"live_migration._post.start\","},{"line_number":8376,"context_line":"                                          network_info\u003dnetwork_info)"},{"line_number":8377,"context_line":"        compute_utils.notify_about_instance_action("},{"line_number":8378,"context_line":"            ctxt, instance, self.host,"},{"line_number":8379,"context_line":"            action\u003dfields.NotificationAction.LIVE_MIGRATION_POST,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_617fd7df","line":8376,"range":{"start_line":8374,"start_character":0,"end_line":8376,"end_character":68},"in_reply_to":"9f560f44_d61d0c74","updated":"2020-08-05 20:11:48.000000000","message":"\u003e hmm I wonder if we want to assert that network_info is omitted in\n \u003e the func test?\n\nSo initially I said \"yeah ok\", but then I started actually doing it, and it\u0027s just too much work methinks. In order to obtain the correct un-versioned notification, we\u0027d have to refactor [1] or [2] to return the notification we want, and also give the instance we\u0027re booting a network interface, in order to give meaning to the eventual assertion that fixed_ips in the notification [3] is empty. Doens\u0027t sound worth it, though [1] and [2] should probably be factored into the integrated base in a subsequent patch.\n\n[1] https://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/regressions/test_bug_1713783.py#L69\n[2] https://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/regressions/test_bug_1837955.py#L30\n[3] https://opendev.org/openstack/nova/src/branch/master/nova/notifications/base.py#L453-L460","commit_id":"148c3574c373b97c204623b35ea64bb40bfefbc3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4c24743dd0f975ea8df87c4de1661ae4a06f3012","unresolved":false,"context_lines":[{"line_number":8371,"context_line":"                     \u0027live.migration._post.start notification will be \u0027"},{"line_number":8372,"context_line":"                     \u0027omitted.\u0027, e, instance\u003dinstance)"},{"line_number":8373,"context_line":""},{"line_number":8374,"context_line":"        self._notify_about_instance_usage(ctxt, instance,"},{"line_number":8375,"context_line":"                                          \"live_migration._post.start\","},{"line_number":8376,"context_line":"                                          network_info\u003dnetwork_info)"},{"line_number":8377,"context_line":"        compute_utils.notify_about_instance_action("},{"line_number":8378,"context_line":"            ctxt, instance, self.host,"},{"line_number":8379,"context_line":"            action\u003dfields.NotificationAction.LIVE_MIGRATION_POST,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_a192d49c","line":8376,"range":{"start_line":8374,"start_character":0,"end_line":8376,"end_character":68},"in_reply_to":"9f560f44_d61d0c74","updated":"2020-07-31 13:55:22.000000000","message":"It\u0027s not totally relevant and shouldn\u0027t stop this backportable fix, but how did we even decide to include this network info in the usage information in the first place? We only do it in a handful of places, and it\u0027s basically proxying stuff from neutron, right? I wonder if this is this a hangover from nova-network days and, if so, whether we could think about dropping it now?\n\nPS: I haven\u0027t read back through the previous convo that lyarwood referenced above, so maybe this has been discussed","commit_id":"148c3574c373b97c204623b35ea64bb40bfefbc3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0d86e046423d095001c8ec5d4943284e1a5ac61a","unresolved":false,"context_lines":[{"line_number":8394,"context_line":"            # host and is already bound and active, so we need to use the"},{"line_number":8395,"context_line":"            # stashed source vifs in migrate_data.vifs (if present) to unplug"},{"line_number":8396,"context_line":"            # on the source host."},{"line_number":8397,"context_line":"            unplug_nw_info \u003d network_info"},{"line_number":8398,"context_line":"            if migrate_data and \u0027vifs\u0027 in migrate_data:"},{"line_number":8399,"context_line":"                nw_info \u003d []"},{"line_number":8400,"context_line":"                for migrate_vif in migrate_data.vifs:"},{"line_number":8401,"context_line":"                    nw_info.append(migrate_vif.source_vif)"},{"line_number":8402,"context_line":"                unplug_nw_info \u003d network_model.NetworkInfo.hydrate(nw_info)"},{"line_number":8403,"context_line":"                LOG.debug(\u0027Calling driver.post_live_migration_at_source \u0027"},{"line_number":8404,"context_line":"                          \u0027with original source VIFs from migrate_data: %s\u0027,"},{"line_number":8405,"context_line":"                          unplug_nw_info, instance\u003dinstance)"},{"line_number":8406,"context_line":"            self.driver.post_live_migration_at_source(ctxt, instance,"},{"line_number":8407,"context_line":"                                                      unplug_nw_info)"},{"line_number":8408,"context_line":"        except NotImplementedError as ex:"},{"line_number":8409,"context_line":"            LOG.debug(ex, instance\u003dinstance)"},{"line_number":8410,"context_line":"            # For all hypervisors other than libvirt, there is a possibility"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f560f44_887c6cec","line":8407,"range":{"start_line":8397,"start_character":7,"end_line":8407,"end_character":69},"updated":"2020-08-13 00:18:27.000000000","message":"not that if we raise an exception on line 8367\nnetwork_info will be none and therefore unplug_nw_info\nwill be none.\n\nif we are using a neutron before rocky we will not be using multiple port bindings so we will not take the if and rebuild unplug_nw_info from the migrate data so if this is backported it will only work with a neutron that is at least rocky and only if we are using the multiple prot binding workflow.\n\notherwias we will call\n\nself.driver.post_live_migration_at_source(ctxt, instance, unplug_nw_info)\n\npassing none.\n\nthis means we wont unplug the vifs on teh source node.\n\nthis is only an issue before rocky e.g. queens\nform rocky on we will always the vifs in the migrate data.","commit_id":"9f205c620e57c9af760d3b52c93a4ff6d5c0e618"}]}
