)]}'
{"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e16d7b213e0b00d5f77fe32376e1d6fcef47d2d4","unresolved":false,"context_lines":[{"line_number":17700,"context_line":"            mock.patch.object(domain, \u0027blockRebase\u0027),"},{"line_number":17701,"context_line":"            mock.patch.object(domain, \u0027blockJobInfo\u0027,"},{"line_number":17702,"context_line":"                              return_value\u003d{"},{"line_number":17703,"context_line":"                                  \u0027type\u0027: 0,"},{"line_number":17704,"context_line":"                                  \u0027bandwidth\u0027: 0,"},{"line_number":17705,"context_line":"                                  \u0027cur\u0027: 1000,"},{"line_number":17706,"context_line":"                                  \u0027end\u0027: 1000})"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_3141a80c","line":17703,"updated":"2016-09-28 16:25:57.000000000","message":"please check Evgeny\u0027s comment in https://review.openstack.org/#/c/365756/5/nova/tests/unit/virt/libvirt/test_guest.py@654 - maybe better would be to change \u0027type\u0027 value to other than 0 (even if it is not hadrcoded in is_block_job_complete() method anymore","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"c242e9e9c6ededef10618b8f83fc5e1554d96e8b","unresolved":false,"context_lines":[{"line_number":17700,"context_line":"            mock.patch.object(domain, \u0027blockRebase\u0027),"},{"line_number":17701,"context_line":"            mock.patch.object(domain, \u0027blockJobInfo\u0027,"},{"line_number":17702,"context_line":"                              return_value\u003d{"},{"line_number":17703,"context_line":"                                  \u0027type\u0027: 0,"},{"line_number":17704,"context_line":"                                  \u0027bandwidth\u0027: 0,"},{"line_number":17705,"context_line":"                                  \u0027cur\u0027: 1000,"},{"line_number":17706,"context_line":"                                  \u0027end\u0027: 1000})"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_d17404ff","line":17703,"in_reply_to":"3a98d1f4_3141a80c","updated":"2016-09-28 16:34:35.000000000","message":"Gah, I just copied this straight from your patch without review. Thanks for the eagle eyes. I\u0027ll change it.","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":13431,"name":"Evgeny Antyshev","email":"eantyshev@gmail.com","username":"eantyshev"},"change_message_id":"12e0842ba2cdddabd79187c750d671d3e876a080","unresolved":false,"context_lines":[{"line_number":1182,"context_line":"            # allow writing to existing external volume file"},{"line_number":1183,"context_line":"            dev.rebase(new_path, copy\u003dTrue, reuse_ext\u003dTrue)"},{"line_number":1184,"context_line":""},{"line_number":1185,"context_line":"            while not dev.is_job_complete():"},{"line_number":1186,"context_line":"                time.sleep(0.5)"},{"line_number":1187,"context_line":""},{"line_number":1188,"context_line":"            dev.abort_job(pivot\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_2f783cf2","line":1185,"updated":"2016-09-29 10:05:34.000000000","message":"see http://libvirt.org/git/?p\u003dlibvirt.git;a\u003dblob;f\u003dsrc/libvirt-domain.c;h\u003d9ba9f49a2d52c6b9feb534a4e6dc371bcd4bb8aa;hb\u003dHEAD#l10142\n\n\"The job transitions to the second phase when the job info states cur \u003d\u003d end, and remains alive to\nmirror all further changes to both source and destination.  The user must call virDomainBlockJobAbort() to end the mirroring while choosing whether to revert to source or pivot to the destination\"\n\nso, if copy\u003dTrue, the rebase job won\u0027t finish until the job is aborted with pivot flag (to pivot to the new base)","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"7617feb535402687e0d19cc36d920de5d1ea42c8","unresolved":false,"context_lines":[{"line_number":1182,"context_line":"            # allow writing to existing external volume file"},{"line_number":1183,"context_line":"            dev.rebase(new_path, copy\u003dTrue, reuse_ext\u003dTrue)"},{"line_number":1184,"context_line":""},{"line_number":1185,"context_line":"            while not dev.is_job_complete():"},{"line_number":1186,"context_line":"                time.sleep(0.5)"},{"line_number":1187,"context_line":""},{"line_number":1188,"context_line":"            dev.abort_job(pivot\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_5726f6ce","line":1185,"in_reply_to":"3a98d1f4_2f783cf2","updated":"2016-09-29 12:32:37.000000000","message":"There is call to pivot operation on line 1188 below [\"dev.abort_job(pivot\u003dTrue\")].","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"8c429feaf2628fbae26101adadf9213405527543","unresolved":false,"context_lines":[{"line_number":1182,"context_line":"            # allow writing to existing external volume file"},{"line_number":1183,"context_line":"            dev.rebase(new_path, copy\u003dTrue, reuse_ext\u003dTrue)"},{"line_number":1184,"context_line":""},{"line_number":1185,"context_line":"            while not dev.is_job_complete():"},{"line_number":1186,"context_line":"                time.sleep(0.5)"},{"line_number":1187,"context_line":""},{"line_number":1188,"context_line":"            dev.abort_job(pivot\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_ddc78710","line":1185,"in_reply_to":"3a98d1f4_2f783cf2","updated":"2016-09-29 12:38:19.000000000","message":"is_job_complete() will return True if either the job disappears, or cur \u003d\u003d end, so I think we\u0027re good here, right?\n\nThe job will transition from pull to mirror. On the next round of this loop is_job_complete will pull blockJobInfo, which will return cur \u003d\u003d end. is_job_complete() will return True, and we\u0027ll move on to do the pivot below. Is that not right?","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":13431,"name":"Evgeny Antyshev","email":"eantyshev@gmail.com","username":"eantyshev"},"change_message_id":"afb87a1f7f3495c4e6a4908f6df063164a38c044","unresolved":false,"context_lines":[{"line_number":1182,"context_line":"            # allow writing to existing external volume file"},{"line_number":1183,"context_line":"            dev.rebase(new_path, copy\u003dTrue, reuse_ext\u003dTrue)"},{"line_number":1184,"context_line":""},{"line_number":1185,"context_line":"            while not dev.is_job_complete():"},{"line_number":1186,"context_line":"                time.sleep(0.5)"},{"line_number":1187,"context_line":""},{"line_number":1188,"context_line":"            dev.abort_job(pivot\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_83692028","line":1185,"in_reply_to":"3a98d1f4_ddc78710","updated":"2016-09-29 13:07:35.000000000","message":"This is correct, and it will work","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"e220e4d34c1cd0e5bc50557f53f3c27a674a4c30","unresolved":false,"context_lines":[{"line_number":1182,"context_line":"            # allow writing to existing external volume file"},{"line_number":1183,"context_line":"            dev.rebase(new_path, copy\u003dTrue, reuse_ext\u003dTrue)"},{"line_number":1184,"context_line":""},{"line_number":1185,"context_line":"            while not dev.is_job_complete():"},{"line_number":1186,"context_line":"                time.sleep(0.5)"},{"line_number":1187,"context_line":""},{"line_number":1188,"context_line":"            dev.abort_job(pivot\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_e331040d","line":1185,"in_reply_to":"3a98d1f4_ddc78710","updated":"2016-09-29 13:11:00.000000000","message":"s/pull/copy/ :-)  (Because virDomainBlockPull() is just virDomainBlockRebase() without the ability to set flags; hence we use the latter)\n\nBut to your main point: yes, it is correct.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"46a6aad9e976037e2e5ea141e0329988d2e5358a","unresolved":false,"context_lines":[{"line_number":1182,"context_line":"            # allow writing to existing external volume file"},{"line_number":1183,"context_line":"            dev.rebase(new_path, copy\u003dTrue, reuse_ext\u003dTrue)"},{"line_number":1184,"context_line":""},{"line_number":1185,"context_line":"            while not dev.is_job_complete():"},{"line_number":1186,"context_line":"                time.sleep(0.5)"},{"line_number":1187,"context_line":""},{"line_number":1188,"context_line":"            dev.abort_job(pivot\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_ff6922fa","line":1185,"in_reply_to":"3a98d1f4_e331040d","updated":"2016-10-02 14:23:34.000000000","message":"BTW, I\u0027m testing out trying to get a swap volume test in tempest and it fails intermittently because we move on before the block copy job is complete:\n\nhttp://logs.openstack.org/73/374373/1/check/gate-tempest-dsvm-full-ubuntu-xenial/fdbe68e/logs/screen-n-cpu.txt.gz?level\u003dDEBUG#_2016-10-02_00_26_34_671\n\nSo I\u0027ll add this fix as a dependency and see if that helps anything.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"}],"nova/virt/libvirt/guest.py":[{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"f7038090a922bf5b9362913910885d4c45d0fb83","unresolved":false,"context_lines":[{"line_number":666,"context_line":"            bandwidth\u003dstatus[\u0027bandwidth\u0027],"},{"line_number":667,"context_line":"            cur\u003dstatus[\u0027cur\u0027],"},{"line_number":668,"context_line":"            end\u003dstatus[\u0027end\u0027])"},{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    def rebase(self, base, shallow\u003dFalse, reuse_ext\u003dFalse,"},{"line_number":671,"context_line":"               copy\u003dFalse, relative\u003dFalse):"},{"line_number":672,"context_line":"        \"\"\"Rebases block to new base"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_1152ec13","line":669,"updated":"2016-09-28 17:04:38.000000000","message":"Nice!  This (reporting the actual values from libvirt, which in turn is just reporting whatever QEMU is feeding it) is much, much cleaner than Nova \"fudging\" the return values, which, as we\u0027ve seen so far, has been a source of grief.","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e16d7b213e0b00d5f77fe32376e1d6fcef47d2d4","unresolved":false,"context_lines":[{"line_number":699,"context_line":"        \"\"\"Resizes block device to Kib size.\"\"\""},{"line_number":700,"context_line":"        self._guest._domain.blockResize(self._disk, size_kb)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    def is_block_job_complete(self, abort_on_error\u003dFalse):"},{"line_number":703,"context_line":"        \"\"\"Return True if the job is complete, False otherwise"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        :param abort_on_error: Whether to stop process and raise NovaException"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_ee30410a","line":702,"updated":"2016-09-28 16:25:57.000000000","message":"nit: according to pattern of naming other methods in this class maybe better would be \"is_job_complete\", what You think?","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"c242e9e9c6ededef10618b8f83fc5e1554d96e8b","unresolved":false,"context_lines":[{"line_number":699,"context_line":"        \"\"\"Resizes block device to Kib size.\"\"\""},{"line_number":700,"context_line":"        self._guest._domain.blockResize(self._disk, size_kb)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    def is_block_job_complete(self, abort_on_error\u003dFalse):"},{"line_number":703,"context_line":"        \"\"\"Return True if the job is complete, False otherwise"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        :param abort_on_error: Whether to stop process and raise NovaException"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_913c3c31","line":702,"in_reply_to":"3a98d1f4_ee30410a","updated":"2016-09-28 16:34:35.000000000","message":"Yep.","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e16d7b213e0b00d5f77fe32376e1d6fcef47d2d4","unresolved":false,"context_lines":[{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        :param abort_on_error: Whether to stop process and raise NovaException"},{"line_number":706,"context_line":"                               on error (default: False)"},{"line_number":707,"context_line":"        :returns: True iff the job is complete"},{"line_number":708,"context_line":"        \"\"\""},{"line_number":709,"context_line":"        try:"},{"line_number":710,"context_line":"            status \u003d self.get_job_info()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_ce2c65ae","line":707,"updated":"2016-09-28 16:25:57.000000000","message":"nit: s/iff/if","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"89d7cd3bb5c4b950a93ea7c326a8bdb4edf67a3c","unresolved":false,"context_lines":[{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        :param abort_on_error: Whether to stop process and raise NovaException"},{"line_number":706,"context_line":"                               on error (default: False)"},{"line_number":707,"context_line":"        :returns: True iff the job is complete"},{"line_number":708,"context_line":"        \"\"\""},{"line_number":709,"context_line":"        try:"},{"line_number":710,"context_line":"            status \u003d self.get_job_info()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_e6cf64ae","line":707,"in_reply_to":"3a98d1f4_5149d48a","updated":"2016-09-29 08:13:12.000000000","message":"Oh, ok then. I thought iff was common usage, but no point using it if it\u0027s not.","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"c242e9e9c6ededef10618b8f83fc5e1554d96e8b","unresolved":false,"context_lines":[{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        :param abort_on_error: Whether to stop process and raise NovaException"},{"line_number":706,"context_line":"                               on error (default: False)"},{"line_number":707,"context_line":"        :returns: True iff the job is complete"},{"line_number":708,"context_line":"        \"\"\""},{"line_number":709,"context_line":"        try:"},{"line_number":710,"context_line":"            status \u003d self.get_job_info()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_f17820fb","line":707,"in_reply_to":"3a98d1f4_ce2c65ae","updated":"2016-09-28 16:34:35.000000000","message":"https://en.wikipedia.org/wiki/Iff ;)","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"f7038090a922bf5b9362913910885d4c45d0fb83","unresolved":false,"context_lines":[{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        :param abort_on_error: Whether to stop process and raise NovaException"},{"line_number":706,"context_line":"                               on error (default: False)"},{"line_number":707,"context_line":"        :returns: True iff the job is complete"},{"line_number":708,"context_line":"        \"\"\""},{"line_number":709,"context_line":"        try:"},{"line_number":710,"context_line":"            status \u003d self.get_job_info()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_5149d48a","line":707,"in_reply_to":"3a98d1f4_f17820fb","updated":"2016-09-28 17:04:38.000000000","message":"I didn\u0027t miss it, but you might want to spell it out, not everyone is bestowed with eagle eyes ;-)","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e16d7b213e0b00d5f77fe32376e1d6fcef47d2d4","unresolved":false,"context_lines":[{"line_number":708,"context_line":"        \"\"\""},{"line_number":709,"context_line":"        try:"},{"line_number":710,"context_line":"            status \u003d self.get_job_info()"},{"line_number":711,"context_line":"        except Exception:"},{"line_number":712,"context_line":"            if abort_on_error:"},{"line_number":713,"context_line":"                raise"},{"line_number":714,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_aefc793f","line":711,"updated":"2016-09-28 16:25:57.000000000","message":"shouldn\u0027t You catch here only exceptions.NovaException() as it is what get_job_info can throw in case of error?\nif for some reason there will be any other exception I\u0027m not sure if it should \"silently ignore\" it and just return True","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b5478dd8e654d745037666527872183ff8f5cecf","unresolved":false,"context_lines":[{"line_number":708,"context_line":"        \"\"\""},{"line_number":709,"context_line":"        try:"},{"line_number":710,"context_line":"            status \u003d self.get_job_info()"},{"line_number":711,"context_line":"        except Exception:"},{"line_number":712,"context_line":"            if abort_on_error:"},{"line_number":713,"context_line":"                raise"},{"line_number":714,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_524c5670","line":711,"in_reply_to":"3a98d1f4_11e2ecd5","updated":"2016-09-29 09:51:05.000000000","message":"sorry, but I don\u0027t understand. Better is like now to catch all exceptions or better would be to catch only NovaException()?","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"c242e9e9c6ededef10618b8f83fc5e1554d96e8b","unresolved":false,"context_lines":[{"line_number":708,"context_line":"        \"\"\""},{"line_number":709,"context_line":"        try:"},{"line_number":710,"context_line":"            status \u003d self.get_job_info()"},{"line_number":711,"context_line":"        except Exception:"},{"line_number":712,"context_line":"            if abort_on_error:"},{"line_number":713,"context_line":"                raise"},{"line_number":714,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_11e2ecd5","line":711,"in_reply_to":"3a98d1f4_aefc793f","updated":"2016-09-28 16:34:35.000000000","message":"Yup, that\u0027s better.","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e16d7b213e0b00d5f77fe32376e1d6fcef47d2d4","unresolved":false,"context_lines":[{"line_number":714,"context_line":""},{"line_number":715,"context_line":"            # NOTE(mdbooth): Would be better to just log this without adding"},{"line_number":716,"context_line":"            # anything, but it seems that\u0027s not allowed."},{"line_number":717,"context_line":"            LOG.exception(_LE(\u0027Error retrieving libvirt job info\u0027))"},{"line_number":718,"context_line":""},{"line_number":719,"context_line":"            # NOTE(mdbooth): Treat an error as job completion"},{"line_number":720,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_9110bc50","line":717,"updated":"2016-09-28 16:25:57.000000000","message":"shouldn\u0027t it be above L712?","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"c242e9e9c6ededef10618b8f83fc5e1554d96e8b","unresolved":false,"context_lines":[{"line_number":714,"context_line":""},{"line_number":715,"context_line":"            # NOTE(mdbooth): Would be better to just log this without adding"},{"line_number":716,"context_line":"            # anything, but it seems that\u0027s not allowed."},{"line_number":717,"context_line":"            LOG.exception(_LE(\u0027Error retrieving libvirt job info\u0027))"},{"line_number":718,"context_line":""},{"line_number":719,"context_line":"            # NOTE(mdbooth): Treat an error as job completion"},{"line_number":720,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_91f51c85","line":717,"in_reply_to":"3a98d1f4_9110bc50","updated":"2016-09-28 16:34:35.000000000","message":"No. We only log the exception here if we\u0027re going to ignore it. If we raise the exception then it\u0027s the caller\u0027s problem.","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"f7038090a922bf5b9362913910885d4c45d0fb83","unresolved":false,"context_lines":[{"line_number":721,"context_line":""},{"line_number":722,"context_line":"        # If the job no longer exists, it is because it has completed"},{"line_number":723,"context_line":"        if status is None:"},{"line_number":724,"context_line":"            return True"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"        # NOTE(slaweq): because of bug in libvirt, which is described in"},{"line_number":727,"context_line":"        # http://www.redhat.com/archives/libvir-list/2016-September/msg00017.html"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_315a8884","line":724,"updated":"2016-09-28 17:04:38.000000000","message":"Just a note for future: Does not affect this patch, Eric mentioned on IRC that a future libvirt could track that a job has finished and provide state even when QEMU didn\u0027t report any.","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"f7038090a922bf5b9362913910885d4c45d0fb83","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        # NOTE(slaweq): because of bug in libvirt, which is described in"},{"line_number":727,"context_line":"        # http://www.redhat.com/archives/libvir-list/2016-September/msg00017.html"},{"line_number":728,"context_line":"        # if status.end \u003d\u003d 0 job is not started yet so it is not finished"},{"line_number":729,"context_line":"        return status.end !\u003d 0 and status.cur \u003d\u003d status.end"},{"line_number":730,"context_line":""},{"line_number":731,"context_line":""},{"line_number":732,"context_line":"class VCPUInfo(object):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_74b07e8c","line":729,"updated":"2016-09-28 17:04:38.000000000","message":"Do we want to mention here that upstream libvirt has fixed the above case (the \"some management applications\" in the below commit message is referring to OpenStack), and at a future MIN_LIBVIRT_VERSION bump in Nova, we need to revisit this code.\n\nhttp://libvirt.org/git/?p\u003dlibvirt.git;a\u003dcommitdiff;h\u003d988218c -- virDomainGetBlockJobInfo: Fix corner case when qemu reports no info\n\n   - - -\n\nAlso: in future, if we could wait for the event BLOCK_JOB_COMPLETED, which is issued only when libvirt detects cur \u003d\u003d end, that\u0027s a solid confirmation of the blockRebase() job completion.","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"89d7cd3bb5c4b950a93ea7c326a8bdb4edf67a3c","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        # NOTE(slaweq): because of bug in libvirt, which is described in"},{"line_number":727,"context_line":"        # http://www.redhat.com/archives/libvir-list/2016-September/msg00017.html"},{"line_number":728,"context_line":"        # if status.end \u003d\u003d 0 job is not started yet so it is not finished"},{"line_number":729,"context_line":"        return status.end !\u003d 0 and status.cur \u003d\u003d status.end"},{"line_number":730,"context_line":""},{"line_number":731,"context_line":""},{"line_number":732,"context_line":"class VCPUInfo(object):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_86e9401d","line":729,"in_reply_to":"3a98d1f4_74b07e8c","updated":"2016-09-29 08:13:12.000000000","message":"Yes to all of the above.","commit_id":"be5639b8b530077ba5bb3759c7b10ab98db956b7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a261c09b5c574ad103c6f449df138bb32da2270b","unresolved":false,"context_lines":[{"line_number":650,"context_line":"        :returns: BlockDeviceJobInfo, or None if no job exists"},{"line_number":651,"context_line":"        \"\"\""},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        # This returns None if there was an error, {} if the job no longer"},{"line_number":654,"context_line":"        # exists, or a fully populated dict if the job exists."},{"line_number":655,"context_line":"        status \u003d self._guest._domain.blockJobInfo(self._disk, flags\u003d0)"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        if status is None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_701e3cbc","line":654,"range":{"start_line":653,"start_character":51,"end_line":654,"end_character":62},"updated":"2016-09-30 19:43:34.000000000","message":"This needs to be updated, it doesn\u0027t return a dict at all, it returns None or the BlockDeviceJobInfo object as pointed out above.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"365fc02da2a3df8bc398841d81c8fbe08e9648fc","unresolved":false,"context_lines":[{"line_number":650,"context_line":"        :returns: BlockDeviceJobInfo, or None if no job exists"},{"line_number":651,"context_line":"        \"\"\""},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        # This returns None if there was an error, {} if the job no longer"},{"line_number":654,"context_line":"        # exists, or a fully populated dict if the job exists."},{"line_number":655,"context_line":"        status \u003d self._guest._domain.blockJobInfo(self._disk, flags\u003d0)"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        if status is None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_ac575656","line":654,"range":{"start_line":653,"start_character":51,"end_line":654,"end_character":62},"in_reply_to":"3a98d1f4_046cb333","updated":"2016-10-03 16:42:33.000000000","message":"Ah no my fault, I was totally lumping it in with the docstring on the method.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"8322b42f37402137e5af8b53e311c89da85e725a","unresolved":false,"context_lines":[{"line_number":650,"context_line":"        :returns: BlockDeviceJobInfo, or None if no job exists"},{"line_number":651,"context_line":"        \"\"\""},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        # This returns None if there was an error, {} if the job no longer"},{"line_number":654,"context_line":"        # exists, or a fully populated dict if the job exists."},{"line_number":655,"context_line":"        status \u003d self._guest._domain.blockJobInfo(self._disk, flags\u003d0)"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        if status is None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_046cb333","line":654,"range":{"start_line":653,"start_character":51,"end_line":654,"end_character":62},"in_reply_to":"3a98d1f4_6b08eada","updated":"2016-10-03 11:32:54.000000000","message":"Yeah, worth it, since I had the context I was reading it the same way as MBooth.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"59e8e670c60516fe34c6798aea9e074942c30f3d","unresolved":false,"context_lines":[{"line_number":650,"context_line":"        :returns: BlockDeviceJobInfo, or None if no job exists"},{"line_number":651,"context_line":"        \"\"\""},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        # This returns None if there was an error, {} if the job no longer"},{"line_number":654,"context_line":"        # exists, or a fully populated dict if the job exists."},{"line_number":655,"context_line":"        status \u003d self._guest._domain.blockJobInfo(self._disk, flags\u003d0)"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        if status is None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_6b08eada","line":654,"range":{"start_line":653,"start_character":51,"end_line":654,"end_character":62},"in_reply_to":"3a98d1f4_701e3cbc","updated":"2016-10-03 10:20:31.000000000","message":"Ah, context :) This comment refers to _domain.blockJobInfo(), not the method return. If you misread it I should probably rewrite it, because that shouldn\u0027t be ambiguous.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a261c09b5c574ad103c6f449df138bb32da2270b","unresolved":false,"context_lines":[{"line_number":654,"context_line":"        # exists, or a fully populated dict if the job exists."},{"line_number":655,"context_line":"        status \u003d self._guest._domain.blockJobInfo(self._disk, flags\u003d0)"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        if status is None:"},{"line_number":658,"context_line":"            msg \u003d _(\u0027libvirt error while requesting blockjob info.\u0027)"},{"line_number":659,"context_line":"            raise exception.NovaException(msg)"},{"line_number":660,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_d069d0fa","line":657,"updated":"2016-09-30 19:43:34.000000000","message":"Does this mean an actual error, or that the job just might no longer exist and is considered complete?","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"8322b42f37402137e5af8b53e311c89da85e725a","unresolved":false,"context_lines":[{"line_number":654,"context_line":"        # exists, or a fully populated dict if the job exists."},{"line_number":655,"context_line":"        status \u003d self._guest._domain.blockJobInfo(self._disk, flags\u003d0)"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        if status is None:"},{"line_number":658,"context_line":"            msg \u003d _(\u0027libvirt error while requesting blockjob info.\u0027)"},{"line_number":659,"context_line":"            raise exception.NovaException(msg)"},{"line_number":660,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_e4618706","line":657,"in_reply_to":"3a98d1f4_44679bb6","updated":"2016-10-03 11:32:54.000000000","message":"Eagle eyes, Matt Riedemann.  Followed the IRC discussion b/n Matt Booth \u0026 DanPB, and learnt the libvirt Python binding of virDomainBlockJobInfo(), blockJobInfo() indeed returns:\n\n[...]\nif ret is None: raise libvirtError (\u0027virDomainGetBlockJobInfo() failed\u0027, dom\u003dself) \n[...]","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"346f9e464cb26c112e7281194b5b8e432efdad1b","unresolved":false,"context_lines":[{"line_number":654,"context_line":"        # exists, or a fully populated dict if the job exists."},{"line_number":655,"context_line":"        status \u003d self._guest._domain.blockJobInfo(self._disk, flags\u003d0)"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        if status is None:"},{"line_number":658,"context_line":"            msg \u003d _(\u0027libvirt error while requesting blockjob info.\u0027)"},{"line_number":659,"context_line":"            raise exception.NovaException(msg)"},{"line_number":660,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_44679bb6","line":657,"in_reply_to":"3a98d1f4_6ee0f8cd","updated":"2016-10-03 11:08:01.000000000","message":"I asked danpb to double check this for me. It occurred to me that I hadn\u0027t worked out the complete code flow, and it was possible there was some layer somewhere which converted a return of VIR_PY_NONE to an actual error.\n\nIt turns out there is, and Dan pointed it out. The upshot is that this doesn\u0027t really return None on error: it raises libvirt.libvirtError instead, which is exactly what we want. I\u0027ll update accordingly.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"59e8e670c60516fe34c6798aea9e074942c30f3d","unresolved":false,"context_lines":[{"line_number":654,"context_line":"        # exists, or a fully populated dict if the job exists."},{"line_number":655,"context_line":"        status \u003d self._guest._domain.blockJobInfo(self._disk, flags\u003d0)"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        if status is None:"},{"line_number":658,"context_line":"            msg \u003d _(\u0027libvirt error while requesting blockjob info.\u0027)"},{"line_number":659,"context_line":"            raise exception.NovaException(msg)"},{"line_number":660,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_6ee0f8cd","line":657,"in_reply_to":"3a98d1f4_d069d0fa","updated":"2016-10-03 10:20:31.000000000","message":"This means an actual error. Ideally the binding would raise an exception in this case, but to the best of my reading of the python binding it just returns None. AFAICT it should raise a libvirtError, which would call libvirtmod.virGetLastError() and return useful error info, but I don\u0027t see where it does this. IIUC this would be a bug in the bindings, imho.\n\nAnyway, None returned here is a real error, but we don\u0027t know what it was and have no way of finding out.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a261c09b5c574ad103c6f449df138bb32da2270b","unresolved":false,"context_lines":[{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        if status is None:"},{"line_number":658,"context_line":"            msg \u003d _(\u0027libvirt error while requesting blockjob info.\u0027)"},{"line_number":659,"context_line":"            raise exception.NovaException(msg)"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"        if len(status) \u003d\u003d 0:"},{"line_number":662,"context_line":"            return None"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_506ce034","line":659,"range":{"start_line":659,"start_character":28,"end_line":659,"end_character":41},"updated":"2016-09-30 19:43:34.000000000","message":"Might as well point this out in the docstring with a :raises: comment.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"59e8e670c60516fe34c6798aea9e074942c30f3d","unresolved":false,"context_lines":[{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        if status is None:"},{"line_number":658,"context_line":"            msg \u003d _(\u0027libvirt error while requesting blockjob info.\u0027)"},{"line_number":659,"context_line":"            raise exception.NovaException(msg)"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"        if len(status) \u003d\u003d 0:"},{"line_number":662,"context_line":"            return None"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_cee90cb9","line":659,"range":{"start_line":659,"start_character":28,"end_line":659,"end_character":41},"in_reply_to":"3a98d1f4_506ce034","updated":"2016-10-03 10:20:31.000000000","message":"Ack.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a261c09b5c574ad103c6f449df138bb32da2270b","unresolved":false,"context_lines":[{"line_number":727,"context_line":""},{"line_number":728,"context_line":"            # NOTE(mdbooth): Would be better to just log this without adding"},{"line_number":729,"context_line":"            # anything, but it seems that\u0027s not allowed."},{"line_number":730,"context_line":"            LOG.exception(_LE(\u0027Error retrieving libvirt job info\u0027))"},{"line_number":731,"context_line":""},{"line_number":732,"context_line":"            # NOTE(mdbooth): Treat an error as job completion"},{"line_number":733,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_707f1cab","line":730,"updated":"2016-09-30 19:43:34.000000000","message":"Is this something we should stacktrace on? I\u0027m confused as to whether or not this is a real error that needs investigation, or something we could hit in a race window where the job is complete and gone, i.e. similar to the len(status) \u003d\u003d 0 case.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"8322b42f37402137e5af8b53e311c89da85e725a","unresolved":false,"context_lines":[{"line_number":727,"context_line":""},{"line_number":728,"context_line":"            # NOTE(mdbooth): Would be better to just log this without adding"},{"line_number":729,"context_line":"            # anything, but it seems that\u0027s not allowed."},{"line_number":730,"context_line":"            LOG.exception(_LE(\u0027Error retrieving libvirt job info\u0027))"},{"line_number":731,"context_line":""},{"line_number":732,"context_line":"            # NOTE(mdbooth): Treat an error as job completion"},{"line_number":733,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_ff4e7039","line":730,"in_reply_to":"3a98d1f4_0eda14e8","updated":"2016-10-03 11:32:54.000000000","message":"Worth mentioning that the admin should refer to libvirt debug log that has log filters which capture the QMP traffic between libvirt \u0026 QEMU? It may fall on the border of \"too much detail\". However, sometimes, without the said log filters (like we enable on Gate jobs), people have to explicitly enable them, reproduce the issue, then examine the logs, to get to the root cause.\n\nFor example, from the same bug this patch is fixing), where I had to request the reporter to reproduce the bug again with log filters:\n\nhttps://bugs.launchpad.net/nova/+bug/1627134/comments/8\n\nThis a \"nice to have\", so feel free to do as you see fit.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"da774fe30b5a761a51b631513446aa2d48b21807","unresolved":false,"context_lines":[{"line_number":727,"context_line":""},{"line_number":728,"context_line":"            # NOTE(mdbooth): Would be better to just log this without adding"},{"line_number":729,"context_line":"            # anything, but it seems that\u0027s not allowed."},{"line_number":730,"context_line":"            LOG.exception(_LE(\u0027Error retrieving libvirt job info\u0027))"},{"line_number":731,"context_line":""},{"line_number":732,"context_line":"            # NOTE(mdbooth): Treat an error as job completion"},{"line_number":733,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_90539916","line":730,"in_reply_to":"3a98d1f4_35f2b79a","updated":"2016-10-03 12:33:37.000000000","message":"Oh, you\u0027re right, not sure what I was thinking -- the libvirtError discussed above raised by blockJobInfo() is sufficient indeed.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"59e8e670c60516fe34c6798aea9e074942c30f3d","unresolved":false,"context_lines":[{"line_number":727,"context_line":""},{"line_number":728,"context_line":"            # NOTE(mdbooth): Would be better to just log this without adding"},{"line_number":729,"context_line":"            # anything, but it seems that\u0027s not allowed."},{"line_number":730,"context_line":"            LOG.exception(_LE(\u0027Error retrieving libvirt job info\u0027))"},{"line_number":731,"context_line":""},{"line_number":732,"context_line":"            # NOTE(mdbooth): Treat an error as job completion"},{"line_number":733,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_0eda14e8","line":730,"in_reply_to":"3a98d1f4_707f1cab","updated":"2016-10-03 10:20:31.000000000","message":"So, as I commented above, this does indicate a real error, but we don\u0027t have any useful information about it. Thinking about it, we should probably specifically direct the admin to the libvirt logs.... I\u0027ll do that.\n\nAlthough, is it useful to print the stack trace in this case? Probably not now that I think about it. I\u0027ll turn this into an error log instead.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"aa07c2225d650350e174fde24ef46272cecde701","unresolved":false,"context_lines":[{"line_number":727,"context_line":""},{"line_number":728,"context_line":"            # NOTE(mdbooth): Would be better to just log this without adding"},{"line_number":729,"context_line":"            # anything, but it seems that\u0027s not allowed."},{"line_number":730,"context_line":"            LOG.exception(_LE(\u0027Error retrieving libvirt job info\u0027))"},{"line_number":731,"context_line":""},{"line_number":732,"context_line":"            # NOTE(mdbooth): Treat an error as job completion"},{"line_number":733,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_35f2b79a","line":730,"in_reply_to":"3a98d1f4_ff4e7039","updated":"2016-10-03 12:26:37.000000000","message":"When I proposed adding a \u0027check the logs\u0027 message I still thought we were going to be missing the context in the exception, so the logs are going to be the only place to get that. Turns out that\u0027s wrong and we do have the context, so we can log it here.\n\nGiven that we don\u0027t need to point the admin to logs for context here, I feel like adding a comment asking for debug messages to be switched on would be too much. Apart from anything, if we were to add it here we should add it everywhere. I think that\u0027s better handled in docs.","commit_id":"1d4cb263f35a21f3a8d4474c87c697079e464863"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"75d16b568403a277bf0d2f8375a19f9aeb6a9dfa","unresolved":false,"context_lines":[{"line_number":648,"context_line":"        \"\"\"Returns information about job currently running"},{"line_number":649,"context_line":""},{"line_number":650,"context_line":"        :returns: BlockDeviceJobInfo, or None if no job exists"},{"line_number":651,"context_line":"        :raises: libvirt.libvirtError on error fetching block job info"},{"line_number":652,"context_line":"        \"\"\""},{"line_number":653,"context_line":""},{"line_number":654,"context_line":"        # libvirt\u0027s blockJobInfo() raises libvirt.libvirtError if there was an"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a98d1f4_075023d3","line":651,"updated":"2016-10-03 16:51:27.000000000","message":"Is this true going back to libvirt-python 1.2.5 since that\u0027s our minimum in g-r:\n\nhttp://git.openstack.org/cgit/openstack/requirements/tree/global-requirements.txt#n83","commit_id":"0f4bd241665c287e49f2d30ca79be96298217b7e"}]}