)]}'
{"os_brick/initiator/connectors/iscsi.py":[{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"15a807a38b5179e38923a8d098299104eadff754","unresolved":false,"context_lines":[{"line_number":108,"context_line":"    def _get_iscsi_sessions_full(self):"},{"line_number":109,"context_line":"        \"\"\"Get iSCSI session information as a list of tuples."},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        Uses iscsi_adm -m session and from a command output like"},{"line_number":112,"context_line":"            tcp: [1] 192.168.121.250:3260,1 iqn.2010-10.org.openstack:volume-"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        This method will drop the node type and return a list like this:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_43688d1b","line":111,"range":{"start_line":111,"start_character":13,"end_line":111,"end_character":22},"updated":"2017-04-11 20:27:58.000000000","message":"s/iscsi_adm/iscsiadm/ ?","commit_id":"dddc7fbe334cca207b1e96b615484fca4fdcaa3d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"282d164cc882311f70f99c6ea1639a5f000b6e96","unresolved":false,"context_lines":[{"line_number":108,"context_line":"    def _get_iscsi_sessions_full(self):"},{"line_number":109,"context_line":"        \"\"\"Get iSCSI session information as a list of tuples."},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        Uses iscsi_adm -m session and from a command output like"},{"line_number":112,"context_line":"            tcp: [1] 192.168.121.250:3260,1 iqn.2010-10.org.openstack:volume-"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        This method will drop the node type and return a list like this:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_ff6b9481","line":111,"range":{"start_line":111,"start_character":13,"end_line":111,"end_character":22},"in_reply_to":"bff0334d_43688d1b","updated":"2017-04-12 18:24:12.000000000","message":"Done","commit_id":"dddc7fbe334cca207b1e96b615484fca4fdcaa3d"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"15a807a38b5179e38923a8d098299104eadff754","unresolved":false,"context_lines":[{"line_number":508,"context_line":"        \"\"\"Get map of devices by sessions from our connection."},{"line_number":509,"context_line":""},{"line_number":510,"context_line":"        For each of the TCP sessions that correspond to our connection"},{"line_number":511,"context_line":"        properties we generate a map of (ip, iqn) to (belong, other) where"},{"line_number":512,"context_line":"        belong is a set of devices in that session that belong to our"},{"line_number":513,"context_line":"        connection and other are other devices that are also connected to that"},{"line_number":514,"context_line":"        session but are from other connections."},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"        We also include all nodes from our connection that don\u0027t have a"},{"line_number":517,"context_line":"        session."}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_74a58546","line":514,"range":{"start_line":511,"start_character":69,"end_line":514,"end_character":47},"updated":"2017-04-11 20:27:58.000000000","message":"We might want to clarify that \"belong\" are the devices for some particular volume attachment/LUN. I was confused for a while reading this because I assumed connection meant the iscsi connection, not the volume connection.","commit_id":"dddc7fbe334cca207b1e96b615484fca4fdcaa3d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"282d164cc882311f70f99c6ea1639a5f000b6e96","unresolved":false,"context_lines":[{"line_number":508,"context_line":"        \"\"\"Get map of devices by sessions from our connection."},{"line_number":509,"context_line":""},{"line_number":510,"context_line":"        For each of the TCP sessions that correspond to our connection"},{"line_number":511,"context_line":"        properties we generate a map of (ip, iqn) to (belong, other) where"},{"line_number":512,"context_line":"        belong is a set of devices in that session that belong to our"},{"line_number":513,"context_line":"        connection and other are other devices that are also connected to that"},{"line_number":514,"context_line":"        session but are from other connections."},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"        We also include all nodes from our connection that don\u0027t have a"},{"line_number":517,"context_line":"        session."}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_5fde003e","line":514,"range":{"start_line":511,"start_character":69,"end_line":514,"end_character":47},"in_reply_to":"bff0334d_74a58546","updated":"2017-04-12 18:24:12.000000000","message":"Yeah, reading it now I see it\u0027s not clear.","commit_id":"dddc7fbe334cca207b1e96b615484fca4fdcaa3d"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"15a807a38b5179e38923a8d098299104eadff754","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        self._disconnect_connection(connection_properties, disconnect, force,"},{"line_number":588,"context_line":"                                    exc)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        # If flushing the multipath failed we try to delete it from multipathd"},{"line_number":591,"context_line":"        if multipath_name:"},{"line_number":592,"context_line":"            LOG.debug(\u0027Deleting multipath %s directly in multipathd.\u0027,"},{"line_number":593,"context_line":"                      multipath_name)"},{"line_number":594,"context_line":"            self._linuxscsi.del_multipath_map(multipath_name)"},{"line_number":595,"context_line":""},{"line_number":596,"context_line":"        if exc:"},{"line_number":597,"context_line":"            LOG.warning(\u0027There were errors removing %s, leftovers may remain \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_d4dd71e0","line":594,"range":{"start_line":590,"start_character":8,"end_line":594,"end_character":61},"updated":"2017-04-11 20:27:58.000000000","message":"The only times I\u0027ve seen it fail is when its either still \"in use\" or multipathd had some internal error. Does doing it again like this fix the issue? Or is this handling some other case?\n\n\nWould it make more sense to just do a retry-loop when we do the multipath -f earlier on in the flow?","commit_id":"dddc7fbe334cca207b1e96b615484fca4fdcaa3d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"282d164cc882311f70f99c6ea1639a5f000b6e96","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        self._disconnect_connection(connection_properties, disconnect, force,"},{"line_number":588,"context_line":"                                    exc)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        # If flushing the multipath failed we try to delete it from multipathd"},{"line_number":591,"context_line":"        if multipath_name:"},{"line_number":592,"context_line":"            LOG.debug(\u0027Deleting multipath %s directly in multipathd.\u0027,"},{"line_number":593,"context_line":"                      multipath_name)"},{"line_number":594,"context_line":"            self._linuxscsi.del_multipath_map(multipath_name)"},{"line_number":595,"context_line":""},{"line_number":596,"context_line":"        if exc:"},{"line_number":597,"context_line":"            LOG.warning(\u0027There were errors removing %s, leftovers may remain \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_1f53d86c","line":594,"range":{"start_line":590,"start_character":8,"end_line":594,"end_character":61},"in_reply_to":"bff0334d_d4dd71e0","updated":"2017-04-12 18:24:12.000000000","message":"multipath -f has been called with 3 retries, but we are using a timeout there of 2 minutes, which means that in some cases, for example when multipath just hangs (I\u0027ve seen it happen) or when the connection is really poor, we may reach this point unable to actually flush the device.\n\nOf course, we only reach this point if `force` is True, otherwise we would have left by an exception.  So we want to do our best to remove the multipath even if it means not flushing data.","commit_id":"dddc7fbe334cca207b1e96b615484fca4fdcaa3d"}],"os_brick/initiator/linuxscsi.py":[{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"15a807a38b5179e38923a8d098299104eadff754","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        :param device_names: Iterable with device names, not paths. ie: [\u0027sda\u0027]"},{"line_number":148,"context_line":"        :returns: String with the dm name or None if not found. ie: \u0027dm-0\u0027"},{"line_number":149,"context_line":"        \"\"\""},{"line_number":150,"context_line":"        multipaths \u003d glob.glob(\u0027/sys/block/*/holders/dm-*\u0027)"},{"line_number":151,"context_line":"        for multipath in multipaths:"},{"line_number":152,"context_line":"            __, device_name, __, dm \u003d multipath.rsplit(\u0027/\u0027, 3)"},{"line_number":153,"context_line":"            if device_name in device_names:"},{"line_number":154,"context_line":"                return dm"},{"line_number":155,"context_line":"        return None"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def remove_connection(self, devices_names, is_multipath, force\u003dFalse,"},{"line_number":158,"context_line":"                          exc\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_190d548f","line":155,"range":{"start_line":150,"start_character":7,"end_line":155,"end_character":19},"updated":"2017-04-11 20:27:58.000000000","message":"I think we can simplify this, for each device (sdX) in device_names you see the dm-Y under holders, so why glob for /sys/block/* when we can just do like os.list_dir(\"/sys/block/%s/holders/\" % device) on each of the devices passed in? Maybe even just break after we find one successfully. Should be less things to loop through than all devices\n\nThe only entry in there should be the dm-Y device they belong too, right?","commit_id":"dddc7fbe334cca207b1e96b615484fca4fdcaa3d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"282d164cc882311f70f99c6ea1639a5f000b6e96","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        :param device_names: Iterable with device names, not paths. ie: [\u0027sda\u0027]"},{"line_number":148,"context_line":"        :returns: String with the dm name or None if not found. ie: \u0027dm-0\u0027"},{"line_number":149,"context_line":"        \"\"\""},{"line_number":150,"context_line":"        multipaths \u003d glob.glob(\u0027/sys/block/*/holders/dm-*\u0027)"},{"line_number":151,"context_line":"        for multipath in multipaths:"},{"line_number":152,"context_line":"            __, device_name, __, dm \u003d multipath.rsplit(\u0027/\u0027, 3)"},{"line_number":153,"context_line":"            if device_name in device_names:"},{"line_number":154,"context_line":"                return dm"},{"line_number":155,"context_line":"        return None"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def remove_connection(self, devices_names, is_multipath, force\u003dFalse,"},{"line_number":158,"context_line":"                          exc\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_99c021e3","line":155,"range":{"start_line":150,"start_character":7,"end_line":155,"end_character":19},"in_reply_to":"bff0334d_190d548f","updated":"2017-04-12 18:24:12.000000000","message":"Using glob is easier to read than using os.listdir, because it automatically handles the exception of non existent path.\n\nI will change it to doing the glob for each each device name, since it\u0027s less work for systems with a lot of block devices.","commit_id":"dddc7fbe334cca207b1e96b615484fca4fdcaa3d"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"15a807a38b5179e38923a8d098299104eadff754","unresolved":false,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    def del_multipath_map(self, multipath_name):"},{"line_number":193,"context_line":"        \"\"\"Directly delete a map via multipathd.\"\"\""},{"line_number":194,"context_line":"        self._execute(\u0027multipathd\u0027, \u0027del\u0027, \u0027map\u0027, multipath_name,"},{"line_number":195,"context_line":"                      check_exit_code\u003dFalse, run_as_root\u003dTrue,"},{"line_number":196,"context_line":"                      root_helper\u003dself._root_helper)"},{"line_number":197,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_f9d2b0d0","line":194,"updated":"2017-04-11 20:27:58.000000000","message":"What does this do differently than \"multipath -f \u003cmap\u003e\"? I thought they were pretty much the same thing.","commit_id":"dddc7fbe334cca207b1e96b615484fca4fdcaa3d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"282d164cc882311f70f99c6ea1639a5f000b6e96","unresolved":false,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    def del_multipath_map(self, multipath_name):"},{"line_number":193,"context_line":"        \"\"\"Directly delete a map via multipathd.\"\"\""},{"line_number":194,"context_line":"        self._execute(\u0027multipathd\u0027, \u0027del\u0027, \u0027map\u0027, multipath_name,"},{"line_number":195,"context_line":"                      check_exit_code\u003dFalse, run_as_root\u003dTrue,"},{"line_number":196,"context_line":"                      root_helper\u003dself._root_helper)"},{"line_number":197,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_658fee29","line":194,"in_reply_to":"bff0334d_f9d2b0d0","updated":"2017-04-12 18:24:12.000000000","message":"Just had a look at the source code and you are right, I though this would skip the flushing part, I could get the dm cleaned up using udevadm and simulating the triggering of the removal, but that\u0027s a slippery slope, so I\u0027ll just flush the multipath again once the individual paths have been removed.","commit_id":"dddc7fbe334cca207b1e96b615484fca4fdcaa3d"}],"os_brick/tests/initiator/connectors/test_iscsi.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8efb7c5ab97d65ff4bb04d570135cb89450e24fa","unresolved":false,"context_lines":[{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    @testtools.skipUnless(os.path.exists(\u0027/dev/disk/by-path\u0027),"},{"line_number":393,"context_line":"                          \u0027Test requires /dev/disk/by-path\u0027)"},{"line_number":394,"context_line":"    @mock.patch(\u0027os.path.exists\u0027, side_effect\u003d(True,) * 4 + (False, False))"},{"line_number":395,"context_line":"    @mock.patch.object(iscsi.ISCSIConnector, \u0027_run_iscsiadm\u0027)"},{"line_number":396,"context_line":"    def test_connect_volume_with_alternative_targets_primary_error("},{"line_number":397,"context_line":"            self, mock_iscsiadm, mock_exists):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f0e0f27_c0c56f5d","line":394,"updated":"2017-06-09 12:57:08.000000000","message":"nit: Don\u0027t change this line since lambda on L415 already takes care of this.  This is a nit because we are removing this whole test in the next patch [1], I have removed it in my local copy in case I need to push a new patch.\n[1] https://review.openstack.org/#/c/455393/13/os_brick/tests/initiator/connectors/test_iscsi.py@370","commit_id":"400ca5d6db818b966065001571e59198c6966e2f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8efb7c5ab97d65ff4bb04d570135cb89450e24fa","unresolved":false,"context_lines":[{"line_number":578,"context_line":"                \u0027/dev/disk/by-path/ip-%s-iscsi-%s-lun-2\u0027 % (dev_loc2, iqn2)]"},{"line_number":579,"context_line":"        mock_devices.return_value \u003d devs"},{"line_number":580,"context_line":"        # mock_iscsi_devices.return_value \u003d devs"},{"line_number":581,"context_line":"        # mock_get_iqn.return_value \u003d [self._iqn, iqn2]"},{"line_number":582,"context_line":"        mock_discover_mpath_device.return_value \u003d ("},{"line_number":583,"context_line":"            fake_multipath_dev, test_connector.FAKE_SCSI_WWN)"},{"line_number":584,"context_line":"        mock_process_lun_id.return_value \u003d [self._lun, lun2]"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f0e0f27_40139f0e","line":581,"updated":"2017-06-09 12:57:08.000000000","message":"nit: Remove these 2 lines.  This is a nit because we are removing this whole test in the next patch [1], I have removed it in my local copy in case I need to push a new patch.\n\n[1] https://review.openstack.org/#/c/455393/13/os_brick/tests/initiator/connectors/test_iscsi.py@504","commit_id":"400ca5d6db818b966065001571e59198c6966e2f"}]}