)]}'
{"os_brick/initiator/linuxscsi.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5aa5ab59747c91ee6aada8528c5857f94164b0bd","unresolved":true,"context_lines":[{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        for device_name in devices_names:"},{"line_number":316,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":317,"context_line":"            if multipath_name:"},{"line_number":318,"context_line":"                # Because multipathd can delete a path device when a device"},{"line_number":319,"context_line":"                # is removed based on udev events, result of path removal is"},{"line_number":320,"context_line":"                # not stricktly checked at this point"}],"source_content_type":"text/x-python","patch_set":3,"id":"974dfaa0_141c92bc","line":317,"range":{"start_line":317,"start_character":15,"end_line":317,"end_character":29},"updated":"2021-04-27 12:31:38.000000000","message":"after flushing the multipath device at L#312, we set multipath_name to None on L#313.\nDon\u0027t we need to remove L#313 to make this work?","commit_id":"042e5fd55a86bb2907f445e4b243e9f2d9f8dadc"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"cfb3ec0aa5aa6a7ee23506a89a588f715baeedec","unresolved":true,"context_lines":[{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        for device_name in devices_names:"},{"line_number":316,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":317,"context_line":"            if multipath_name:"},{"line_number":318,"context_line":"                # Because multipathd can delete a path device when a device"},{"line_number":319,"context_line":"                # is removed based on udev events, result of path removal is"},{"line_number":320,"context_line":"                # not stricktly checked at this point"}],"source_content_type":"text/x-python","patch_set":3,"id":"cd418213_4e0f629a","line":317,"range":{"start_line":317,"start_character":15,"end_line":317,"end_character":29},"in_reply_to":"974dfaa0_141c92bc","updated":"2021-04-27 13:30:44.000000000","message":"Thank you for pointing that out. You are correct and I think I made a wrong change when I rebased my test patch on recent fix for multipath flushing.\nI\u0027ll update the patch asap.","commit_id":"042e5fd55a86bb2907f445e4b243e9f2d9f8dadc"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a3e274703ac11b4800c4544905a2b87a5707f9b6","unresolved":false,"context_lines":[{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        for device_name in devices_names:"},{"line_number":316,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":317,"context_line":"            if multipath_name:"},{"line_number":318,"context_line":"                # Because multipathd can delete a path device when a device"},{"line_number":319,"context_line":"                # is removed based on udev events, result of path removal is"},{"line_number":320,"context_line":"                # not stricktly checked at this point"}],"source_content_type":"text/x-python","patch_set":3,"id":"2deead42_f7cd0e67","line":317,"range":{"start_line":317,"start_character":15,"end_line":317,"end_character":29},"in_reply_to":"cd418213_4e0f629a","updated":"2021-04-27 13:56:01.000000000","message":"Done","commit_id":"042e5fd55a86bb2907f445e4b243e9f2d9f8dadc"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5aa5ab59747c91ee6aada8528c5857f94164b0bd","unresolved":true,"context_lines":[{"line_number":317,"context_line":"            if multipath_name:"},{"line_number":318,"context_line":"                # Because multipathd can delete a path device when a device"},{"line_number":319,"context_line":"                # is removed based on udev events, result of path removal is"},{"line_number":320,"context_line":"                # not stricktly checked at this point"},{"line_number":321,"context_line":"                self.multipath_del_path(dev_path)"},{"line_number":322,"context_line":"            flush \u003d self.requires_flush(dev_path, path_used, was_multipath)"},{"line_number":323,"context_line":"            self.remove_scsi_device(dev_path, force, exc, flush)"}],"source_content_type":"text/x-python","patch_set":3,"id":"04a84128_e985d9e7","line":320,"range":{"start_line":320,"start_character":22,"end_line":320,"end_character":31},"updated":"2021-04-27 12:31:38.000000000","message":"strictly","commit_id":"042e5fd55a86bb2907f445e4b243e9f2d9f8dadc"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a3e274703ac11b4800c4544905a2b87a5707f9b6","unresolved":false,"context_lines":[{"line_number":317,"context_line":"            if multipath_name:"},{"line_number":318,"context_line":"                # Because multipathd can delete a path device when a device"},{"line_number":319,"context_line":"                # is removed based on udev events, result of path removal is"},{"line_number":320,"context_line":"                # not stricktly checked at this point"},{"line_number":321,"context_line":"                self.multipath_del_path(dev_path)"},{"line_number":322,"context_line":"            flush \u003d self.requires_flush(dev_path, path_used, was_multipath)"},{"line_number":323,"context_line":"            self.remove_scsi_device(dev_path, force, exc, flush)"}],"source_content_type":"text/x-python","patch_set":3,"id":"0753be95_9f53e3f8","line":320,"range":{"start_line":320,"start_character":22,"end_line":320,"end_character":31},"in_reply_to":"04a84128_e985d9e7","updated":"2021-04-27 13:56:01.000000000","message":"Done","commit_id":"042e5fd55a86bb2907f445e4b243e9f2d9f8dadc"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5aa5ab59747c91ee6aada8528c5857f94164b0bd","unresolved":true,"context_lines":[{"line_number":318,"context_line":"                # Because multipathd can delete a path device when a device"},{"line_number":319,"context_line":"                # is removed based on udev events, result of path removal is"},{"line_number":320,"context_line":"                # not stricktly checked at this point"},{"line_number":321,"context_line":"                self.multipath_del_path(dev_path)"},{"line_number":322,"context_line":"            flush \u003d self.requires_flush(dev_path, path_used, was_multipath)"},{"line_number":323,"context_line":"            self.remove_scsi_device(dev_path, force, exc, flush)"},{"line_number":324,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"b82a9fd2_885afd77","line":321,"range":{"start_line":321,"start_character":21,"end_line":321,"end_character":39},"updated":"2021-04-27 12:31:38.000000000","message":"Shouldn\u0027t this be done after we flush any remaining IO with blockdev --flushbufs on L#72 ? i.e. in remove_scsi_device method","commit_id":"042e5fd55a86bb2907f445e4b243e9f2d9f8dadc"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a3e274703ac11b4800c4544905a2b87a5707f9b6","unresolved":true,"context_lines":[{"line_number":318,"context_line":"                # Because multipathd can delete a path device when a device"},{"line_number":319,"context_line":"                # is removed based on udev events, result of path removal is"},{"line_number":320,"context_line":"                # not stricktly checked at this point"},{"line_number":321,"context_line":"                self.multipath_del_path(dev_path)"},{"line_number":322,"context_line":"            flush \u003d self.requires_flush(dev_path, path_used, was_multipath)"},{"line_number":323,"context_line":"            self.remove_scsi_device(dev_path, force, exc, flush)"},{"line_number":324,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"c33ee38d_f097da5e","line":321,"range":{"start_line":321,"start_character":21,"end_line":321,"end_character":39},"in_reply_to":"b82a9fd2_885afd77","updated":"2021-04-27 13:56:01.000000000","message":"Because this just removes device from multipathd monitoring and doesn\u0027t touch device, it should be good to run this command once multipath device is flushed.","commit_id":"042e5fd55a86bb2907f445e4b243e9f2d9f8dadc"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a7c087b9f16d55392c8a3461fc2e8bd3800da922","unresolved":true,"context_lines":[{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"                for device_name in devices_names:"},{"line_number":316,"context_line":"                    # Ensure that the devices are removed from multipathd."},{"line_number":317,"context_line":"                    # Because multipathd can delete a path device based on udev"},{"line_number":318,"context_line":"                    # events later even if delete fails here, result of this"},{"line_number":319,"context_line":"                    # delete is not strictly checked at this point"},{"line_number":320,"context_line":"                    dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":321,"context_line":"                    self.multipath_del_path(dev_path)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        for device_name in devices_names:"},{"line_number":324,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":4,"id":"b77a5d3c_fe0b18f3","line":321,"range":{"start_line":315,"start_character":0,"end_line":321,"end_character":53},"updated":"2021-04-27 19:48:11.000000000","message":"-1: In my opinion devices should not be removed from multipathd at this point, and they should be removed *after* we have actually removed them from the system.\n\nI think we should add the call to multipath_del_path right after remove_scsi_device method, and making this call should be conditional to the multipathd running.\n\nIf we only remove it when we have multipath_name, we could still run into problems, because paths are always generated in multipathd, even when it doesn\u0027t form a multipath.","commit_id":"4cd508497c3d2e7ac6b05eb196c05e84f5ac48ee"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"16cb558e3349ce75e0db720a8498506ce610a8c6","unresolved":true,"context_lines":[{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"                for device_name in devices_names:"},{"line_number":316,"context_line":"                    # Ensure that the devices are removed from multipathd."},{"line_number":317,"context_line":"                    # Because multipathd can delete a path device based on udev"},{"line_number":318,"context_line":"                    # events later even if delete fails here, result of this"},{"line_number":319,"context_line":"                    # delete is not strictly checked at this point"},{"line_number":320,"context_line":"                    dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":321,"context_line":"                    self.multipath_del_path(dev_path)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        for device_name in devices_names:"},{"line_number":324,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":4,"id":"d7c9df15_40a9374f","line":321,"range":{"start_line":315,"start_character":0,"end_line":321,"end_character":53},"in_reply_to":"b77a5d3c_fe0b18f3","updated":"2021-04-27 22:55:19.000000000","message":"\u003e -1: In my opinion devices should not be removed from multipathd at this point, and they should be removed *after* we have actually removed them from the system.\n\nI disagree with this because of the following two points.\n\n1. The multipath device has been flushed already at this point and it should be safe to remove its path devices here.\n\n2. After we remove the path devices we can\u0027t guarantee that the same device name(like sdb) is not used by any other connection. So removing path after device removal might conflict with simultaneous volume attachment, though I think it\u0027s very much rare case.\n\n\n\u003e I think we should add the call to multipath_del_path right after remove_scsi_device method, and making this call should be conditional to the multipathd running.\n\u003e\n\u003e If we only remove it when we have multipath_name, we could still run into problems, because paths are always generated in multipathd, even when it doesn\u0027t form a multipath.\n\nI agree with this point, and will update the patch.","commit_id":"4cd508497c3d2e7ac6b05eb196c05e84f5ac48ee"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"01d2d89ee31d8475e3ae591f4bcb94334469f75b","unresolved":false,"context_lines":[{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"                for device_name in devices_names:"},{"line_number":316,"context_line":"                    # Ensure that the devices are removed from multipathd."},{"line_number":317,"context_line":"                    # Because multipathd can delete a path device based on udev"},{"line_number":318,"context_line":"                    # events later even if delete fails here, result of this"},{"line_number":319,"context_line":"                    # delete is not strictly checked at this point"},{"line_number":320,"context_line":"                    dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":321,"context_line":"                    self.multipath_del_path(dev_path)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        for device_name in devices_names:"},{"line_number":324,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":4,"id":"72bc89b1_9ce54bd0","line":321,"range":{"start_line":315,"start_character":0,"end_line":321,"end_character":53},"in_reply_to":"d7c9df15_40a9374f","updated":"2021-04-28 12:36:33.000000000","message":"You are right, and I thought about it while preparing dinner last night and forgot to update the comments, sorry about that.","commit_id":"4cd508497c3d2e7ac6b05eb196c05e84f5ac48ee"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a7c087b9f16d55392c8a3461fc2e8bd3800da922","unresolved":true,"context_lines":[{"line_number":735,"context_line":"        \"\"\"Remove a path from multipathd for monitoring.\"\"\""},{"line_number":736,"context_line":"        stdout, stderr \u003d self._execute(\u0027multipathd\u0027, \u0027del\u0027, \u0027path\u0027, realpath,"},{"line_number":737,"context_line":"                                       run_as_root\u003dTrue, timeout\u003d5,"},{"line_number":738,"context_line":"                                       check_exit_code\u003dFalse,"},{"line_number":739,"context_line":"                                       root_helper\u003dself._root_helper)"},{"line_number":740,"context_line":"        return stdout.strip() \u003d\u003d \u0027ok\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"ebdcdf64_5a37e44a","line":738,"updated":"2021-04-27 19:48:11.000000000","message":"I\u0027m ok with this, but not 100% sure if we should check for specific codes instead.","commit_id":"4cd508497c3d2e7ac6b05eb196c05e84f5ac48ee"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"01d2d89ee31d8475e3ae591f4bcb94334469f75b","unresolved":false,"context_lines":[{"line_number":735,"context_line":"        \"\"\"Remove a path from multipathd for monitoring.\"\"\""},{"line_number":736,"context_line":"        stdout, stderr \u003d self._execute(\u0027multipathd\u0027, \u0027del\u0027, \u0027path\u0027, realpath,"},{"line_number":737,"context_line":"                                       run_as_root\u003dTrue, timeout\u003d5,"},{"line_number":738,"context_line":"                                       check_exit_code\u003dFalse,"},{"line_number":739,"context_line":"                                       root_helper\u003dself._root_helper)"},{"line_number":740,"context_line":"        return stdout.strip() \u003d\u003d \u0027ok\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"80356053_21314a0e","line":738,"in_reply_to":"71a7a58c_9f4913cf","updated":"2021-04-28 12:36:33.000000000","message":"Ack","commit_id":"4cd508497c3d2e7ac6b05eb196c05e84f5ac48ee"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"16cb558e3349ce75e0db720a8498506ce610a8c6","unresolved":true,"context_lines":[{"line_number":735,"context_line":"        \"\"\"Remove a path from multipathd for monitoring.\"\"\""},{"line_number":736,"context_line":"        stdout, stderr \u003d self._execute(\u0027multipathd\u0027, \u0027del\u0027, \u0027path\u0027, realpath,"},{"line_number":737,"context_line":"                                       run_as_root\u003dTrue, timeout\u003d5,"},{"line_number":738,"context_line":"                                       check_exit_code\u003dFalse,"},{"line_number":739,"context_line":"                                       root_helper\u003dself._root_helper)"},{"line_number":740,"context_line":"        return stdout.strip() \u003d\u003d \u0027ok\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"71a7a58c_9f4913cf","line":738,"in_reply_to":"ebdcdf64_5a37e44a","updated":"2021-04-27 22:55:19.000000000","message":"This follows the implementation of multipath_add_path. It might be more safe to expect specific codes but I\u0027ll leave it now if it\u0027s ok.","commit_id":"4cd508497c3d2e7ac6b05eb196c05e84f5ac48ee"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6a37ccd41d0b1ced30ae0632aafd404904832e2a","unresolved":true,"context_lines":[{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        if LinuxSCSI.is_multipath_running(False, self._root_helper):"},{"line_number":316,"context_line":"            for device_name in devices_names:"},{"line_number":317,"context_line":"                # Ensure that the devices are removed from multipathd."},{"line_number":318,"context_line":"                # Because multipathd can delete a path device based on udev"},{"line_number":319,"context_line":"                # events later even if delete fails here, result of this"},{"line_number":320,"context_line":"                # delete is not strictly checked at this point"},{"line_number":321,"context_line":"                dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":322,"context_line":"                self.multipath_del_path(dev_path)"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        for device_name in devices_names:"},{"line_number":325,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":5,"id":"d48fdd51_bc452acd","line":322,"range":{"start_line":315,"start_character":0,"end_line":322,"end_character":49},"updated":"2021-04-28 10:57:30.000000000","message":"Since this now doesn\u0027t require multipath_name, can we include this inside the for loop on L#324\nwe can check if multipath is running and store it in a variable and use that variable while removing devices from multipathd, something like:\n\n    multipath_running \u003d False\n    if LinuxSCSI.is_multipath_running(False, self._root_helper):\n        multipath_running \u003d True\n\nand use it inside the loop below:\n\n    for device_name in devices_names:\n        dev_path \u003d \u0027/dev/\u0027 + device_name\n        if multipath_running:\n            self.multipath_del_path(dev_path)\n\nAnd I\u0027m still not sure about the consequence if this is successful and and flushing the io[1] or removing the scsi devices[2] fails somehow\n\n[1] https://github.com/openstack/os-brick/blob/master/os_brick/initiator/linuxscsi.py#L355-L357\n[2] https://github.com/openstack/os-brick/blob/master/os_brick/initiator/linuxscsi.py#L76-L77","commit_id":"59d764ab0eec77853839ba41d56ed3f089346711"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"98dc559932511c13265f1e36c70e054c639e5da1","unresolved":true,"context_lines":[{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        if LinuxSCSI.is_multipath_running(False, self._root_helper):"},{"line_number":316,"context_line":"            for device_name in devices_names:"},{"line_number":317,"context_line":"                # Ensure that the devices are removed from multipathd."},{"line_number":318,"context_line":"                # Because multipathd can delete a path device based on udev"},{"line_number":319,"context_line":"                # events later even if delete fails here, result of this"},{"line_number":320,"context_line":"                # delete is not strictly checked at this point"},{"line_number":321,"context_line":"                dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":322,"context_line":"                self.multipath_del_path(dev_path)"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        for device_name in devices_names:"},{"line_number":325,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":5,"id":"829a79c8_3d0a8238","line":322,"range":{"start_line":315,"start_character":0,"end_line":322,"end_character":49},"in_reply_to":"d48fdd51_bc452acd","updated":"2021-04-28 12:03:46.000000000","message":"\u003e Since this now doesn\u0027t require multipath_name, can we include this inside the for loop on L#324\n\nI\u0027ve updated the patch according to the suggestion. Thanks !.\n\n\u003e And I\u0027m still not sure about the consequence if this is successful and and flushing the io[1] or removing the scsi devices[2] fails somehow\n\nIf flush or removal of iscsi device fails, then the device is incompletely left even without any change, and we need manual cleanup or retrying detachment.\n\nIf we need manual cleanup, only difference made by this change is that the iscsi device is no longer monitored by multipathd. The required clean up steps are same and we need to flush and remove the iscsi device manually.\n\nIf we can rerun detachment (for example if volume detachiment from an instance failed), retried detachment triggers path removal again. However because we don\u0027t validate its result, the multipath_del_path completes without any failure even the device is already removed from multipathd and subsequent iscsi device flush/removal is executed.\n\nSo the change doesn\u0027t make any big change in the situation we expect when iscsi device is not properly removed.","commit_id":"59d764ab0eec77853839ba41d56ed3f089346711"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"01d2d89ee31d8475e3ae591f4bcb94334469f75b","unresolved":true,"context_lines":[{"line_number":311,"context_line":"            with exc.context(force, \u0027Flushing %s failed\u0027, multipath_name):"},{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        multipath_running \u003d LinuxSCSI.is_multipath_running("},{"line_number":316,"context_line":"            False, self._root_helper)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        for device_name in devices_names:"},{"line_number":319,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":6,"id":"16061ba9_0c2c727b","line":316,"range":{"start_line":314,"start_character":0,"end_line":316,"end_character":37},"updated":"2021-04-28 12:36:33.000000000","message":"-1: optimize the common case where we have multipath_name so we don\u0027t have to make an additional command line call.\n\nnit: use `self` instead of `LinuxSCSI`\n\nnit: use named parameter so it\u0027s easy to know what False is referring to.\n\nThis could lead to something like:\n\n        multipath_running \u003d multipath_name or self.is_multipath_running(\n            enforce_multipath\u003dFalse, root_helper\u003dself._root_helper)","commit_id":"3817406f91a12064b6c87969b5a661c89c2b1bca"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a38a515bf844637edf3c2c58efe910be5f974d0d","unresolved":true,"context_lines":[{"line_number":311,"context_line":"            with exc.context(force, \u0027Flushing %s failed\u0027, multipath_name):"},{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        multipath_running \u003d LinuxSCSI.is_multipath_running("},{"line_number":316,"context_line":"            False, self._root_helper)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        for device_name in devices_names:"},{"line_number":319,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":6,"id":"25e19ac6_cde0dfca","line":316,"range":{"start_line":314,"start_character":0,"end_line":316,"end_character":37},"in_reply_to":"16061ba9_0c2c727b","updated":"2021-04-28 13:00:28.000000000","message":"Thanks for useful suggestions. Expecially the optimization looks very smart to me.\n\nI\u0027ve updated to address all points.","commit_id":"3817406f91a12064b6c87969b5a661c89c2b1bca"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b7c700e5b4d68c68cbb041c1508fad5ec2eec085","unresolved":false,"context_lines":[{"line_number":311,"context_line":"            with exc.context(force, \u0027Flushing %s failed\u0027, multipath_name):"},{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        multipath_running \u003d LinuxSCSI.is_multipath_running("},{"line_number":316,"context_line":"            False, self._root_helper)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        for device_name in devices_names:"},{"line_number":319,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":6,"id":"82274c7c_b3991d18","line":316,"range":{"start_line":314,"start_character":0,"end_line":316,"end_character":37},"in_reply_to":"25e19ac6_cde0dfca","updated":"2021-04-28 13:03:39.000000000","message":"thanks, looks good","commit_id":"3817406f91a12064b6c87969b5a661c89c2b1bca"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"01d2d89ee31d8475e3ae591f4bcb94334469f75b","unresolved":true,"context_lines":[{"line_number":319,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":320,"context_line":"            if multipath_running:"},{"line_number":321,"context_line":"                # Ensure that the devices are removed from multipathd."},{"line_number":322,"context_line":"                # Because multipathd can delete a path device based on udev"},{"line_number":323,"context_line":"                # events later even if delete fails here, result of this"},{"line_number":324,"context_line":"                # delete is not strictly checked at this point"},{"line_number":325,"context_line":"                self.multipath_del_path(dev_path)"},{"line_number":326,"context_line":"            flush \u003d self.requires_flush(dev_path, path_used, was_multipath)"}],"source_content_type":"text/x-python","patch_set":6,"id":"413966be_2463853f","line":323,"range":{"start_line":322,"start_character":0,"end_line":323,"end_character":58},"updated":"2021-04-28 12:36:33.000000000","message":"If I understood correctly the issue is that multipathd doesn\u0027t notice in time that the device has been removed from the system by the time we use it again for a new connection.  Maybe we should mention it here so it\u0027s clear for other developers why this forced removal is necessary.","commit_id":"3817406f91a12064b6c87969b5a661c89c2b1bca"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a38a515bf844637edf3c2c58efe910be5f974d0d","unresolved":true,"context_lines":[{"line_number":319,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":320,"context_line":"            if multipath_running:"},{"line_number":321,"context_line":"                # Ensure that the devices are removed from multipathd."},{"line_number":322,"context_line":"                # Because multipathd can delete a path device based on udev"},{"line_number":323,"context_line":"                # events later even if delete fails here, result of this"},{"line_number":324,"context_line":"                # delete is not strictly checked at this point"},{"line_number":325,"context_line":"                self.multipath_del_path(dev_path)"},{"line_number":326,"context_line":"            flush \u003d self.requires_flush(dev_path, path_used, was_multipath)"}],"source_content_type":"text/x-python","patch_set":6,"id":"812e9bf6_d12c5e7b","line":323,"range":{"start_line":322,"start_character":0,"end_line":323,"end_character":58},"in_reply_to":"413966be_2463853f","updated":"2021-04-28 13:00:28.000000000","message":"I updated the comment here. Does it look better to you now ?","commit_id":"3817406f91a12064b6c87969b5a661c89c2b1bca"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b7c700e5b4d68c68cbb041c1508fad5ec2eec085","unresolved":false,"context_lines":[{"line_number":319,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"},{"line_number":320,"context_line":"            if multipath_running:"},{"line_number":321,"context_line":"                # Ensure that the devices are removed from multipathd."},{"line_number":322,"context_line":"                # Because multipathd can delete a path device based on udev"},{"line_number":323,"context_line":"                # events later even if delete fails here, result of this"},{"line_number":324,"context_line":"                # delete is not strictly checked at this point"},{"line_number":325,"context_line":"                self.multipath_del_path(dev_path)"},{"line_number":326,"context_line":"            flush \u003d self.requires_flush(dev_path, path_used, was_multipath)"}],"source_content_type":"text/x-python","patch_set":6,"id":"008766cb_ddfd0e91","line":323,"range":{"start_line":322,"start_character":0,"end_line":323,"end_character":58},"in_reply_to":"812e9bf6_d12c5e7b","updated":"2021-04-28 13:03:39.000000000","message":"Looks great.","commit_id":"3817406f91a12064b6c87969b5a661c89c2b1bca"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5ac319dbe6242eec2cc8135b4b31e9cfd2eb59b1","unresolved":true,"context_lines":[{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        multipath_running \u003d multipath_name or self.is_multipath_running("},{"line_number":316,"context_line":"            enforce_multipath\u003dFalse, root_helper\u003dself._root_helper)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        for device_name in devices_names:"},{"line_number":319,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":8,"id":"8df9d178_a66cd07d","line":316,"range":{"start_line":315,"start_character":0,"end_line":316,"end_character":67},"updated":"2021-04-28 13:16:03.000000000","message":"whoami-rajat pointed out that multipath_name will be set to None on line 313 in most cases, so this is pointless.\nSorry about that. :-(\n\nIn that case we need to change the code to account for that by setting multipath_running to True inside the \"if multipath_name:\" part and add an else clause to it.\n\n        if multipath_name:\n            with exc.context(force, \u0027Flushing %s failed\u0027, multipath_name):\n                self.flush_multipath_device(multipath_name)\n                multipath_name \u003d None\n                multipath_running \u003d True\n        else:\n            multipath_running \u003d self.is_multipath_running(\n                enforce_multipath\u003dFalse, root_helper\u003dself._root_helper)","commit_id":"023d96917a09b5c8825f17d9a48cfb0a12ff5cd4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f899d99e128cdff02e522ad0564dd01e077868fb","unresolved":false,"context_lines":[{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        multipath_running \u003d multipath_name or self.is_multipath_running("},{"line_number":316,"context_line":"            enforce_multipath\u003dFalse, root_helper\u003dself._root_helper)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        for device_name in devices_names:"},{"line_number":319,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":8,"id":"0bc59c55_78e593f4","line":316,"range":{"start_line":315,"start_character":0,"end_line":316,"end_character":67},"in_reply_to":"045af744_adc77e81","updated":"2021-04-28 16:02:40.000000000","message":"that\u0027s even better  :-)","commit_id":"023d96917a09b5c8825f17d9a48cfb0a12ff5cd4"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"cbf3270b97fab479fd66117dd2ac76a6d86e3a2a","unresolved":true,"context_lines":[{"line_number":312,"context_line":"                self.flush_multipath_device(multipath_name)"},{"line_number":313,"context_line":"                multipath_name \u003d None"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        multipath_running \u003d multipath_name or self.is_multipath_running("},{"line_number":316,"context_line":"            enforce_multipath\u003dFalse, root_helper\u003dself._root_helper)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        for device_name in devices_names:"},{"line_number":319,"context_line":"            dev_path \u003d \u0027/dev/\u0027 + device_name"}],"source_content_type":"text/x-python","patch_set":8,"id":"045af744_adc77e81","line":316,"range":{"start_line":315,"start_character":0,"end_line":316,"end_character":67},"in_reply_to":"8df9d178_a66cd07d","updated":"2021-04-28 13:32:57.000000000","message":"Done. Note I put multipath_running \u003d True outside of the with block to ensure that variable is initialized when flush_multipath_device fails but force\u003dTrue.\n\nOne unit test case to simulate the situation where dm device doesn\u0027t exist was added to test this behavior.","commit_id":"023d96917a09b5c8825f17d9a48cfb0a12ff5cd4"}],"releasenotes/notes/bug-1924652-2323f905f62ef8ba.yaml":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a7c087b9f16d55392c8a3461fc2e8bd3800da922","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug #1924652 \u003chttps://bugs.launchpad.net/os-brick/+bug/1924652\u003e`_: Ensure"},{"line_number":5,"context_line":"    path devices are removed during detaching a multipath volume, to avoid"},{"line_number":6,"context_line":"    orphan paths left which sometimes prevent subsequent multipath device"},{"line_number":7,"context_line":"    creation."}],"source_content_type":"text/x-yaml","patch_set":4,"id":"e3a21842_0a838e33","line":7,"range":{"start_line":4,"start_character":72,"end_line":7,"end_character":13},"updated":"2021-04-27 19:48:11.000000000","message":"I think this could be misleading, because path can refer to the multipathd path entity as well as the path of the actual device that is reused.\n\nI would rephrase it to mention the issue and just that is fixed.  Something in the lines of: \"Fix issue with newer multipathd implementations where quick reuse of an removed device path would make multipathd not notice it as a new device, preventing it from creating a multipath device for the new device path.\"","commit_id":"4cd508497c3d2e7ac6b05eb196c05e84f5ac48ee"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"01d2d89ee31d8475e3ae591f4bcb94334469f75b","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":"    `Bug #1924652 \u003chttps://bugs.launchpad.net/os-brick/+bug/1924652\u003e`_: Ensure"},{"line_number":5,"context_line":"    path devices are removed during detaching a multipath volume, to avoid"},{"line_number":6,"context_line":"    orphan paths left which sometimes prevent subsequent multipath device"},{"line_number":7,"context_line":"    creation."}],"source_content_type":"text/x-yaml","patch_set":4,"id":"86614d61_8f009d52","line":7,"range":{"start_line":4,"start_character":72,"end_line":7,"end_character":13},"in_reply_to":"e3a21842_0a838e33","updated":"2021-04-28 12:36:33.000000000","message":"Done","commit_id":"4cd508497c3d2e7ac6b05eb196c05e84f5ac48ee"}]}