)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"156a4f9800a46c6039606b7717d5eb99ff286f89","unresolved":false,"context_lines":[{"line_number":2567,"context_line":"            context, instance_uuid)"},{"line_number":2568,"context_line":"        if not bdms:"},{"line_number":2569,"context_line":"            return False"},{"line_number":2570,"context_line":"        for bdm in bdms:"},{"line_number":2571,"context_line":"            if bdm.get(\u0027connection_info\u0027) and \\"},{"line_number":2572,"context_line":"                    (bdm[\u0027connection_info\u0027] !\u003d \u0027null\u0027):"},{"line_number":2573,"context_line":"                if self.driver.check_refresh_connection_info("}],"source_content_type":"text/x-python","patch_set":25,"id":"1af94dfe_0aacde8a","line":2570,"updated":"2016-03-17 14:41:45.000000000","message":"I suspect that \u0027connection_info\u0027 may be a proxy for this, but I think you want to explicitly select only volume bdms here. Even if connection_info is a proxy for this state, please put a comment in to point this out for the unwary and/or explicitly select only volumes anyway.","commit_id":"45ebb25bafb3a2b566ffe9f2c840805bcce37c9d"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"156a4f9800a46c6039606b7717d5eb99ff286f89","unresolved":false,"context_lines":[{"line_number":2580,"context_line":"        refresh_conn_info \u003d False"},{"line_number":2581,"context_line":"        if self.check_refresh_connection_info(context, instance.uuid):"},{"line_number":2582,"context_line":"            refresh_conn_info \u003d True"},{"line_number":2583,"context_line":"        LOG.info(_LI(\"Refresh_instance_block_device_info:%s\"),"},{"line_number":2584,"context_line":"                 refresh_conn_info)"},{"line_number":2585,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":2586,"context_line":"            context, instance, refresh_conn_info)"}],"source_content_type":"text/x-python","patch_set":25,"id":"1af94dfe_2da8d48c","line":2583,"updated":"2016-03-17 14:41:45.000000000","message":"This is only INFO if it\u0027s true, otherwise it\u0027s just noise that\u0027s best left at debug or even better, omitted. Please only output a log message if we\u0027re actually going to refresh. Also, the underscores here aren\u0027t appropriate for a user log message. Suggest maybe:\n\n\"Refreshing volume connection_info for instance %(uuid)s\"","commit_id":"45ebb25bafb3a2b566ffe9f2c840805bcce37c9d"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"156a4f9800a46c6039606b7717d5eb99ff286f89","unresolved":false,"context_lines":[{"line_number":2582,"context_line":"            refresh_conn_info \u003d True"},{"line_number":2583,"context_line":"        LOG.info(_LI(\"Refresh_instance_block_device_info:%s\"),"},{"line_number":2584,"context_line":"                 refresh_conn_info)"},{"line_number":2585,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":2586,"context_line":"            context, instance, refresh_conn_info)"},{"line_number":2587,"context_line":"        self.driver.power_on(context, instance,"},{"line_number":2588,"context_line":"                             network_info,"}],"source_content_type":"text/x-python","patch_set":25,"id":"1af94dfe_6d713ca8","line":2585,"updated":"2016-03-17 14:41:45.000000000","message":"See below: fetching bdms twice.","commit_id":"45ebb25bafb3a2b566ffe9f2c840805bcce37c9d"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"156a4f9800a46c6039606b7717d5eb99ff286f89","unresolved":false,"context_lines":[{"line_number":3044,"context_line":"        context \u003d context.elevated()"},{"line_number":3045,"context_line":"        LOG.info(_LI(\"Rebooting instance\"), context\u003dcontext, instance\u003dinstance)"},{"line_number":3046,"context_line":""},{"line_number":3047,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":3048,"context_line":"            context, instance, refresh_conn_info)"},{"line_number":3049,"context_line":""},{"line_number":3050,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"}],"source_content_type":"text/x-python","patch_set":25,"id":"1af94dfe_6a7002dd","line":3047,"updated":"2016-03-17 14:41:45.000000000","message":"We\u0027re now fetching bdms twice in quick succession. Could you please refactor this to only fetch bdms once. Perhaps you could pass bdms into check_refresh_connection_info().","commit_id":"45ebb25bafb3a2b566ffe9f2c840805bcce37c9d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e4a3593790556c53721f8d22aa0cb02dc73a7bb","unresolved":false,"context_lines":[{"line_number":1832,"context_line":"                                        bdms\u003dNone):"},{"line_number":1833,"context_line":"        \"\"\"Transform block devices to the driver block_device format.\"\"\""},{"line_number":1834,"context_line":""},{"line_number":1835,"context_line":"        if not bdms:"},{"line_number":1836,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":1837,"context_line":"                    context, instance.uuid)"},{"line_number":1838,"context_line":"        block_device_info \u003d driver.get_block_device_info(instance, bdms)"},{"line_number":1839,"context_line":""},{"line_number":1840,"context_line":"        if not refresh_conn_info:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa0719c6_678bc71f","line":1837,"range":{"start_line":1835,"start_character":8,"end_line":1837,"end_character":43},"updated":"2016-03-24 12:38:53.000000000","message":"_get_instance_block_device_info is called from reboot_instance, and this is then a 2nd time that we\u0027re going back to the database to get BDMs. It would be better to just use the BDM list here to check if you need to refresh (especially in the case where refresh_conn_info is False, because if it\u0027s True, you\u0027re already going to be refreshing).","commit_id":"249ccee7e43dafb9bd7c02db6f6634b52af3278b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e4a3593790556c53721f8d22aa0cb02dc73a7bb","unresolved":false,"context_lines":[{"line_number":2586,"context_line":"            refresh_conn_info \u003d True"},{"line_number":2587,"context_line":"        LOG.info(_LI(\"Refresh_instance_block_device_info:%s\"),"},{"line_number":2588,"context_line":"                 refresh_conn_info)"},{"line_number":2589,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":2590,"context_line":"            context, instance, refresh_conn_info)"},{"line_number":2591,"context_line":"        self.driver.power_on(context, instance,"},{"line_number":2592,"context_line":"                             network_info,"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa0719c6_e7d4f7ec","line":2589,"range":{"start_line":2589,"start_character":33,"end_line":2589,"end_character":64},"updated":"2016-03-24 12:38:53.000000000","message":"Again, we already get the BDMs in this method if not provided, so just re-use that list in the case that refresh_connection_info is False in there.","commit_id":"249ccee7e43dafb9bd7c02db6f6634b52af3278b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e4a3593790556c53721f8d22aa0cb02dc73a7bb","unresolved":false,"context_lines":[{"line_number":3048,"context_line":"        context \u003d context.elevated()"},{"line_number":3049,"context_line":"        LOG.info(_LI(\"Rebooting instance\"), context\u003dcontext, instance\u003dinstance)"},{"line_number":3050,"context_line":""},{"line_number":3051,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":3052,"context_line":"            context, instance, refresh_conn_info)"},{"line_number":3053,"context_line":""},{"line_number":3054,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa0719c6_c7a1f395","line":3051,"range":{"start_line":3051,"start_character":33,"end_line":3051,"end_character":64},"updated":"2016-03-24 12:38:53.000000000","message":"Just do your check in this method where we\u0027re already getting the BDMs.","commit_id":"249ccee7e43dafb9bd7c02db6f6634b52af3278b"}],"nova/virt/libvirt/volume/net.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"156a4f9800a46c6039606b7717d5eb99ff286f89","unresolved":false,"context_lines":[{"line_number":111,"context_line":"              self).disconnect_volume(connection_info, disk_dev)"},{"line_number":112,"context_line":"        self._delete_secret_by_name(connection_info)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def check_refresh_connection_info(self, connection_info):"},{"line_number":115,"context_line":"        \"\"\"check if it is needed to refresh connection info.\"\"\""},{"line_number":116,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver("},{"line_number":117,"context_line":"            pool\u003dCONF.libvirt.images_rbd_pool,"}],"source_content_type":"text/x-python","patch_set":25,"id":"1af94dfe_8d6be8f0","line":114,"updated":"2016-03-17 14:41:45.000000000","message":"This will execute for both iscsi and rbd volumes, which isn\u0027t what\u0027s intended.","commit_id":"45ebb25bafb3a2b566ffe9f2c840805bcce37c9d"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"156a4f9800a46c6039606b7717d5eb99ff286f89","unresolved":false,"context_lines":[{"line_number":114,"context_line":"    def check_refresh_connection_info(self, connection_info):"},{"line_number":115,"context_line":"        \"\"\"check if it is needed to refresh connection info.\"\"\""},{"line_number":116,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver("},{"line_number":117,"context_line":"            pool\u003dCONF.libvirt.images_rbd_pool,"},{"line_number":118,"context_line":"            ceph_conf\u003dCONF.libvirt.images_rbd_ceph_conf,"},{"line_number":119,"context_line":"            rbd_user\u003dCONF.libvirt.rbd_user)"},{"line_number":120,"context_line":"        ceph_hosts, ceph_ports \u003d rbd_driver.get_mon_addrs()"}],"source_content_type":"text/x-python","patch_set":25,"id":"1af94dfe_cd04d04b","line":117,"updated":"2016-03-17 14:41:45.000000000","message":"This pool isn\u0027t relevant here.","commit_id":"45ebb25bafb3a2b566ffe9f2c840805bcce37c9d"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"156a4f9800a46c6039606b7717d5eb99ff286f89","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        \"\"\"check if it is needed to refresh connection info.\"\"\""},{"line_number":116,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver("},{"line_number":117,"context_line":"            pool\u003dCONF.libvirt.images_rbd_pool,"},{"line_number":118,"context_line":"            ceph_conf\u003dCONF.libvirt.images_rbd_ceph_conf,"},{"line_number":119,"context_line":"            rbd_user\u003dCONF.libvirt.rbd_user)"},{"line_number":120,"context_line":"        ceph_hosts, ceph_ports \u003d rbd_driver.get_mon_addrs()"},{"line_number":121,"context_line":"        data_info \u003d connection_info.get(\u0027data\u0027)"}],"source_content_type":"text/x-python","patch_set":25,"id":"1af94dfe_6da2dc4c","line":118,"updated":"2016-03-17 14:41:45.000000000","message":"This ceph configuration isn\u0027t relevant here.","commit_id":"45ebb25bafb3a2b566ffe9f2c840805bcce37c9d"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"156a4f9800a46c6039606b7717d5eb99ff286f89","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver("},{"line_number":117,"context_line":"            pool\u003dCONF.libvirt.images_rbd_pool,"},{"line_number":118,"context_line":"            ceph_conf\u003dCONF.libvirt.images_rbd_ceph_conf,"},{"line_number":119,"context_line":"            rbd_user\u003dCONF.libvirt.rbd_user)"},{"line_number":120,"context_line":"        ceph_hosts, ceph_ports \u003d rbd_driver.get_mon_addrs()"},{"line_number":121,"context_line":"        data_info \u003d connection_info.get(\u0027data\u0027)"},{"line_number":122,"context_line":"        if data_info:"}],"source_content_type":"text/x-python","patch_set":25,"id":"1af94dfe_68392aa4","line":119,"updated":"2016-03-17 14:41:45.000000000","message":"It\u0027s not clear to me that this rbd_user is relevant here.","commit_id":"45ebb25bafb3a2b566ffe9f2c840805bcce37c9d"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"156a4f9800a46c6039606b7717d5eb99ff286f89","unresolved":false,"context_lines":[{"line_number":123,"context_line":"            bdm_hosts_ports_mapping \u003d zip(data_info.get(\u0027hosts\u0027),"},{"line_number":124,"context_line":"                                          data_info.get(\u0027ports\u0027))"},{"line_number":125,"context_line":"            ceph_hosts_ports_mapping \u003d zip(ceph_hosts, ceph_ports)"},{"line_number":126,"context_line":"            if set(bdm_hosts_ports_mapping) !\u003d set(ceph_hosts_ports_mapping):"},{"line_number":127,"context_line":"                return True"},{"line_number":128,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":25,"id":"1af94dfe_8829163a","line":126,"updated":"2016-03-17 14:41:45.000000000","message":"We seem to be asking ceph if the set of hosts which service images are the same as the ones being used by this volume.\n\nWhile this may often be true, I don\u0027t understand why it must be true architecturally. This seems wrong to me.","commit_id":"45ebb25bafb3a2b566ffe9f2c840805bcce37c9d"},{"author":{"_account_id":3189,"name":"Chet Burgess","email":"cfb@metacloud.com","username":"cfb-n"},"change_message_id":"28a5106477bd1822d177946fd9ee24b4ba24cc52","unresolved":false,"context_lines":[{"line_number":111,"context_line":"              self).disconnect_volume(connection_info, disk_dev)"},{"line_number":112,"context_line":"        self._delete_secret_by_name(connection_info)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def check_refresh_connection_info(self, connection_info):"},{"line_number":115,"context_line":"        \"\"\"check if it is needed to refresh connection info.\"\"\""},{"line_number":116,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver("},{"line_number":117,"context_line":"            pool\u003dCONF.libvirt.images_rbd_pool,"}],"source_content_type":"text/x-python","patch_set":29,"id":"da0c15f0_87e7c411","line":114,"range":{"start_line":114,"start_character":0,"end_line":114,"end_character":61},"updated":"2016-03-24 22:56:48.000000000","message":"The LibvirtNetVolumeDriver can be used for iscsi or rbd. Your logic appears to only work for RBD. What will happen if this gets called for a volume that is attached with iscsi?","commit_id":"249ccee7e43dafb9bd7c02db6f6634b52af3278b"},{"author":{"_account_id":3189,"name":"Chet Burgess","email":"cfb@metacloud.com","username":"cfb-n"},"change_message_id":"28a5106477bd1822d177946fd9ee24b4ba24cc52","unresolved":false,"context_lines":[{"line_number":111,"context_line":"              self).disconnect_volume(connection_info, disk_dev)"},{"line_number":112,"context_line":"        self._delete_secret_by_name(connection_info)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def check_refresh_connection_info(self, connection_info):"},{"line_number":115,"context_line":"        \"\"\"check if it is needed to refresh connection info.\"\"\""},{"line_number":116,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver("},{"line_number":117,"context_line":"            pool\u003dCONF.libvirt.images_rbd_pool,"},{"line_number":118,"context_line":"            ceph_conf\u003dCONF.libvirt.images_rbd_ceph_conf,"},{"line_number":119,"context_line":"            rbd_user\u003dCONF.libvirt.rbd_user)"},{"line_number":120,"context_line":"        ceph_hosts, ceph_ports \u003d rbd_driver.get_mon_addrs()"},{"line_number":121,"context_line":"        data_info \u003d connection_info.get(\u0027data\u0027)"},{"line_number":122,"context_line":"        if data_info:"},{"line_number":123,"context_line":"            bdm_hosts_ports_mapping \u003d zip(data_info.get(\u0027hosts\u0027),"},{"line_number":124,"context_line":"                                          data_info.get(\u0027ports\u0027))"},{"line_number":125,"context_line":"            ceph_hosts_ports_mapping \u003d zip(ceph_hosts, ceph_ports)"},{"line_number":126,"context_line":"            if set(bdm_hosts_ports_mapping) !\u003d set(ceph_hosts_ports_mapping):"},{"line_number":127,"context_line":"                return True"},{"line_number":128,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":29,"id":"da0c15f0_a72f69a0","line":128,"range":{"start_line":114,"start_character":0,"end_line":128,"end_character":20},"updated":"2016-03-24 22:56:48.000000000","message":"As Josh Durgen already said, this is absolutely incorrect. There is no requirement that the CEPH cluster or pool that cinder used to create the original volume is the one that nova is configured to use. cinder supports having multiple volume backends that each could be configured with an entirely separate ceph.conf allowing you to place volumes on multiple clusters or pools. The only thing required to be the same between nova and cinder with regard to CEPH is that the rbd/secret must be the same on the different CEPH clusters/pools as is configured locally for nova.\n\nAs Josh suggested the proper fix here is to refetch the connection info from cinder.","commit_id":"249ccee7e43dafb9bd7c02db6f6634b52af3278b"}]}