)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9e17f066815508ea848db6fc055f125426af1984","unresolved":false,"context_lines":[{"line_number":23,"context_line":"that point if we should be able to quiesce but it failed, which"},{"line_number":24,"context_line":"is an error that should be investigated."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Also, the unused quiesce parameter is removed from the"},{"line_number":27,"context_line":"test_volume_snapshot_create_libgfapi test while in this code."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Change-Id: I685c592340dc8a3b7d32834a0114f2925dc4305c"},{"line_number":30,"context_line":"Closes-Bug: #1629371"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1a430d35_8d45f8b4","line":27,"range":{"start_line":26,"start_character":0,"end_line":27,"end_character":61},"updated":"2017-02-09 18:03:05.000000000","message":"Is this still valid?","commit_id":"e659a6e7cbb3078b47c0860465530eab05acf73e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"335a574897e1453aaebeb2e3401fbf96f031e6b3","unresolved":false,"context_lines":[{"line_number":23,"context_line":"that point if we should be able to quiesce but it failed, which"},{"line_number":24,"context_line":"is an error that should be investigated."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Also, the unused quiesce parameter is removed from the"},{"line_number":27,"context_line":"test_volume_snapshot_create_libgfapi test while in this code."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Change-Id: I685c592340dc8a3b7d32834a0114f2925dc4305c"},{"line_number":30,"context_line":"Closes-Bug: #1629371"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"fa31d9ce_6bdd64df","line":27,"range":{"start_line":26,"start_character":0,"end_line":27,"end_character":61},"in_reply_to":"1a430d35_8d45f8b4","updated":"2017-02-14 20:59:13.000000000","message":"Yeah:\n\nhttps://review.openstack.org/#/c/380433/3/nova/tests/unit/virt/libvirt/test_driver.py@17811\n\nRight?","commit_id":"e659a6e7cbb3078b47c0860465530eab05acf73e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9560faf0f127589204394da4c6e4e46e08520cee","unresolved":false,"context_lines":[{"line_number":23,"context_line":"that point if we should be able to quiesce but it failed, which"},{"line_number":24,"context_line":"is an error that should be investigated."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Also, the unused quiesce parameter is removed from the"},{"line_number":27,"context_line":"test_volume_snapshot_create_libgfapi test while in this code."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Change-Id: I685c592340dc8a3b7d32834a0114f2925dc4305c"},{"line_number":30,"context_line":"Closes-Bug: #1629371"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba2be162_4a1bc172","line":27,"range":{"start_line":26,"start_character":0,"end_line":27,"end_character":61},"in_reply_to":"fa31d9ce_6bdd64df","updated":"2017-03-01 13:19:32.000000000","message":"I read that as \u0027unused_quiesce\u0027, and couldn\u0027t find it when grepping. Cool cool","commit_id":"e659a6e7cbb3078b47c0860465530eab05acf73e"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2ee8b374120c1fa3552e928c022a4e2d983903c8","unresolved":false,"context_lines":[{"line_number":17202,"context_line":"        mock_refresh_connection_info.assert_called_once_with(self.c, instance,"},{"line_number":17203,"context_line":"            self.drvr._volume_api, self.drvr)"},{"line_number":17204,"context_line":""},{"line_number":17205,"context_line":"    def test_volume_snapshot_create(self, quiesce\u003dTrue, can_quiesce\u003dTrue,"},{"line_number":17206,"context_line":"                                    quiesce_required\u003dFalse):"},{"line_number":17207,"context_line":"        \"\"\"Test snapshot creation with file-based disk.\"\"\""},{"line_number":17208,"context_line":"        self.flags(instance_name_template\u003d\u0027instance-%s\u0027)"},{"line_number":17209,"context_line":"        self.mox.StubOutWithMock(self.drvr._host, \u0027get_domain\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_e47fecdd","line":17206,"range":{"start_line":17205,"start_character":0,"end_line":17206,"end_character":60},"updated":"2016-10-07 14:32:05.000000000","message":"nit: Could you make this a private function and add an explicit call below, i.e.\n\n    def _test_volume_snapshot_create(...\n\n    def test_volume_snapshot_create(...\n\n    def test_volume_snapshot_create_cannot_quiesce(self):\n\nJust makes this a little easier to read (why are we recalling a normal test)","commit_id":"63dc29cce647536701206b1929b6c7756d636552"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"068d65dc2d277f5d3fed805e8dfdd3c774ce5dcd","unresolved":false,"context_lines":[{"line_number":17202,"context_line":"        mock_refresh_connection_info.assert_called_once_with(self.c, instance,"},{"line_number":17203,"context_line":"            self.drvr._volume_api, self.drvr)"},{"line_number":17204,"context_line":""},{"line_number":17205,"context_line":"    def test_volume_snapshot_create(self, quiesce\u003dTrue, can_quiesce\u003dTrue,"},{"line_number":17206,"context_line":"                                    quiesce_required\u003dFalse):"},{"line_number":17207,"context_line":"        \"\"\"Test snapshot creation with file-based disk.\"\"\""},{"line_number":17208,"context_line":"        self.flags(instance_name_template\u003d\u0027instance-%s\u0027)"},{"line_number":17209,"context_line":"        self.mox.StubOutWithMock(self.drvr._host, \u0027get_domain\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_8cf7aef4","line":17206,"range":{"start_line":17205,"start_character":0,"end_line":17206,"end_character":60},"in_reply_to":"1a95cdbc_e47fecdd","updated":"2016-11-30 12:22:41.000000000","message":"+1, but its a nit.","commit_id":"63dc29cce647536701206b1929b6c7756d636552"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"67c24410355fec8ec1ce3832192f6a4497d15d28","unresolved":false,"context_lines":[{"line_number":17202,"context_line":"        mock_refresh_connection_info.assert_called_once_with(self.c, instance,"},{"line_number":17203,"context_line":"            self.drvr._volume_api, self.drvr)"},{"line_number":17204,"context_line":""},{"line_number":17205,"context_line":"    def test_volume_snapshot_create(self, quiesce\u003dTrue, can_quiesce\u003dTrue,"},{"line_number":17206,"context_line":"                                    quiesce_required\u003dFalse):"},{"line_number":17207,"context_line":"        \"\"\"Test snapshot creation with file-based disk.\"\"\""},{"line_number":17208,"context_line":"        self.flags(instance_name_template\u003d\u0027instance-%s\u0027)"},{"line_number":17209,"context_line":"        self.mox.StubOutWithMock(self.drvr._host, \u0027get_domain\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a430d35_a284b10a","line":17206,"range":{"start_line":17205,"start_character":0,"end_line":17206,"end_character":60},"in_reply_to":"1a95cdbc_e47fecdd","updated":"2017-02-08 19:56:22.000000000","message":"Done","commit_id":"63dc29cce647536701206b1929b6c7756d636552"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9e17f066815508ea848db6fc055f125426af1984","unresolved":false,"context_lines":[{"line_number":17873,"context_line":""},{"line_number":17874,"context_line":"        self.mox.VerifyAll()"},{"line_number":17875,"context_line":""},{"line_number":17876,"context_line":"    def test_volume_snapshot_create_cannot_quiesce(self):"},{"line_number":17877,"context_line":"        # We can\u0027t quiesce so we don\u0027t try."},{"line_number":17878,"context_line":"        self._test_volume_snapshot_create(can_quiesce\u003dFalse)"},{"line_number":17879,"context_line":""},{"line_number":17880,"context_line":"    def test_volume_snapshot_create_cannot_quiesce_quiesce_required(self):"},{"line_number":17881,"context_line":"        # We can\u0027t quiesce but it\u0027s required so we fail."},{"line_number":17882,"context_line":"        self._test_volume_snapshot_create(can_quiesce\u003dFalse,"},{"line_number":17883,"context_line":"                                          quiesce_required\u003dTrue)"},{"line_number":17884,"context_line":""},{"line_number":17885,"context_line":"    def test_volume_snapshot_create_can_quiesce_quiesce_required_fails(self):"},{"line_number":17886,"context_line":"        # We can quiesce but it fails and it\u0027s required so we fail."},{"line_number":17887,"context_line":"        self._test_volume_snapshot_create("},{"line_number":17888,"context_line":"            quiesce\u003dFalse, can_quiesce\u003dTrue, quiesce_required\u003dTrue)"},{"line_number":17889,"context_line":""},{"line_number":17890,"context_line":"    def test_volume_snapshot_create_noquiesce(self):"},{"line_number":17891,"context_line":"        # We can quiesce but it fails but it\u0027s not required so we don\u0027t fail."},{"line_number":17892,"context_line":"        self._test_volume_snapshot_create(quiesce\u003dFalse)"},{"line_number":17893,"context_line":""},{"line_number":17894,"context_line":"    def test_volume_snapshot_create_noquiesce_cannot_quiesce(self):"},{"line_number":17895,"context_line":"        # We can\u0027t quiesce so we don\u0027t try, and if we did we\u0027d fail."},{"line_number":17896,"context_line":"        self._test_volume_snapshot_create(quiesce\u003dFalse, can_quiesce\u003dFalse)"},{"line_number":17897,"context_line":""},{"line_number":17898,"context_line":"    @mock.patch.object(host.Host,"},{"line_number":17899,"context_line":"                       \u0027has_min_version\u0027, return_value\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_edfcbc74","line":17896,"range":{"start_line":17876,"start_character":0,"end_line":17896,"end_character":75},"updated":"2017-02-09 18:03:05.000000000","message":"nit: These look like prime \u0027ddt\u0027 targets, were you to respin","commit_id":"e659a6e7cbb3078b47c0860465530eab05acf73e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"335a574897e1453aaebeb2e3401fbf96f031e6b3","unresolved":false,"context_lines":[{"line_number":17873,"context_line":""},{"line_number":17874,"context_line":"        self.mox.VerifyAll()"},{"line_number":17875,"context_line":""},{"line_number":17876,"context_line":"    def test_volume_snapshot_create_cannot_quiesce(self):"},{"line_number":17877,"context_line":"        # We can\u0027t quiesce so we don\u0027t try."},{"line_number":17878,"context_line":"        self._test_volume_snapshot_create(can_quiesce\u003dFalse)"},{"line_number":17879,"context_line":""},{"line_number":17880,"context_line":"    def test_volume_snapshot_create_cannot_quiesce_quiesce_required(self):"},{"line_number":17881,"context_line":"        # We can\u0027t quiesce but it\u0027s required so we fail."},{"line_number":17882,"context_line":"        self._test_volume_snapshot_create(can_quiesce\u003dFalse,"},{"line_number":17883,"context_line":"                                          quiesce_required\u003dTrue)"},{"line_number":17884,"context_line":""},{"line_number":17885,"context_line":"    def test_volume_snapshot_create_can_quiesce_quiesce_required_fails(self):"},{"line_number":17886,"context_line":"        # We can quiesce but it fails and it\u0027s required so we fail."},{"line_number":17887,"context_line":"        self._test_volume_snapshot_create("},{"line_number":17888,"context_line":"            quiesce\u003dFalse, can_quiesce\u003dTrue, quiesce_required\u003dTrue)"},{"line_number":17889,"context_line":""},{"line_number":17890,"context_line":"    def test_volume_snapshot_create_noquiesce(self):"},{"line_number":17891,"context_line":"        # We can quiesce but it fails but it\u0027s not required so we don\u0027t fail."},{"line_number":17892,"context_line":"        self._test_volume_snapshot_create(quiesce\u003dFalse)"},{"line_number":17893,"context_line":""},{"line_number":17894,"context_line":"    def test_volume_snapshot_create_noquiesce_cannot_quiesce(self):"},{"line_number":17895,"context_line":"        # We can\u0027t quiesce so we don\u0027t try, and if we did we\u0027d fail."},{"line_number":17896,"context_line":"        self._test_volume_snapshot_create(quiesce\u003dFalse, can_quiesce\u003dFalse)"},{"line_number":17897,"context_line":""},{"line_number":17898,"context_line":"    @mock.patch.object(host.Host,"},{"line_number":17899,"context_line":"                       \u0027has_min_version\u0027, return_value\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa31d9ce_6b048449","line":17896,"range":{"start_line":17876,"start_character":0,"end_line":17896,"end_character":75},"in_reply_to":"1a430d35_edfcbc74","updated":"2017-02-14 20:59:13.000000000","message":"IMO these are confusing enough and ddt would just make them more confusing.","commit_id":"e659a6e7cbb3078b47c0860465530eab05acf73e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"535d74f0814b8ce109f503a2dfe6c35ba9b9839b","unresolved":false,"context_lines":[{"line_number":17873,"context_line":""},{"line_number":17874,"context_line":"        self.mox.VerifyAll()"},{"line_number":17875,"context_line":""},{"line_number":17876,"context_line":"    def test_volume_snapshot_create_cannot_quiesce(self):"},{"line_number":17877,"context_line":"        # We can\u0027t quiesce so we don\u0027t try."},{"line_number":17878,"context_line":"        self._test_volume_snapshot_create(can_quiesce\u003dFalse)"},{"line_number":17879,"context_line":""},{"line_number":17880,"context_line":"    def test_volume_snapshot_create_cannot_quiesce_quiesce_required(self):"},{"line_number":17881,"context_line":"        # We can\u0027t quiesce but it\u0027s required so we fail."},{"line_number":17882,"context_line":"        self._test_volume_snapshot_create(can_quiesce\u003dFalse,"},{"line_number":17883,"context_line":"                                          quiesce_required\u003dTrue)"},{"line_number":17884,"context_line":""},{"line_number":17885,"context_line":"    def test_volume_snapshot_create_can_quiesce_quiesce_required_fails(self):"},{"line_number":17886,"context_line":"        # We can quiesce but it fails and it\u0027s required so we fail."},{"line_number":17887,"context_line":"        self._test_volume_snapshot_create("},{"line_number":17888,"context_line":"            quiesce\u003dFalse, can_quiesce\u003dTrue, quiesce_required\u003dTrue)"},{"line_number":17889,"context_line":""},{"line_number":17890,"context_line":"    def test_volume_snapshot_create_noquiesce(self):"},{"line_number":17891,"context_line":"        # We can quiesce but it fails but it\u0027s not required so we don\u0027t fail."},{"line_number":17892,"context_line":"        self._test_volume_snapshot_create(quiesce\u003dFalse)"},{"line_number":17893,"context_line":""},{"line_number":17894,"context_line":"    def test_volume_snapshot_create_noquiesce_cannot_quiesce(self):"},{"line_number":17895,"context_line":"        # We can\u0027t quiesce so we don\u0027t try, and if we did we\u0027d fail."},{"line_number":17896,"context_line":"        self._test_volume_snapshot_create(quiesce\u003dFalse, can_quiesce\u003dFalse)"},{"line_number":17897,"context_line":""},{"line_number":17898,"context_line":"    @mock.patch.object(host.Host,"},{"line_number":17899,"context_line":"                       \u0027has_min_version\u0027, return_value\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_1b9be2ca","line":17896,"range":{"start_line":17876,"start_character":0,"end_line":17896,"end_character":75},"in_reply_to":"fa31d9ce_6b048449","updated":"2017-03-07 17:01:52.000000000","message":"I like the comments and names, they have value here.","commit_id":"e659a6e7cbb3078b47c0860465530eab05acf73e"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"068d65dc2d277f5d3fed805e8dfdd3c774ce5dcd","unresolved":false,"context_lines":[{"line_number":1834,"context_line":"            except libvirt.libvirtError:"},{"line_number":1835,"context_line":"                LOG.exception(_LE(\u0027Unable to create quiesced VM snapshot, \u0027"},{"line_number":1836,"context_line":"                                  \u0027attempting again with quiescing disabled.\u0027),"},{"line_number":1837,"context_line":"                              instance\u003dinstance)"},{"line_number":1838,"context_line":"        except (exception.InstanceQuiesceNotSupported,"},{"line_number":1839,"context_line":"                exception.QemuGuestAgentNotEnabled) as err:"},{"line_number":1840,"context_line":"            # If the image says that quiesce is required then we need to fail."}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_2c9a1ac6","line":1837,"updated":"2016-11-30 12:22:41.000000000","message":"Should we not also raise here when image_meta.properties.get(\u0027os_require_quiesce\u0027, False) \u003d\u003d True?","commit_id":"63dc29cce647536701206b1929b6c7756d636552"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e5dd0fab9b5b4459f6d70a43f9aca31479e272d0","unresolved":false,"context_lines":[{"line_number":1834,"context_line":"            except libvirt.libvirtError:"},{"line_number":1835,"context_line":"                LOG.exception(_LE(\u0027Unable to create quiesced VM snapshot, \u0027"},{"line_number":1836,"context_line":"                                  \u0027attempting again with quiescing disabled.\u0027),"},{"line_number":1837,"context_line":"                              instance\u003dinstance)"},{"line_number":1838,"context_line":"        except (exception.InstanceQuiesceNotSupported,"},{"line_number":1839,"context_line":"                exception.QemuGuestAgentNotEnabled) as err:"},{"line_number":1840,"context_line":"            # If the image says that quiesce is required then we need to fail."}],"source_content_type":"text/x-python","patch_set":2,"id":"1a430d35_a7468378","line":1837,"in_reply_to":"5a74a57a_2c9a1ac6","updated":"2017-02-08 19:41:10.000000000","message":"Oh so we *can* quiesce because the image has the guest agent but snapshot fails with quiesce\u003dTrue...so if the image says to require quiesce, we should fail. That makes sense. The snapshot could have failed for some other reason, but I suppose it still makes sense to fail rather than continue.","commit_id":"63dc29cce647536701206b1929b6c7756d636552"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2ee8b374120c1fa3552e928c022a4e2d983903c8","unresolved":false,"context_lines":[{"line_number":1837,"context_line":"                              instance\u003dinstance)"},{"line_number":1838,"context_line":"        except (exception.InstanceQuiesceNotSupported,"},{"line_number":1839,"context_line":"                exception.QemuGuestAgentNotEnabled) as err:"},{"line_number":1840,"context_line":"            # If the image says that quiesce is required then we need to fail."},{"line_number":1841,"context_line":"            if image_meta.properties.get(\u0027os_require_quiesce\u0027, False):"},{"line_number":1842,"context_line":"                raise"},{"line_number":1843,"context_line":"            LOG.info(_LI(\u0027Skipping quiescing instance: %(reason)s.\u0027),"},{"line_number":1844,"context_line":"                     {\u0027reason\u0027: err}, instance\u003dinstance)"},{"line_number":1845,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_64dafce9","line":1842,"range":{"start_line":1840,"start_character":0,"end_line":1842,"end_character":21},"updated":"2016-10-07 14:32:05.000000000","message":"This sounds like it should have been a different bug","commit_id":"63dc29cce647536701206b1929b6c7756d636552"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c15d2d09eca06d23202ec468236b24450f80223a","unresolved":false,"context_lines":[{"line_number":1837,"context_line":"                              instance\u003dinstance)"},{"line_number":1838,"context_line":"        except (exception.InstanceQuiesceNotSupported,"},{"line_number":1839,"context_line":"                exception.QemuGuestAgentNotEnabled) as err:"},{"line_number":1840,"context_line":"            # If the image says that quiesce is required then we need to fail."},{"line_number":1841,"context_line":"            if image_meta.properties.get(\u0027os_require_quiesce\u0027, False):"},{"line_number":1842,"context_line":"                raise"},{"line_number":1843,"context_line":"            LOG.info(_LI(\u0027Skipping quiescing instance: %(reason)s.\u0027),"},{"line_number":1844,"context_line":"                     {\u0027reason\u0027: err}, instance\u003dinstance)"},{"line_number":1845,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_f1aac318","line":1842,"range":{"start_line":1840,"start_character":0,"end_line":1842,"end_character":21},"in_reply_to":"1a95cdbc_64dafce9","updated":"2016-10-11 16:06:11.000000000","message":"Yeah, it is separate, but I threw it in here while I was doing this as this error handling is done elsewhere for the same type of failure. I could split this out into a separate patch on top if people find it confusing or trying to do too many things in a single change.","commit_id":"63dc29cce647536701206b1929b6c7756d636552"}]}