)]}'
{"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ea2e2a2ab1d606274691b689fcbeecded0f47e50","unresolved":false,"context_lines":[{"line_number":2269,"context_line":"                sr_ref \u003d self._session.VDI.get_SR(vdi_ref)"},{"line_number":2270,"context_line":"                sr_uuid \u003d self._session.SR.get_uuid(sr_ref)"},{"line_number":2271,"context_line":"                # map volume UUID to SR UUID"},{"line_number":2272,"context_line":"                volume_to_sr_uuids[volume_uuid] \u003d sr_uuid"},{"line_number":2273,"context_line":"            dest_check_data.volume_to_sr_uuids \u003d volume_to_sr_uuids"},{"line_number":2274,"context_line":""},{"line_number":2275,"context_line":"        return dest_check_data"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_09afe281","line":2272,"updated":"2016-04-19 10:14:05.000000000","message":"yeah, that looks correct to me.","commit_id":"1b1ddcd4e3e8ab08eae7bb6f3d8a538c126f212d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ea2e2a2ab1d606274691b689fcbeecded0f47e50","unresolved":false,"context_lines":[{"line_number":2513,"context_line":"                                                \u0027block_device_mapping\u0027]:"},{"line_number":2514,"context_line":"                    sr_uuid, _ \u003d self._volumeops.connect_volume("},{"line_number":2515,"context_line":"                            block_device_map[\u0027connection_info\u0027],"},{"line_number":2516,"context_line":"                            volume_to_sr_uuids)"},{"line_number":2517,"context_line":"                    sr_ref \u003d self._session.call_xenapi(\u0027SR.get_by_uuid\u0027,"},{"line_number":2518,"context_line":"                                                       sr_uuid)"},{"line_number":2519,"context_line":"                    sr_uuid_map[sr_uuid] \u003d sr_ref"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_292f3ef7","line":2516,"updated":"2016-04-19 10:14:05.000000000","message":"So here, I think we should actually look up the specific sr-uuid, so we just pass through the sr uuid down into connect_volume, rather than the dictionary, which is a bit odd given the other arguments here.","commit_id":"1b1ddcd4e3e8ab08eae7bb6f3d8a538c126f212d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fd615b9a6ac011ff1be68e5816b54030016549a2","unresolved":false,"context_lines":[{"line_number":2513,"context_line":"                                                \u0027block_device_mapping\u0027]:"},{"line_number":2514,"context_line":"                    sr_uuid, _ \u003d self._volumeops.connect_volume("},{"line_number":2515,"context_line":"                            block_device_map[\u0027connection_info\u0027],"},{"line_number":2516,"context_line":"                            volume_to_sr_uuids)"},{"line_number":2517,"context_line":"                    sr_ref \u003d self._session.call_xenapi(\u0027SR.get_by_uuid\u0027,"},{"line_number":2518,"context_line":"                                                       sr_uuid)"},{"line_number":2519,"context_line":"                    sr_uuid_map[sr_uuid] \u003d sr_ref"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_8c42d666","line":2516,"in_reply_to":"1a122d0e_183e018c","updated":"2016-04-19 13:48:22.000000000","message":"My take is that the abstraction layer at this point is \"give me all the stuff for this volume to be connected\".\n\nThe dictionary is really for all the volumes for this VM.\n\nThis is something that feels OK: volumeops.connect_all_volumes(block_device_info[\u0027block_device_mapping\u0027], volume_to_sr_uuids), but thats not what we want here.\n\nBut you could argue both ways.","commit_id":"1b1ddcd4e3e8ab08eae7bb6f3d8a538c126f212d"},{"author":{"_account_id":11069,"name":"Corey Wright","email":"undefined+launchpad@pobox.com","username":"coreywright"},"change_message_id":"31f08d4172dee35898ceafaa7cc8242d8587bd39","unresolved":false,"context_lines":[{"line_number":2513,"context_line":"                                                \u0027block_device_mapping\u0027]:"},{"line_number":2514,"context_line":"                    sr_uuid, _ \u003d self._volumeops.connect_volume("},{"line_number":2515,"context_line":"                            block_device_map[\u0027connection_info\u0027],"},{"line_number":2516,"context_line":"                            volume_to_sr_uuids)"},{"line_number":2517,"context_line":"                    sr_ref \u003d self._session.call_xenapi(\u0027SR.get_by_uuid\u0027,"},{"line_number":2518,"context_line":"                                                       sr_uuid)"},{"line_number":2519,"context_line":"                    sr_uuid_map[sr_uuid] \u003d sr_ref"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_183e018c","line":2516,"in_reply_to":"1a122d0e_292f3ef7","updated":"2016-04-19 13:30:11.000000000","message":"tl;dr i was simply maintaining / not violating the existing abstraction levels present in the call tree.\n\nto get the volume id in order to extract the sr uuid from the dict you have to dig down into block_device_map (ie block_device_map-\u003econnection_info-\u003edata-\u003evolume_id), but we progressively peel away the onion that is block_device_map going down the call tree and don\u0027t get to \"data\"/\"params\" (to get the target, port, \u0026 targetIQN) until parse_sr_info at the very end of the call tree.\n\nif you are telling me that the data abstraction / onion peeling is worth violating so as to only pass the sr uuid (and not the entire volume-to-sr-mapping; everything\u0027s a trade-off), or i\u0027m being daft and overlooking a more \"shallow\" (in terms of onion peeling / data abstraction) way of obtaining the volume id, then i\u0027ll readily alter this, but it wasn\u0027t done without reason.","commit_id":"1b1ddcd4e3e8ab08eae7bb6f3d8a538c126f212d"}],"nova/virt/xenapi/volume_utils.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ea2e2a2ab1d606274691b689fcbeecded0f47e50","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        if volume_to_sr_uuids and volume_id in volume_to_sr_uuids:"},{"line_number":59,"context_line":"            # doing a live-migrate, lets make the UUIDs match in case UUID was"},{"line_number":60,"context_line":"            # using the old style UUID scheme"},{"line_number":61,"context_line":"            sr_uuid \u003d volume_to_sr_uuids[volume_id]"},{"line_number":62,"context_line":"        else:"},{"line_number":63,"context_line":"            sr_identity \u003d \"%s/%s/%s\" % (params[\u0027target\u0027], params[\u0027port\u0027],"},{"line_number":64,"context_line":"                                        params[\u0027targetIQN\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_e91c26c3","line":61,"updated":"2016-04-19 10:14:05.000000000","message":"yeah, this is my bad, I think we should do this thing higher up the stack.","commit_id":"1b1ddcd4e3e8ab08eae7bb6f3d8a538c126f212d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fd615b9a6ac011ff1be68e5816b54030016549a2","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        if volume_to_sr_uuids and volume_id in volume_to_sr_uuids:"},{"line_number":59,"context_line":"            # doing a live-migrate, lets make the UUIDs match in case UUID was"},{"line_number":60,"context_line":"            # using the old style UUID scheme"},{"line_number":61,"context_line":"            sr_uuid \u003d volume_to_sr_uuids[volume_id]"},{"line_number":62,"context_line":"        else:"},{"line_number":63,"context_line":"            sr_identity \u003d \"%s/%s/%s\" % (params[\u0027target\u0027], params[\u0027port\u0027],"},{"line_number":64,"context_line":"                                        params[\u0027targetIQN\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_af406435","line":61,"in_reply_to":"1a122d0e_d8e00999","updated":"2016-04-19 13:48:22.000000000","message":"its normal for the higher layers to extract the volume uuid like this, AFAIK:\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L1078","commit_id":"1b1ddcd4e3e8ab08eae7bb6f3d8a538c126f212d"},{"author":{"_account_id":11069,"name":"Corey Wright","email":"undefined+launchpad@pobox.com","username":"coreywright"},"change_message_id":"31f08d4172dee35898ceafaa7cc8242d8587bd39","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        if volume_to_sr_uuids and volume_id in volume_to_sr_uuids:"},{"line_number":59,"context_line":"            # doing a live-migrate, lets make the UUIDs match in case UUID was"},{"line_number":60,"context_line":"            # using the old style UUID scheme"},{"line_number":61,"context_line":"            sr_uuid \u003d volume_to_sr_uuids[volume_id]"},{"line_number":62,"context_line":"        else:"},{"line_number":63,"context_line":"            sr_identity \u003d \"%s/%s/%s\" % (params[\u0027target\u0027], params[\u0027port\u0027],"},{"line_number":64,"context_line":"                                        params[\u0027targetIQN\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_d8e00999","line":61,"in_reply_to":"1a122d0e_e91c26c3","updated":"2016-04-19 13:30:11.000000000","message":"to get the sr_uuid you need the volume_id.  the volume_id is in connection_data, but we don\u0027t access the specifics of it (ie block_device_map-\u003econnection_info-\u003edata/\"params\") until here (ie it\u0027s merely a data abstraction everywhere above in the call tree).\n\nwant to violate the data abstraction in exchange for reducing the argument that\u0027s passed down through multiple calls (ie sr_uuid string vs volume_to_sr_uuids dict), or tell me the data abstraction down through the call tree is incidental and not intentional, then i\u0027ll readily change it, but it wasn\u0027t done without reason (and just want to make sure that reason is being recognized, even if it is thought of as invalid or insignificant).\n\nor am i being daft and there\u0027s an obvious less-deep-into-the-block_device_map way of getting the volume id and i\u0027m stupidly overlooking it?","commit_id":"1b1ddcd4e3e8ab08eae7bb6f3d8a538c126f212d"}]}