)]}'
{"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"9b24053fec41193aeb89ff4970add18a710b7530","unresolved":false,"context_lines":[{"line_number":14961,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._connect_volume\u0027)"},{"line_number":14962,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":14963,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.write_instance_config\u0027)"},{"line_number":14964,"context_line":"    def test_swap_volume_disconnect_new_volume_on_error(self, write_config,"},{"line_number":14965,"context_line":"            get_guest, connect_volume, get_volume_config, disconnect_volume,"},{"line_number":14966,"context_line":"            rebase):"},{"line_number":14967,"context_line":"        \"\"\"Assert that disconnect_volume is called for the new volume if an"}],"source_content_type":"text/x-python","patch_set":5,"id":"7ffa3b31_728b5567","line":14964,"updated":"2017-04-21 16:07:06.000000000","message":"We should also duplicate this test for an exception raised by abort_job.","commit_id":"709d619c75c6d97c80fd8e398e10217251ed8692"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"9b24053fec41193aeb89ff4970add18a710b7530","unresolved":false,"context_lines":[{"line_number":14977,"context_line":"                               \u0027serial\u0027: \u0027new-volume-id\u0027,"},{"line_number":14978,"context_line":"                               \u0027data\u0027: {\u0027device_path\u0027: \u0027/fake-new-volume\u0027,"},{"line_number":14979,"context_line":"                                        \u0027access_mode\u0027: \u0027rw\u0027}}"},{"line_number":14980,"context_line":"        mock_dom \u003d mock.MagicMock()"},{"line_number":14981,"context_line":"        guest \u003d libvirt_guest.Guest(mock_dom)"},{"line_number":14982,"context_line":"        mock_dom.XMLDesc.return_value \u003d \"\"\"\u003cdomain\u003e"},{"line_number":14983,"context_line":"          \u003cdevices\u003e"},{"line_number":14984,"context_line":"            \u003cdisk type\u003d\u0027file\u0027\u003e"},{"line_number":14985,"context_line":"                \u003csource file\u003d\u0027/fake-old-volume\u0027/\u003e"},{"line_number":14986,"context_line":"                \u003ctarget dev\u003d\u0027vdb\u0027 bus\u003d\u0027virtio\u0027/\u003e"},{"line_number":14987,"context_line":"            \u003c/disk\u003e"},{"line_number":14988,"context_line":"          \u003c/devices\u003e"},{"line_number":14989,"context_line":"        \u003c/domain\u003e"},{"line_number":14990,"context_line":"        \"\"\""},{"line_number":14991,"context_line":"        get_guest.return_value \u003d guest"},{"line_number":14992,"context_line":"        disk_info \u003d {\u0027bus\u0027: \u0027virtio\u0027, \u0027type\u0027: \u0027disk\u0027, \u0027dev\u0027: \u0027vdb\u0027}"},{"line_number":14993,"context_line":"        get_volume_config.return_value \u003d mock.MagicMock("},{"line_number":14994,"context_line":"                source_path\u003d\u0027/fake-path\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7ffa3b31_127d316d","line":14991,"range":{"start_line":14980,"start_character":0,"end_line":14991,"end_character":38},"updated":"2017-04-21 16:07:06.000000000","message":"None of this is really relevant to this test. Can we just mock whatever would read this?","commit_id":"709d619c75c6d97c80fd8e398e10217251ed8692"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ab4bc86c1585abff85053801eea781b6f770d891","unresolved":false,"context_lines":[{"line_number":14965,"context_line":"    def test_swap_volume_disconnect_new_volume_on_rebase_error(self,"},{"line_number":14966,"context_line":"            write_config, get_guest, get_disk, get_volume_config,"},{"line_number":14967,"context_line":"            connect_volume, disconnect_volume, rebase):"},{"line_number":14968,"context_line":"        \"\"\"Assert that disconnect_volume is called for the new volume if an"},{"line_number":14969,"context_line":"           error is encountered while rebasing"},{"line_number":14970,"context_line":"        \"\"\""},{"line_number":14971,"context_line":"        conn \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())"},{"line_number":14972,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":14973,"context_line":"        guest \u003d libvirt_guest.Guest(mock.MagicMock())"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ff73747_82815792","line":14970,"range":{"start_line":14968,"start_character":0,"end_line":14970,"end_character":11},"updated":"2017-04-26 10:17:27.000000000","message":"I like test docstrings. We should do more of these.","commit_id":"809065485c19fd535db6740bb21b867c41c008fe"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"740f7501363ef3533cfa49d5fbf51c0c5106993d","unresolved":false,"context_lines":[{"line_number":14984,"context_line":"                mock.sentinel.new_connection_info,"},{"line_number":14985,"context_line":"                {\u0027dev\u0027: \u0027vdb\u0027, \u0027type\u0027: \u0027disk\u0027, \u0027bus\u0027: \u0027virtio\u0027})"},{"line_number":14986,"context_line":"        disconnect_volume.assert_called_once_with("},{"line_number":14987,"context_line":"                mock.sentinel.new_connection_info, \u0027vdb\u0027)"},{"line_number":14988,"context_line":""},{"line_number":14989,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.guest.BlockDevice.is_job_complete\u0027)"},{"line_number":14990,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.guest.BlockDevice.abort_job\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ff73747_3e3b0a39","line":14987,"updated":"2017-04-24 13:44:51.000000000","message":"Prefer this with the sentinels, much easier to see what we\u0027re asserting.","commit_id":"809065485c19fd535db6740bb21b867c41c008fe"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"9a3a802a70f0904447a41ae0f9fc05cffe20bf42","unresolved":false,"context_lines":[{"line_number":1299,"context_line":"                # finished. libvirt bug #1119173"},{"line_number":1300,"context_line":"                while not dev.is_job_complete():"},{"line_number":1301,"context_line":"                    time.sleep(0.5)"},{"line_number":1302,"context_line":"                dev.resize(resize_to * units.Gi / units.Ki)"},{"line_number":1303,"context_line":"        finally:"},{"line_number":1304,"context_line":"            self._host.write_instance_config(xml)"},{"line_number":1305,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_3b87f364","line":1302,"updated":"2017-04-21 12:13:36.000000000","message":"What about a failure here?\n\nI think it would be most robust to abort the new volume on any error here.","commit_id":"b31a49df8878cd7389e879c15f4f10b8f919ac58"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"edcf577329ea6c13fc0801afc00264a8990a9d42","unresolved":false,"context_lines":[{"line_number":1299,"context_line":"                # finished. libvirt bug #1119173"},{"line_number":1300,"context_line":"                while not dev.is_job_complete():"},{"line_number":1301,"context_line":"                    time.sleep(0.5)"},{"line_number":1302,"context_line":"                dev.resize(resize_to * units.Gi / units.Ki)"},{"line_number":1303,"context_line":"        finally:"},{"line_number":1304,"context_line":"            self._host.write_instance_config(xml)"},{"line_number":1305,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_03d049c8","line":1302,"in_reply_to":"7ffa3b31_30d82d79","updated":"2017-04-21 14:08:51.000000000","message":"So it appears that the above polling of is_job_complete isn\u0027t required anymore as since 1.2.7 this has been treated as a sync call by libvirt (If I\u0027m reading the commit correctly). I\u0027ve removed it in the following commit :\n\nhttps://review.openstack.org/458854","commit_id":"b31a49df8878cd7389e879c15f4f10b8f919ac58"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"cc9fbea03dcc399ba0e413698ed10cc4da84fbd7","unresolved":false,"context_lines":[{"line_number":1299,"context_line":"                # finished. libvirt bug #1119173"},{"line_number":1300,"context_line":"                while not dev.is_job_complete():"},{"line_number":1301,"context_line":"                    time.sleep(0.5)"},{"line_number":1302,"context_line":"                dev.resize(resize_to * units.Gi / units.Ki)"},{"line_number":1303,"context_line":"        finally:"},{"line_number":1304,"context_line":"            self._host.write_instance_config(xml)"},{"line_number":1305,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_db47ef76","line":1302,"in_reply_to":"7ffa3b31_3b87f364","updated":"2017-04-21 12:52:52.000000000","message":"I wanted to follow up with an additional fix for failures during resize as we\u0027ve already pivoted at this point to the new volume and would need to find a way pivot back to the original before disconnecting.\n\nI can however extend the rebase try down to L1295 to catch everything up until that point.","commit_id":"b31a49df8878cd7389e879c15f4f10b8f919ac58"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"58cf80badc48dc46b1a50573e9e3cc559a9b6a6a","unresolved":false,"context_lines":[{"line_number":1299,"context_line":"                # finished. libvirt bug #1119173"},{"line_number":1300,"context_line":"                while not dev.is_job_complete():"},{"line_number":1301,"context_line":"                    time.sleep(0.5)"},{"line_number":1302,"context_line":"                dev.resize(resize_to * units.Gi / units.Ki)"},{"line_number":1303,"context_line":"        finally:"},{"line_number":1304,"context_line":"            self._host.write_instance_config(xml)"},{"line_number":1305,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_30d82d79","line":1302,"in_reply_to":"7ffa3b31_db47ef76","updated":"2017-04-21 13:32:58.000000000","message":"That would make sense, but if you intend to include the pivot itself you\u0027d need to go all the way up to line 1301.\n\nHow about adding a \u0027pivoted\u0027 field to the exception class which you set to True if raised after line 1301? Then you only need 1 try block and 1 exception, and you can still defer what to do about pivoted\u003dTrue for a later patch?","commit_id":"b31a49df8878cd7389e879c15f4f10b8f919ac58"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"9a3a802a70f0904447a41ae0f9fc05cffe20bf42","unresolved":false,"context_lines":[{"line_number":1329,"context_line":"        if not conf.source_path:"},{"line_number":1330,"context_line":"            self._disconnect_volume(new_connection_info, disk_dev)"},{"line_number":1331,"context_line":"            raise NotImplementedError(_(\"Swap only supports host devices\"))"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"        try:"},{"line_number":1334,"context_line":"            self._swap_volume(guest, disk_dev, conf.source_path, resize_to)"},{"line_number":1335,"context_line":"        except exception.VolumeRebaseFailed:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_3b3a5398","line":1332,"updated":"2017-04-21 12:13:36.000000000","message":"How about:\n\n  disconnect_info \u003d new_connection_info\n  try:\n    self._swap_volume(guest, disk_dev, conf.source_path, resize_to)\n    disconnect_info \u003d old_connection_info\n  finally:\n    self._disconnect_volume(disconnect_info, disk_dev)\n\nThen you wouldn\u0027t need the additional exception class, and any failure at all would disconnect the new one.","commit_id":"b31a49df8878cd7389e879c15f4f10b8f919ac58"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"cc9fbea03dcc399ba0e413698ed10cc4da84fbd7","unresolved":false,"context_lines":[{"line_number":1329,"context_line":"        if not conf.source_path:"},{"line_number":1330,"context_line":"            self._disconnect_volume(new_connection_info, disk_dev)"},{"line_number":1331,"context_line":"            raise NotImplementedError(_(\"Swap only supports host devices\"))"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"        try:"},{"line_number":1334,"context_line":"            self._swap_volume(guest, disk_dev, conf.source_path, resize_to)"},{"line_number":1335,"context_line":"        except exception.VolumeRebaseFailed:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_ed58ae31","line":1332,"in_reply_to":"7ffa3b31_3b3a5398","updated":"2017-04-21 12:52:52.000000000","message":"Yeah but again the additional rollback logic for resize failures is made easier with these exception classes IMHO.","commit_id":"b31a49df8878cd7389e879c15f4f10b8f919ac58"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"58cf80badc48dc46b1a50573e9e3cc559a9b6a6a","unresolved":false,"context_lines":[{"line_number":1329,"context_line":"        if not conf.source_path:"},{"line_number":1330,"context_line":"            self._disconnect_volume(new_connection_info, disk_dev)"},{"line_number":1331,"context_line":"            raise NotImplementedError(_(\"Swap only supports host devices\"))"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"        try:"},{"line_number":1334,"context_line":"            self._swap_volume(guest, disk_dev, conf.source_path, resize_to)"},{"line_number":1335,"context_line":"        except exception.VolumeRebaseFailed:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_50a4d9b9","line":1332,"in_reply_to":"7ffa3b31_ed58ae31","updated":"2017-04-21 13:32:58.000000000","message":"D\u0027oh! Of course.","commit_id":"b31a49df8878cd7389e879c15f4f10b8f919ac58"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ab4bc86c1585abff85053801eea781b6f770d891","unresolved":false,"context_lines":[{"line_number":1288,"context_line":""},{"line_number":1289,"context_line":"                dev.abort_job(pivot\u003dTrue)"},{"line_number":1290,"context_line":""},{"line_number":1291,"context_line":"            except Exception as exc:"},{"line_number":1292,"context_line":"                LOG.exception(\"Failure rebasing volume %(new_path)s on \""},{"line_number":1293,"context_line":"                    \"%(new_path)s.\", {\u0027new_path\u0027: new_path,"},{"line_number":1294,"context_line":"                                      \u0027old_path\u0027: disk_path})"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ff73747_62552b1d","line":1291,"range":{"start_line":1291,"start_character":19,"end_line":1291,"end_character":28},"updated":"2017-04-26 10:17:27.000000000","message":"nit: Isn\u0027t there a more specific class of exception we could capture?","commit_id":"809065485c19fd535db6740bb21b867c41c008fe"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b63ce0ad915d327214312185e36ef9ef99bc73a9","unresolved":false,"context_lines":[{"line_number":1289,"context_line":"                dev.abort_job(pivot\u003dTrue)"},{"line_number":1290,"context_line":""},{"line_number":1291,"context_line":"            except Exception as exc:"},{"line_number":1292,"context_line":"                LOG.exception(\"Failure rebasing volume %(new_path)s on \""},{"line_number":1293,"context_line":"                    \"%(new_path)s.\", {\u0027new_path\u0027: new_path,"},{"line_number":1294,"context_line":"                                      \u0027old_path\u0027: disk_path})"},{"line_number":1295,"context_line":"                raise exception.VolumeRebaseFailed(reason\u003dsix.text_type(exc))"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f1d235d_807d2c70","line":1292,"range":{"start_line":1292,"start_character":57,"end_line":1292,"end_character":65},"updated":"2017-06-29 16:19:21.000000000","message":"This should be old_path, *sigh*.","commit_id":"809065485c19fd535db6740bb21b867c41c008fe"}]}