)]}'
{"nova/virt/powervm/blockdev.py":[{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"44abd0f92b8fffbdbf18db435721214c73cd2919","unresolved":false,"context_lines":[{"line_number":26,"context_line":"from nova.openstack.common import cfg"},{"line_number":27,"context_line":"from nova.openstack.common import excutils"},{"line_number":28,"context_line":"from nova.openstack.common import log as logging"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"from nova.virt import images"},{"line_number":31,"context_line":"from nova.virt.powervm import command"},{"line_number":32,"context_line":"from nova.virt.powervm import common"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F93E%3D","line":29,"updated":"2012-12-04 17:02:49.000000000","message":"Nit. Is it a trailing space here?","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"44abd0f92b8fffbdbf18db435721214c73cd2919","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        :param file_path: output file path"},{"line_number":250,"context_line":"        \"\"\""},{"line_number":251,"context_line":"        if compress:"},{"line_number":252,"context_line":"            cmd \u003d (\u0027dd if\u003d/dev/%s bs\u003d1024k | gzip -9 \u003e %s\u0027 %"},{"line_number":253,"context_line":"                   (device_name, file_path))"},{"line_number":254,"context_line":"        else:"},{"line_number":255,"context_line":"            cmd \u003d \u0027dd if\u003d/dev/%s of\u003d%s bs\u003d1024k\u0027 % (device_name, file_path)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F92s%3D","line":252,"updated":"2012-12-04 17:02:49.000000000","message":"I think this is fragile. It could be the case that the \u0027dd\u0027 or \u0027gzip\u0027 aren\u0027t in the path for the particular system? Couldn\u0027t it be a new method in the BaseCommand class?","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"44abd0f92b8fffbdbf18db435721214c73cd2919","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        cmd \u003d (\"perl -MDigest::MD5 -e \u0027my $file \u003d \\\"%s\\\"; open(FILE, $file); \""},{"line_number":262,"context_line":"               \"binmode(FILE); \""},{"line_number":263,"context_line":"               \"print Digest::MD5-\u003enew-\u003eaddfile(*FILE)-\u003ehexdigest, \""},{"line_number":264,"context_line":"               \"\\\" $file\\n\\\";\u0027\" % remote_path)"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"        output \u003d self.run_command_as_root(cmd)"},{"line_number":267,"context_line":"        return output[0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9yc%3D","line":264,"updated":"2012-12-04 17:02:49.000000000","message":"This is even more fragile IMO. It is relying on that perl has the Digest::MD5 module installed.","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a5d07dede8c714b57492b2ecd7f224686bc1fe26","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        cmd \u003d (\"perl -MDigest::MD5 -e \u0027my $file \u003d \\\"%s\\\"; open(FILE, $file); \""},{"line_number":262,"context_line":"               \"binmode(FILE); \""},{"line_number":263,"context_line":"               \"print Digest::MD5-\u003enew-\u003eaddfile(*FILE)-\u003ehexdigest, \""},{"line_number":264,"context_line":"               \"\\\" $file\\n\\\";\u0027\" % remote_path)"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"        output \u003d self.run_command_as_root(cmd)"},{"line_number":267,"context_line":"        return output[0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9y0%3D","line":264,"updated":"2012-12-04 16:55:52.000000000","message":"Use hashlib.","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"210070add2a48bffc3c84d86d6a44117df5f5548","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                         if False (default), copy the file as is"},{"line_number":355,"context_line":"        \"\"\""},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"        tempStr \u003d os.path.join(local_dest_dir,"},{"line_number":358,"context_line":"                               os.path.basename(remote_source_path))"},{"line_number":359,"context_line":"        local_file_path \u003d \u0027\u0027.join([tempStr, \u0027.gz\u0027])"},{"line_number":360,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9e4%3D","line":357,"updated":"2012-12-04 20:14:49.000000000","message":"python naming is non camel case, so this should be temp_str or just temp","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"210070add2a48bffc3c84d86d6a44117df5f5548","unresolved":false,"context_lines":[{"line_number":356,"context_line":""},{"line_number":357,"context_line":"        tempStr \u003d os.path.join(local_dest_dir,"},{"line_number":358,"context_line":"                               os.path.basename(remote_source_path))"},{"line_number":359,"context_line":"        local_file_path \u003d \u0027\u0027.join([tempStr, \u0027.gz\u0027])"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"        if compress:"},{"line_number":362,"context_line":"            copy_from_path \u003d \u0027\u0027.join([remote_source_path, \u0027.gz\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9e0%3D","line":359,"updated":"2012-12-04 20:14:49.000000000","message":"why join instead of +?\n\nlocal_file_path \u003d temp + \u0027.gz\u0027","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"210070add2a48bffc3c84d86d6a44117df5f5548","unresolved":false,"context_lines":[{"line_number":359,"context_line":"        local_file_path \u003d \u0027\u0027.join([tempStr, \u0027.gz\u0027])"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"        if compress:"},{"line_number":362,"context_line":"            copy_from_path \u003d \u0027\u0027.join([remote_source_path, \u0027.gz\u0027])"},{"line_number":363,"context_line":"        else:"},{"line_number":364,"context_line":"            copy_from_path \u003d remote_source_path"},{"line_number":365,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9ew%3D","line":362,"updated":"2012-12-04 20:14:49.000000000","message":"same issue as above","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"44abd0f92b8fffbdbf18db435721214c73cd2919","unresolved":false,"context_lines":[{"line_number":402,"context_line":"            raise exception.PowerVMFileTransferFailed()"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        # Cleanup transferred remote file"},{"line_number":405,"context_line":"        cmd \u003d \"/usr/bin/rm -f %s\" % copy_from_path"},{"line_number":406,"context_line":"        output \u003d self.run_command_as_root(cmd)"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"        return local_file_path"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9yY%3D","line":405,"updated":"2012-12-04 17:02:49.000000000","message":"It could take advantage of BaseCommand class as well, couldn\u0027t it?","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1378661ca867f4f3d607cece5f88402c65383509","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        # do the disk copy"},{"line_number":143,"context_line":"        dest_file_path \u003d os.path.join("},{"line_number":144,"context_line":"                CONF.powervm_img_remote_path, image_id)"},{"line_number":145,"context_line":"        self._copy_device_to_file(device_name, dest_file_path, compress\u003dFalse)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        # compress and copy the file back to the nova-compute host"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMX%2F%2F8rs%3D","line":144,"updated":"2012-12-05 15:10:25.000000000","message":"See comment below about using os.path.join() on remote paths.","commit_id":"94d8604160b5e17f588404760db176456bb8dbe8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1378661ca867f4f3d607cece5f88402c65383509","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        # do the disk copy"},{"line_number":143,"context_line":"        dest_file_path \u003d os.path.join("},{"line_number":144,"context_line":"                CONF.powervm_img_remote_path, image_id)"},{"line_number":145,"context_line":"        self._copy_device_to_file(device_name, dest_file_path, compress\u003dFalse)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        # compress and copy the file back to the nova-compute host"},{"line_number":148,"context_line":"        snapshot_file_path \u003d self._copy_image_file_from_host("}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMX%2F%2F8rg%3D","line":145,"updated":"2012-12-05 15:10:25.000000000","message":"If you never use compress\u003dTrue for this method (which I assume would mean that the VIOS does the compression before sending) then I think you should remove the interface.","commit_id":"94d8604160b5e17f588404760db176456bb8dbe8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1378661ca867f4f3d607cece5f88402c65383509","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        # compress and copy the file back to the nova-compute host"},{"line_number":148,"context_line":"        snapshot_file_path \u003d self._copy_image_file_from_host("},{"line_number":149,"context_line":"                dest_file_path, CONF.powervm_img_local_path,"},{"line_number":150,"context_line":"                compress\u003dTrue)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        # get glance service"},{"line_number":153,"context_line":"        glance_service, image_id \u003d glance.get_remote_image_service("}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMX%2F%2F8rc%3D","line":150,"updated":"2012-12-05 15:10:25.000000000","message":"If you always compress the image, why do you need the param to _copy_image_file_from_host()?","commit_id":"94d8604160b5e17f588404760db176456bb8dbe8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1378661ca867f4f3d607cece5f88402c65383509","unresolved":false,"context_lines":[{"line_number":301,"context_line":"                                   remote_path)"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"            # Verify image file checksums match"},{"line_number":304,"context_line":"            output \u003d self._md5sum_remote_file(final_path)"},{"line_number":305,"context_line":"            if not output:"},{"line_number":306,"context_line":"                LOG.error(_(\"Unable to get checksum\"))"},{"line_number":307,"context_line":"                raise exception.PowerVMFileTransferFailed()"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMX%2F%2F8sg%3D","line":304,"updated":"2012-12-05 15:10:25.000000000","message":"Just so I\u0027m clear, this is computing the md5 so that we can verify that the ftp transfer completed, is that right? Isn\u0027t that a bit excessive? Simply unzipping it will tell us if the file is damaged, right? Each block is 32-bit CRC\u0027d...","commit_id":"94d8604160b5e17f588404760db176456bb8dbe8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1378661ca867f4f3d607cece5f88402c65383509","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        \"\"\""},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"        temp_str \u003d os.path.join(local_dest_dir,"},{"line_number":358,"context_line":"                               os.path.basename(remote_source_path))"},{"line_number":359,"context_line":"        local_file_path \u003d temp_str + \u0027.gz\u0027"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"        if compress:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMX%2F%2F8sc%3D","line":358,"updated":"2012-12-05 15:10:25.000000000","message":"I don\u0027t think it makes sense to use os.path.join() on a remote path. If you\u0027re running this from Windows, for example, it won\u0027t split the path elements based on / and this will break. Since you know the remote is AIX/UNIX, perhaps you should just explicitly split on /?","commit_id":"94d8604160b5e17f588404760db176456bb8dbe8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1378661ca867f4f3d607cece5f88402c65383509","unresolved":false,"context_lines":[{"line_number":373,"context_line":"            self.run_vios_command_as_root(cmd)"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"        # Get file checksum"},{"line_number":376,"context_line":"        output \u003d self._md5sum_remote_file(copy_from_path)"},{"line_number":377,"context_line":"        if not output:"},{"line_number":378,"context_line":"            LOG.error(_(\"Unable to get checksum\"))"},{"line_number":379,"context_line":"            msg_args \u003d {\u0027file_path\u0027: copy_from_path}"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMX%2F%2F8sQ%3D","line":376,"updated":"2012-12-05 15:10:25.000000000","message":"I\u0027ve lost track of how many times you\u0027ve read the (potentially huge) image for no reason. Perhaps some crafty use of tee and pipes could facilitate gzip\u0027ing and md5sum\u0027ing simultaneously?","commit_id":"94d8604160b5e17f588404760db176456bb8dbe8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1378661ca867f4f3d607cece5f88402c65383509","unresolved":false,"context_lines":[{"line_number":384,"context_line":"        # Copy file to host"},{"line_number":385,"context_line":"        common.ftp_get_command(self.connection_data,"},{"line_number":386,"context_line":"                               copy_from_path,"},{"line_number":387,"context_line":"                               local_file_path)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        # Calculate copied image checksum"},{"line_number":390,"context_line":"        with open(local_file_path, \u0027r\u0027) as image_file:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMX%2F%2F8sI%3D","line":387,"updated":"2012-12-05 15:10:25.000000000","message":"Can this fail? If it does, you fail trying to open the file below, which may be less obvious in a log than you want... If it could possibly already exist locally, then the confusion is even worse.","commit_id":"94d8604160b5e17f588404760db176456bb8dbe8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1378661ca867f4f3d607cece5f88402c65383509","unresolved":false,"context_lines":[{"line_number":407,"context_line":""},{"line_number":408,"context_line":"        return local_file_path"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"    def run_vios_command(self, cmd, check_exit_code\u003dTrue):"},{"line_number":411,"context_line":"        \"\"\"Run a remote command using an active ssh connection."},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        :param command: String with the command to run."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMX%2F%2F8tI%3D","line":410,"updated":"2012-12-05 15:10:25.000000000","message":"I really would have liked to see the adjustment of this method name as a separate patch before this one.","commit_id":"94d8604160b5e17f588404760db176456bb8dbe8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"854dd518932e08686c01697d9a42eb2f36884459","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        self._copy_device_to_file(device_name, dest_file_path)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        # yield to other greenthreads"},{"line_number":149,"context_line":"        greenthread.sleep(0)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        # compress and copy the file back to the nova-compute host"},{"line_number":152,"context_line":"        snapshot_file_path \u003d self._copy_image_file_from_host("}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2Fzg4%3D","line":149,"updated":"2013-01-04 15:25:38.000000000","message":"All this does is yield between two long-running operations, right? It doesn\u0027t actually let something else run while the file is being copied. I think this looks like a hack being placed right here...","commit_id":"32cdfa7fa0cab38170731887540252f77fb390e4"},{"author":{"_account_id":5371,"name":"Mathew Odden","email":"locke105@gmail.com","username":"locke105"},"change_message_id":"f428373d1af7e544ce24f284ac82ac05692ba7ef","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        self._copy_device_to_file(device_name, dest_file_path)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        # yield to other greenthreads"},{"line_number":149,"context_line":"        greenthread.sleep(0)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        # compress and copy the file back to the nova-compute host"},{"line_number":152,"context_line":"        snapshot_file_path \u003d self._copy_image_file_from_host("}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2Fzc8%3D","line":149,"in_reply_to":"AAAAM3%2F%2Fzg4%3D","updated":"2013-01-04 15:41:37.000000000","message":"Correct, this is just a \u0027yield\u0027 to allow other threads to get some time in between big copy operations. As long as the python threading gets monkey patched, this is really just a courtesy move.","commit_id":"32cdfa7fa0cab38170731887540252f77fb390e4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"854dd518932e08686c01697d9a42eb2f36884459","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        :param file_path: output file path"},{"line_number":257,"context_line":"        \"\"\""},{"line_number":258,"context_line":"        cmd \u003d \u0027dd if\u003d/dev/%s of\u003d%s bs\u003d1024k\u0027 % (device_name, file_path)"},{"line_number":259,"context_line":"        self.run_vios_command_as_root(cmd)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def _md5sum_remote_file(self, remote_path):"},{"line_number":262,"context_line":"        # AIX6/VIOS cannot md5sum files with sizes greater than ~2GB"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2Fzgo%3D","line":259,"updated":"2013-01-04 15:25:38.000000000","message":"This, being the command that can take a long time. I don\u0027t see anything (after a quick google) that implies eventlet support in paramiko. If there is/was then there may not be a problem, but if not, this would block nova for an unreasonably long time, right?\n\nI don\u0027t see \"green\" anywhere in the paramiko source, so I don\u0027t think it\u0027s doing anything to help you here. If I\u0027m wrong, please slap me :)","commit_id":"32cdfa7fa0cab38170731887540252f77fb390e4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e82c1e673cb427d3a2dcd1db7f99f713c1b5114c","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        :param file_path: output file path"},{"line_number":257,"context_line":"        \"\"\""},{"line_number":258,"context_line":"        cmd \u003d \u0027dd if\u003d/dev/%s of\u003d%s bs\u003d1024k\u0027 % (device_name, file_path)"},{"line_number":259,"context_line":"        self.run_vios_command_as_root(cmd)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def _md5sum_remote_file(self, remote_path):"},{"line_number":262,"context_line":"        # AIX6/VIOS cannot md5sum files with sizes greater than ~2GB"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2Fzb4%3D","line":259,"in_reply_to":"AAAAM3%2F%2Fzdg%3D","updated":"2013-01-04 15:57:56.000000000","message":"Hmm, okay, got a pointer to that? If that\u0027s the case, the the yields are unnecessary and just look sloppy, right?","commit_id":"32cdfa7fa0cab38170731887540252f77fb390e4"},{"author":{"_account_id":5371,"name":"Mathew Odden","email":"locke105@gmail.com","username":"locke105"},"change_message_id":"f428373d1af7e544ce24f284ac82ac05692ba7ef","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        :param file_path: output file path"},{"line_number":257,"context_line":"        \"\"\""},{"line_number":258,"context_line":"        cmd \u003d \u0027dd if\u003d/dev/%s of\u003d%s bs\u003d1024k\u0027 % (device_name, file_path)"},{"line_number":259,"context_line":"        self.run_vios_command_as_root(cmd)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def _md5sum_remote_file(self, remote_path):"},{"line_number":262,"context_line":"        # AIX6/VIOS cannot md5sum files with sizes greater than ~2GB"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2Fzdg%3D","line":259,"in_reply_to":"AAAAM3%2F%2Fzgo%3D","updated":"2013-01-04 15:41:37.000000000","message":"I believe, due to the monkey patching of python threading, that paramiko calls get run in a greenlet/greenthread. I have seen some evidence of this when the paramiko thread gets destroyed without finishing properly. Also, I can confirm that I have seen two paramiko SSH commands running at the same time before in nova. It caused a race condition issue that I had to fix with a lock.","commit_id":"32cdfa7fa0cab38170731887540252f77fb390e4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6c5aeca8ef36982a554dd0d1333d82fedf9dfdd","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        self._copy_device_to_file(device_name, dest_file_path)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        # yield to other greenthreads"},{"line_number":149,"context_line":"        greenthread.sleep(0)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        # compress and copy the file back to the nova-compute host"},{"line_number":152,"context_line":"        snapshot_file_path \u003d self._copy_image_file_from_host("}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FwXQ%3D","line":149,"updated":"2013-01-07 15:25:35.000000000","message":"If the paramiko calls are actually playing nice with other threads, then this should be removed, right? There\u0027s not need for it and it distracts from the fact that the operation is not in fact blocking, right?","commit_id":"68b58c43d6970e0ee06f7de3f2698bf6480a35c7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6c5aeca8ef36982a554dd0d1333d82fedf9dfdd","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                compress\u003dTrue)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        # yield to other greenthreads"},{"line_number":157,"context_line":"        greenthread.sleep(0)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        # get glance service"},{"line_number":160,"context_line":"        glance_service, image_id \u003d glance.get_remote_image_service("}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FwXM%3D","line":157,"updated":"2013-01-07 15:25:35.000000000","message":"Same here","commit_id":"68b58c43d6970e0ee06f7de3f2698bf6480a35c7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6c5aeca8ef36982a554dd0d1333d82fedf9dfdd","unresolved":false,"context_lines":[{"line_number":382,"context_line":"            msg_args \u003d {\u0027file_path\u0027: copy_from_path}"},{"line_number":383,"context_line":"            raise exception.PowerVMFileTransferFailed(**msg_args)"},{"line_number":384,"context_line":"        else:"},{"line_number":385,"context_line":"            sourceChksum \u003d output.split(\u0027 \u0027)[0]"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        # Copy file to host"},{"line_number":388,"context_line":"        common.ftp_get_command(self.connection_data,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FwXE%3D","line":385,"updated":"2013-01-07 15:25:35.000000000","message":"style issue: camelCase. Please fix all the instances in this function.","commit_id":"68b58c43d6970e0ee06f7de3f2698bf6480a35c7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a364177ae1774a9424e6b949a82be409cd023f1","unresolved":false,"context_lines":[{"line_number":386,"context_line":"        # Calculate copied image checksum"},{"line_number":387,"context_line":"        with open(local_file_path, \u0027r\u0027) as image_file:"},{"line_number":388,"context_line":"            hasher \u003d hashlib.md5()"},{"line_number":389,"context_line":"            blockSize \u003d 0x10000"},{"line_number":390,"context_line":"            buf \u003d image_file.read(blockSize)"},{"line_number":391,"context_line":"            while len(buf) \u003e 0:"},{"line_number":392,"context_line":"                hasher.update(buf)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAM3%2F%2Fvpo%3D","line":389,"updated":"2013-01-07 22:20:43.000000000","message":"Missed one.","commit_id":"7f238a6e231d21c44e2c0e725c4c5bd3da5f1e60"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"7f11cd295777a6d1692e5799adc0df54b99a7050","unresolved":false,"context_lines":[{"line_number":386,"context_line":"        # Calculate copied image checksum"},{"line_number":387,"context_line":"        with open(local_file_path, \u0027r\u0027) as image_file:"},{"line_number":388,"context_line":"            hasher \u003d hashlib.md5()"},{"line_number":389,"context_line":"            blockSize \u003d 0x10000"},{"line_number":390,"context_line":"            buf \u003d image_file.read(blockSize)"},{"line_number":391,"context_line":"            while len(buf) \u003e 0:"},{"line_number":392,"context_line":"                hasher.update(buf)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAM3%2F%2Fvm0%3D","line":389,"in_reply_to":"AAAAM3%2F%2FvpM%3D","updated":"2013-01-07 22:47:44.000000000","message":"Done","commit_id":"7f238a6e231d21c44e2c0e725c4c5bd3da5f1e60"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"2b3ab767b5b1c9fe421dd8a5009a5ba233bab4c8","unresolved":false,"context_lines":[{"line_number":386,"context_line":"        # Calculate copied image checksum"},{"line_number":387,"context_line":"        with open(local_file_path, \u0027r\u0027) as image_file:"},{"line_number":388,"context_line":"            hasher \u003d hashlib.md5()"},{"line_number":389,"context_line":"            blockSize \u003d 0x10000"},{"line_number":390,"context_line":"            buf \u003d image_file.read(blockSize)"},{"line_number":391,"context_line":"            while len(buf) \u003e 0:"},{"line_number":392,"context_line":"                hasher.update(buf)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAM3%2F%2FvpM%3D","line":389,"in_reply_to":"AAAAM3%2F%2Fvpo%3D","updated":"2013-01-07 22:47:07.000000000","message":"Ahh thanks","commit_id":"7f238a6e231d21c44e2c0e725c4c5bd3da5f1e60"}],"nova/virt/powervm/common.py":[{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"210070add2a48bffc3c84d86d6a44117df5f5548","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                         passwd\u003dconnection.password)"},{"line_number":128,"context_line":"        ftp.cwd(os.path.dirname(remote_path))"},{"line_number":129,"context_line":"        name \u003d os.path.basename(remote_path)"},{"line_number":130,"context_line":"        LOG.debug(_(\"ftp GET %s to: %s\") % (remote_path, local_path))"},{"line_number":131,"context_line":"        with open(local_path, \u0027w\u0027) as ftpfile:"},{"line_number":132,"context_line":"            ftpcmd \u003d \u0027\u0027.join([\u0027RETR \u0027, name])"},{"line_number":133,"context_line":"            ftp.retrbinary(ftpcmd, ftpfile.write)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9eY%3D","line":130,"updated":"2012-12-04 20:14:49.000000000","message":"the pep8 fix for this \n\nLOG.debug(_(\"ftp GET %(remote_path)s to: %(local_path)s\") % locals())","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"210070add2a48bffc3c84d86d6a44117df5f5548","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        name \u003d os.path.basename(remote_path)"},{"line_number":130,"context_line":"        LOG.debug(_(\"ftp GET %s to: %s\") % (remote_path, local_path))"},{"line_number":131,"context_line":"        with open(local_path, \u0027w\u0027) as ftpfile:"},{"line_number":132,"context_line":"            ftpcmd \u003d \u0027\u0027.join([\u0027RETR \u0027, name])"},{"line_number":133,"context_line":"            ftp.retrbinary(ftpcmd, ftpfile.write)"},{"line_number":134,"context_line":"        ftp.close()"},{"line_number":135,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9eU%3D","line":132,"updated":"2012-12-04 20:14:49.000000000","message":"same join vs. + question","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6c5aeca8ef36982a554dd0d1333d82fedf9dfdd","unresolved":false,"context_lines":[{"line_number":126,"context_line":"                         user\u003dconnection.username,"},{"line_number":127,"context_line":"                         passwd\u003dconnection.password)"},{"line_number":128,"context_line":"        ftp.cwd(os.path.dirname(remote_path))"},{"line_number":129,"context_line":"        name \u003d os.path.basename(remote_path)"},{"line_number":130,"context_line":"        LOG.debug(_(\"ftp GET %(remote_path)s to: %(local_path)s\") % locals())"},{"line_number":131,"context_line":"        with open(local_path, \u0027w\u0027) as ftpfile:"},{"line_number":132,"context_line":"            ftpcmd \u003d \u0027RETR %s\u0027 % name"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FwWM%3D","line":129,"updated":"2013-01-07 15:25:35.000000000","message":"Neither of these OS abstractions will yield a working implementation on Windows, right? Since the os.path abstractions are only valid for the local system, and we know we\u0027re administrating a UNIX remote, we should provide a specific implementation of these two functions, IMHO.","commit_id":"68b58c43d6970e0ee06f7de3f2698bf6480a35c7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a364177ae1774a9424e6b949a82be409cd023f1","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                         user\u003dconnection.username,"},{"line_number":128,"context_line":"                         passwd\u003dconnection.password)"},{"line_number":129,"context_line":"        ftp.cwd(os.path.dirname(remote_path))"},{"line_number":130,"context_line":"        name \u003d os.path.basename(remote_path)"},{"line_number":131,"context_line":"        LOG.debug(_(\"ftp GET %(remote_path)s to: %(local_path)s\") % locals())"},{"line_number":132,"context_line":"        with open(local_path, \u0027w\u0027) as ftpfile:"},{"line_number":133,"context_line":"            ftpcmd \u003d \u0027RETR %s\u0027 % name"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAM3%2F%2Fvpg%3D","line":130,"updated":"2013-01-07 22:20:43.000000000","message":"You didn\u0027t fix this.","commit_id":"7f238a6e231d21c44e2c0e725c4c5bd3da5f1e60"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6cfced3162530eaa035c904ffbde8c3018cd23e4","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                         user\u003dconnection.username,"},{"line_number":128,"context_line":"                         passwd\u003dconnection.password)"},{"line_number":129,"context_line":"        ftp.cwd(os.path.dirname(remote_path))"},{"line_number":130,"context_line":"        name \u003d os.path.basename(remote_path)"},{"line_number":131,"context_line":"        LOG.debug(_(\"ftp GET %(remote_path)s to: %(local_path)s\") % locals())"},{"line_number":132,"context_line":"        with open(local_path, \u0027w\u0027) as ftpfile:"},{"line_number":133,"context_line":"            ftpcmd \u003d \u0027RETR %s\u0027 % name"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAM3%2F%2Fvms%3D","line":130,"in_reply_to":"AAAAM3%2F%2Fvm8%3D","updated":"2013-01-07 22:47:59.000000000","message":"Alright, fair enough, thanks!","commit_id":"7f238a6e231d21c44e2c0e725c4c5bd3da5f1e60"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"2b3ab767b5b1c9fe421dd8a5009a5ba233bab4c8","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                         user\u003dconnection.username,"},{"line_number":128,"context_line":"                         passwd\u003dconnection.password)"},{"line_number":129,"context_line":"        ftp.cwd(os.path.dirname(remote_path))"},{"line_number":130,"context_line":"        name \u003d os.path.basename(remote_path)"},{"line_number":131,"context_line":"        LOG.debug(_(\"ftp GET %(remote_path)s to: %(local_path)s\") % locals())"},{"line_number":132,"context_line":"        with open(local_path, \u0027w\u0027) as ftpfile:"},{"line_number":133,"context_line":"            ftpcmd \u003d \u0027RETR %s\u0027 % name"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAM3%2F%2Fvm8%3D","line":130,"in_reply_to":"AAAAM3%2F%2Fvpg%3D","updated":"2013-01-07 22:47:07.000000000","message":"Turns out after some testing on Windows systems, os.path.dirname and os.path.basename are os.path.sep agnostic. So even running on windows \"/some/long/file/path.py\" will return as \"/some/long/file\" when using os.path.dirname. Same goes for using os.path.basename().","commit_id":"7f238a6e231d21c44e2c0e725c4c5bd3da5f1e60"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a364177ae1774a9424e6b949a82be409cd023f1","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                source_path\u003dremote_path, dest_path\u003dlocal_path)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"def aix_path_join(path_one, path_two):"},{"line_number":143,"context_line":"    \"\"\"Ensures file path is built correctly for remote UNIX system"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    :param path_one: string of the first file path"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAM3%2F%2Fvpc%3D","line":142,"updated":"2013-01-07 22:20:43.000000000","message":"This is fine, but I didn\u0027t think it needed to be done, I just think you need to use this above.","commit_id":"7f238a6e231d21c44e2c0e725c4c5bd3da5f1e60"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"2b3ab767b5b1c9fe421dd8a5009a5ba233bab4c8","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                source_path\u003dremote_path, dest_path\u003dlocal_path)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"def aix_path_join(path_one, path_two):"},{"line_number":143,"context_line":"    \"\"\"Ensures file path is built correctly for remote UNIX system"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    :param path_one: string of the first file path"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAM3%2F%2FvnE%3D","line":142,"in_reply_to":"AAAAM3%2F%2Fvpc%3D","updated":"2013-01-07 22:47:07.000000000","message":"So we us this utility to make sure both paths are joined properly for the remote system and that \u0027final_path\u0027 is returned as a uniform filepath name for the remote system.","commit_id":"7f238a6e231d21c44e2c0e725c4c5bd3da5f1e60"}],"nova/virt/powervm/driver.py":[{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"210070add2a48bffc3c84d86d6a44117df5f5548","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        self._powervm.capture_image(context, instance, image_id,"},{"line_number":159,"context_line":"                                    new_snapshot_meta)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        LOG.info(_(\"Snapshot done in %s seconds\") %"},{"line_number":162,"context_line":"                 (time.time() - snapshot_start),"},{"line_number":163,"context_line":"                 instance\u003dinstance)"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9eQ%3D","line":161,"updated":"2012-12-04 20:14:49.000000000","message":"you really want this as an info log level? If so it seems like you at least want the image name in here, otherwise I\u0027m not sure there will be much value in it.","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1378661ca867f4f3d607cece5f88402c65383509","unresolved":false,"context_lines":[{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        # disk capture and glance upload"},{"line_number":158,"context_line":"        self._powervm.capture_image(context, instance, image_id,"},{"line_number":159,"context_line":"                                    new_snapshot_meta)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        snapshot_time \u003d time.time() - snapshot_start"},{"line_number":162,"context_line":"        inst_name \u003d instance[\u0027name\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMX%2F%2F8rY%3D","line":159,"updated":"2012-12-05 15:10:25.000000000","message":"So, by my reading of capture_image(), if someone does a snapshot on a VM, the entire nova-compute service is blocked for some number of minutes and completely unable to do anything else. I.e. however long it takes to shut down the instance, grab the snapshot, copy it to the host, compress it, md5sum it, etc.\n\nSeems..like..bad to me. Am I missing something?","commit_id":"94d8604160b5e17f588404760db176456bb8dbe8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6c5aeca8ef36982a554dd0d1333d82fedf9dfdd","unresolved":false,"context_lines":[{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        # disk capture and glance upload"},{"line_number":158,"context_line":"        self._powervm.capture_image(context, instance, image_id,"},{"line_number":159,"context_line":"                                    new_snapshot_meta)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        snapshot_time \u003d time.time() - snapshot_start"},{"line_number":162,"context_line":"        inst_name \u003d instance[\u0027name\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FwWI%3D","line":159,"updated":"2013-01-07 15:25:35.000000000","message":"This is your call, but it sure seems like the generic operation of uploading to glance should happen in driver.py and not in the deeper layer.","commit_id":"68b58c43d6970e0ee06f7de3f2698bf6480a35c7"}],"nova/virt/powervm/operator.py":[{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"210070add2a48bffc3c84d86d6a44117df5f5548","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        :param image_id: uuid of pre-created snapshot image"},{"line_number":296,"context_line":"        :param image_meta: metadata to upload with captured image"},{"line_number":297,"context_line":"        \"\"\""},{"line_number":298,"context_line":"        lpar_obj \u003d self._operator.get_lpar(instance[\u0027name\u0027])"},{"line_number":299,"context_line":"        previous_state \u003d lpar_obj[\u0027state\u0027]"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        # stop the instance if it is running"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9eE%3D","line":298,"updated":"2012-12-04 20:14:49.000000000","message":"remove the _obj notation, typically not used in the rest of the project.","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"210070add2a48bffc3c84d86d6a44117df5f5548","unresolved":false,"context_lines":[{"line_number":299,"context_line":"        previous_state \u003d lpar_obj[\u0027state\u0027]"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        # stop the instance if it is running"},{"line_number":302,"context_line":"        if (previous_state \u003d\u003d \u0027Running\u0027):"},{"line_number":303,"context_line":"            LOG.debug(_(\"Stopping instance %s for snapshot.\") %"},{"line_number":304,"context_line":"                      instance[\u0027name\u0027])"},{"line_number":305,"context_line":"            # wait up to 2 minutes for shutdown"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9eM%3D","line":302,"updated":"2012-12-04 20:14:49.000000000","message":"parens not needed","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"210070add2a48bffc3c84d86d6a44117df5f5548","unresolved":false,"context_lines":[{"line_number":314,"context_line":"                disk_name, context, image_id, image_meta)"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        # restart instance if it was running before"},{"line_number":317,"context_line":"        if (previous_state \u003d\u003d \u0027Running\u0027):"},{"line_number":318,"context_line":"            self.power_on(instance[\u0027name\u0027])"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def _cleanup(self, instance_name):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMX%2F%2F9eI%3D","line":317,"updated":"2012-12-04 20:14:49.000000000","message":"parens not needed","commit_id":"e8970ae28d81aa3e0a3507ca59a36a3d89407590"}]}