)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"681c28684276ad123dd4135f31022cc44c57f860","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add a lock to prevent race during detach_interface"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Only allow one detach at a time with the same pattern instance-port_id"},{"line_number":10,"context_line":"in order to avoid race condition when multiple detach/attach are run"},{"line_number":11,"context_line":"concurrently."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"When multiple detach run concurrently on a specific instance-port_id,"},{"line_number":14,"context_line":"manager consider many of them as valid because info_cache still contains"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9f560f44_2cc94225","line":11,"range":{"start_line":10,"start_character":38,"end_line":11,"end_character":12},"updated":"2020-08-25 14:29:21.000000000","message":"you mention attach and detach here but only add the lock for detach.\nshould we lock both operations","commit_id":"2a5e0174d6d5be41686b96fe982fc922fcdd0262"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"9e95671276b787cb1e98dc193eeabae33b27b2c1","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add a lock to prevent race during detach_interface"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Only allow one detach at a time with the same pattern instance-port_id"},{"line_number":10,"context_line":"in order to avoid race condition when multiple detach/attach are run"},{"line_number":11,"context_line":"concurrently."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"When multiple detach run concurrently on a specific instance-port_id,"},{"line_number":14,"context_line":"manager consider many of them as valid because info_cache still contains"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9f560f44_801ac5c2","line":11,"range":{"start_line":10,"start_character":38,"end_line":11,"end_character":12},"in_reply_to":"9f560f44_2cc94225","updated":"2020-08-26 08:06:09.000000000","message":"Done","commit_id":"2a5e0174d6d5be41686b96fe982fc922fcdd0262"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0ecdbe79a4bcb3be6faf29f1faeb41929c3d36a2","unresolved":false,"context_lines":[{"line_number":15,"context_line":"the port and info_cache is refreshed only once the first request complete."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"So during this gap of time, while the first request accomplishes the task,"},{"line_number":18,"context_line":"all subsequent requests are destined to failed and caught as a Warning [1] in"},{"line_number":19,"context_line":"different location of code, according proper advancement of the first request."},{"line_number":20,"context_line":"The Issue is that all those caught requests finally run a"},{"line_number":21,"context_line":"deallocate_port_for_instance which will unbind the port."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f560f44_2c1627a9","line":18,"range":{"start_line":18,"start_character":40,"end_line":18,"end_character":46},"updated":"2020-08-28 10:29:43.000000000","message":"fail","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0ecdbe79a4bcb3be6faf29f1faeb41929c3d36a2","unresolved":false,"context_lines":[{"line_number":15,"context_line":"the port and info_cache is refreshed only once the first request complete."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"So during this gap of time, while the first request accomplishes the task,"},{"line_number":18,"context_line":"all subsequent requests are destined to failed and caught as a Warning [1] in"},{"line_number":19,"context_line":"different location of code, according proper advancement of the first request."},{"line_number":20,"context_line":"The Issue is that all those caught requests finally run a"},{"line_number":21,"context_line":"deallocate_port_for_instance which will unbind the port."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f560f44_8c0bf37b","line":18,"range":{"start_line":18,"start_character":51,"end_line":18,"end_character":70},"updated":"2020-08-28 10:29:43.000000000","message":"log a warning","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"900604db93a48ac9752c369df2adf484b6512654","unresolved":false,"context_lines":[{"line_number":15,"context_line":"the port and info_cache is refreshed only once the first request complete."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"So during this gap of time, while the first request accomplishes the task,"},{"line_number":18,"context_line":"all subsequent requests are destined to failed and caught as a Warning [1] in"},{"line_number":19,"context_line":"different location of code, according proper advancement of the first request."},{"line_number":20,"context_line":"The Issue is that all those caught requests finally run a"},{"line_number":21,"context_line":"deallocate_port_for_instance which will unbind the port."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f560f44_54867d67","line":18,"range":{"start_line":18,"start_character":40,"end_line":18,"end_character":46},"in_reply_to":"9f560f44_2c1627a9","updated":"2020-08-28 13:38:29.000000000","message":"Done","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"900604db93a48ac9752c369df2adf484b6512654","unresolved":false,"context_lines":[{"line_number":15,"context_line":"the port and info_cache is refreshed only once the first request complete."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"So during this gap of time, while the first request accomplishes the task,"},{"line_number":18,"context_line":"all subsequent requests are destined to failed and caught as a Warning [1] in"},{"line_number":19,"context_line":"different location of code, according proper advancement of the first request."},{"line_number":20,"context_line":"The Issue is that all those caught requests finally run a"},{"line_number":21,"context_line":"deallocate_port_for_instance which will unbind the port."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f560f44_7481014e","line":18,"range":{"start_line":18,"start_character":51,"end_line":18,"end_character":70},"in_reply_to":"9f560f44_8c0bf37b","updated":"2020-08-28 13:38:29.000000000","message":"Done","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0ecdbe79a4bcb3be6faf29f1faeb41929c3d36a2","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"So during this gap of time, while the first request accomplishes the task,"},{"line_number":18,"context_line":"all subsequent requests are destined to failed and caught as a Warning [1] in"},{"line_number":19,"context_line":"different location of code, according proper advancement of the first request."},{"line_number":20,"context_line":"The Issue is that all those caught requests finally run a"},{"line_number":21,"context_line":"deallocate_port_for_instance which will unbind the port."},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f560f44_cc24cb11","line":19,"range":{"start_line":19,"start_character":28,"end_line":19,"end_character":56},"updated":"2020-08-28 10:29:43.000000000","message":"depending on the outcome","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"900604db93a48ac9752c369df2adf484b6512654","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"So during this gap of time, while the first request accomplishes the task,"},{"line_number":18,"context_line":"all subsequent requests are destined to failed and caught as a Warning [1] in"},{"line_number":19,"context_line":"different location of code, according proper advancement of the first request."},{"line_number":20,"context_line":"The Issue is that all those caught requests finally run a"},{"line_number":21,"context_line":"deallocate_port_for_instance which will unbind the port."},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f560f44_b47b9979","line":19,"range":{"start_line":19,"start_character":28,"end_line":19,"end_character":56},"in_reply_to":"9f560f44_cc24cb11","updated":"2020-08-28 13:38:29.000000000","message":"Done","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0ecdbe79a4bcb3be6faf29f1faeb41929c3d36a2","unresolved":false,"context_lines":[{"line_number":25,"context_line":"inconsistency."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"[1] \u0027Detaching interface %(mac)s failed because the device is no longer found"},{"line_number":28,"context_line":"     on the guest.\u0027)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Closes-Bug: #1892870"},{"line_number":31,"context_line":"Change-Id: Iea5969d0bd16dc9a6f1ba950224b0115e466ce66"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f560f44_e67a90b8","line":28,"range":{"start_line":28,"start_character":19,"end_line":28,"end_character":20},"updated":"2020-08-28 10:29:43.000000000","message":"nit","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"900604db93a48ac9752c369df2adf484b6512654","unresolved":false,"context_lines":[{"line_number":25,"context_line":"inconsistency."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"[1] \u0027Detaching interface %(mac)s failed because the device is no longer found"},{"line_number":28,"context_line":"     on the guest.\u0027)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Closes-Bug: #1892870"},{"line_number":31,"context_line":"Change-Id: Iea5969d0bd16dc9a6f1ba950224b0115e466ce66"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f560f44_94801549","line":28,"range":{"start_line":28,"start_character":19,"end_line":28,"end_character":20},"in_reply_to":"9f560f44_e67a90b8","updated":"2020-08-28 13:38:29.000000000","message":"Done","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"}],"nova/compute/manager.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"681c28684276ad123dd4135f31022cc44c57f860","unresolved":false,"context_lines":[{"line_number":6957,"context_line":"        \"\"\"Attach a volume to an instance.\"\"\""},{"line_number":6958,"context_line":"        driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":6959,"context_line":""},{"line_number":6960,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":6961,"context_line":"        def do_attach_volume(context, instance, driver_bdm):"},{"line_number":6962,"context_line":"            try:"},{"line_number":6963,"context_line":"                return self._attach_volume(context, instance, driver_bdm)"},{"line_number":6964,"context_line":"            except Exception:"},{"line_number":6965,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":6966,"context_line":"                    bdm.destroy()"},{"line_number":6967,"context_line":""},{"line_number":6968,"context_line":"        do_attach_volume(context, instance, driver_bdm)"},{"line_number":6969,"context_line":""},{"line_number":6970,"context_line":"    def _attach_volume(self, context, instance, bdm):"},{"line_number":6971,"context_line":"        context \u003d context.elevated()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_8c85eeaf","line":6968,"range":{"start_line":6960,"start_character":2,"end_line":6968,"end_character":55},"updated":"2020-08-25 14:29:21.000000000","message":"and we lock on attach","commit_id":"2a5e0174d6d5be41686b96fe982fc922fcdd0262"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"9e95671276b787cb1e98dc193eeabae33b27b2c1","unresolved":false,"context_lines":[{"line_number":6957,"context_line":"        \"\"\"Attach a volume to an instance.\"\"\""},{"line_number":6958,"context_line":"        driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":6959,"context_line":""},{"line_number":6960,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":6961,"context_line":"        def do_attach_volume(context, instance, driver_bdm):"},{"line_number":6962,"context_line":"            try:"},{"line_number":6963,"context_line":"                return self._attach_volume(context, instance, driver_bdm)"},{"line_number":6964,"context_line":"            except Exception:"},{"line_number":6965,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":6966,"context_line":"                    bdm.destroy()"},{"line_number":6967,"context_line":""},{"line_number":6968,"context_line":"        do_attach_volume(context, instance, driver_bdm)"},{"line_number":6969,"context_line":""},{"line_number":6970,"context_line":"    def _attach_volume(self, context, instance, bdm):"},{"line_number":6971,"context_line":"        context \u003d context.elevated()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_4069cd22","line":6968,"range":{"start_line":6960,"start_character":2,"end_line":6968,"end_character":55},"in_reply_to":"9f560f44_8c85eeaf","updated":"2020-08-26 08:06:09.000000000","message":"ok","commit_id":"2a5e0174d6d5be41686b96fe982fc922fcdd0262"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"681c28684276ad123dd4135f31022cc44c57f860","unresolved":false,"context_lines":[{"line_number":7109,"context_line":"    @wrap_exception()"},{"line_number":7110,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"},{"line_number":7111,"context_line":"    @wrap_instance_fault"},{"line_number":7112,"context_line":"    def detach_volume(self, context, volume_id, instance, attachment_id):"},{"line_number":7113,"context_line":"        \"\"\"Detach a volume from an instance."},{"line_number":7114,"context_line":""},{"line_number":7115,"context_line":"        :param context: security context"},{"line_number":7116,"context_line":"        :param volume_id: the volume id"},{"line_number":7117,"context_line":"        :param instance: the Instance object to detach the volume from"},{"line_number":7118,"context_line":"        :param attachment_id: The volume attachment_id for the given instance"},{"line_number":7119,"context_line":"                              and volume."},{"line_number":7120,"context_line":""},{"line_number":7121,"context_line":"        \"\"\""},{"line_number":7122,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":7123,"context_line":"        def do_detach_volume(context, volume_id, instance, attachment_id):"},{"line_number":7124,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":7125,"context_line":"                    context, volume_id, instance.uuid)"},{"line_number":7126,"context_line":"            self._detach_volume(context, bdm, instance,"},{"line_number":7127,"context_line":"                                attachment_id\u003dattachment_id)"},{"line_number":7128,"context_line":""},{"line_number":7129,"context_line":"        do_detach_volume(context, volume_id, instance, attachment_id)"},{"line_number":7130,"context_line":""},{"line_number":7131,"context_line":"    def _init_volume_connection(self, context, new_volume,"},{"line_number":7132,"context_line":"                                old_volume_id, connector, bdm,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_ac8a72bb","line":7129,"range":{"start_line":7112,"start_character":3,"end_line":7129,"end_character":69},"updated":"2020-08-25 14:29:21.000000000","message":"note this is adding the same behavior we have for volumes already","commit_id":"2a5e0174d6d5be41686b96fe982fc922fcdd0262"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"9e95671276b787cb1e98dc193eeabae33b27b2c1","unresolved":false,"context_lines":[{"line_number":7109,"context_line":"    @wrap_exception()"},{"line_number":7110,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"},{"line_number":7111,"context_line":"    @wrap_instance_fault"},{"line_number":7112,"context_line":"    def detach_volume(self, context, volume_id, instance, attachment_id):"},{"line_number":7113,"context_line":"        \"\"\"Detach a volume from an instance."},{"line_number":7114,"context_line":""},{"line_number":7115,"context_line":"        :param context: security context"},{"line_number":7116,"context_line":"        :param volume_id: the volume id"},{"line_number":7117,"context_line":"        :param instance: the Instance object to detach the volume from"},{"line_number":7118,"context_line":"        :param attachment_id: The volume attachment_id for the given instance"},{"line_number":7119,"context_line":"                              and volume."},{"line_number":7120,"context_line":""},{"line_number":7121,"context_line":"        \"\"\""},{"line_number":7122,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":7123,"context_line":"        def do_detach_volume(context, volume_id, instance, attachment_id):"},{"line_number":7124,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":7125,"context_line":"                    context, volume_id, instance.uuid)"},{"line_number":7126,"context_line":"            self._detach_volume(context, bdm, instance,"},{"line_number":7127,"context_line":"                                attachment_id\u003dattachment_id)"},{"line_number":7128,"context_line":""},{"line_number":7129,"context_line":"        do_detach_volume(context, volume_id, instance, attachment_id)"},{"line_number":7130,"context_line":""},{"line_number":7131,"context_line":"    def _init_volume_connection(self, context, new_volume,"},{"line_number":7132,"context_line":"                                old_volume_id, connector, bdm,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_208139c4","line":7129,"range":{"start_line":7112,"start_character":3,"end_line":7129,"end_character":69},"in_reply_to":"9f560f44_ac8a72bb","updated":"2020-08-26 08:06:09.000000000","message":"ok","commit_id":"2a5e0174d6d5be41686b96fe982fc922fcdd0262"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"681c28684276ad123dd4135f31022cc44c57f860","unresolved":false,"context_lines":[{"line_number":7540,"context_line":"    @wrap_instance_fault"},{"line_number":7541,"context_line":"    def detach_interface(self, context, instance, port_id):"},{"line_number":7542,"context_line":"        \"\"\"Detach a network adapter from an instance.\"\"\""},{"line_number":7543,"context_line":"        lockname \u003d \u0027detach_interface-%s-%s\u0027 % (instance.uuid, port_id)"},{"line_number":7544,"context_line":""},{"line_number":7545,"context_line":"        @utils.synchronized(lockname)"},{"line_number":7546,"context_line":"        def _lock_do_detach_interface(context, instance, port_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_ccdd26de","line":7543,"range":{"start_line":7543,"start_character":19,"end_line":7543,"end_character":27},"updated":"2020-08-25 14:29:21.000000000","message":"if we do lock in attach too then we should just drop this prefix and use the same lockname for both cases.","commit_id":"2a5e0174d6d5be41686b96fe982fc922fcdd0262"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"9e95671276b787cb1e98dc193eeabae33b27b2c1","unresolved":false,"context_lines":[{"line_number":7540,"context_line":"    @wrap_instance_fault"},{"line_number":7541,"context_line":"    def detach_interface(self, context, instance, port_id):"},{"line_number":7542,"context_line":"        \"\"\"Detach a network adapter from an instance.\"\"\""},{"line_number":7543,"context_line":"        lockname \u003d \u0027detach_interface-%s-%s\u0027 % (instance.uuid, port_id)"},{"line_number":7544,"context_line":""},{"line_number":7545,"context_line":"        @utils.synchronized(lockname)"},{"line_number":7546,"context_line":"        def _lock_do_detach_interface(context, instance, port_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_40526d68","line":7543,"range":{"start_line":7543,"start_character":19,"end_line":7543,"end_character":27},"in_reply_to":"9f560f44_ccdd26de","updated":"2020-08-26 08:06:09.000000000","message":"I aggree.","commit_id":"2a5e0174d6d5be41686b96fe982fc922fcdd0262"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5ada97acefaafced7e68f2800d67cc7fe562c31c","unresolved":false,"context_lines":[{"line_number":7479,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"},{"line_number":7480,"context_line":"    @wrap_instance_fault"},{"line_number":7481,"context_line":"    def attach_interface(self, context, instance, network_id, port_id,"},{"line_number":7482,"context_line":"                         requested_ip, tag):"},{"line_number":7483,"context_line":"        lockname \u003d \u0027interface-%s-%s\u0027 % (instance.uuid, port_id)"},{"line_number":7484,"context_line":""},{"line_number":7485,"context_line":"        @utils.synchronized(lockname)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_a937d6e2","line":7482,"updated":"2020-08-27 14:18:55.000000000","message":"Rather than doing this mass indentation, could we add a second inner function and simply call that from the lock? See \u0027attach_volume\u0027 for an example","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"0eb094659342d7697e00d089696b4b5eb4ca4496","unresolved":false,"context_lines":[{"line_number":7479,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"},{"line_number":7480,"context_line":"    @wrap_instance_fault"},{"line_number":7481,"context_line":"    def attach_interface(self, context, instance, network_id, port_id,"},{"line_number":7482,"context_line":"                         requested_ip, tag):"},{"line_number":7483,"context_line":"        lockname \u003d \u0027interface-%s-%s\u0027 % (instance.uuid, port_id)"},{"line_number":7484,"context_line":""},{"line_number":7485,"context_line":"        @utils.synchronized(lockname)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_cbae752e","line":7482,"in_reply_to":"9f560f44_a937d6e2","updated":"2020-08-28 07:49:00.000000000","message":"That\u0027s true. done","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0ecdbe79a4bcb3be6faf29f1faeb41929c3d36a2","unresolved":false,"context_lines":[{"line_number":7562,"context_line":"        do_detach_interface(context, instance, port_id)"},{"line_number":7563,"context_line":""},{"line_number":7564,"context_line":"    def _detach_interface(self, context, instance, port_id):"},{"line_number":7565,"context_line":"        compute_utils.refresh_info_cache_for_instance(context, instance)"},{"line_number":7566,"context_line":"        network_info \u003d instance.info_cache.network_info"},{"line_number":7567,"context_line":"        condemned \u003d None"},{"line_number":7568,"context_line":"        for vif in network_info:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_4cda7b1d","line":7565,"updated":"2020-08-28 10:29:43.000000000","message":"So this will simply update the in-memory object by retrieving things from the DB. I assume the actual DB entry has already been updated by the earlier call?\n\nLater: Okay, so there\u0027s a call to \u0027update_instance_cache_with_nw_info\u0027 in \u0027nova.network.neutron.API.deallocate_for_instance\u0027, which is called at the bottom of this function. Can we not rely on that to set things properly for us?","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"900604db93a48ac9752c369df2adf484b6512654","unresolved":false,"context_lines":[{"line_number":7562,"context_line":"        do_detach_interface(context, instance, port_id)"},{"line_number":7563,"context_line":""},{"line_number":7564,"context_line":"    def _detach_interface(self, context, instance, port_id):"},{"line_number":7565,"context_line":"        compute_utils.refresh_info_cache_for_instance(context, instance)"},{"line_number":7566,"context_line":"        network_info \u003d instance.info_cache.network_info"},{"line_number":7567,"context_line":"        condemned \u003d None"},{"line_number":7568,"context_line":"        for vif in network_info:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_7498e1cb","line":7565,"in_reply_to":"9f560f44_4cda7b1d","updated":"2020-08-28 13:38:29.000000000","message":"Exactly, instances objects in memory does not contains very last network cache present in DB, because object is created from DB at the time of API call I guess.\n\nIf we comment out this line I reproduce the issue as explain in bug(I just retested)\n\nAt the end of the deallocate_port_for_instance, there is a called to update_instance_cache_with_nw_info which is updating DB (not refreshing instance from DB) so DB is in sync, and existing instance object in memory have to refresh from DB.","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a3a55815e79955a68aa2988956963011a936833e","unresolved":false,"context_lines":[{"line_number":7562,"context_line":"        do_detach_interface(context, instance, port_id)"},{"line_number":7563,"context_line":""},{"line_number":7564,"context_line":"    def _detach_interface(self, context, instance, port_id):"},{"line_number":7565,"context_line":"        compute_utils.refresh_info_cache_for_instance(context, instance)"},{"line_number":7566,"context_line":"        network_info \u003d instance.info_cache.network_info"},{"line_number":7567,"context_line":"        condemned \u003d None"},{"line_number":7568,"context_line":"        for vif in network_info:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_f4007138","line":7565,"in_reply_to":"9f560f44_7498e1cb","updated":"2020-08-28 14:08:33.000000000","message":"Okay, could we get a comment indicating this is necessary?","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"3df048e21bada23c231ac685648edf1eb7f7bfed","unresolved":false,"context_lines":[{"line_number":7562,"context_line":"        do_detach_interface(context, instance, port_id)"},{"line_number":7563,"context_line":""},{"line_number":7564,"context_line":"    def _detach_interface(self, context, instance, port_id):"},{"line_number":7565,"context_line":"        compute_utils.refresh_info_cache_for_instance(context, instance)"},{"line_number":7566,"context_line":"        network_info \u003d instance.info_cache.network_info"},{"line_number":7567,"context_line":"        condemned \u003d None"},{"line_number":7568,"context_line":"        for vif in network_info:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_cfa838db","line":7565,"in_reply_to":"9f560f44_f4007138","updated":"2020-08-28 14:26:00.000000000","message":"It is necessary to refresh instance network info cache object here with content of DB, because previous interface attach/detach that was holding the lock just updated it.","commit_id":"7c3eb10c7fade8a4cbab17aa50d7a8164da30664"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5ada97acefaafced7e68f2800d67cc7fe562c31c","unresolved":false,"context_lines":[{"line_number":10292,"context_line":"        with test.nested("},{"line_number":10293,"context_line":"            mock.patch.dict(self.compute.driver.capabilities,"},{"line_number":10294,"context_line":"                             supports_attach_interface\u003dTrue),"},{"line_number":10295,"context_line":"            mock.patch(\u0027oslo_concurrency.lockutils.lock\u0027)) as (cap, mock_lock):"},{"line_number":10296,"context_line":"            vif \u003d self.compute.attach_interface(self.context,"},{"line_number":10297,"context_line":"                                                instance,"},{"line_number":10298,"context_line":"                                                network_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_69577e02","line":10295,"updated":"2020-08-27 14:18:55.000000000","message":"nit:\n\n      mock.patch(\u0027oslo_concurrency.lockutils.lock\u0027)\n  ) as (cap, mock_lock):\n      vif \u003d self.compute.attach_interface(self.context,\n\n(makes it easier to visually distinguish","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"0eb094659342d7697e00d089696b4b5eb4ca4496","unresolved":false,"context_lines":[{"line_number":10292,"context_line":"        with test.nested("},{"line_number":10293,"context_line":"            mock.patch.dict(self.compute.driver.capabilities,"},{"line_number":10294,"context_line":"                             supports_attach_interface\u003dTrue),"},{"line_number":10295,"context_line":"            mock.patch(\u0027oslo_concurrency.lockutils.lock\u0027)) as (cap, mock_lock):"},{"line_number":10296,"context_line":"            vif \u003d self.compute.attach_interface(self.context,"},{"line_number":10297,"context_line":"                                                instance,"},{"line_number":10298,"context_line":"                                                network_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_6b6f69fc","line":10295,"in_reply_to":"9f560f44_69577e02","updated":"2020-08-28 07:49:00.000000000","message":"Done","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5ada97acefaafced7e68f2800d67cc7fe562c31c","unresolved":false,"context_lines":[{"line_number":10293,"context_line":"            mock.patch.dict(self.compute.driver.capabilities,"},{"line_number":10294,"context_line":"                             supports_attach_interface\u003dTrue),"},{"line_number":10295,"context_line":"            mock.patch(\u0027oslo_concurrency.lockutils.lock\u0027)) as (cap, mock_lock):"},{"line_number":10296,"context_line":"            vif \u003d self.compute.attach_interface(self.context,"},{"line_number":10297,"context_line":"                                                instance,"},{"line_number":10298,"context_line":"                                                network_id,"},{"line_number":10299,"context_line":"                                                port_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_895492ff","line":10296,"updated":"2020-08-27 14:18:55.000000000","message":"If you used an inner function, you wouldn\u0027t need the mock of the lock. You could simply call \u0027_attach_interface\u0027 instead","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e7736ee6877dfc96d6804cd9c62f93ebda63d61c","unresolved":false,"context_lines":[{"line_number":10293,"context_line":"            mock.patch.dict(self.compute.driver.capabilities,"},{"line_number":10294,"context_line":"                             supports_attach_interface\u003dTrue),"},{"line_number":10295,"context_line":"            mock.patch(\u0027oslo_concurrency.lockutils.lock\u0027)) as (cap, mock_lock):"},{"line_number":10296,"context_line":"            vif \u003d self.compute.attach_interface(self.context,"},{"line_number":10297,"context_line":"                                                instance,"},{"line_number":10298,"context_line":"                                                network_id,"},{"line_number":10299,"context_line":"                                                port_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_a97236e1","line":10296,"in_reply_to":"9f560f44_09d8821b","updated":"2020-08-27 14:50:23.000000000","message":"Ah, no, that\u0027s fair. I was thinking that checking that we called the lock was mostly noise but it probably does make sense to verify. Ignore this","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"115e47183d876128455c20b019b12219dca58123","unresolved":false,"context_lines":[{"line_number":10293,"context_line":"            mock.patch.dict(self.compute.driver.capabilities,"},{"line_number":10294,"context_line":"                             supports_attach_interface\u003dTrue),"},{"line_number":10295,"context_line":"            mock.patch(\u0027oslo_concurrency.lockutils.lock\u0027)) as (cap, mock_lock):"},{"line_number":10296,"context_line":"            vif \u003d self.compute.attach_interface(self.context,"},{"line_number":10297,"context_line":"                                                instance,"},{"line_number":10298,"context_line":"                                                network_id,"},{"line_number":10299,"context_line":"                                                port_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_09d8821b","line":10296,"in_reply_to":"9f560f44_895492ff","updated":"2020-08-27 14:38:58.000000000","message":"Not sure to understand:\n\nThe purpose here is to mock_lock to ensure at the end we go through this lock, as suggested by gibi comment.\n\nOr Maybe you mean replace test.nested with an inner like this:\n\n@mock.patch.dict(self.compute.driver.capabili....\n@mock.patch(\u0027oslo_concurrency.lockutils.lock\u0027)\ndef _attach_interface (mock_lock, cap)\n    vif \u003d ....","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"0eb094659342d7697e00d089696b4b5eb4ca4496","unresolved":false,"context_lines":[{"line_number":10293,"context_line":"            mock.patch.dict(self.compute.driver.capabilities,"},{"line_number":10294,"context_line":"                             supports_attach_interface\u003dTrue),"},{"line_number":10295,"context_line":"            mock.patch(\u0027oslo_concurrency.lockutils.lock\u0027)) as (cap, mock_lock):"},{"line_number":10296,"context_line":"            vif \u003d self.compute.attach_interface(self.context,"},{"line_number":10297,"context_line":"                                                instance,"},{"line_number":10298,"context_line":"                                                network_id,"},{"line_number":10299,"context_line":"                                                port_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_4b49c58c","line":10296,"in_reply_to":"9f560f44_a97236e1","updated":"2020-08-28 07:49:00.000000000","message":"yeah that\u0027s part of tested stuff.","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1ae0af35ddaa5df8203e15fe741769407ba643fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9f560f44_63dcdbe3","updated":"2020-08-26 08:50:58.000000000","message":"Could you please assert that the lock is grabbed for both attach and detach? You can mock and assert the call on \u0027oslo_concurrency.lockutils.lock\u0027 function.","commit_id":"1306f89937a1c9e2331afcb3a2972ab817872d13"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"da15b8d6ab0e9f81b9f49529d8d35e22ba46cb58","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9f560f44_5fbd1f00","in_reply_to":"9f560f44_63dcdbe3","updated":"2020-08-26 14:34:10.000000000","message":"makes sense, I added lock mock in both attach/detach test","commit_id":"1306f89937a1c9e2331afcb3a2972ab817872d13"}],"releasenotes/notes/bug-1892870-eb894956bf04713d.yaml":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5ada97acefaafced7e68f2800d67cc7fe562c31c","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    This release contains a fix for `bug 1892870`_ where a race condition"},{"line_number":5,"context_line":"    may occur during concurrent ``interface detach/attach``, resulting in an"},{"line_number":6,"context_line":"    interface accidentally unbind after attached."},{"line_number":7,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":4,"id":"9f560f44_c9416a3b","line":4,"range":{"start_line":4,"start_character":4,"end_line":4,"end_character":73},"updated":"2020-08-27 14:18:55.000000000","message":"Resolve a race condition that may occur...","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"0eb094659342d7697e00d089696b4b5eb4ca4496","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    This release contains a fix for `bug 1892870`_ where a race condition"},{"line_number":5,"context_line":"    may occur during concurrent ``interface detach/attach``, resulting in an"},{"line_number":6,"context_line":"    interface accidentally unbind after attached."},{"line_number":7,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":4,"id":"9f560f44_c6e30c06","line":4,"range":{"start_line":4,"start_character":4,"end_line":4,"end_character":73},"in_reply_to":"9f560f44_c9416a3b","updated":"2020-08-28 07:49:00.000000000","message":"Done","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5ada97acefaafced7e68f2800d67cc7fe562c31c","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    This release contains a fix for `bug 1892870`_ where a race condition"},{"line_number":5,"context_line":"    may occur during concurrent ``interface detach/attach``, resulting in an"},{"line_number":6,"context_line":"    interface accidentally unbind after attached."},{"line_number":7,"context_line":""},{"line_number":8,"context_line":"    .. _bug 1892870: https://bugs.launchpad.net/nova/+bug/1892870"}],"source_content_type":"text/x-yaml","patch_set":4,"id":"9f560f44_09488259","line":6,"updated":"2020-08-27 14:18:55.000000000","message":"See `bug 1892870`_ for more details.","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"0eb094659342d7697e00d089696b4b5eb4ca4496","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    This release contains a fix for `bug 1892870`_ where a race condition"},{"line_number":5,"context_line":"    may occur during concurrent ``interface detach/attach``, resulting in an"},{"line_number":6,"context_line":"    interface accidentally unbind after attached."},{"line_number":7,"context_line":""},{"line_number":8,"context_line":"    .. _bug 1892870: https://bugs.launchpad.net/nova/+bug/1892870"}],"source_content_type":"text/x-yaml","patch_set":4,"id":"9f560f44_a6e098fa","line":6,"in_reply_to":"9f560f44_09488259","updated":"2020-08-28 07:49:00.000000000","message":"Done","commit_id":"27f4ebaeee75049dea3236a2ef06527dc3ff0892"}]}