)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8338,"name":"Eric Blake","email":"eblake@redhat.com","username":"eblake"},"change_message_id":"e9352b10668b2da7247077c6bb34544874eb2d11","unresolved":false,"context_lines":[{"line_number":10,"context_line":"operation in libvirt there is possibility that block_job is"},{"line_number":11,"context_line":"not yet started and libvirt blockJobInfo method will return"},{"line_number":12,"context_line":"status.end \u003d 0 and status.cur \u003d 0. In such case libvirt driver"},{"line_number":13,"context_line":"not waits to finish block rebase operation and that cause"},{"line_number":14,"context_line":"problem because created snapshot is corrupted."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch adds check if status.end !\u003d 0 to return information"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa7ab95a_c34cb944","line":13,"updated":"2016-09-01 21:13:36.000000000","message":"s/not waits/does not wait/\ns/cause/causes a/","commit_id":"62585d7e8060239bcc70a064dbc862722fa9c1a4"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"01d89d860dbe71ed153ca9a2788568a1c9eea005","unresolved":false,"context_lines":[{"line_number":10,"context_line":"operation in libvirt there is possibility that block_job is"},{"line_number":11,"context_line":"not yet started and libvirt blockJobInfo method will return"},{"line_number":12,"context_line":"status.end \u003d 0 and status.cur \u003d 0. In such case libvirt driver"},{"line_number":13,"context_line":"not waits to finish block rebase operation and that cause"},{"line_number":14,"context_line":"problem because created snapshot is corrupted."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch adds check if status.end !\u003d 0 to return information"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa7ab95a_fbd23aff","line":13,"in_reply_to":"fa7ab95a_c34cb944","updated":"2016-09-02 06:32:25.000000000","message":"Done","commit_id":"62585d7e8060239bcc70a064dbc862722fa9c1a4"}],"nova/tests/unit/virt/libvirt/test_guest.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"ea651928e99a43120512e1fab05d66e5e785968d","unresolved":false,"context_lines":[{"line_number":631,"context_line":"        in_progress \u003d self.gblock.wait_for_job()"},{"line_number":632,"context_line":"        self.assertTrue(in_progress)"},{"line_number":633,"context_line":""},{"line_number":634,"context_line":"        self.domain.blockJobInfo.return_value \u003d {"},{"line_number":635,"context_line":"            \"type\": 4,"},{"line_number":636,"context_line":"            \"bandwidth\": 18,"},{"line_number":637,"context_line":"            \"cur\": 95,"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_cb083683","line":634,"updated":"2016-09-02 11:08:36.000000000","message":"nit: These should really be 4 different tests. I\u0027d be -1 on this, but you\u0027re following the existing pattern. A fixup would be nice if you feel so inclined, though.","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"c368b09711951e8b852fdfee3c2d2b01ec28c591","unresolved":false,"context_lines":[{"line_number":631,"context_line":"        in_progress \u003d self.gblock.wait_for_job()"},{"line_number":632,"context_line":"        self.assertTrue(in_progress)"},{"line_number":633,"context_line":""},{"line_number":634,"context_line":"        self.domain.blockJobInfo.return_value \u003d {"},{"line_number":635,"context_line":"            \"type\": 4,"},{"line_number":636,"context_line":"            \"bandwidth\": 18,"},{"line_number":637,"context_line":"            \"cur\": 95,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a89bdaa_448f4023","line":634,"in_reply_to":"fa7ab95a_cb083683","updated":"2016-09-05 08:09:23.000000000","message":"I also thought that but I doesn\u0027t want to change existing \"pattern\". But now when I said the same, I changed it :)","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"}],"nova/virt/libvirt/guest.py":[{"author":{"_account_id":8338,"name":"Eric Blake","email":"eblake@redhat.com","username":"eblake"},"change_message_id":"e9352b10668b2da7247077c6bb34544874eb2d11","unresolved":false,"context_lines":[{"line_number":698,"context_line":"        Libvirt may return either cur\u003d\u003dend or an empty dict when"},{"line_number":699,"context_line":"        the job is complete, depending on whether the job has been"},{"line_number":700,"context_line":"        cleaned up by libvirt yet, or not."},{"line_number":701,"context_line":"        It can also return end\u003d0 if qemu not start yet block operation."},{"line_number":702,"context_line":""},{"line_number":703,"context_line":"        :param abort_on_error: Whether to stop process and raise NovaException"},{"line_number":704,"context_line":"                               on error (default: False)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_c3faf9bc","line":701,"updated":"2016-09-01 21:13:36.000000000","message":"s/not start yet/has not yet started the/","commit_id":"62585d7e8060239bcc70a064dbc862722fa9c1a4"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"01d89d860dbe71ed153ca9a2788568a1c9eea005","unresolved":false,"context_lines":[{"line_number":698,"context_line":"        Libvirt may return either cur\u003d\u003dend or an empty dict when"},{"line_number":699,"context_line":"        the job is complete, depending on whether the job has been"},{"line_number":700,"context_line":"        cleaned up by libvirt yet, or not."},{"line_number":701,"context_line":"        It can also return end\u003d0 if qemu not start yet block operation."},{"line_number":702,"context_line":""},{"line_number":703,"context_line":"        :param abort_on_error: Whether to stop process and raise NovaException"},{"line_number":704,"context_line":"                               on error (default: False)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_5bbfa6a9","line":701,"in_reply_to":"fa7ab95a_c3faf9bc","updated":"2016-09-02 06:32:25.000000000","message":"Done","commit_id":"62585d7e8060239bcc70a064dbc862722fa9c1a4"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"99616368369d44dc14e524da82e16d37b8a77123","unresolved":false,"context_lines":[{"line_number":653,"context_line":"        if status !\u003d -1:"},{"line_number":654,"context_line":"            # NOTE(slaweq): as libvirt can return end\u003d0 when block job is not"},{"line_number":655,"context_line":"            # yet started by qemu by default will be set -1 value if end is not"},{"line_number":656,"context_line":"            # given at all"},{"line_number":657,"context_line":"            return BlockDeviceJobInfo("},{"line_number":658,"context_line":"                job\u003dstatus.get(\"type\", 0),"},{"line_number":659,"context_line":"                bandwidth\u003dstatus.get(\"bandwidth\", 0),"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_94ebb8db","line":656,"updated":"2016-09-02 08:56:51.000000000","message":"Minor rephrasing (feel free to take it or not): \"As libvirt can return end\u003d0 when block job is not yet started by QEMU. If \u0027end\u0027 is not given at all, set the default to -1.\"","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"cb9521e831fc951197215ace8d632c11162e020d","unresolved":false,"context_lines":[{"line_number":653,"context_line":"        if status !\u003d -1:"},{"line_number":654,"context_line":"            # NOTE(slaweq): as libvirt can return end\u003d0 when block job is not"},{"line_number":655,"context_line":"            # yet started by qemu by default will be set -1 value if end is not"},{"line_number":656,"context_line":"            # given at all"},{"line_number":657,"context_line":"            return BlockDeviceJobInfo("},{"line_number":658,"context_line":"                job\u003dstatus.get(\"type\", 0),"},{"line_number":659,"context_line":"                bandwidth\u003dstatus.get(\"bandwidth\", 0),"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_839c586b","line":656,"in_reply_to":"fa7ab95a_94ebb8db","updated":"2016-09-02 10:55:22.000000000","message":"Done","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"ea651928e99a43120512e1fab05d66e5e785968d","unresolved":false,"context_lines":[{"line_number":658,"context_line":"                job\u003dstatus.get(\"type\", 0),"},{"line_number":659,"context_line":"                bandwidth\u003dstatus.get(\"bandwidth\", 0),"},{"line_number":660,"context_line":"                cur\u003dstatus.get(\"cur\", -1),"},{"line_number":661,"context_line":"                end\u003dstatus.get(\"end\", -1))"},{"line_number":662,"context_line":""},{"line_number":663,"context_line":"    def rebase(self, base, shallow\u003dFalse, reuse_ext\u003dFalse,"},{"line_number":664,"context_line":"               copy\u003dFalse, relative\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_8b746e9c","line":661,"updated":"2016-09-02 11:08:36.000000000","message":"These 2 changes don\u0027t appear to have any effect, as these values will always be populated.","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"c368b09711951e8b852fdfee3c2d2b01ec28c591","unresolved":false,"context_lines":[{"line_number":658,"context_line":"                job\u003dstatus.get(\"type\", 0),"},{"line_number":659,"context_line":"                bandwidth\u003dstatus.get(\"bandwidth\", 0),"},{"line_number":660,"context_line":"                cur\u003dstatus.get(\"cur\", -1),"},{"line_number":661,"context_line":"                end\u003dstatus.get(\"end\", -1))"},{"line_number":662,"context_line":""},{"line_number":663,"context_line":"    def rebase(self, base, shallow\u003dFalse, reuse_ext\u003dFalse,"},{"line_number":664,"context_line":"               copy\u003dFalse, relative\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a89bdaa_70a40757","line":661,"in_reply_to":"fa7ab95a_8b746e9c","updated":"2016-09-05 08:09:23.000000000","message":"I made it like that because I assumed that for some reason there is method \"get\" used and default value (was 0) is set for both of those parameters. In such case if libvirt can return dict without those values it would be set to 0, and later in line 722 it would never return True.\n\nTo avoid such potential issue I set default values here to -1 so if \"cur\" and \"end\" key will not be in \"status\" dict, then it will be set to -1 and then in L722 it will return True.\n\nIf You are sure that it can\u0027t be like that, I can remove it.","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"ea651928e99a43120512e1fab05d66e5e785968d","unresolved":false,"context_lines":[{"line_number":692,"context_line":"        \"\"\"Resizes block device to Kib size.\"\"\""},{"line_number":693,"context_line":"        self._guest._domain.blockResize(self._disk, size_kb)"},{"line_number":694,"context_line":""},{"line_number":695,"context_line":"    def wait_for_job(self, abort_on_error\u003dFalse, wait_for_job_clean\u003dFalse):"},{"line_number":696,"context_line":"        \"\"\"Wait for libvirt block job to complete."},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"        Libvirt may return either cur\u003d\u003dend or an empty dict when"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_d831ee5a","line":695,"updated":"2016-09-02 11:08:36.000000000","message":"Eww. Why doesn\u0027t this method wait?\n\nObviously not related to this patch.","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"99616368369d44dc14e524da82e16d37b8a77123","unresolved":false,"context_lines":[{"line_number":705,"context_line":"                               on error (default: False)"},{"line_number":706,"context_line":"        :param wait_for_job_clean: Whether to force wait to ensure job is"},{"line_number":707,"context_line":"                                   finished (see bug: LP#1119173)"},{"line_number":708,"context_line":""},{"line_number":709,"context_line":"        :returns: True if still in progress"},{"line_number":710,"context_line":"                  False if completed"},{"line_number":711,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_54ccd088","line":708,"updated":"2016-09-02 08:56:51.000000000","message":"While we\u0027re touching this file, worth fixing the wrong bug reference from \"LP#1119173\" to \"RH Bugzilla#1119173\"?\n\nBecause, if someone goes to look at Launchpad currently, it turns up a wrong bug.\n\nCorrect bug:\n\nhttps://bugzilla.redhat.com/show_bug.cgi?id\u003d1119173 -- The default behavor of abort block job with pivot flag isn\u0027t sync","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"cb9521e831fc951197215ace8d632c11162e020d","unresolved":false,"context_lines":[{"line_number":705,"context_line":"                               on error (default: False)"},{"line_number":706,"context_line":"        :param wait_for_job_clean: Whether to force wait to ensure job is"},{"line_number":707,"context_line":"                                   finished (see bug: LP#1119173)"},{"line_number":708,"context_line":""},{"line_number":709,"context_line":"        :returns: True if still in progress"},{"line_number":710,"context_line":"                  False if completed"},{"line_number":711,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_a3995c7b","line":708,"in_reply_to":"fa7ab95a_54ccd088","updated":"2016-09-02 10:55:22.000000000","message":"Done","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"ea651928e99a43120512e1fab05d66e5e785968d","unresolved":false,"context_lines":[{"line_number":719,"context_line":"        if wait_for_job_clean:"},{"line_number":720,"context_line":"            job_ended \u003d status.job \u003d\u003d 0"},{"line_number":721,"context_line":"        else:"},{"line_number":722,"context_line":"            job_ended \u003d status.end !\u003d 0 and status.cur \u003d\u003d status.end"},{"line_number":723,"context_line":""},{"line_number":724,"context_line":"        return not job_ended"},{"line_number":725,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_cbe3d6d6","line":722,"updated":"2016-09-02 11:08:36.000000000","message":"IIUC, this is the only required fix here.\n\nHowever, this is working around a \u0027bug\u0027 in libvirt which is described here: http://www.redhat.com/archives/libvir-list/2016-September/msg00017.html . It sounds like this bug/ambiguity will change at some point in the future. Consequently, we should have a comment block here thoroughly describing why we\u0027re doing something weird and temporary. The comment block should link to the above ML post by kashyap/eblake. As a maintainer, this provides me with all the relevant context.\n\nWe should also note that until libvirt\u0027s behaviour changes, end \u003d\u003d cur \u003d\u003d 0 is potentially ambiguous for a block job whose end has a \u0027legitimate\u0027 value of zero. In that case we\u0027d hang forever here. I don\u0027t know if it makes sense for such a block job to exist, but it\u0027s worth at least noting here as a marker in case anybody hits it. For example, in the absence of a comment block, a future maintainer may well just remove the offending \u0027status.end !\u003d 0\u0027, and reintroduce this bug.","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"c368b09711951e8b852fdfee3c2d2b01ec28c591","unresolved":false,"context_lines":[{"line_number":719,"context_line":"        if wait_for_job_clean:"},{"line_number":720,"context_line":"            job_ended \u003d status.job \u003d\u003d 0"},{"line_number":721,"context_line":"        else:"},{"line_number":722,"context_line":"            job_ended \u003d status.end !\u003d 0 and status.cur \u003d\u003d status.end"},{"line_number":723,"context_line":""},{"line_number":724,"context_line":"        return not job_ended"},{"line_number":725,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a89bdaa_44c74012","line":722,"in_reply_to":"fa7ab95a_cbe3d6d6","updated":"2016-09-05 08:09:23.000000000","message":"Done","commit_id":"2c8dc10b965d611da5adf1a2fefd3ded8efb1fa3"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"308ae23436fe66471da85ec5be1c376fb4adebb6","unresolved":false,"context_lines":[{"line_number":658,"context_line":"                job\u003dstatus.get(\"type\", 0),"},{"line_number":659,"context_line":"                bandwidth\u003dstatus.get(\"bandwidth\", 0),"},{"line_number":660,"context_line":"                cur\u003dstatus.get(\"cur\", -1),"},{"line_number":661,"context_line":"                end\u003dstatus.get(\"end\", -1))"},{"line_number":662,"context_line":""},{"line_number":663,"context_line":"    def rebase(self, base, shallow\u003dFalse, reuse_ext\u003dFalse,"},{"line_number":664,"context_line":"               copy\u003dFalse, relative\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_dabec0fe","line":661,"updated":"2016-09-05 09:25:34.000000000","message":"I just checked the code in libvirt and its python bindings. \u0027cur\u0027 and \u0027end\u0027 aren\u0027t pointers in virDomainBlockJobInfo; they\u0027re unsigned long long. Consequently, they can\u0027t obviously represent \u0027unset\u0027. Sure enough, checking in the libvirt bindings they are set unconditionally in libvirt_virDomainGetBlockJobInfo(). This means that the existing code using .get() is actively misleading.\n\nI\u0027m definitely -1 on changing dead code as it makes it harder for the maintainer to find the original intent in git history. I would prefer that you changed these lookups to be unconditional, which correctly represents the data structure. However, if you just want to leave them alone (i.e. leave it with a default of 0) I\u0027ll submit a follow-on patch. I think this is a reasonable change to make in this patch, though.","commit_id":"711734f8bf9b6856fde9c2710e4c4fda876983bf"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"7d558ca83e2b03ff45dd89e2a3949b611de5f39a","unresolved":false,"context_lines":[{"line_number":658,"context_line":"                job\u003dstatus.get(\"type\", 0),"},{"line_number":659,"context_line":"                bandwidth\u003dstatus.get(\"bandwidth\", 0),"},{"line_number":660,"context_line":"                cur\u003dstatus.get(\"cur\", -1),"},{"line_number":661,"context_line":"                end\u003dstatus.get(\"end\", -1))"},{"line_number":662,"context_line":""},{"line_number":663,"context_line":"    def rebase(self, base, shallow\u003dFalse, reuse_ext\u003dFalse,"},{"line_number":664,"context_line":"               copy\u003dFalse, relative\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_8ebb1a72","line":661,"in_reply_to":"9a89bdaa_dabec0fe","updated":"2016-09-05 10:43:48.000000000","message":"Ok. I left it unchanged here and I will propose another patch to change all those parameters to be unconditional","commit_id":"711734f8bf9b6856fde9c2710e4c4fda876983bf"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"308ae23436fe66471da85ec5be1c376fb4adebb6","unresolved":false,"context_lines":[{"line_number":721,"context_line":"        else:"},{"line_number":722,"context_line":"            # NOTE(slaweq): because of bug in libvirt, which is described in"},{"line_number":723,"context_line":"            # http://www.redhat.com/archives/libvir-list/2016-September/msg00017.html"},{"line_number":724,"context_line":"            # if status.end \u003d\u003d 0 job is not started yet so it is not finished"},{"line_number":725,"context_line":"            job_ended \u003d status.end !\u003d 0 and status.cur \u003d\u003d status.end"},{"line_number":726,"context_line":""},{"line_number":727,"context_line":"        return not job_ended"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_7a469424","line":724,"updated":"2016-09-05 09:25:34.000000000","message":"Nice, thanks.","commit_id":"711734f8bf9b6856fde9c2710e4c4fda876983bf"}]}