)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9e582745db99b8664654280a96775ea0ab7219fe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c3d7ac73_d2ebf2cb","updated":"2023-03-27 11:22:56.000000000","message":"Thanks for working on this fix.","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"742855a6466c7bd6f73ad5d0579e463b311e4c34","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7a0b12c8_505d42d9","updated":"2023-04-20 16:08:47.000000000","message":"Thanks Gorka for the review. I\u0027ve modified the logic for the iSCSI and FC connector to pass multipath-dm (dm-*) and we use that to flush and inquire if multipath device exists or not","commit_id":"be4d4e191e78cb7df0c439054cdb937f9db73ea2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"885522ec7ddd147675dd3298b61de821ddb7c992","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"31b1f412_c313f363","updated":"2023-05-26 08:20:45.000000000","message":"Thanks for fixing this issue","commit_id":"be4d4e191e78cb7df0c439054cdb937f9db73ea2"}],"os_brick/initiator/linuxscsi.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9e582745db99b8664654280a96775ea0ab7219fe","unresolved":true,"context_lines":[{"line_number":384,"context_line":"        def _flush_multipath_device(device_map_name):"},{"line_number":385,"context_line":"            try:"},{"line_number":386,"context_line":"                self._execute(\u0027multipath\u0027, \u0027-f\u0027, device_map_name, run_as_root\u003dTrue,"},{"line_number":387,"context_line":"                              root_helper\u003dself._root_helper)"},{"line_number":388,"context_line":"            except Exception as ex:"},{"line_number":389,"context_line":"                self.tries \u003d self.tries + 1"},{"line_number":390,"context_line":"                if self.tries \u003e\u003d 3:"}],"source_content_type":"text/x-python","patch_set":1,"id":"a5b4a916_3911fba8","line":387,"updated":"2023-03-27 11:22:56.000000000","message":"-1: You have removed the `timeout\u003d300` which was added to prevent the call from blocking (as mentioned in comment L381-L383).","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"742855a6466c7bd6f73ad5d0579e463b311e4c34","unresolved":false,"context_lines":[{"line_number":384,"context_line":"        def _flush_multipath_device(device_map_name):"},{"line_number":385,"context_line":"            try:"},{"line_number":386,"context_line":"                self._execute(\u0027multipath\u0027, \u0027-f\u0027, device_map_name, run_as_root\u003dTrue,"},{"line_number":387,"context_line":"                              root_helper\u003dself._root_helper)"},{"line_number":388,"context_line":"            except Exception as ex:"},{"line_number":389,"context_line":"                self.tries \u003d self.tries + 1"},{"line_number":390,"context_line":"                if self.tries \u003e\u003d 3:"}],"source_content_type":"text/x-python","patch_set":1,"id":"80c8d4d4_3c2744ab","line":387,"in_reply_to":"a5b4a916_3911fba8","updated":"2023-04-20 16:08:47.000000000","message":"Done","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9e582745db99b8664654280a96775ea0ab7219fe","unresolved":true,"context_lines":[{"line_number":387,"context_line":"                              root_helper\u003dself._root_helper)"},{"line_number":388,"context_line":"            except Exception as ex:"},{"line_number":389,"context_line":"                self.tries \u003d self.tries + 1"},{"line_number":390,"context_line":"                if self.tries \u003e\u003d 3:"},{"line_number":391,"context_line":"                    raise ex"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"            (out, _err) \u003d self._execute(\u0027multipath\u0027, \u0027-l\u0027, device_map_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"29accb74_4a6334fc","line":390,"updated":"2023-03-27 11:22:56.000000000","message":"-1: If the last failure is cause by the device having been removed the call will fail instead of succeed.  This should go after L398","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"742855a6466c7bd6f73ad5d0579e463b311e4c34","unresolved":false,"context_lines":[{"line_number":387,"context_line":"                              root_helper\u003dself._root_helper)"},{"line_number":388,"context_line":"            except Exception as ex:"},{"line_number":389,"context_line":"                self.tries \u003d self.tries + 1"},{"line_number":390,"context_line":"                if self.tries \u003e\u003d 3:"},{"line_number":391,"context_line":"                    raise ex"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"            (out, _err) \u003d self._execute(\u0027multipath\u0027, \u0027-l\u0027, device_map_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"f5bc7c5f_5044f3a9","line":390,"in_reply_to":"29accb74_4a6334fc","updated":"2023-04-20 16:08:47.000000000","message":"Done","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9e582745db99b8664654280a96775ea0ab7219fe","unresolved":true,"context_lines":[{"line_number":390,"context_line":"                if self.tries \u003e\u003d 3:"},{"line_number":391,"context_line":"                    raise ex"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"            (out, _err) \u003d self._execute(\u0027multipath\u0027, \u0027-l\u0027, device_map_name,"},{"line_number":394,"context_line":"                                        run_as_root\u003dTrue,"},{"line_number":395,"context_line":"                                        check_exit_code\u003dFalse,"},{"line_number":396,"context_line":"                                        root_helper\u003dself._root_helper)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9363aa5c_77259df2","line":393,"updated":"2023-03-27 11:22:56.000000000","message":"-1: Calling `multipath -l` is an \"expensive\" operation because we have to call os-brick, which has to fork a process to run it.  We should use sysfs to do this more efficiently.","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"742855a6466c7bd6f73ad5d0579e463b311e4c34","unresolved":false,"context_lines":[{"line_number":390,"context_line":"                if self.tries \u003e\u003d 3:"},{"line_number":391,"context_line":"                    raise ex"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"            (out, _err) \u003d self._execute(\u0027multipath\u0027, \u0027-l\u0027, device_map_name,"},{"line_number":394,"context_line":"                                        run_as_root\u003dTrue,"},{"line_number":395,"context_line":"                                        check_exit_code\u003dFalse,"},{"line_number":396,"context_line":"                                        root_helper\u003dself._root_helper)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bc2c5ce8_0482e22e","line":393,"in_reply_to":"9363aa5c_77259df2","updated":"2023-04-20 16:08:47.000000000","message":"Done","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9e582745db99b8664654280a96775ea0ab7219fe","unresolved":true,"context_lines":[{"line_number":398,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        self.tries \u003d 0"},{"line_number":401,"context_line":"        timer \u003d loopingcall.FixedIntervalWithTimeoutLoopingCall("},{"line_number":402,"context_line":"            _flush_multipath_device, device_map_name)"},{"line_number":403,"context_line":"        timer.start(interval\u003d10, timeout\u003d300).wait()"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    @utils.retry(exception.VolumeDeviceNotFound)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7c5974f5_27a2a5b3","line":402,"range":{"start_line":401,"start_character":0,"end_line":402,"end_character":53},"updated":"2023-03-27 11:22:56.000000000","message":"-1: I think the `@utils.retry` decorator is a cleaner way of doing this, like we do in other methods, like the `wait_for_path` one.","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"742855a6466c7bd6f73ad5d0579e463b311e4c34","unresolved":false,"context_lines":[{"line_number":398,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        self.tries \u003d 0"},{"line_number":401,"context_line":"        timer \u003d loopingcall.FixedIntervalWithTimeoutLoopingCall("},{"line_number":402,"context_line":"            _flush_multipath_device, device_map_name)"},{"line_number":403,"context_line":"        timer.start(interval\u003d10, timeout\u003d300).wait()"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    @utils.retry(exception.VolumeDeviceNotFound)"}],"source_content_type":"text/x-python","patch_set":1,"id":"323aefef_45eb8289","line":402,"range":{"start_line":401,"start_character":0,"end_line":402,"end_character":53},"in_reply_to":"7c5974f5_27a2a5b3","updated":"2023-04-20 16:08:47.000000000","message":"Done","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9e582745db99b8664654280a96775ea0ab7219fe","unresolved":true,"context_lines":[{"line_number":400,"context_line":"        self.tries \u003d 0"},{"line_number":401,"context_line":"        timer \u003d loopingcall.FixedIntervalWithTimeoutLoopingCall("},{"line_number":402,"context_line":"            _flush_multipath_device, device_map_name)"},{"line_number":403,"context_line":"        timer.start(interval\u003d10, timeout\u003d300).wait()"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    @utils.retry(exception.VolumeDeviceNotFound)"},{"line_number":406,"context_line":"    def wait_for_path(self, volume_path):"}],"source_content_type":"text/x-python","patch_set":1,"id":"592ba651_7493dc74","line":403,"range":{"start_line":403,"start_character":33,"end_line":403,"end_character":44},"updated":"2023-03-27 11:22:56.000000000","message":"This timeout is not equivalent to the `_execute` `timeout` parameter, because that other one actually sends a signal to the process [1], where this will most likely won\u0027t.\n\n\n[1]: https://github.com/openstack/os-brick/blob/28ffcdbfa138859859beca2f80627c076269be56/os_brick/privileged/rootwrap.py#L96","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"742855a6466c7bd6f73ad5d0579e463b311e4c34","unresolved":false,"context_lines":[{"line_number":400,"context_line":"        self.tries \u003d 0"},{"line_number":401,"context_line":"        timer \u003d loopingcall.FixedIntervalWithTimeoutLoopingCall("},{"line_number":402,"context_line":"            _flush_multipath_device, device_map_name)"},{"line_number":403,"context_line":"        timer.start(interval\u003d10, timeout\u003d300).wait()"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    @utils.retry(exception.VolumeDeviceNotFound)"},{"line_number":406,"context_line":"    def wait_for_path(self, volume_path):"}],"source_content_type":"text/x-python","patch_set":1,"id":"109271ae_ba83e36d","line":403,"range":{"start_line":403,"start_character":33,"end_line":403,"end_character":44},"in_reply_to":"592ba651_7493dc74","updated":"2023-04-20 16:08:47.000000000","message":"Done","commit_id":"bb49b412db72b9c07b9463450fdb920833c072f4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"885522ec7ddd147675dd3298b61de821ddb7c992","unresolved":true,"context_lines":[{"line_number":309,"context_line":"        LOG.debug(\u0027Removing %(type)s devices %(devices)s\u0027,"},{"line_number":310,"context_line":"                  {\u0027type\u0027: \u0027multipathed\u0027 if multipath_dm else \u0027single pathed\u0027,"},{"line_number":311,"context_line":"                   \u0027devices\u0027: \u0027, \u0027.join(devices_names)})"},{"line_number":312,"context_line":"        multipath_name \u003d self.get_dm_name(multipath_dm) and multipath_dm"},{"line_number":313,"context_line":"        if multipath_name:"},{"line_number":314,"context_line":"            with exc.context(force, \u0027Flushing %s failed\u0027, multipath_name):"},{"line_number":315,"context_line":"                self.flush_multipath_device(multipath_name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3bd4e803_1eae9672","line":312,"updated":"2023-05-26 08:20:45.000000000","message":"-1: We don\u0027t need to get the dm name, we can just use the `mutlipath_dm` directly.","commit_id":"be4d4e191e78cb7df0c439054cdb937f9db73ea2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"885522ec7ddd147675dd3298b61de821ddb7c992","unresolved":true,"context_lines":[{"line_number":385,"context_line":"            dm_path \u003d os.path.join(\u0027/dev/\u0027, device_map_name)"},{"line_number":386,"context_line":"            self._execute(\u0027multipath\u0027, \u0027-f\u0027, dm_path, run_as_root\u003dTrue,"},{"line_number":387,"context_line":"                          root_helper\u003dself._root_helper, timeout\u003d300)"},{"line_number":388,"context_line":"            # We are only concerned with the multipath device here so"},{"line_number":389,"context_line":"            # sending empty device list"},{"line_number":390,"context_line":"            if not self.get_sysfs_wwn([], device_map_name):"},{"line_number":391,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":2,"id":"037982b6_0667126e","line":388,"updated":"2023-05-26 08:20:45.000000000","message":"nit: Mention how the execution call can fail while the multipath is still being flushed by multipathd asynchronously, so the device can actually disappear after a failure.","commit_id":"be4d4e191e78cb7df0c439054cdb937f9db73ea2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"885522ec7ddd147675dd3298b61de821ddb7c992","unresolved":true,"context_lines":[{"line_number":387,"context_line":"                          root_helper\u003dself._root_helper, timeout\u003d300)"},{"line_number":388,"context_line":"            # We are only concerned with the multipath device here so"},{"line_number":389,"context_line":"            # sending empty device list"},{"line_number":390,"context_line":"            if not self.get_sysfs_wwn([], device_map_name):"},{"line_number":391,"context_line":"                return"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"        except putils.ProcessExecutionError as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"4c035de3_45ca6861","line":390,"updated":"2023-05-26 08:20:45.000000000","message":"-1: Better use:\n```\n   if not os.path.exists(dm_path):\n       return\n```","commit_id":"be4d4e191e78cb7df0c439054cdb937f9db73ea2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"885522ec7ddd147675dd3298b61de821ddb7c992","unresolved":true,"context_lines":[{"line_number":391,"context_line":"                return"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"        except putils.ProcessExecutionError as exc:"},{"line_number":394,"context_line":"            if not self.get_sysfs_wwn([], device_map_name):"},{"line_number":395,"context_line":"                return"},{"line_number":396,"context_line":"            raise exc"},{"line_number":397,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1e2511d4_cb3ea831","line":394,"updated":"2023-05-26 08:20:45.000000000","message":"ditto","commit_id":"be4d4e191e78cb7df0c439054cdb937f9db73ea2"}]}