)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"5110d32dea6977b35d556fce307a64519fc5fa00","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Gleb Stepanov \u003cgstepanov@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-02-26 18:14:05 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Console log copy for block live migratiob"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Fix problem of console log file lost during"},{"line_number":10,"context_line":"block live migration by copying this file"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9aed3d3a_5e16d7f1","line":7,"range":{"start_line":7,"start_character":32,"end_line":7,"end_character":41},"updated":"2016-02-26 20:56:35.000000000","message":"nit: migratiob -\u003e migration","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ff8bc0d9a5021e33d192e0b3e44301878de1dc08","unresolved":false,"context_lines":[{"line_number":15549,"context_line":"        mock_log_chown.return_value \u003d console_log"},{"line_number":15550,"context_line":"        mock_rename.return_value \u003d \"\""},{"line_number":15551,"context_line":""},{"line_number":15552,"context_line":"        class FakeInstance(object):"},{"line_number":15553,"context_line":"            pass"},{"line_number":15554,"context_line":""},{"line_number":15555,"context_line":"        instance \u003d FakeInstance()"},{"line_number":15556,"context_line":"        self.drvr._restore_log(instance)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_60c7ee12","line":15553,"range":{"start_line":15552,"start_character":8,"end_line":15553,"end_character":16},"updated":"2016-03-11 21:27:10.000000000","message":"Please no, we have fake nova.objects.Instance objects for stuff like this, see how self.test_instance (or fake_instance) is used throughout this test module.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"19410d0cc7c7cf2a922152106bf5445fb70b50bc","unresolved":false,"context_lines":[{"line_number":15549,"context_line":"        mock_log_chown.return_value \u003d console_log"},{"line_number":15550,"context_line":"        mock_rename.return_value \u003d \"\""},{"line_number":15551,"context_line":""},{"line_number":15552,"context_line":"        class FakeInstance(object):"},{"line_number":15553,"context_line":"            pass"},{"line_number":15554,"context_line":""},{"line_number":15555,"context_line":"        instance \u003d FakeInstance()"},{"line_number":15556,"context_line":"        self.drvr._restore_log(instance)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_9b7884c6","line":15553,"range":{"start_line":15552,"start_character":8,"end_line":15553,"end_character":16},"in_reply_to":"3afc51ec_60c7ee12","updated":"2016-03-14 08:36:23.000000000","message":"ok","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"23207bbe58558c5e8d7aa1abf2df08b9c78a5bf0","unresolved":false,"context_lines":[{"line_number":6887,"context_line":"                                  instance.image_meta,"},{"line_number":6888,"context_line":"                                  block_device_info\u003dblock_device_info,"},{"line_number":6889,"context_line":"                                  write_to_disk\u003dTrue)"},{"line_number":6890,"context_line":"        if block_migration:"},{"line_number":6891,"context_line":"            # Restore log file from source."},{"line_number":6892,"context_line":"            self._restore_log(instance)"},{"line_number":6893,"context_line":"        self._host.write_instance_config(xml)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bae84128_095ef844","line":6890,"updated":"2016-02-25 15:01:42.000000000","message":"We might allow live migration to broken host and it may  lead to an exception in _get_guest_xml. Therefore I\u0027m thinking whether we should put this in final block","commit_id":"8d6f10cf8edd087b957e7664d62adb4f4b7f03e3"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"23207bbe58558c5e8d7aa1abf2df08b9c78a5bf0","unresolved":false,"context_lines":[{"line_number":6896,"context_line":"        \"\"\"Restore log file received from source"},{"line_number":6897,"context_line":"        node and swap it with newly created console.log."},{"line_number":6898,"context_line":""},{"line_number":6899,"context_line":"        :param instance:"},{"line_number":6900,"context_line":"        :return:"},{"line_number":6901,"context_line":"        \"\"\""},{"line_number":6902,"context_line":"        console_file \u003d self._get_console_log_path(instance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bae84128_697d04e7","line":6899,"range":{"start_line":6899,"start_character":8,"end_line":6899,"end_character":24},"updated":"2016-02-25 15:01:42.000000000","message":"You are missing description here","commit_id":"8d6f10cf8edd087b957e7664d62adb4f4b7f03e3"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"23207bbe58558c5e8d7aa1abf2df08b9c78a5bf0","unresolved":false,"context_lines":[{"line_number":6897,"context_line":"        node and swap it with newly created console.log."},{"line_number":6898,"context_line":""},{"line_number":6899,"context_line":"        :param instance:"},{"line_number":6900,"context_line":"        :return:"},{"line_number":6901,"context_line":"        \"\"\""},{"line_number":6902,"context_line":"        console_file \u003d self._get_console_log_path(instance)"},{"line_number":6903,"context_line":"        console_file_copy \u003d \"{console_file}{suffix}\".format(**{"}],"source_content_type":"text/x-python","patch_set":2,"id":"bae84128_895fa874","line":6900,"range":{"start_line":6900,"start_character":8,"end_line":6900,"end_character":16},"updated":"2016-02-25 15:01:42.000000000","message":"And you can remove this line, because this function does not return anything","commit_id":"8d6f10cf8edd087b957e7664d62adb4f4b7f03e3"},{"author":{"_account_id":3068,"name":"Arata Notsu","email":"notsu@virtualtech.jp","username":"arata776"},"change_message_id":"8814d558fbbad1ad7ddc6fa2b5f3fb7e80432902","unresolved":false,"context_lines":[{"line_number":340,"context_line":"    \"aarch64\": \"/usr/share/AAVMF/AAVMF_CODE.fd\""},{"line_number":341,"context_line":"}"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"MAX_CONSOLE_BYTES \u003d 100 * units.Ki"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"# The libvirt driver will prefix any disable reason codes with this string."},{"line_number":346,"context_line":"DISABLE_PREFIX \u003d \u0027AUTO: \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9aed3d3a_6590c2ed","line":343,"range":{"start_line":343,"start_character":0,"end_line":343,"end_character":34},"updated":"2016-02-25 17:33:20.000000000","message":"here","commit_id":"e27279686121173024314611ac3c2e1d79fa07ed"},{"author":{"_account_id":3068,"name":"Arata Notsu","email":"notsu@virtualtech.jp","username":"arata776"},"change_message_id":"8814d558fbbad1ad7ddc6fa2b5f3fb7e80432902","unresolved":false,"context_lines":[{"line_number":2831,"context_line":"                libvirt_utils.chown(path, os.getuid())"},{"line_number":2832,"context_line":""},{"line_number":2833,"context_line":"                with libvirt_utils.file_open(path, \u0027rb\u0027) as fp:"},{"line_number":2834,"context_line":"                    log_data, remaining \u003d utils.last_bytes(fp,"},{"line_number":2835,"context_line":"                                                           MAX_CONSOLE_BYTES)"},{"line_number":2836,"context_line":"                    if remaining \u003e 0:"},{"line_number":2837,"context_line":"                        LOG.info(_LI(\u0027Truncated console log returned, \u0027"},{"line_number":2838,"context_line":"                                     \u0027%d bytes ignored\u0027), remaining,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9aed3d3a_45777ed4","line":2835,"range":{"start_line":2834,"start_character":20,"end_line":2835,"end_character":77},"updated":"2016-02-25 17:33:20.000000000","message":"here","commit_id":"e27279686121173024314611ac3c2e1d79fa07ed"},{"author":{"_account_id":3068,"name":"Arata Notsu","email":"notsu@virtualtech.jp","username":"arata776"},"change_message_id":"8814d558fbbad1ad7ddc6fa2b5f3fb7e80432902","unresolved":false,"context_lines":[{"line_number":5981,"context_line":"                        \"console_file\": console_file,"},{"line_number":5982,"context_line":"                        \"suffix\": \".copy\"})"},{"line_number":5983,"context_line":"            try:"},{"line_number":5984,"context_line":"                self._remotefs.copy_file(console_file, dest_path)"},{"line_number":5985,"context_line":"            except processutils.ProcessExecutionError as e:"},{"line_number":5986,"context_line":"                LOG.exception(_LE(\u0027Trying to copy file %(path) from \u0027"},{"line_number":5987,"context_line":"                                  \u0027%(host) failed \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9aed3d3a_059f3604","line":5984,"range":{"start_line":5984,"start_character":16,"end_line":5984,"end_character":65},"updated":"2016-02-25 17:33:20.000000000","message":"I hope you will consider dealing with the fact that console.log can become super large due to https://bugs.launchpad.net/nova/+bug/832507. In such case, simply copying console.log results in consumption of a huge amount of bandwidth and time.\n\nI guess this method will work: check the size of the file and if it is unacceptably large, copy only the last MAX_CONSOLE_BYTES bytes of the file. We can do such truncation safely because it is invisible from users, as only the last MAX_CONSOLE_BYTES bytes are available via the console-log API (please see the lines I marked \u0027here\u0027).","commit_id":"e27279686121173024314611ac3c2e1d79fa07ed"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"e674597ae888e2d6295a6f951685009a12151720","unresolved":false,"context_lines":[{"line_number":5981,"context_line":"                        \"console_file\": console_file,"},{"line_number":5982,"context_line":"                        \"suffix\": \".copy\"})"},{"line_number":5983,"context_line":"            try:"},{"line_number":5984,"context_line":"                self._remotefs.copy_file(console_file, dest_path)"},{"line_number":5985,"context_line":"            except processutils.ProcessExecutionError as e:"},{"line_number":5986,"context_line":"                LOG.exception(_LE(\u0027Trying to copy file %(path) from \u0027"},{"line_number":5987,"context_line":"                                  \u0027%(host) failed \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9aed3d3a_e802e76e","line":5984,"range":{"start_line":5984,"start_character":16,"end_line":5984,"end_character":65},"in_reply_to":"9aed3d3a_059f3604","updated":"2016-02-26 09:57:00.000000000","message":"Thanks, i will add this check.","commit_id":"e27279686121173024314611ac3c2e1d79fa07ed"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"dc8f8ae4228cddaee544b8afff0c2cfa3467e2a0","unresolved":false,"context_lines":[{"line_number":5989,"context_line":"                                     \u0027%d bytes ignored\u0027),"},{"line_number":5990,"context_line":"                                 remaining, instance\u003dinstance)"},{"line_number":5991,"context_line":"                    # Write truncated console.log file data to copy."},{"line_number":5992,"context_line":"                    libvirt_utils.write_to_file(console_file_copy, log_data)"},{"line_number":5993,"context_line":""},{"line_number":5994,"context_line":"                path_on_destination \u003d \"%(dest)s:%(path)s\". \\"},{"line_number":5995,"context_line":"                    format(dest\u003ddest,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aed3d3a_998ff822","line":5992,"updated":"2016-02-26 12:57:07.000000000","message":"Looks like you don\u0027t remove this file if live migration fails for any reason or i\u0027m missing something","commit_id":"db851130443a04d2c1bf5d48b7ea308358c9c798"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"a40bc57bccb99aa6fa2840be2e11b943842ee077","unresolved":false,"context_lines":[{"line_number":5989,"context_line":"                                     \u0027%d bytes ignored\u0027),"},{"line_number":5990,"context_line":"                                 remaining, instance\u003dinstance)"},{"line_number":5991,"context_line":"                    # Write truncated console.log file data to copy."},{"line_number":5992,"context_line":"                    libvirt_utils.write_to_file(console_file_copy, log_data)"},{"line_number":5993,"context_line":""},{"line_number":5994,"context_line":"                path_on_destination \u003d \"%(dest)s:%(path)s\". \\"},{"line_number":5995,"context_line":"                    format(dest\u003ddest,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aed3d3a_db8690b8","line":5992,"in_reply_to":"9aed3d3a_0f555a4f","updated":"2016-02-26 14:23:26.000000000","message":"Hmm, we don\u0027t have rollback_live_migration_at_source here in driver, but it looks like we will need because of this logic.\n\nOr we won\u0027t if others agree that we don\u0027t need to remove .copy file, but it is a garbage.","commit_id":"db851130443a04d2c1bf5d48b7ea308358c9c798"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"2e9c2a10a9ae16f0f10b82b4f906b0f62a58dfdb","unresolved":false,"context_lines":[{"line_number":5989,"context_line":"                                     \u0027%d bytes ignored\u0027),"},{"line_number":5990,"context_line":"                                 remaining, instance\u003dinstance)"},{"line_number":5991,"context_line":"                    # Write truncated console.log file data to copy."},{"line_number":5992,"context_line":"                    libvirt_utils.write_to_file(console_file_copy, log_data)"},{"line_number":5993,"context_line":""},{"line_number":5994,"context_line":"                path_on_destination \u003d \"%(dest)s:%(path)s\". \\"},{"line_number":5995,"context_line":"                    format(dest\u003ddest,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aed3d3a_0f555a4f","line":5992,"in_reply_to":"9aed3d3a_998ff822","updated":"2016-02-26 13:28:23.000000000","message":"I think that may be useful, but i\u0027m not sure where should i pu this code.","commit_id":"db851130443a04d2c1bf5d48b7ea308358c9c798"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"bfb032c5cb7323615a0616f36d3fccff2c075c4d","unresolved":false,"context_lines":[{"line_number":5989,"context_line":"                                     \u0027%d bytes ignored\u0027),"},{"line_number":5990,"context_line":"                                 remaining, instance\u003dinstance)"},{"line_number":5991,"context_line":"                    # Write truncated console.log file data to copy."},{"line_number":5992,"context_line":"                    libvirt_utils.write_to_file(console_file_copy, log_data)"},{"line_number":5993,"context_line":""},{"line_number":5994,"context_line":"                path_on_destination \u003d \"%(dest)s:%(path)s\". \\"},{"line_number":5995,"context_line":"                    format(dest\u003ddest,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9aed3d3a_1138d49f","line":5992,"in_reply_to":"9aed3d3a_db8690b8","updated":"2016-02-26 15:52:25.000000000","message":"i will try to add copy removal to _live_migration_operation in finally statement, so we can consider it like rollback_live_migration_at_source if something inside live migration operation fails.","commit_id":"db851130443a04d2c1bf5d48b7ea308358c9c798"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"d4532a69cc42049e98d93cac340646acfc37e7c1","unresolved":false,"context_lines":[{"line_number":6000,"context_line":"                    self._remotefs.copy_file(console_file_copy,"},{"line_number":6001,"context_line":"                                             path_on_destination)"},{"line_number":6002,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":6003,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path) from \u0027"},{"line_number":6004,"context_line":"                                      \u0027%(host) failed \u0027"},{"line_number":6005,"context_line":"                                      \u0027with %(exc)\u0027),"},{"line_number":6006,"context_line":"                                      {\"path\": console_file,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_c6c90fae","line":6003,"range":{"start_line":6003,"start_character":59,"end_line":6003,"end_character":66},"updated":"2016-03-01 05:31:12.000000000","message":"The conversion type is required.\nFor example, %(path)s","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"696a5bb909505f6606180c3ec206b278663f9dfa","unresolved":false,"context_lines":[{"line_number":6000,"context_line":"                    self._remotefs.copy_file(console_file_copy,"},{"line_number":6001,"context_line":"                                             path_on_destination)"},{"line_number":6002,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":6003,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path) from \u0027"},{"line_number":6004,"context_line":"                                      \u0027%(host) failed \u0027"},{"line_number":6005,"context_line":"                                      \u0027with %(exc)\u0027),"},{"line_number":6006,"context_line":"                                      {\"path\": console_file,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_e2beb6ac","line":6003,"range":{"start_line":6003,"start_character":59,"end_line":6003,"end_character":66},"in_reply_to":"9aed3d3a_c6c90fae","updated":"2016-03-01 09:35:53.000000000","message":"Ok","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"d4532a69cc42049e98d93cac340646acfc37e7c1","unresolved":false,"context_lines":[{"line_number":6001,"context_line":"                                             path_on_destination)"},{"line_number":6002,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":6003,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path) from \u0027"},{"line_number":6004,"context_line":"                                      \u0027%(host) failed \u0027"},{"line_number":6005,"context_line":"                                      \u0027with %(exc)\u0027),"},{"line_number":6006,"context_line":"                                      {\"path\": console_file,"},{"line_number":6007,"context_line":"                                       \"host\": instance.host,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_a6cccbbb","line":6004,"range":{"start_line":6004,"start_character":39,"end_line":6004,"end_character":46},"updated":"2016-03-01 05:31:12.000000000","message":"ditto","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"696a5bb909505f6606180c3ec206b278663f9dfa","unresolved":false,"context_lines":[{"line_number":6001,"context_line":"                                             path_on_destination)"},{"line_number":6002,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":6003,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path) from \u0027"},{"line_number":6004,"context_line":"                                      \u0027%(host) failed \u0027"},{"line_number":6005,"context_line":"                                      \u0027with %(exc)\u0027),"},{"line_number":6006,"context_line":"                                      {\"path\": console_file,"},{"line_number":6007,"context_line":"                                       \"host\": instance.host,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_c2d452ed","line":6004,"range":{"start_line":6004,"start_character":39,"end_line":6004,"end_character":46},"in_reply_to":"9aed3d3a_a6cccbbb","updated":"2016-03-01 09:35:53.000000000","message":"ok","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"d4532a69cc42049e98d93cac340646acfc37e7c1","unresolved":false,"context_lines":[{"line_number":6002,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":6003,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path) from \u0027"},{"line_number":6004,"context_line":"                                      \u0027%(host) failed \u0027"},{"line_number":6005,"context_line":"                                      \u0027with %(exc)\u0027),"},{"line_number":6006,"context_line":"                                      {\"path\": console_file,"},{"line_number":6007,"context_line":"                                       \"host\": instance.host,"},{"line_number":6008,"context_line":"                                       \"exc\": e})"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_06c817a5","line":6005,"range":{"start_line":6005,"start_character":44,"end_line":6005,"end_character":50},"updated":"2016-03-01 05:31:12.000000000","message":"ditto","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"696a5bb909505f6606180c3ec206b278663f9dfa","unresolved":false,"context_lines":[{"line_number":6002,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":6003,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path) from \u0027"},{"line_number":6004,"context_line":"                                      \u0027%(host) failed \u0027"},{"line_number":6005,"context_line":"                                      \u0027with %(exc)\u0027),"},{"line_number":6006,"context_line":"                                      {\"path\": console_file,"},{"line_number":6007,"context_line":"                                       \"host\": instance.host,"},{"line_number":6008,"context_line":"                                       \"exc\": e})"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_4207a281","line":6005,"range":{"start_line":6005,"start_character":44,"end_line":6005,"end_character":50},"in_reply_to":"9aed3d3a_06c817a5","updated":"2016-03-01 09:35:53.000000000","message":"ok","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"5110d32dea6977b35d556fce307a64519fc5fa00","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":6092,"context_line":"                LOG.error(_LE(\"Live Migration failure: %s\"), e,"},{"line_number":6093,"context_line":"                          instance\u003dinstance)"},{"line_number":6094,"context_line":"        finally:"},{"line_number":6095,"context_line":"            try:"},{"line_number":6096,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6097,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_1e3baf39","line":6094,"updated":"2016-02-26 20:56:35.000000000","message":"It seems like this finally block could appear above with the other code that is added? Why do it here instead?","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"73f6dac749e1669c3925c1af0bb7fd9fccd51a65","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":6092,"context_line":"                LOG.error(_LE(\"Live Migration failure: %s\"), e,"},{"line_number":6093,"context_line":"                          instance\u003dinstance)"},{"line_number":6094,"context_line":"        finally:"},{"line_number":6095,"context_line":"            try:"},{"line_number":6096,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6097,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7af24918_7b8df87a","line":6094,"in_reply_to":"7af24918_d387d610","updated":"2016-03-01 13:26:27.000000000","message":"Thanks, i was a bit confused about that, i think implement log copy removal on source in finally block is most appropriate.","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"98dbc88cc3e98adb3760f6f2a49aa2ff6f325a11","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":6092,"context_line":"                LOG.error(_LE(\"Live Migration failure: %s\"), e,"},{"line_number":6093,"context_line":"                          instance\u003dinstance)"},{"line_number":6094,"context_line":"        finally:"},{"line_number":6095,"context_line":"            try:"},{"line_number":6096,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6097,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_421379b8","line":6094,"in_reply_to":"9aed3d3a_1e3baf39","updated":"2016-02-28 18:49:30.000000000","message":"I\u0027m not sure about it. In case on-going live migration is aborted this code will not be executed. It should be part of rollback logic","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"696a5bb909505f6606180c3ec206b278663f9dfa","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":6092,"context_line":"                LOG.error(_LE(\"Live Migration failure: %s\"), e,"},{"line_number":6093,"context_line":"                          instance\u003dinstance)"},{"line_number":6094,"context_line":"        finally:"},{"line_number":6095,"context_line":"            try:"},{"line_number":6096,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6097,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_a23dce2c","line":6094,"in_reply_to":"9aed3d3a_1e3baf39","updated":"2016-03-01 09:35:53.000000000","message":"TO be sure that console_log copy will be eventually removed.","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"0b6e85af998d57b8fcbcbcc87426c88c56e08c70","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":6092,"context_line":"                LOG.error(_LE(\"Live Migration failure: %s\"), e,"},{"line_number":6093,"context_line":"                          instance\u003dinstance)"},{"line_number":6094,"context_line":"        finally:"},{"line_number":6095,"context_line":"            try:"},{"line_number":6096,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6097,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_8c476676","line":6094,"in_reply_to":"9aed3d3a_1e3baf39","updated":"2016-02-29 09:32:31.000000000","message":"this code is placed here because console_log_copy should be deleted any way if migration fails.","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"0b6e85af998d57b8fcbcbcc87426c88c56e08c70","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":6092,"context_line":"                LOG.error(_LE(\"Live Migration failure: %s\"), e,"},{"line_number":6093,"context_line":"                          instance\u003dinstance)"},{"line_number":6094,"context_line":"        finally:"},{"line_number":6095,"context_line":"            try:"},{"line_number":6096,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6097,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_e73c4dca","line":6094,"in_reply_to":"9aed3d3a_421379b8","updated":"2016-02-29 09:32:31.000000000","message":"Ok, i will research how to implement such logic for rollbacking migration on source node.But what do you think about if i will remove console_log_copy right after it is copied in line 6002?","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"696a5bb909505f6606180c3ec206b278663f9dfa","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":6092,"context_line":"                LOG.error(_LE(\"Live Migration failure: %s\"), e,"},{"line_number":6093,"context_line":"                          instance\u003dinstance)"},{"line_number":6094,"context_line":"        finally:"},{"line_number":6095,"context_line":"            try:"},{"line_number":6096,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6097,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_cd176343","line":6094,"in_reply_to":"9aed3d3a_c315af49","updated":"2016-03-01 09:35:53.000000000","message":"So would it be a good idea to implement rollback on source logic in separate review?","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"2338f2c9b3fc5bcba730f1ddbcffa510ef9b7151","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":6092,"context_line":"                LOG.error(_LE(\"Live Migration failure: %s\"), e,"},{"line_number":6093,"context_line":"                          instance\u003dinstance)"},{"line_number":6094,"context_line":"        finally:"},{"line_number":6095,"context_line":"            try:"},{"line_number":6096,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6097,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7af24918_d387d610","line":6094,"in_reply_to":"9aed3d3a_cd176343","updated":"2016-03-01 12:32:24.000000000","message":"Sorry, I just realized that on source you remove this file just after it\u0027s transferred to destination. And rollback_on_destination will remove it, because it removes whole instance dir. So it\u0027s fine without implementing it","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"d381070dfd617fe332de9dd6e22a6b6d9f15bb78","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":6092,"context_line":"                LOG.error(_LE(\"Live Migration failure: %s\"), e,"},{"line_number":6093,"context_line":"                          instance\u003dinstance)"},{"line_number":6094,"context_line":"        finally:"},{"line_number":6095,"context_line":"            try:"},{"line_number":6096,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6097,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_c315af49","line":6094,"in_reply_to":"9aed3d3a_e73c4dca","updated":"2016-02-29 13:51:10.000000000","message":"I think it\u0027s fine to have it there, this code can fail in multiple places. However, i\u0027d make sure before removing this file that it exists. Once rollback method is added it might be racy.","commit_id":"520fcda70116f7d92e2f3858a4b5167de8f78bcb"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"37c552c2fe7aecc87b6ca32728a98c801746e7d6","unresolved":false,"context_lines":[{"line_number":5999,"context_line":"                               path\u003dconsole_file_copy)"},{"line_number":6000,"context_line":""},{"line_number":6001,"context_line":"                    self._remotefs.copy_file(console_file_copy,"},{"line_number":6002,"context_line":"                                         path_on_destination)"},{"line_number":6003,"context_line":"                    try:"},{"line_number":6004,"context_line":"                        libvirt_utils.file_delete(console_file_copy)"},{"line_number":6005,"context_line":"                    except Exception:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7af24918_b3d7aa22","line":6002,"range":{"start_line":6002,"start_character":41,"end_line":6002,"end_character":61},"updated":"2016-03-01 12:32:40.000000000","message":"nit, indentation","commit_id":"f6ae0c7ced9ef1cd8dca2e402052bdb893356d56"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"56744241e44fc97aca54302c381c5113e300a07c","unresolved":false,"context_lines":[{"line_number":5999,"context_line":"                               path\u003dconsole_file_copy)"},{"line_number":6000,"context_line":""},{"line_number":6001,"context_line":"                    self._remotefs.copy_file(console_file_copy,"},{"line_number":6002,"context_line":"                                         path_on_destination)"},{"line_number":6003,"context_line":"                    try:"},{"line_number":6004,"context_line":"                        libvirt_utils.file_delete(console_file_copy)"},{"line_number":6005,"context_line":"                    except Exception:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7af24918_1b579403","line":6002,"range":{"start_line":6002,"start_character":41,"end_line":6002,"end_character":61},"in_reply_to":"7af24918_b3d7aa22","updated":"2016-03-01 13:26:12.000000000","message":"Yep","commit_id":"f6ae0c7ced9ef1cd8dca2e402052bdb893356d56"},{"author":{"_account_id":3068,"name":"Arata Notsu","email":"notsu@virtualtech.jp","username":"arata776"},"change_message_id":"bb6d3138e8f01b81aac27c38e997310e1bcca094","unresolved":false,"context_lines":[{"line_number":5991,"context_line":"                                         \u0027%d bytes ignored\u0027),"},{"line_number":5992,"context_line":"                                     remaining, instance\u003dinstance)"},{"line_number":5993,"context_line":"                        # Write truncated console.log file data to copy."},{"line_number":5994,"context_line":"                        libvirt_utils.write_to_file(console_file_copy,"},{"line_number":5995,"context_line":"                                                    log_data)"},{"line_number":5996,"context_line":""},{"line_number":5997,"context_line":"                    path_on_destination \u003d \"%(dest)s:%(path)s\". \\"},{"line_number":5998,"context_line":"                        format(dest\u003ddest,"},{"line_number":5999,"context_line":"                               path\u003dconsole_file_copy)"},{"line_number":6000,"context_line":""},{"line_number":6001,"context_line":"                    self._remotefs.copy_file(console_file_copy,"},{"line_number":6002,"context_line":"                                         path_on_destination)"},{"line_number":6003,"context_line":"                    try:"},{"line_number":6004,"context_line":"                        libvirt_utils.file_delete(console_file_copy)"},{"line_number":6005,"context_line":"                    except Exception:"},{"line_number":6006,"context_line":"                        pass"},{"line_number":6007,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":6008,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path)s from \u0027"},{"line_number":6009,"context_line":"                                      \u0027%(host)s failed \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"7af24918_583f6525","line":6006,"range":{"start_line":5994,"start_character":0,"end_line":6006,"end_character":28},"updated":"2016-03-01 12:27:59.000000000","message":"Can we delete the truncated file in a finally block? And, it seems good to log something if an exception occurs.\n\n    libvirt_utils.write_to_file(console_file_copy, log_data)\n    try:\n        path_on_destination \u003d ...\n        self._remotefs.copy_file(console_file_copy, path_on_destination)\n    finally:\n        try:\n            libvirt_utils.file_delete(console_file_copy)\n        except Exception:\n            LOG.warning(...)","commit_id":"f6ae0c7ced9ef1cd8dca2e402052bdb893356d56"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"56744241e44fc97aca54302c381c5113e300a07c","unresolved":false,"context_lines":[{"line_number":5991,"context_line":"                                         \u0027%d bytes ignored\u0027),"},{"line_number":5992,"context_line":"                                     remaining, instance\u003dinstance)"},{"line_number":5993,"context_line":"                        # Write truncated console.log file data to copy."},{"line_number":5994,"context_line":"                        libvirt_utils.write_to_file(console_file_copy,"},{"line_number":5995,"context_line":"                                                    log_data)"},{"line_number":5996,"context_line":""},{"line_number":5997,"context_line":"                    path_on_destination \u003d \"%(dest)s:%(path)s\". \\"},{"line_number":5998,"context_line":"                        format(dest\u003ddest,"},{"line_number":5999,"context_line":"                               path\u003dconsole_file_copy)"},{"line_number":6000,"context_line":""},{"line_number":6001,"context_line":"                    self._remotefs.copy_file(console_file_copy,"},{"line_number":6002,"context_line":"                                         path_on_destination)"},{"line_number":6003,"context_line":"                    try:"},{"line_number":6004,"context_line":"                        libvirt_utils.file_delete(console_file_copy)"},{"line_number":6005,"context_line":"                    except Exception:"},{"line_number":6006,"context_line":"                        pass"},{"line_number":6007,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":6008,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path)s from \u0027"},{"line_number":6009,"context_line":"                                      \u0027%(host)s failed \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"7af24918_887eb8ac","line":6006,"range":{"start_line":5994,"start_character":0,"end_line":6006,"end_character":28},"in_reply_to":"7af24918_583f6525","updated":"2016-03-01 13:26:12.000000000","message":"I\u0027ve tried to do that in finally block in the end of the function, this solution had a few comments, it seems that it is needed to implement separate rollback_on_source function.","commit_id":"f6ae0c7ced9ef1cd8dca2e402052bdb893356d56"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"da85192f2c551a548c5ffacf4ec1c56755480702","unresolved":false,"context_lines":[{"line_number":5991,"context_line":"                                         \u0027%d bytes ignored\u0027),"},{"line_number":5992,"context_line":"                                     remaining, instance\u003dinstance)"},{"line_number":5993,"context_line":"                        # Write truncated console.log file data to copy."},{"line_number":5994,"context_line":"                        libvirt_utils.write_to_file(console_file_copy,"},{"line_number":5995,"context_line":"                                                    log_data)"},{"line_number":5996,"context_line":""},{"line_number":5997,"context_line":"                    path_on_destination \u003d \"%(dest)s:%(path)s\". \\"},{"line_number":5998,"context_line":"                        format(dest\u003ddest,"},{"line_number":5999,"context_line":"                               path\u003dconsole_file_copy)"},{"line_number":6000,"context_line":""},{"line_number":6001,"context_line":"                    self._remotefs.copy_file(console_file_copy,"},{"line_number":6002,"context_line":"                                         path_on_destination)"},{"line_number":6003,"context_line":"                    try:"},{"line_number":6004,"context_line":"                        libvirt_utils.file_delete(console_file_copy)"},{"line_number":6005,"context_line":"                    except Exception:"},{"line_number":6006,"context_line":"                        pass"},{"line_number":6007,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":6008,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path)s from \u0027"},{"line_number":6009,"context_line":"                                      \u0027%(host)s failed \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"7af24918_beedae27","line":6006,"range":{"start_line":5994,"start_character":0,"end_line":6006,"end_character":28},"in_reply_to":"7af24918_887eb8ac","updated":"2016-03-01 13:27:45.000000000","message":"Forget my comment about rollback_on_source, it\u0027s completely not needed, because you already remove this file on source once it is transferred.","commit_id":"f6ae0c7ced9ef1cd8dca2e402052bdb893356d56"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"c2e432d9358399be3e115a49c3e7951f5cd192aa","unresolved":false,"context_lines":[{"line_number":5983,"context_line":"                    suffix\u003d\".copy\")"},{"line_number":5984,"context_line":"                try:"},{"line_number":5985,"context_line":"                    with libvirt_utils.file_open(console_file, \u0027rb\u0027) as fp:"},{"line_number":5986,"context_line":"                        log_data, remaining \u003d \\"},{"line_number":5987,"context_line":"                            utils.last_bytes(fp,"},{"line_number":5988,"context_line":"                                             MAX_CONSOLE_BYTES)"},{"line_number":5989,"context_line":"                        if remaining \u003e 0:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7af24918_3409bd2d","line":5986,"updated":"2016-03-01 13:51:05.000000000","message":"You can apply the same here","commit_id":"2c4ceb78f0082286641325d5459ed01d36771197"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"2996883c3d06515ba2985c1980b1be0400b9571c","unresolved":false,"context_lines":[{"line_number":5983,"context_line":"                    suffix\u003d\".copy\")"},{"line_number":5984,"context_line":"                try:"},{"line_number":5985,"context_line":"                    with libvirt_utils.file_open(console_file, \u0027rb\u0027) as fp:"},{"line_number":5986,"context_line":"                        log_data, remaining \u003d \\"},{"line_number":5987,"context_line":"                            utils.last_bytes(fp,"},{"line_number":5988,"context_line":"                                             MAX_CONSOLE_BYTES)"},{"line_number":5989,"context_line":"                        if remaining \u003e 0:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7af24918_34353d46","line":5986,"in_reply_to":"7af24918_3409bd2d","updated":"2016-03-01 14:02:16.000000000","message":"ok","commit_id":"2c4ceb78f0082286641325d5459ed01d36771197"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"5336fdb352c1eb5e9c5dee6e2d482d34800f6384","unresolved":false,"context_lines":[{"line_number":5994,"context_line":"                        libvirt_utils.write_to_file(console_file_copy,"},{"line_number":5995,"context_line":"                                                    log_data)"},{"line_number":5996,"context_line":""},{"line_number":5997,"context_line":"                    path_on_destination \u003d \"%(dest)s:%(path)s\". \\"},{"line_number":5998,"context_line":"                        format(dest\u003ddest,"},{"line_number":5999,"context_line":"                               path\u003dconsole_file_copy)"},{"line_number":6000,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7af24918_193afc36","line":5997,"updated":"2016-03-01 13:50:26.000000000","message":"To avoid breaking line here you can do:\n\"%(dest)s:%(path)s\".format(\ndest\u003ddest, path\u003dconsole_file_copy)","commit_id":"2c4ceb78f0082286641325d5459ed01d36771197"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"2996883c3d06515ba2985c1980b1be0400b9571c","unresolved":false,"context_lines":[{"line_number":5994,"context_line":"                        libvirt_utils.write_to_file(console_file_copy,"},{"line_number":5995,"context_line":"                                                    log_data)"},{"line_number":5996,"context_line":""},{"line_number":5997,"context_line":"                    path_on_destination \u003d \"%(dest)s:%(path)s\". \\"},{"line_number":5998,"context_line":"                        format(dest\u003ddest,"},{"line_number":5999,"context_line":"                               path\u003dconsole_file_copy)"},{"line_number":6000,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7af24918_b4494dd3","line":5997,"in_reply_to":"7af24918_193afc36","updated":"2016-03-01 14:02:16.000000000","message":"ok","commit_id":"2c4ceb78f0082286641325d5459ed01d36771197"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"72477905a29e20c04cdb4f50edf8c4f4cbfe4065","unresolved":false,"context_lines":[{"line_number":5968,"context_line":"        try:"},{"line_number":5969,"context_line":"            if block_migration:"},{"line_number":5970,"context_line":"                migration_flags \u003d self._block_migration_flags"},{"line_number":5971,"context_line":"                self._chown_console_log_for_instance(instance)"},{"line_number":5972,"context_line":"                console_file \u003d self._get_console_log_path(instance)"},{"line_number":5973,"context_line":"                console_file_copy \u003d \"{console_file}{suffix}\".format("},{"line_number":5974,"context_line":"                    console_file\u003dconsole_file,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_e0ddbe71","line":5971,"updated":"2016-03-11 21:26:15.000000000","message":"Can we please move this entirely new block of code into a new private method so we\u0027re not blowing up the size of the method we\u0027re in? Something like _live_migration_copy_console_log?\n\nAlso, from what I can tell, none of this code is tested.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"19410d0cc7c7cf2a922152106bf5445fb70b50bc","unresolved":false,"context_lines":[{"line_number":5968,"context_line":"        try:"},{"line_number":5969,"context_line":"            if block_migration:"},{"line_number":5970,"context_line":"                migration_flags \u003d self._block_migration_flags"},{"line_number":5971,"context_line":"                self._chown_console_log_for_instance(instance)"},{"line_number":5972,"context_line":"                console_file \u003d self._get_console_log_path(instance)"},{"line_number":5973,"context_line":"                console_file_copy \u003d \"{console_file}{suffix}\".format("},{"line_number":5974,"context_line":"                    console_file\u003dconsole_file,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_1b059479","line":5971,"in_reply_to":"3afc51ec_e0ddbe71","updated":"2016-03-14 08:36:23.000000000","message":"Yes, thanks. I think it will be reasonable to write separate method and separate tests.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"72477905a29e20c04cdb4f50edf8c4f4cbfe4065","unresolved":false,"context_lines":[{"line_number":5993,"context_line":"                    self._remotefs.copy_file(console_file_copy,"},{"line_number":5994,"context_line":"                                             path_on_destination)"},{"line_number":5995,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":5996,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path)s from \u0027"},{"line_number":5997,"context_line":"                                      \u0027%(host)s failed \u0027"},{"line_number":5998,"context_line":"                                      \u0027with %(exc)s\u0027),"},{"line_number":5999,"context_line":"                                      {\"path\": console_file,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_40d59241","line":5996,"updated":"2016-03-11 21:26:15.000000000","message":"Is this an expected/acceptable failure? If so, it shouldn\u0027t stacktrace an error level log.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"19410d0cc7c7cf2a922152106bf5445fb70b50bc","unresolved":false,"context_lines":[{"line_number":5993,"context_line":"                    self._remotefs.copy_file(console_file_copy,"},{"line_number":5994,"context_line":"                                             path_on_destination)"},{"line_number":5995,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":5996,"context_line":"                    LOG.exception(_LE(\u0027Trying to copy file %(path)s from \u0027"},{"line_number":5997,"context_line":"                                      \u0027%(host)s failed \u0027"},{"line_number":5998,"context_line":"                                      \u0027with %(exc)s\u0027),"},{"line_number":5999,"context_line":"                                      {\"path\": console_file,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_db784cf5","line":5996,"in_reply_to":"3afc51ec_40d59241","updated":"2016-03-14 08:36:23.000000000","message":"This is expectable failure so, maybe you are right. So there should be different type of logging?","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"72477905a29e20c04cdb4f50edf8c4f4cbfe4065","unresolved":false,"context_lines":[{"line_number":6086,"context_line":"                          instance\u003dinstance)"},{"line_number":6087,"context_line":"        finally:"},{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_4063d2b1","line":6089,"range":{"start_line":6089,"start_character":42,"end_line":6089,"end_character":59},"updated":"2016-03-11 21:26:15.000000000","message":"This variable doesn\u0027t exist if not block_migration, so you should at least declare it outside the if block_migration block that creates it, or check if block_migration before trying to delete with a variable that\u0027s not in scope.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"19410d0cc7c7cf2a922152106bf5445fb70b50bc","unresolved":false,"context_lines":[{"line_number":6086,"context_line":"                          instance\u003dinstance)"},{"line_number":6087,"context_line":"        finally:"},{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_1b9634f0","line":6089,"range":{"start_line":6089,"start_character":42,"end_line":6089,"end_character":59},"in_reply_to":"3afc51ec_4063d2b1","updated":"2016-03-14 08:36:23.000000000","message":"Ok.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"92ba4b0146673eac4990feb5ace842dd20226eac","unresolved":false,"context_lines":[{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""},{"line_number":6093,"context_line":"        # If \u0027migrateToURI\u0027 fails we don\u0027t know what state the"},{"line_number":6094,"context_line":"        # VM instances on each host are in. Possibilities include"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aef4532_38acdd9f","line":6091,"updated":"2016-03-08 07:13:29.000000000","message":"I don\u0027t think silently dropping the exceptions is a good idea. If issues arise with the file copies there\u0027s no way to find out about it. Perhaps use INFO or DEBUG level for this?","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"19410d0cc7c7cf2a922152106bf5445fb70b50bc","unresolved":false,"context_lines":[{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""},{"line_number":6093,"context_line":"        # If \u0027migrateToURI\u0027 fails we don\u0027t know what state the"},{"line_number":6094,"context_line":"        # VM instances on each host are in. Possibilities include"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_5ba0bc43","line":6091,"in_reply_to":"3afc51ec_20ab261f","updated":"2016-03-14 08:36:23.000000000","message":"Ok","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"0a0463e30d3a9354b26dd758805d777dbfa11850","unresolved":false,"context_lines":[{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""},{"line_number":6093,"context_line":"        # If \u0027migrateToURI\u0027 fails we don\u0027t know what state the"},{"line_number":6094,"context_line":"        # VM instances on each host are in. Possibilities include"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aef4532_d6a7fca4","line":6091,"in_reply_to":"5aef4532_02677e4d","updated":"2016-03-10 13:04:38.000000000","message":"Yeah, but why it nonwritable? console_file_copy is created by nova process and owned by stack user. So it is always writable and deletable.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"d474312f99d6deb132434f9e548d204144226e5e","unresolved":false,"context_lines":[{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""},{"line_number":6093,"context_line":"        # If \u0027migrateToURI\u0027 fails we don\u0027t know what state the"},{"line_number":6094,"context_line":"        # VM instances on each host are in. Possibilities include"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aef4532_53ea2cad","line":6091,"in_reply_to":"5aef4532_38acdd9f","updated":"2016-03-09 09:59:29.000000000","message":"Yep, but there are so many places in nova when exceptions\nare passed silently. And information that nova cannot delete redundant console_file_copy would not affect neither cloud user nor cloud operator.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"6c02a61214936c39743924f9113711f20d82cf41","unresolved":false,"context_lines":[{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""},{"line_number":6093,"context_line":"        # If \u0027migrateToURI\u0027 fails we don\u0027t know what state the"},{"line_number":6094,"context_line":"        # VM instances on each host are in. Possibilities include"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aef4532_8280d8f8","line":6091,"in_reply_to":"5aef4532_4201b004","updated":"2016-03-09 11:14:08.000000000","message":"Thanks, but i can\u0027t get what next time do you mean?\nconsole_file_copy is created once for being copyed to destination.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"3ed0e207627cc0cc05ec4b0a281b28991d5b639b","unresolved":false,"context_lines":[{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""},{"line_number":6093,"context_line":"        # If \u0027migrateToURI\u0027 fails we don\u0027t know what state the"},{"line_number":6094,"context_line":"        # VM instances on each host are in. Possibilities include"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aef4532_4201b004","line":6091,"in_reply_to":"5aef4532_53ea2cad","updated":"2016-03-09 11:08:07.000000000","message":"I fear things will hit the wall not upon failing deletion but upon trying to access it next time. :-D\nHaving said that, if that approach is ok with the rest of Nova i\u0027ll remove the -1.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2a5d52f5c3688cc47dae7bd1773c0634a6408b29","unresolved":false,"context_lines":[{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""},{"line_number":6093,"context_line":"        # If \u0027migrateToURI\u0027 fails we don\u0027t know what state the"},{"line_number":6094,"context_line":"        # VM instances on each host are in. Possibilities include"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aef4532_02677e4d","line":6091,"in_reply_to":"5aef4532_8280d8f8","updated":"2016-03-09 20:56:09.000000000","message":"A follow up migration might try to write to the path where an old, e.g. non-writable console_file_copy, file remains is crossing my mind.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e1ebc2fda7e53fea11148b231f5ba189a6f7575b","unresolved":false,"context_lines":[{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""},{"line_number":6093,"context_line":"        # If \u0027migrateToURI\u0027 fails we don\u0027t know what state the"},{"line_number":6094,"context_line":"        # VM instances on each host are in. Possibilities include"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_20ab261f","line":6091,"in_reply_to":"5aef4532_a9be41ab","updated":"2016-03-11 21:29:21.000000000","message":"FTR, nova is not OK with silently ignoring failures, you should at least be logging at debug level if you fail to clean something up. If there are other places in nova that are silently ignoring failures, and aren\u0027t just using the exceptions as flow control, then those are wrong and should be cleaned up. Live migration is super complicated and easy to break, so making it harder to debug is not cool.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"35721babae3edac13492c496e3f33ff87af20c5e","unresolved":false,"context_lines":[{"line_number":6088,"context_line":"            try:"},{"line_number":6089,"context_line":"                libvirt_utils.file_delete(console_file_copy)"},{"line_number":6090,"context_line":"            except Exception:"},{"line_number":6091,"context_line":"                pass"},{"line_number":6092,"context_line":""},{"line_number":6093,"context_line":"        # If \u0027migrateToURI\u0027 fails we don\u0027t know what state the"},{"line_number":6094,"context_line":"        # VM instances on each host are in. Possibilities include"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aef4532_a9be41ab","line":6091,"in_reply_to":"5aef4532_d6a7fca4","updated":"2016-03-10 15:34:36.000000000","message":"That should be ok as long as no other process touches it.\nI\u0027m still in a story where libvirt chowns files in migration and probably a bit paranoid because of that.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"72477905a29e20c04cdb4f50edf8c4f4cbfe4065","unresolved":false,"context_lines":[{"line_number":6906,"context_line":"                # Restore log file from source. even if exception"},{"line_number":6907,"context_line":"                # occured in _get_guest_xml in case of"},{"line_number":6908,"context_line":"                # migration to broken host."},{"line_number":6909,"context_line":"                self._restore_log(instance)"},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"    def _restore_log(self, instance):"},{"line_number":6912,"context_line":"        \"\"\"Restore log file received from source"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_60dd2e14","line":6909,"updated":"2016-03-11 21:26:15.000000000","message":"This can fail, right? Like when doing chown? Point is, if this fails, it\u0027s going to mask whatever might fail from _get_guest_xml. We probably shouldn\u0027t let that happen.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"72477905a29e20c04cdb4f50edf8c4f4cbfe4065","unresolved":false,"context_lines":[{"line_number":6915,"context_line":"        :param instance: nova.objects.instance.Instance object"},{"line_number":6916,"context_line":"        \"\"\""},{"line_number":6917,"context_line":"        console_file \u003d self._get_console_log_path(instance)"},{"line_number":6918,"context_line":"        console_file_copy \u003d \"{console_file}{suffix}\".format(**{"},{"line_number":6919,"context_line":"            \"console_file\": console_file,"},{"line_number":6920,"context_line":"            \"suffix\": \".copy\"})"},{"line_number":6921,"context_line":"        self._chown_console_log_for_instance(instance)"},{"line_number":6922,"context_line":"        try:"},{"line_number":6923,"context_line":"            os.rename(console_file_copy, console_file)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_602b6eec","line":6920,"range":{"start_line":6918,"start_character":28,"end_line":6920,"end_character":31},"updated":"2016-03-11 21:26:15.000000000","message":"This is duplicated code, we should probably have a little private helper method for this so it can be re-used in both places.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"72477905a29e20c04cdb4f50edf8c4f4cbfe4065","unresolved":false,"context_lines":[{"line_number":6922,"context_line":"        try:"},{"line_number":6923,"context_line":"            os.rename(console_file_copy, console_file)"},{"line_number":6924,"context_line":"        except OSError as e:"},{"line_number":6925,"context_line":"            LOG.exception(_LE(\"Exception has been occured during \""},{"line_number":6926,"context_line":"                              \"file substitution %(ex)s\"),"},{"line_number":6927,"context_line":"                          ex\u003de)"},{"line_number":6928,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_a0abb688","line":6925,"range":{"start_line":6925,"start_character":41,"end_line":6925,"end_character":57},"updated":"2016-03-11 21:26:15.000000000","message":"s/has been occurred/occurred/\n\nSame question as before, but is this an expected/acceptable failure? Could the file not exist?","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"72477905a29e20c04cdb4f50edf8c4f4cbfe4065","unresolved":false,"context_lines":[{"line_number":6924,"context_line":"        except OSError as e:"},{"line_number":6925,"context_line":"            LOG.exception(_LE(\"Exception has been occured during \""},{"line_number":6926,"context_line":"                              \"file substitution %(ex)s\"),"},{"line_number":6927,"context_line":"                          ex\u003de)"},{"line_number":6928,"context_line":""},{"line_number":6929,"context_line":"    def _get_instance_disk_info(self, instance_name, xml,"},{"line_number":6930,"context_line":"                                block_device_info\u003dNone):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afc51ec_e07c9e22","line":6927,"updated":"2016-03-11 21:26:15.000000000","message":"You don\u0027t need this if you\u0027re using LOG.exception, but I\u0027m not sure we should be using LOG.exception.\n\nYou should, however, pass instance\u003dinstance to whatever log method so we have context.","commit_id":"fe485545f3f2cb351463fd59392413a423281e11"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"03a20d8cd0209e3fff27cdfd0fe584d0c5cc748d","unresolved":false,"context_lines":[{"line_number":5968,"context_line":"        try:"},{"line_number":5969,"context_line":"            if block_migration:"},{"line_number":5970,"context_line":"                migration_flags \u003d self._block_migration_flags"},{"line_number":5971,"context_line":"                self._copy_console_log(dest, instance)"},{"line_number":5972,"context_line":"            else:"},{"line_number":5973,"context_line":"                migration_flags \u003d self._live_migration_flags"},{"line_number":5974,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1af94dfe_51d7edbe","line":5971,"updated":"2016-03-16 17:51:03.000000000","message":"Are you intending to do a check for shared instance storage here? It\u0027s not the same as a shared instance path. For example, ceph wouldn\u0027t require block migration, but typically has non-shared instance storage. You can get this from migrate_data, assuming it\u0027s populated at this point in the flow (I haven\u0027t checked).","commit_id":"1dd46118ce44d6660dbdade68796385123e08ad1"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"69142c959f84ee777d696a4bff153d5435e5eb25","unresolved":false,"context_lines":[{"line_number":5968,"context_line":"        try:"},{"line_number":5969,"context_line":"            if block_migration:"},{"line_number":5970,"context_line":"                migration_flags \u003d self._block_migration_flags"},{"line_number":5971,"context_line":"                self._copy_console_log(dest, instance)"},{"line_number":5972,"context_line":"            else:"},{"line_number":5973,"context_line":"                migration_flags \u003d self._live_migration_flags"},{"line_number":5974,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1af94dfe_61167756","line":5971,"in_reply_to":"1af94dfe_51d7edbe","updated":"2016-03-17 10:40:02.000000000","message":"At this point the guest on the source is still running and quite possibly still writing to the console log file. So by copying it at this point you\u0027re going to be loosing any data written while migration is taking place.\n\nIt needs to be copied after the source QEMU is paused but before target QEMU starts. This is essentially impossible todo at the Nova level - it would need to be done by libvirt in co-ordination with QEMU","commit_id":"1dd46118ce44d6660dbdade68796385123e08ad1"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"03a20d8cd0209e3fff27cdfd0fe584d0c5cc748d","unresolved":false,"context_lines":[{"line_number":6099,"context_line":"                  instance\u003dinstance)"},{"line_number":6100,"context_line":""},{"line_number":6101,"context_line":"    def _copy_console_log(self, dest, instance):"},{"line_number":6102,"context_line":"        self._chown_console_log_for_instance(instance)"},{"line_number":6103,"context_line":"        console_file \u003d self._get_console_log_path(instance)"},{"line_number":6104,"context_line":"        console_file_copy \u003d self._get_console_log_copy_name(console_file)"},{"line_number":6105,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1af94dfe_c477bd9d","line":6102,"updated":"2016-03-16 17:51:03.000000000","message":"Remember this is running on the source. This shouldn\u0027t be required.","commit_id":"1dd46118ce44d6660dbdade68796385123e08ad1"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"03a20d8cd0209e3fff27cdfd0fe584d0c5cc748d","unresolved":false,"context_lines":[{"line_number":6101,"context_line":"    def _copy_console_log(self, dest, instance):"},{"line_number":6102,"context_line":"        self._chown_console_log_for_instance(instance)"},{"line_number":6103,"context_line":"        console_file \u003d self._get_console_log_path(instance)"},{"line_number":6104,"context_line":"        console_file_copy \u003d self._get_console_log_copy_name(console_file)"},{"line_number":6105,"context_line":""},{"line_number":6106,"context_line":"        try:"},{"line_number":6107,"context_line":"            with libvirt_utils.file_open(console_file, \u0027rb\u0027) as fp:"}],"source_content_type":"text/x-python","patch_set":11,"id":"1af94dfe_04d525b8","line":6104,"updated":"2016-03-16 17:51:03.000000000","message":"You should use a temporary file instead. It will also clean itself up afterwards, and does not suffer from potential races from concurrent operations[1].\n\n[1] Not that that\u0027s an issue here, but by not using a temporary file you require the reader to check.","commit_id":"1dd46118ce44d6660dbdade68796385123e08ad1"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"03a20d8cd0209e3fff27cdfd0fe584d0c5cc748d","unresolved":false,"context_lines":[{"line_number":6114,"context_line":"                             remaining, instance\u003dinstance)"},{"line_number":6115,"context_line":"                # Write truncated console.log file data to copy."},{"line_number":6116,"context_line":"                libvirt_utils.write_to_file(console_file_copy,"},{"line_number":6117,"context_line":"                                            log_data)"},{"line_number":6118,"context_line":""},{"line_number":6119,"context_line":"            path_on_destination \u003d \"%(dest)s:%(path)s\".format("},{"line_number":6120,"context_line":"                dest\u003ddest,"}],"source_content_type":"text/x-python","patch_set":11,"id":"1af94dfe_64034943","line":6117,"updated":"2016-03-16 17:51:03.000000000","message":"See above: write to a temp file.","commit_id":"1dd46118ce44d6660dbdade68796385123e08ad1"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"03a20d8cd0209e3fff27cdfd0fe584d0c5cc748d","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                                      write_to_disk\u003dTrue)"},{"line_number":6906,"context_line":"            self._host.write_instance_config(xml)"},{"line_number":6907,"context_line":"        finally:"},{"line_number":6908,"context_line":"            if block_migration:"},{"line_number":6909,"context_line":"                # Restore log file from source. even if exception"},{"line_number":6910,"context_line":"                # occured in _get_guest_xml in case of"},{"line_number":6911,"context_line":"                # migration to broken host."}],"source_content_type":"text/x-python","patch_set":11,"id":"1af94dfe_a4ab911c","line":6908,"updated":"2016-03-16 17:51:03.000000000","message":"Again, pretty sure you don\u0027t mean block_migration here.","commit_id":"1dd46118ce44d6660dbdade68796385123e08ad1"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"03a20d8cd0209e3fff27cdfd0fe584d0c5cc748d","unresolved":false,"context_lines":[{"line_number":6922,"context_line":"        \"\"\""},{"line_number":6923,"context_line":"        console_file \u003d self._get_console_log_path(instance)"},{"line_number":6924,"context_line":"        console_file_copy \u003d self._get_console_log_copy_name(console_file)"},{"line_number":6925,"context_line":"        self._chown_console_log_for_instance(instance)"},{"line_number":6926,"context_line":"        try:"},{"line_number":6927,"context_line":"            os.rename(console_file_copy, console_file)"},{"line_number":6928,"context_line":"        except OSError as e:"}],"source_content_type":"text/x-python","patch_set":11,"id":"1af94dfe_87ff77a9","line":6925,"updated":"2016-03-16 17:51:03.000000000","message":"This is just going to change the permissions on the existing console log, which we\u0027re about to replace. I don\u0027t think this is required here. It may be required later.","commit_id":"1dd46118ce44d6660dbdade68796385123e08ad1"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"03a20d8cd0209e3fff27cdfd0fe584d0c5cc748d","unresolved":false,"context_lines":[{"line_number":6924,"context_line":"        console_file_copy \u003d self._get_console_log_copy_name(console_file)"},{"line_number":6925,"context_line":"        self._chown_console_log_for_instance(instance)"},{"line_number":6926,"context_line":"        try:"},{"line_number":6927,"context_line":"            os.rename(console_file_copy, console_file)"},{"line_number":6928,"context_line":"        except OSError as e:"},{"line_number":6929,"context_line":"            LOG.debug(\"Exception has been occured during \""},{"line_number":6930,"context_line":"                      \"file %(console_file_copy)s substitution \","}],"source_content_type":"text/x-python","patch_set":11,"id":"1af94dfe_a759d35c","line":6927,"updated":"2016-03-16 17:51:03.000000000","message":"This looks wrong to me. I haven\u0027t tested this, but surely qemu will already have the old console log open, so it will continue writing to old file even after it has been replaced? I\u0027d expect you to have to send qemu a monitor command or something when doing this.","commit_id":"1dd46118ce44d6660dbdade68796385123e08ad1"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"69142c959f84ee777d696a4bff153d5435e5eb25","unresolved":false,"context_lines":[{"line_number":6924,"context_line":"        console_file_copy \u003d self._get_console_log_copy_name(console_file)"},{"line_number":6925,"context_line":"        self._chown_console_log_for_instance(instance)"},{"line_number":6926,"context_line":"        try:"},{"line_number":6927,"context_line":"            os.rename(console_file_copy, console_file)"},{"line_number":6928,"context_line":"        except OSError as e:"},{"line_number":6929,"context_line":"            LOG.debug(\"Exception has been occured during \""},{"line_number":6930,"context_line":"                      \"file %(console_file_copy)s substitution \","}],"source_content_type":"text/x-python","patch_set":11,"id":"1af94dfe_015a134f","line":6927,"in_reply_to":"1af94dfe_a759d35c","updated":"2016-03-17 10:40:02.000000000","message":"Yes indeed, QEMU (or virtlogd) on the target will just keep writing to its original open, but now deleted, file handle. So you\u0027ve just succeeded to copying across old data and preventing any access to newly written data.","commit_id":"1dd46118ce44d6660dbdade68796385123e08ad1"}]}