)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bdc8673300e8629f1480185ac2d9b2a597d771a9","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: update the logic to configure volume with scsi controller"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This commit is fixing the issue by adding the address element to the"},{"line_number":10,"context_line":"volumes configuration."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1686116"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1f013ff3_7d4cdcc6","line":9,"range":{"start_line":9,"start_character":15,"end_line":9,"end_character":31},"updated":"2017-05-16 09:06:27.000000000","message":"Super nit, as you\u0027re going to respin this to resolve the current merge conflict can you add what the issue is here?","commit_id":"64d20ba94794a0a469381a35d07f7d8a0b0bad0a"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bdc8673300e8629f1480185ac2d9b2a597d771a9","unresolved":false,"context_lines":[{"line_number":1066,"context_line":"        for source in tcp_devices:"},{"line_number":1067,"context_line":"            yield (source.get(\"host\"), int(source.get(\"service\")))"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    def _get_scsi_controller_max_unit(self, guest):"},{"line_number":1070,"context_line":"        xml \u003d guest.get_xml_desc()"},{"line_number":1071,"context_line":"        tree \u003d etree.fromstring(xml)"},{"line_number":1072,"context_line":"        addrs \u003d \"./devices/disk[@device\u003d\u0027disk\u0027]/address[@type\u003d\u0027drive\u0027]\""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f013ff3_1d6dd803","line":1069,"range":{"start_line":1069,"start_character":8,"end_line":1069,"end_character":51},"updated":"2017-05-16 09:06:27.000000000","message":"super nit, a short docs would be really useful here.","commit_id":"64d20ba94794a0a469381a35d07f7d8a0b0bad0a"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"faa7fd236f1b1f0fe466354716f497b5c7a638e3","unresolved":false,"context_lines":[{"line_number":1066,"context_line":"        for source in tcp_devices:"},{"line_number":1067,"context_line":"            yield (source.get(\"host\"), int(source.get(\"service\")))"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    def _get_scsi_controller_max_unit(self, guest):"},{"line_number":1070,"context_line":"        xml \u003d guest.get_xml_desc()"},{"line_number":1071,"context_line":"        tree \u003d etree.fromstring(xml)"},{"line_number":1072,"context_line":"        addrs \u003d \"./devices/disk[@device\u003d\u0027disk\u0027]/address[@type\u003d\u0027drive\u0027]\""}],"source_content_type":"text/x-python","patch_set":1,"id":"ff0f0b1f_f999371f","line":1069,"in_reply_to":"1f013ff3_1d6dd803","updated":"2017-05-18 07:45:10.000000000","message":"Sure I will add one","commit_id":"64d20ba94794a0a469381a35d07f7d8a0b0bad0a"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bdc8673300e8629f1480185ac2d9b2a597d771a9","unresolved":false,"context_lines":[{"line_number":1066,"context_line":"        for source in tcp_devices:"},{"line_number":1067,"context_line":"            yield (source.get(\"host\"), int(source.get(\"service\")))"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    def _get_scsi_controller_max_unit(self, guest):"},{"line_number":1070,"context_line":"        xml \u003d guest.get_xml_desc()"},{"line_number":1071,"context_line":"        tree \u003d etree.fromstring(xml)"},{"line_number":1072,"context_line":"        addrs \u003d \"./devices/disk[@device\u003d\u0027disk\u0027]/address[@type\u003d\u0027drive\u0027]\""},{"line_number":1073,"context_line":""},{"line_number":1074,"context_line":"        ret \u003d []"},{"line_number":1075,"context_line":"        for obj in tree.findall(addrs):"},{"line_number":1076,"context_line":"            ret.append(int(obj.get(\u0027unit\u0027, 0)))"},{"line_number":1077,"context_line":"        return max(ret)"},{"line_number":1078,"context_line":""},{"line_number":1079,"context_line":"    @staticmethod"},{"line_number":1080,"context_line":"    def _get_rbd_driver():"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f013ff3_5d95a0e3","line":1077,"range":{"start_line":1069,"start_character":0,"end_line":1077,"end_character":23},"updated":"2017-05-16 09:06:27.000000000","message":"Can we add a test for this?","commit_id":"64d20ba94794a0a469381a35d07f7d8a0b0bad0a"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bdc8673300e8629f1480185ac2d9b2a597d771a9","unresolved":false,"context_lines":[{"line_number":1239,"context_line":"        disk_info \u003d blockinfo.get_info_from_bdm("},{"line_number":1240,"context_line":"            instance, CONF.libvirt.virt_type, instance.image_meta, bdm)"},{"line_number":1241,"context_line":"        self._connect_volume(connection_info, disk_info)"},{"line_number":1242,"context_line":"        if disk_info[\u0027bus\u0027] \u003d\u003d \u0027scsi\u0027:"},{"line_number":1243,"context_line":"            disk_info[\u0027unit\u0027] \u003d self._get_scsi_controller_max_unit(guest) + 1"},{"line_number":1244,"context_line":"        LOG.debug(\"Volume disk_info: %s\", disk_info, instance\u003dinstance)"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"        conf \u003d self._get_volume_config(connection_info, disk_info)"},{"line_number":1247,"context_line":"        self._set_cache_mode(conf)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f013ff3_bd85c4af","line":1244,"range":{"start_line":1242,"start_character":0,"end_line":1244,"end_character":71},"updated":"2017-05-16 09:06:27.000000000","message":"As above, this isn\u0027t tested at the moment.","commit_id":"64d20ba94794a0a469381a35d07f7d8a0b0bad0a"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"faa7fd236f1b1f0fe466354716f497b5c7a638e3","unresolved":false,"context_lines":[{"line_number":1241,"context_line":"        self._connect_volume(connection_info, disk_info)"},{"line_number":1242,"context_line":"        if disk_info[\u0027bus\u0027] \u003d\u003d \u0027scsi\u0027:"},{"line_number":1243,"context_line":"            disk_info[\u0027unit\u0027] \u003d self._get_scsi_controller_max_unit(guest) + 1"},{"line_number":1244,"context_line":"        LOG.debug(\"Volume disk_info: %s\", disk_info, instance\u003dinstance)"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"        conf \u003d self._get_volume_config(connection_info, disk_info)"},{"line_number":1247,"context_line":"        self._set_cache_mode(conf)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff0f0b1f_19950be5","line":1244,"in_reply_to":"1f013ff3_bd85c4af","updated":"2017-05-18 07:45:10.000000000","message":"Well all of that part is already exercised [0]. As you can see the property \"bus\u003dscsi\" has been added to make the process to pass in this path, then the method is reading the XML to return the last disk unit, adding one, and so that is asserted.\n\nSince it\u0027s a private method I don\u0027t think we need to have a specific test which is going to do exactly what the test is doing.\n\ndoes that make sense?\n\n[0] https://review.openstack.org/#/c/459741/1/nova/tests/unit/virt/libvirt/test_driver.py","commit_id":"64d20ba94794a0a469381a35d07f7d8a0b0bad0a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"12ca53127cf27f606dcf9ea1997f0b2b8cc8c0ab","unresolved":false,"context_lines":[{"line_number":1058,"context_line":"        \"\"\"Returns the max disk unit used by scsi controller\"\"\""},{"line_number":1059,"context_line":"        xml \u003d guest.get_xml_desc()"},{"line_number":1060,"context_line":"        tree \u003d etree.fromstring(xml)"},{"line_number":1061,"context_line":"        addrs \u003d \"./devices/disk[@device\u003d\u0027disk\u0027]/address[@type\u003d\u0027drive\u0027]\""},{"line_number":1062,"context_line":""},{"line_number":1063,"context_line":"        ret \u003d []"},{"line_number":1064,"context_line":"        for obj in tree.findall(addrs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_5d7368e8","line":1061,"updated":"2017-05-18 11:36:47.000000000","message":"As I\u0027ve mention elsewhere, this assumes we only have 1 disk controller. Can we make that assertion explicit, eg with a comment, or handle it if it might not be true?","commit_id":"d1b111539ec6ff6030e423c237eafe8a06950583"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dfcd8db5df212d5fc93e3678d5030f89e3bb7963","unresolved":false,"context_lines":[{"line_number":1233,"context_line":"        self._connect_volume(connection_info, disk_info, instance)"},{"line_number":1234,"context_line":"        if disk_info[\u0027bus\u0027] \u003d\u003d \u0027scsi\u0027:"},{"line_number":1235,"context_line":"            disk_info[\u0027unit\u0027] \u003d self._get_scsi_controller_max_unit(guest) + 1"},{"line_number":1236,"context_line":"        LOG.debug(\"Volume disk_info: %s\", disk_info, instance\u003dinstance)"},{"line_number":1237,"context_line":""},{"line_number":1238,"context_line":"        conf \u003d self._get_volume_config(connection_info, disk_info)"},{"line_number":1239,"context_line":"        self._set_cache_mode(conf)"}],"source_content_type":"text/x-python","patch_set":6,"id":"df140735_43edc023","line":1236,"updated":"2017-06-07 18:57:56.000000000","message":"Not sure this log message is all that useful. Kind of seems like leftover test debugging? If not, perhaps add something like \"In attach_volume(), after connect_volume(), disk_info is ...\".","commit_id":"0ac83e44ed0de6b97f45005209e402d1305d208d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dfcd8db5df212d5fc93e3678d5030f89e3bb7963","unresolved":false,"context_lines":[{"line_number":3736,"context_line":"            self._connect_volume(connection_info, info, instance)"},{"line_number":3737,"context_line":"            if scsi_controller and scsi_controller.model \u003d\u003d \u0027virtio-scsi\u0027:"},{"line_number":3738,"context_line":"                info[\u0027unit\u0027] \u003d disk_mapping[\u0027unit\u0027]"},{"line_number":3739,"context_line":"                disk_mapping[\u0027unit\u0027] +\u003d 1"},{"line_number":3740,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":3741,"context_line":"            devices.append(cfg)"},{"line_number":3742,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"}],"source_content_type":"text/x-python","patch_set":6,"id":"df140735_43d6e051","line":3739,"updated":"2017-06-07 18:57:56.000000000","message":"heh. see my comment on the last patch. you didn\u0027t actually have to pass unit in disk_mapping... you could have just passed disk_unit to _get_volume_config() below and tracked the \"current\" disk unit here in the for loop :)","commit_id":"0ac83e44ed0de6b97f45005209e402d1305d208d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1501b51cce9a035172862b15aa9dc1ec8b36eb29","unresolved":false,"context_lines":[{"line_number":1054,"context_line":"        addrs \u003d \"./devices/disk[@device\u003d\u0027disk\u0027]/address[@type\u003d\u0027drive\u0027]\""},{"line_number":1055,"context_line":""},{"line_number":1056,"context_line":"        ret \u003d []"},{"line_number":1057,"context_line":"        for obj in tree.findall(addrs):"},{"line_number":1058,"context_line":"            ret.append(int(obj.get(\u0027unit\u0027, 0)))"},{"line_number":1059,"context_line":"        return max(ret)"},{"line_number":1060,"context_line":""},{"line_number":1061,"context_line":"    @staticmethod"},{"line_number":1062,"context_line":"    def _get_rbd_driver():"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f1d235d_6f32b112","line":1059,"range":{"start_line":1057,"start_character":8,"end_line":1059,"end_character":23},"updated":"2017-07-03 13:31:12.000000000","message":"Same comments as previous patch: are we able to hotplug SCSI devices, and if so, should be find the first free index/unit rather than always using the max?","commit_id":"c25629f85feb53b5be0347f68c43b3b55fb9f137"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"534172e2330238372485f020d0666237116c480e","unresolved":false,"context_lines":[{"line_number":1056,"context_line":"        ret \u003d []"},{"line_number":1057,"context_line":"        for obj in tree.findall(addrs):"},{"line_number":1058,"context_line":"            ret.append(int(obj.get(\u0027unit\u0027, 0)))"},{"line_number":1059,"context_line":"        return max(ret)"},{"line_number":1060,"context_line":""},{"line_number":1061,"context_line":"    @staticmethod"},{"line_number":1062,"context_line":"    def _get_rbd_driver():"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f1d235d_fa60bd03","line":1059,"in_reply_to":"3f1d235d_6f32b112","updated":"2017-07-03 13:59:28.000000000","message":"Yes we are able to hotplug, that part of code is basically for that purpose. libvirt is not going to validate the limit of number of devices attached by looking at unit. The unit could be any 20digit number.","commit_id":"c25629f85feb53b5be0347f68c43b3b55fb9f137"}],"nova/virt/libvirt/volume/volume.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"12ca53127cf27f606dcf9ea1997f0b2b8cc8c0ab","unresolved":false,"context_lines":[{"line_number":99,"context_line":"            # In order to allow up to 256 disks handled by one iscsi"},{"line_number":100,"context_line":"            # controller, the device addr should be specified."},{"line_number":101,"context_line":"            conf.device_addr \u003d vconfig.LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":102,"context_line":"            conf.device_addr.controller \u003d 0"},{"line_number":103,"context_line":"            conf.device_addr.bus \u003d 0"},{"line_number":104,"context_line":"            conf.device_addr.target \u003d 0"},{"line_number":105,"context_line":"            if \u0027unit\u0027 in disk_info:"},{"line_number":106,"context_line":"                conf.device_addr.unit \u003d disk_info[\u0027unit\u0027]"},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_9dbc606a","line":104,"range":{"start_line":102,"start_character":0,"end_line":104,"end_character":39},"updated":"2017-05-18 11:36:47.000000000","message":"As in the previous patch, I\u0027d like to see either a comment explaining why this is correct, or passing in a controller device with the values to copy.","commit_id":"d1b111539ec6ff6030e423c237eafe8a06950583"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0f0cfa3b1a2f28800bf37cc1cf85d887c706dfae","unresolved":false,"context_lines":[{"line_number":94,"context_line":"            conf.driver_discard \u003d \u0027unmap\u0027"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        if disk_info[\u0027bus\u0027] \u003d\u003d \u0027scsi\u0027:"},{"line_number":97,"context_line":"            # The driver is responsible to create the SCSI controller"},{"line_number":98,"context_line":"            # at index 0."},{"line_number":99,"context_line":"            conf.device_addr \u003d vconfig.LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":100,"context_line":"            conf.device_addr.controller \u003d 0"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f1d235d_293cb9e0","line":97,"range":{"start_line":97,"start_character":40,"end_line":97,"end_character":49},"updated":"2017-07-03 12:34:47.000000000","message":"s/to create/for creating/","commit_id":"c25629f85feb53b5be0347f68c43b3b55fb9f137"}]}
