)]}'
{"nova/virt/hyperv/basevolumeutils.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1ddd1a5f3e56ea8eb3c768fa3a30e2824bc34ab2","unresolved":false,"context_lines":[{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    def get_target_lun_count(self, target_iqn):"},{"line_number":147,"context_line":"        devices \u003d self._get_devices_for_target(target_iqn)"},{"line_number":148,"context_line":"        return len(devices)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    def get_target_from_disk_path(self, disk_path):"},{"line_number":151,"context_line":"        initiator_sessions \u003d self._conn_wmi.MSiSCSIInitiator_SessionClass()"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_78c6c79b","line":148,"updated":"2014-09-25 19:29:50.000000000","message":"Ooops :) devices can be None, which would raise an exception here. I think you want to either return [] from _get_devices_for_target(), or check for None here.","commit_id":"d9b0bfecf90a6370e5068687b66c733dcbd174c0"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"7514c8405ae1beeb48a6d6c714ca9efe0795d524","unresolved":false,"context_lines":[{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    def get_target_lun_count(self, target_iqn):"},{"line_number":147,"context_line":"        devices \u003d self._get_devices_for_target(target_iqn)"},{"line_number":148,"context_line":"        return len(devices)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    def get_target_from_disk_path(self, disk_path):"},{"line_number":151,"context_line":"        initiator_sessions \u003d self._conn_wmi.MSiSCSIInitiator_SessionClass()"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_a3e56039","line":148,"in_reply_to":"da9df570_78c6c79b","updated":"2014-09-25 20:15:25.000000000","message":"Whops, you are right about this. I was lured by the fact that initiator_sessions[0].Devices is always a list and forgot to take a look 2 lines above :)","commit_id":"d9b0bfecf90a6370e5068687b66c733dcbd174c0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                                                  \"Class WHERE TargetName\u003d\u0027%s\u0027\""},{"line_number":125,"context_line":"                                                  % target_iqn)"},{"line_number":126,"context_line":"        if not initiator_sessions:"},{"line_number":127,"context_line":"            return []"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        return initiator_sessions[0].Devices"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_f219f9d7","line":127,"updated":"2014-10-23 16:25:00.000000000","message":"I don\u0027t see anything in test_basevolumeutils covering this case.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"763c19b59030a36230b109d4deded91bd3698b29","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                                                  \"Class WHERE TargetName\u003d\u0027%s\u0027\""},{"line_number":125,"context_line":"                                                  % target_iqn)"},{"line_number":126,"context_line":"        if not initiator_sessions:"},{"line_number":127,"context_line":"            return []"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        return initiator_sessions[0].Devices"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_3157fecb","line":127,"in_reply_to":"9aa7fdbe_f219f9d7","updated":"2014-10-23 17:06:19.000000000","message":"Done","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            if device.ScsiLun \u003d\u003d target_lun:"},{"line_number":136,"context_line":"                return device.DeviceNumber"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def get_target_lun_count(self, target_iqn):"},{"line_number":139,"context_line":"        devices \u003d self._get_devices_for_target(target_iqn)"},{"line_number":140,"context_line":"        return len(devices)"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_b2d691ef","line":138,"updated":"2014-10-23 16:25:00.000000000","message":"I should see a unit test for this in test_basevolumeutils.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"763c19b59030a36230b109d4deded91bd3698b29","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            if device.ScsiLun \u003d\u003d target_lun:"},{"line_number":136,"context_line":"                return device.DeviceNumber"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def get_target_lun_count(self, target_iqn):"},{"line_number":139,"context_line":"        devices \u003d self._get_devices_for_target(target_iqn)"},{"line_number":140,"context_line":"        return len(devices)"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_71742668","line":138,"in_reply_to":"9aa7fdbe_b2d691ef","updated":"2014-10-23 17:06:19.000000000","message":"Done","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"}],"nova/virt/hyperv/livemigrationops.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        self._imagecache \u003d imagecache.ImageCache()"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    @check_os_version_requirement"},{"line_number":61,"context_line":"    def live_migration(self, context, instance_ref, dest, post_method,"},{"line_number":62,"context_line":"                       recover_method, block_migration\u003dFalse,"},{"line_number":63,"context_line":"                       migrate_data\u003dNone):"},{"line_number":64,"context_line":"        LOG.debug(\"live_migration called\", instance\u003dinstance_ref)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_d2c95527","line":61,"updated":"2014-10-23 16:25:00.000000000","message":"I guess this is all tested in test_hypervapi...would be nice if we didn\u0027t have to mock everything out at such a high level to unit test these changes.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"763c19b59030a36230b109d4deded91bd3698b29","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        self._imagecache \u003d imagecache.ImageCache()"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    @check_os_version_requirement"},{"line_number":61,"context_line":"    def live_migration(self, context, instance_ref, dest, post_method,"},{"line_number":62,"context_line":"                       recover_method, block_migration\u003dFalse,"},{"line_number":63,"context_line":"                       migrate_data\u003dNone):"},{"line_number":64,"context_line":"        LOG.debug(\"live_migration called\", instance\u003dinstance_ref)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_d15072d6","line":61,"in_reply_to":"9aa7fdbe_d2c95527","updated":"2014-10-23 17:06:19.000000000","message":"see answer in volumeops.py","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"519564919689eb6c9b590c1d0851a58bc7fdfd26","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            self._vmops.copy_vm_console_logs(instance_name, dest)"},{"line_number":69,"context_line":"            iscsi_targets \u003d self._livemigrutils.live_migrate_vm(instance_name,"},{"line_number":70,"context_line":"                                                                dest)"},{"line_number":71,"context_line":"            for target_iqn, target_luns_count in iscsi_targets.items():"},{"line_number":72,"context_line":"                self._volumeops.logout_storage_target(target_iqn,"},{"line_number":73,"context_line":"                                                      target_luns_count)"},{"line_number":74,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a8c0949_b0779439","line":71,"updated":"2014-11-06 05:07:42.000000000","message":"Noted that the return type change from live_migrate_vm is handled here (as an int rather than a list now).","commit_id":"7cb0d9f8754e4f6b20c709a72ff87126c9972f51"}],"nova/virt/hyperv/livemigrationutils.py":[{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"124afb9f529f38a8141ea7b10425f50249e2cb4e","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        for (rasd_rel_path, disk_path) in disk_paths.items():"},{"line_number":125,"context_line":"            (target_iqn,"},{"line_number":126,"context_line":"             target_lun) \u003d self._volutils.get_target_from_disk_path(disk_path)"},{"line_number":127,"context_line":"            if iscsi_targets.get(target_iqn):"},{"line_number":128,"context_line":"                iscsi_targets[target_iqn].append(target_lun)"},{"line_number":129,"context_line":"            else:"},{"line_number":130,"context_line":"                iscsi_targets[target_iqn] \u003d [target_lun]"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_a1be4fd2","line":127,"updated":"2014-09-25 07:52:08.000000000","message":"I think it would look better:\n\nif target_iqn in iscsi_targets:\n\nIt does the same thing, but it looks more readable.","commit_id":"6cde85b40c176ca0ef5d1a2994a0d837127ad78e"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"a9e3aaab9511aeb1bb66868e94a1215663431707","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        for (rasd_rel_path, disk_path) in disk_paths.items():"},{"line_number":125,"context_line":"            (target_iqn,"},{"line_number":126,"context_line":"             target_lun) \u003d self._volutils.get_target_from_disk_path(disk_path)"},{"line_number":127,"context_line":"            if iscsi_targets.get(target_iqn):"},{"line_number":128,"context_line":"                iscsi_targets[target_iqn].append(target_lun)"},{"line_number":129,"context_line":"            else:"},{"line_number":130,"context_line":"                iscsi_targets[target_iqn] \u003d [target_lun]"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_611c67c2","line":127,"in_reply_to":"da9df570_a1be4fd2","updated":"2014-09-25 08:05:55.000000000","message":"Thanks for the suggestion!","commit_id":"6cde85b40c176ca0ef5d1a2994a0d837127ad78e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1ddd1a5f3e56ea8eb3c768fa3a30e2824bc34ab2","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        volutils_remote \u003d volumeutilsv2.VolumeUtilsV2(dest_host)"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        disk_paths_remote \u003d {}"},{"line_number":123,"context_line":"        iscsi_targets \u003d {}"},{"line_number":124,"context_line":"        for (rasd_rel_path, disk_path) in disk_paths.items():"},{"line_number":125,"context_line":"            (target_iqn,"},{"line_number":126,"context_line":"             target_lun) \u003d self._volutils.get_target_from_disk_path(disk_path)"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_38c99fac","line":123,"updated":"2014-09-25 19:29:50.000000000","message":"Instead of lines 127-130, you can use collections.defaultdict(list) here. Like so:\n\n import collections\n ...\n\n iscsi_targets \u003d collections.defaultdict(list)\n ...\n    iscsi_targets[target_iqn].append(target_lun)","commit_id":"d9b0bfecf90a6370e5068687b66c733dcbd174c0"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"7514c8405ae1beeb48a6d6c714ca9efe0795d524","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        volutils_remote \u003d volumeutilsv2.VolumeUtilsV2(dest_host)"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        disk_paths_remote \u003d {}"},{"line_number":123,"context_line":"        iscsi_targets \u003d {}"},{"line_number":124,"context_line":"        for (rasd_rel_path, disk_path) in disk_paths.items():"},{"line_number":125,"context_line":"            (target_iqn,"},{"line_number":126,"context_line":"             target_lun) \u003d self._volutils.get_target_from_disk_path(disk_path)"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_a36ac07e","line":123,"in_reply_to":"da9df570_38c99fac","updated":"2014-09-25 20:15:25.000000000","message":"I haven\u0027t thought about that, great suggestion!","commit_id":"d9b0bfecf90a6370e5068687b66c733dcbd174c0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        return dict(ide_paths.items() + scsi_paths.items())"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    def _get_remote_disk_data(self, vmutils_remote, disk_paths, dest_host):"},{"line_number":121,"context_line":"        volutils_remote \u003d volumeutilsv2.VolumeUtilsV2(dest_host)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"        disk_paths_remote \u003d {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_b24931b9","line":120,"updated":"2014-10-23 16:25:00.000000000","message":"I guess this is all tested in test_hypervapi...would be nice if we didn\u0027t have to mock everything out at such a high level to unit test these changes.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"763c19b59030a36230b109d4deded91bd3698b29","unresolved":false,"context_lines":[{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        return dict(ide_paths.items() + scsi_paths.items())"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    def _get_remote_disk_data(self, vmutils_remote, disk_paths, dest_host):"},{"line_number":121,"context_line":"        volutils_remote \u003d volumeutilsv2.VolumeUtilsV2(dest_host)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"        disk_paths_remote \u003d {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_714d063e","line":120,"in_reply_to":"9aa7fdbe_b24931b9","updated":"2014-10-23 17:06:19.000000000","message":"See answer on volumeops.py","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"                iscsi_targets[target_iqn].append(target_lun)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"                dev_num \u003d volutils_remote.get_device_number_for_target("},{"line_number":133,"context_line":"                    target_iqn, target_lun)"},{"line_number":134,"context_line":"                disk_path_remote \u003d ("},{"line_number":135,"context_line":"                    vmutils_remote.get_mounted_disk_by_drive_number(dev_num))"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_b7012308","line":132,"updated":"2014-10-23 16:25:00.000000000","message":"This line didn\u0027t need to change.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"4d1c397978e060de51123a5c7443d01600edc2e1","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"                iscsi_targets[target_iqn].append(target_lun)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"                dev_num \u003d volutils_remote.get_device_number_for_target("},{"line_number":133,"context_line":"                    target_iqn, target_lun)"},{"line_number":134,"context_line":"                disk_path_remote \u003d ("},{"line_number":135,"context_line":"                    vmutils_remote.get_mounted_disk_by_drive_number(dev_num))"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_9b44371c","line":132,"in_reply_to":"9aa7fdbe_b7012308","updated":"2014-10-24 09:47:35.000000000","message":"Well, otherwise this line would  have exceeded 80 characters. Same for the one below.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"                dev_num \u003d volutils_remote.get_device_number_for_target("},{"line_number":133,"context_line":"                    target_iqn, target_lun)"},{"line_number":134,"context_line":"                disk_path_remote \u003d ("},{"line_number":135,"context_line":"                    vmutils_remote.get_mounted_disk_by_drive_number(dev_num))"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"                disk_paths_remote[rasd_rel_path] \u003d disk_path_remote"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_d2015506","line":134,"updated":"2014-10-23 16:25:00.000000000","message":"This line didn\u0027t need to change.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"7245b4f1a55d7cf2c3e48dd61929aeb45d6cbbac","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                disk_paths_remote[rasd_rel_path] \u003d disk_path_remote"},{"line_number":138,"context_line":"            else:"},{"line_number":139,"context_line":"                LOG.debug(\"Could not retrieve iSCSI target from disk path: \","},{"line_number":140,"context_line":"                          disk_path)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        return (disk_paths_remote, iscsi_targets)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_7404144a","line":140,"updated":"2014-10-23 18:04:36.000000000","message":"you missed a %s in the log message.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"4d1c397978e060de51123a5c7443d01600edc2e1","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                disk_paths_remote[rasd_rel_path] \u003d disk_path_remote"},{"line_number":138,"context_line":"            else:"},{"line_number":139,"context_line":"                LOG.debug(\"Could not retrieve iSCSI target from disk path: \","},{"line_number":140,"context_line":"                          disk_path)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        return (disk_paths_remote, iscsi_targets)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_bb4c9b43","line":140,"in_reply_to":"9aa7fdbe_7404144a","updated":"2014-10-24 09:47:35.000000000","message":"Done","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"519564919689eb6c9b590c1d0851a58bc7fdfd26","unresolved":false,"context_lines":[{"line_number":127,"context_line":"            if target:"},{"line_number":128,"context_line":"                (target_iqn, target_lun) \u003d target"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"                iscsi_targets[target_iqn] +\u003d 1"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"                dev_num \u003d volutils_remote.get_device_number_for_target("},{"line_number":133,"context_line":"                    target_iqn, target_lun)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a8c0949_707d2c1c","line":130,"updated":"2014-11-06 05:07:42.000000000","message":"Changes the return type and this is an internal interface but live_migrate_vm is using it and is public, so we have to be careful about what\u0027s calling live_migrate_vm now...","commit_id":"7cb0d9f8754e4f6b20c709a72ff87126c9972f51"}],"nova/virt/hyperv/vmops.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                (disk_files, volume_drives) \u003d storage"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"                self._vmutils.destroy_vm(instance_name)"},{"line_number":411,"context_line":"                self._volumeops.disconnect_volumes(volume_drives)"},{"line_number":412,"context_line":"            else:"},{"line_number":413,"context_line":"                LOG.debug(\"Instance not found\", instance\u003dinstance)"},{"line_number":414,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_d2f07569","line":411,"updated":"2014-10-23 16:25:00.000000000","message":"I don\u0027t see anything in test_vmops for this destroy case.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"540eb9bb382df65c67e8a7f33e1c3f07c1ed4ac4","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                (disk_files, volume_drives) \u003d storage"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"                self._vmutils.destroy_vm(instance_name)"},{"line_number":411,"context_line":"                self._volumeops.disconnect_volumes(volume_drives)"},{"line_number":412,"context_line":"            else:"},{"line_number":413,"context_line":"                LOG.debug(\"Instance not found\", instance\u003dinstance)"},{"line_number":414,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_c18a52be","line":411,"in_reply_to":"9aa7fdbe_d1b95216","updated":"2014-10-24 11:46:50.000000000","message":"FWIW, this is tested as well in test_hypervapi by test_destroy. The only thing is that ATM in the test case, there are no volumes to be disconnected.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"763c19b59030a36230b109d4deded91bd3698b29","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                (disk_files, volume_drives) \u003d storage"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"                self._vmutils.destroy_vm(instance_name)"},{"line_number":411,"context_line":"                self._volumeops.disconnect_volumes(volume_drives)"},{"line_number":412,"context_line":"            else:"},{"line_number":413,"context_line":"                LOG.debug(\"Instance not found\", instance\u003dinstance)"},{"line_number":414,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_d1b95216","line":411,"in_reply_to":"9aa7fdbe_d2f07569","updated":"2014-10-23 17:06:19.000000000","message":"Done","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"}],"nova/virt/hyperv/volumeops.py":[{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"124afb9f529f38a8141ea7b10425f50249e2cb4e","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        if total_available_luns \u003d\u003d 1:"},{"line_number":198,"context_line":"            self.logout_storage_target(target_iqn)"},{"line_number":199,"context_line":"        else:"},{"line_number":200,"context_line":"            LOG.debug(\"Skipping disconnecting target %s as there \""},{"line_number":201,"context_line":"                      \"are LUNs still being used.\" % target_iqn)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def get_volume_connector(self, instance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_216edf96","line":200,"updated":"2014-09-25 07:52:08.000000000","message":"Hmm, wouldn\u0027t this be better at info level?","commit_id":"6cde85b40c176ca0ef5d1a2994a0d837127ad78e"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"a9e3aaab9511aeb1bb66868e94a1215663431707","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        if total_available_luns \u003d\u003d 1:"},{"line_number":198,"context_line":"            self.logout_storage_target(target_iqn)"},{"line_number":199,"context_line":"        else:"},{"line_number":200,"context_line":"            LOG.debug(\"Skipping disconnecting target %s as there \""},{"line_number":201,"context_line":"                      \"are LUNs still being used.\" % target_iqn)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def get_volume_connector(self, instance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_01271b13","line":200,"in_reply_to":"da9df570_216edf96","updated":"2014-09-25 08:05:55.000000000","message":"Well, since almost all the logs in this module are at debug level, it would not make sense to put those at info level, considering the fact that there are more relevant logs which are at debug level.","commit_id":"6cde85b40c176ca0ef5d1a2994a0d837127ad78e"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"124afb9f529f38a8141ea7b10425f50249e2cb4e","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            if targets[target_iqn] \u003d\u003d total_available_luns:"},{"line_number":264,"context_line":"                self.logout_storage_target(target_iqn)"},{"line_number":265,"context_line":"            else:"},{"line_number":266,"context_line":"                LOG.debug(\"Skipping disconnecting target %s as there \""},{"line_number":267,"context_line":"                          \"are LUNs still being used.\" % target_iqn)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def get_target_from_disk_path(self, physical_drive_path):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_01c55b8d","line":266,"updated":"2014-09-25 07:52:08.000000000","message":"Hmm, wouldn\u0027t this be better at info level?","commit_id":"6cde85b40c176ca0ef5d1a2994a0d837127ad78e"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"a9e3aaab9511aeb1bb66868e94a1215663431707","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            if targets[target_iqn] \u003d\u003d total_available_luns:"},{"line_number":264,"context_line":"                self.logout_storage_target(target_iqn)"},{"line_number":265,"context_line":"            else:"},{"line_number":266,"context_line":"                LOG.debug(\"Skipping disconnecting target %s as there \""},{"line_number":267,"context_line":"                          \"are LUNs still being used.\" % target_iqn)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def get_target_from_disk_path(self, physical_drive_path):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_21225f01","line":266,"in_reply_to":"da9df570_01c55b8d","updated":"2014-09-25 08:05:55.000000000","message":"Same here.","commit_id":"6cde85b40c176ca0ef5d1a2994a0d837127ad78e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1ddd1a5f3e56ea8eb3c768fa3a30e2824bc34ab2","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            self.logout_storage_target(target_iqn)"},{"line_number":199,"context_line":"        else:"},{"line_number":200,"context_line":"            LOG.debug(\"Skipping disconnecting target %s as there \""},{"line_number":201,"context_line":"                      \"are LUNs still being used.\" % target_iqn)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def get_volume_connector(self, instance):"},{"line_number":204,"context_line":"        if not self._initiator:"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_58ce63b5","line":201,"updated":"2014-09-25 19:29:50.000000000","message":"Here and below, use a comma instead of % to avoid interpolation of arguments when DEBUG log level not used...","commit_id":"d9b0bfecf90a6370e5068687b66c733dcbd174c0"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"7514c8405ae1beeb48a6d6c714ca9efe0795d524","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            self.logout_storage_target(target_iqn)"},{"line_number":199,"context_line":"        else:"},{"line_number":200,"context_line":"            LOG.debug(\"Skipping disconnecting target %s as there \""},{"line_number":201,"context_line":"                      \"are LUNs still being used.\" % target_iqn)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def get_volume_connector(self, instance):"},{"line_number":204,"context_line":"        if not self._initiator:"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_c31504e6","line":201,"in_reply_to":"da9df570_58ce63b5","updated":"2014-09-25 20:15:25.000000000","message":"bad habit :) thanks for pointing it out!","commit_id":"d9b0bfecf90a6370e5068687b66c733dcbd174c0"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"ba140574fe2e8ec3dbb935088d09b9759e160ea7","unresolved":false,"context_lines":[{"line_number":252,"context_line":"        targets \u003d {}"},{"line_number":253,"context_line":"        for volume_drive in volume_drives:"},{"line_number":254,"context_line":"            target_iqn \u003d self._volutils.get_target_from_disk_path("},{"line_number":255,"context_line":"                volume_drive)[0]"},{"line_number":256,"context_line":"            targets[target_iqn] \u003d targets.get(target_iqn, 0) + 1"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        for target_iqn in targets:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_1b0c2e94","line":255,"updated":"2014-10-11 17:09:53.000000000","message":"Fails if get_target_from_disk_path() returns None. \n\nThe following test fails on Hyper-V 2008 R2 and on 2012 using V1 utils:\n\ntempest.scenario.test_volume_boot_pattern.TestVolumeBootPattern.test_volume_boot_pattern","commit_id":"cf6f033fdd3f3184ced25819368087dee123ad57"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"8532e07950ec2789e54bf52c10037c2689f74eb0","unresolved":false,"context_lines":[{"line_number":252,"context_line":"        targets \u003d {}"},{"line_number":253,"context_line":"        for volume_drive in volume_drives:"},{"line_number":254,"context_line":"            target_iqn \u003d self._volutils.get_target_from_disk_path("},{"line_number":255,"context_line":"                volume_drive)[0]"},{"line_number":256,"context_line":"            targets[target_iqn] \u003d targets.get(target_iqn, 0) + 1"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        for target_iqn in targets:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_5dee9f8c","line":255,"in_reply_to":"baa201ad_1b0c2e94","updated":"2014-10-13 08:52:25.000000000","message":"Thanks for pointing this out! I\u0027m adding a check here.","commit_id":"cf6f033fdd3f3184ced25819368087dee123ad57"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":172,"context_line":"    def logout_storage_target(self, target_iqn):"},{"line_number":173,"context_line":"        LOG.debug(\"Logging off storage target %s\", target_iqn)"},{"line_number":174,"context_line":"        self._volutils.logout_storage_target(target_iqn)"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    def detach_volume(self, connection_info, instance_name):"},{"line_number":177,"context_line":"        \"\"\"Detach a volume to the SCSI controller.\"\"\""},{"line_number":178,"context_line":"        LOG.debug(\"Detach_volume: %(connection_info)s \""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_12e75dad","line":175,"updated":"2014-10-23 16:25:00.000000000","message":"I guess this is all tested in test_hypervapi...would be nice if we didn\u0027t have to mock everything out at such a high level to unit test these changes.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"763c19b59030a36230b109d4deded91bd3698b29","unresolved":false,"context_lines":[{"line_number":172,"context_line":"    def logout_storage_target(self, target_iqn):"},{"line_number":173,"context_line":"        LOG.debug(\"Logging off storage target %s\", target_iqn)"},{"line_number":174,"context_line":"        self._volutils.logout_storage_target(target_iqn)"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    def detach_volume(self, connection_info, instance_name):"},{"line_number":177,"context_line":"        \"\"\"Detach a volume to the SCSI controller.\"\"\""},{"line_number":178,"context_line":"        LOG.debug(\"Detach_volume: %(connection_info)s \""}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_31c9beac","line":175,"in_reply_to":"9aa7fdbe_12e75dad","updated":"2014-10-23 17:06:19.000000000","message":"note: test_hypervapi is going away (there\u0027s a Kilo BP for that)\n\nTests for ops classes will go in dedicated test cases e.g. test_volumeops.py, mocking of course the utils class (as by definition of unit test).","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9954bb6fd6d0835cad2b66d6b4ad1550eddbaadf","unresolved":false,"context_lines":[{"line_number":172,"context_line":"    def logout_storage_target(self, target_iqn):"},{"line_number":173,"context_line":"        LOG.debug(\"Logging off storage target %s\", target_iqn)"},{"line_number":174,"context_line":"        self._volutils.logout_storage_target(target_iqn)"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    def detach_volume(self, connection_info, instance_name):"},{"line_number":177,"context_line":"        \"\"\"Detach a volume to the SCSI controller.\"\"\""},{"line_number":178,"context_line":"        LOG.debug(\"Detach_volume: %(connection_info)s \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8c0949_9078d82c","line":175,"in_reply_to":"9aa7fdbe_31c9beac","updated":"2014-11-06 05:05:10.000000000","message":"Things don\u0027t go away until there is actually a patch up to remove them, so it\u0027s not a good reason to not add test coverage until that happens.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":249,"context_line":"                                       \u0027for target_iqn: %s\u0027) % target_iqn)"},{"line_number":250,"context_line":"        return mounted_disk_path"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    def disconnect_volumes(self, volume_drives):"},{"line_number":253,"context_line":"        targets \u003d collections.defaultdict(int)"},{"line_number":254,"context_line":"        for volume_drive in volume_drives:"},{"line_number":255,"context_line":"            target \u003d self._volutils.get_target_from_disk_path("}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_b2f77158","line":252,"updated":"2014-10-23 16:25:00.000000000","message":"Where is this unit tested since there is no test_volumeops and I don\u0027t see disconnect_volumes in test_hypervapi?","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        for target_iqn in targets:"},{"line_number":265,"context_line":"            # Disconnect the target only if we disconnect all the"},{"line_number":266,"context_line":"            # available LUNs."},{"line_number":267,"context_line":"            total_available_luns \u003d self._volutils.get_target_lun_count("},{"line_number":268,"context_line":"                target_iqn)"},{"line_number":269,"context_line":"            if targets[target_iqn] \u003d\u003d total_available_luns:"},{"line_number":270,"context_line":"                self.logout_storage_target(target_iqn)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_32b8e1e4","line":267,"updated":"2014-10-23 16:25:00.000000000","message":"nit: this code and the same in detach_volume could be combined into a private utility method.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"4d1c397978e060de51123a5c7443d01600edc2e1","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        for target_iqn in targets:"},{"line_number":265,"context_line":"            # Disconnect the target only if we disconnect all the"},{"line_number":266,"context_line":"            # available LUNs."},{"line_number":267,"context_line":"            total_available_luns \u003d self._volutils.get_target_lun_count("},{"line_number":268,"context_line":"                target_iqn)"},{"line_number":269,"context_line":"            if targets[target_iqn] \u003d\u003d total_available_luns:"},{"line_number":270,"context_line":"                self.logout_storage_target(target_iqn)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_643ff9d6","line":267,"in_reply_to":"9aa7fdbe_32b8e1e4","updated":"2014-10-24 09:47:35.000000000","message":"Done","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71005a6f8194bc6e92635a108f0335dc85ef1fd6","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                instance_name, ctrller_path, disk_address, mounted_disk_path)"},{"line_number":295,"context_line":"            disk_address +\u003d 1"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def get_target_lun_count(self, target_iqn):"},{"line_number":298,"context_line":"        return self._volutils.get_target_lun_count(target_iqn)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_12aafd84","line":297,"updated":"2014-10-23 16:25:00.000000000","message":"What is using this?  Looks like everything else is using _volutils.get_target_lun_count.\n\n...\n\nOh I guess livemigrationops is using this as a delegation method.\n\nThen I\u0027d say detach_volume and disconnect_volumes in this class should be using this method rather than calling _volutils directly.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"763c19b59030a36230b109d4deded91bd3698b29","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                instance_name, ctrller_path, disk_address, mounted_disk_path)"},{"line_number":295,"context_line":"            disk_address +\u003d 1"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def get_target_lun_count(self, target_iqn):"},{"line_number":298,"context_line":"        return self._volutils.get_target_lun_count(target_iqn)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aa7fdbe_91950a89","line":297,"in_reply_to":"9aa7fdbe_12aafd84","updated":"2014-10-23 17:06:19.000000000","message":"the idea is that utils classes are exposed only to the ops layer. It this case it\u0027s a mere delegation.\n\nI don\u0027t see a big point in having other methods in this class calling it instead of the underlying utils method.","commit_id":"788d2469a7ab16256d30e066849cd70d327f1b81"}]}