)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bb6ac8736671f95343bd24e156f0c9a987b6ab2c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: Use the mirror ready element to detect when to pivot"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This element was first introduced in libvirt v1.2.7 and provides an"},{"line_number":10,"context_line":"alternative method for detecting when a blockjob is complete. A value of"},{"line_number":11,"context_line":"\u0027yes\u0027 meaning that it is safe to pivot to the new disk."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5a74a57a_0cc73413","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":51},"updated":"2016-11-23 11:49:40.000000000","message":"It was actually introduced much earlier, I was just confused by the reference to 1.2.7 in the docs. I\u0027ll remove this in a future respin.","commit_id":"217627a92666f63c6f018afb1b864bfe2030dadb"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b78c1c6d97560397bf34bec59836a3f567d989bb","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Matthew Booth \u003cmbooth@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-01-25 11:35:47 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: Use the mirror element to detect job completion"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The mirror element was introduced in Libvirt 0.10.0 [1] and provides a"},{"line_number":10,"context_line":"ready attribute that helps to determine when a blockjob is complete."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"3a461143_e6d6064e","line":7,"range":{"start_line":7,"start_character":17,"end_line":7,"end_character":23},"updated":"2017-01-30 11:02:26.000000000","message":"A little info on what the mirror element is would be helpful. It looks like mirroring is a QEMU-inspired synonym for copying, but I couldn\u0027t find any libvirt docs on the subject oddly enough","commit_id":"9ed782641970dd371adc151e7d3913a402369dd3"}],"nova/tests/unit/virt/libvirt/test_config.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"a1cc13a588e57556136b5df20a272e93fb84fce9","unresolved":false,"context_lines":[{"line_number":788,"context_line":"        xmldoc \u003d etree.fromstring(xml)"},{"line_number":789,"context_line":"        obj \u003d config.LibvirtConfigGuestDisk()"},{"line_number":790,"context_line":"        obj.parse_dom(xmldoc)"},{"line_number":791,"context_line":"        self.assertEqual(obj.mirror.ready, \"yes\")"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"    def test_config_boot_order_parse(self):"},{"line_number":794,"context_line":"        xml \u003d \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a74a57a_58dd6b6f","line":791,"updated":"2016-11-23 11:05:34.000000000","message":"Isn\u0027t the usual pattern expected, observed?\n\n(I personally don\u0027t like that pattern, but I guess it\u0027s the standard round these parts)","commit_id":"217627a92666f63c6f018afb1b864bfe2030dadb"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b69a7f175bef81c9693ee8578875645ebab034b7","unresolved":false,"context_lines":[{"line_number":788,"context_line":"        xmldoc \u003d etree.fromstring(xml)"},{"line_number":789,"context_line":"        obj \u003d config.LibvirtConfigGuestDisk()"},{"line_number":790,"context_line":"        obj.parse_dom(xmldoc)"},{"line_number":791,"context_line":"        self.assertEqual(obj.mirror.ready, \"yes\")"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"    def test_config_boot_order_parse(self):"},{"line_number":794,"context_line":"        xml \u003d \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a74a57a_b803df0e","line":791,"in_reply_to":"5a74a57a_58dd6b6f","updated":"2016-11-23 11:24:01.000000000","message":"It doesn\u0027t appear to be in this particular test file, the majority of assertEqual\u0027s  I can see in here use observed and then expected. Happy to switch these around if I respin, I should really drop this test down below test_config_boot_order_parse as well if I get a chance.","commit_id":"217627a92666f63c6f018afb1b864bfe2030dadb"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b78c1c6d97560397bf34bec59836a3f567d989bb","unresolved":false,"context_lines":[{"line_number":774,"context_line":""},{"line_number":775,"context_line":"    def test_config_mirror_parse(self):"},{"line_number":776,"context_line":"        xml \u003d \"\"\""},{"line_number":777,"context_line":"\u003cdisk type\u003d\"file\" device\u003d\"disk\"\u003e"},{"line_number":778,"context_line":"  \u003cdriver name\u003d\"qemu\" type\u003d\"qcow2\" cache\u003d\"none\" discard\u003d\"unmap\"/\u003e"},{"line_number":779,"context_line":"  \u003csource file\u003d\"/tmp/hello.qcow2\"/\u003e"},{"line_number":780,"context_line":"  \u003ctarget bus\u003d\"ide\" dev\u003d\"/dev/hda\"/\u003e"},{"line_number":781,"context_line":"  \u003cserial\u003e7a97c4a3-6f59-41d4-bf47-191d7f97f8e9\u003c/serial\u003e"},{"line_number":782,"context_line":"  \u003cmirror type\u003d\u0027file\u0027 file\u003d\u0027/tmp/new.img\u0027 format\u003d\u0027raw\u0027 job\u003d\u0027copy\u0027 ready\u003d\u0027yes\u0027\u003e"},{"line_number":783,"context_line":"    \u003cformat type\u003d\u0027raw\u0027/\u003e"},{"line_number":784,"context_line":"    \u003csource file\u003d\u0027/tmp/new.img\u0027/\u003e"},{"line_number":785,"context_line":"  \u003c/mirror\u003e"},{"line_number":786,"context_line":"  \u003cboot order\u003d\"1\"/\u003e"},{"line_number":787,"context_line":"\u003c/disk\u003e\"\"\""},{"line_number":788,"context_line":"        xmldoc \u003d etree.fromstring(xml)"},{"line_number":789,"context_line":"        obj \u003d config.LibvirtConfigGuestDisk()"},{"line_number":790,"context_line":"        obj.parse_dom(xmldoc)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a461143_26448e19","line":787,"range":{"start_line":777,"start_character":0,"end_line":787,"end_character":10},"updated":"2017-01-30 11:02:26.000000000","message":"nit: You can indent this as seen below","commit_id":"9ed782641970dd371adc151e7d3913a402369dd3"}],"nova/tests/unit/virt/libvirt/test_guest.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"120c5a1e06266ac60caab06bc347f1e4c619b234","unresolved":false,"context_lines":[{"line_number":661,"context_line":"            \"cur\": 95,"},{"line_number":662,"context_line":"            \"end\": 100}"},{"line_number":663,"context_line":"        is_complete \u003d self.gblock.is_job_complete()"},{"line_number":664,"context_line":"        self.assertFalse(is_complete)"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"    def test_is_job_complete_not_ready(self):"},{"line_number":667,"context_line":"        gblock \u003d self.guest.get_block_device(\u0027vda\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5a3905b3_ff0d508f","line":664,"updated":"2017-01-24 16:45:56.000000000","message":"If you wanted to gold plate it, you could assert here that we don\u0027t attempt to pull down the domain xml from libvirt. I guess it\u0027s implicit, though, as the test would presumably fail because it\u0027s not mocked.","commit_id":"3810da160ec35e93c12f4b61e9c98a0df02079e1"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b78c1c6d97560397bf34bec59836a3f567d989bb","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        is_complete \u003d self.gblock.is_job_complete()"},{"line_number":683,"context_line":"        self.assertFalse(is_complete)"},{"line_number":684,"context_line":""},{"line_number":685,"context_line":"    def test_is_job_complete_not_ready(self):"},{"line_number":686,"context_line":"        gblock \u003d self.guest.get_block_device(\u0027vda\u0027)"},{"line_number":687,"context_line":"        disk \u003d vconfig.LibvirtConfigGuestDisk()"},{"line_number":688,"context_line":"        disk.mirror \u003d vconfig.LibvirtConfigGuestDiskMirror()"},{"line_number":689,"context_line":"        with mock.patch.object(self.guest, \u0027get_disk\u0027, return_value\u003ddisk):"},{"line_number":690,"context_line":"            self.domain.blockJobInfo.return_value \u003d {"},{"line_number":691,"context_line":"                \"type\": 4,"},{"line_number":692,"context_line":"                \"bandwidth\": 18,"},{"line_number":693,"context_line":"                \"cur\": 100,"},{"line_number":694,"context_line":"                \"end\": 100}"},{"line_number":695,"context_line":"            is_complete \u003d gblock.is_job_complete()"},{"line_number":696,"context_line":"            self.assertFalse(is_complete)"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"    def test_is_job_complete_ready(self):"},{"line_number":699,"context_line":"        gblock \u003d self.guest.get_block_device(\u0027vda\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a461143_46fb5ac1","line":696,"range":{"start_line":685,"start_character":0,"end_line":696,"end_character":41},"updated":"2017-01-30 11:02:26.000000000","message":"nit: Not entirely related to this, but is there a \u0027test_is_job_complete_pass\u0027 test that ensures cur\u003d100 and end\u003d100 passes? I\u0027d like to see one if so, just these tests focus entirely on the mirror element.","commit_id":"9ed782641970dd371adc151e7d3913a402369dd3"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"fc2cfb7334476e95c2f22611c3757dba794b4dac","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        is_complete \u003d self.gblock.is_job_complete()"},{"line_number":683,"context_line":"        self.assertFalse(is_complete)"},{"line_number":684,"context_line":""},{"line_number":685,"context_line":"    def test_is_job_complete_not_ready(self):"},{"line_number":686,"context_line":"        gblock \u003d self.guest.get_block_device(\u0027vda\u0027)"},{"line_number":687,"context_line":"        disk \u003d vconfig.LibvirtConfigGuestDisk()"},{"line_number":688,"context_line":"        disk.mirror \u003d vconfig.LibvirtConfigGuestDiskMirror()"},{"line_number":689,"context_line":"        with mock.patch.object(self.guest, \u0027get_disk\u0027, return_value\u003ddisk):"},{"line_number":690,"context_line":"            self.domain.blockJobInfo.return_value \u003d {"},{"line_number":691,"context_line":"                \"type\": 4,"},{"line_number":692,"context_line":"                \"bandwidth\": 18,"},{"line_number":693,"context_line":"                \"cur\": 100,"},{"line_number":694,"context_line":"                \"end\": 100}"},{"line_number":695,"context_line":"            is_complete \u003d gblock.is_job_complete()"},{"line_number":696,"context_line":"            self.assertFalse(is_complete)"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"    def test_is_job_complete_ready(self):"},{"line_number":699,"context_line":"        gblock \u003d self.guest.get_block_device(\u0027vda\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a461143_a9a5b147","line":696,"range":{"start_line":685,"start_character":0,"end_line":696,"end_character":41},"in_reply_to":"3a461143_46fb5ac1","updated":"2017-01-30 13:10:57.000000000","message":"That\u0027s a really obvious omission. Not this patch, though. I\u0027ll knock up another patch to add it later.","commit_id":"9ed782641970dd371adc151e7d3913a402369dd3"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b78c1c6d97560397bf34bec59836a3f567d989bb","unresolved":false,"context_lines":[{"line_number":877,"context_line":"            elif c.tag \u003d\u003d \u0027boot\u0027:"},{"line_number":878,"context_line":"                self.boot_order \u003d c.get(\u0027order\u0027)"},{"line_number":879,"context_line":"            elif c.tag \u003d\u003d \u0027mirror\u0027:"},{"line_number":880,"context_line":"                m \u003d LibvirtConfigGuestDiskMirror()"},{"line_number":881,"context_line":"                m.parse_dom(c)"},{"line_number":882,"context_line":"                self.mirror \u003d m"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3a461143_661f7608","line":881,"range":{"start_line":880,"start_character":20,"end_line":881,"end_character":30},"updated":"2017-01-30 11:02:26.000000000","message":"femtonit:\n\n  m \u003d LibvirtConfigGuestDiskMirror().parse_dom(c)\n\n...for consistency with previous sections :)","commit_id":"9ed782641970dd371adc151e7d3913a402369dd3"}],"nova/virt/libvirt/guest.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"87ba014a6ad5c40c2fcc0f2a868e9ad4f2a4cf7a","unresolved":false,"context_lines":[{"line_number":736,"context_line":"        # The earliest tag which contains this commit is v2.3.0-rc1, so we"},{"line_number":737,"context_line":"        # should be able to remove this workaround when MIN_LIBVIRT_VERSION"},{"line_number":738,"context_line":"        # reaches 2.3.0, or we move to handling job events instead."},{"line_number":739,"context_line":"        else:"},{"line_number":740,"context_line":"            return status.cur !\u003d 0 and status.cur \u003d\u003d status.end"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_b2c7b462","line":739,"range":{"start_line":739,"start_character":8,"end_line":739,"end_character":12},"updated":"2016-11-18 19:03:49.000000000","message":"This is unnecessary. If the above condition is true it\u0027s going to return regardless.","commit_id":"08f71b7cd06ea69a11a2cc75f4d520e41ca7b423"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"87ba014a6ad5c40c2fcc0f2a868e9ad4f2a4cf7a","unresolved":false,"context_lines":[{"line_number":737,"context_line":"        # should be able to remove this workaround when MIN_LIBVIRT_VERSION"},{"line_number":738,"context_line":"        # reaches 2.3.0, or we move to handling job events instead."},{"line_number":739,"context_line":"        else:"},{"line_number":740,"context_line":"            return status.cur !\u003d 0 and status.cur \u003d\u003d status.end"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":""},{"line_number":743,"context_line":"class VCPUInfo(object):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_f2a3ecaf","line":740,"range":{"start_line":740,"start_character":26,"end_line":740,"end_character":29},"updated":"2016-11-18 19:03:49.000000000","message":"Why did this change?","commit_id":"08f71b7cd06ea69a11a2cc75f4d520e41ca7b423"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"87ba014a6ad5c40c2fcc0f2a868e9ad4f2a4cf7a","unresolved":false,"context_lines":[{"line_number":765,"context_line":"        :param bandwidth: Used in MiB/s"},{"line_number":766,"context_line":"        :param cur: Indicates the position between 0 and \u0027end\u0027"},{"line_number":767,"context_line":"        :param end: Indicates the position for this operation"},{"line_number":768,"context_line":"        :param ready: Indicates if the job is complete"},{"line_number":769,"context_line":"        \"\"\""},{"line_number":770,"context_line":"        self.job \u003d job"},{"line_number":771,"context_line":"        self.bandwidth \u003d bandwidth"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_d299907a","line":768,"updated":"2016-11-18 19:03:49.000000000","message":"But based on the logic this isn\u0027t a boolean, and it could be !\u003d \u0027yes\u0027 right? Indicating it\u0027s not ready.","commit_id":"08f71b7cd06ea69a11a2cc75f4d520e41ca7b423"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bb6ac8736671f95343bd24e156f0c9a987b6ab2c","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        if status is None:"},{"line_number":727,"context_line":"            return True"},{"line_number":728,"context_line":""},{"line_number":729,"context_line":"        # NOTE(lyarwood): If available use the mirror ready flag to determine"},{"line_number":730,"context_line":"        # if the block job is complete and ready to pivot."},{"line_number":731,"context_line":"        disk \u003d self._guest.get_disk(self._disk)"},{"line_number":732,"context_line":"        if disk and disk.mirror:"},{"line_number":733,"context_line":"            return disk.mirror.ready \u003d\u003d \u0027yes\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a74a57a_8c6d0435","line":730,"range":{"start_line":729,"start_character":25,"end_line":730,"end_character":57},"updated":"2016-11-23 11:49:40.000000000","message":"s/flag/element/g","commit_id":"217627a92666f63c6f018afb1b864bfe2030dadb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0cf6b7974f454faa47886fd2fefb5fac392d51b2","unresolved":false,"context_lines":[{"line_number":728,"context_line":""},{"line_number":729,"context_line":"        # NOTE(lyarwood): If available use the mirror ready flag to determine"},{"line_number":730,"context_line":"        # if the block job is complete and ready to pivot."},{"line_number":731,"context_line":"        disk \u003d self._guest.get_disk(self._disk)"},{"line_number":732,"context_line":"        if disk and disk.mirror:"},{"line_number":733,"context_line":"            return disk.mirror.ready \u003d\u003d \u0027yes\u0027"},{"line_number":734,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a74a57a_2f306256","line":731,"updated":"2016-11-23 11:58:28.000000000","message":"This fetches the domain\u0027s xml every time we go round this loop. We don\u0027t need to do this, though, because the ready flag will only be \u0027yes\u0027 if (but not iff) status.end \u003d\u003d status.cur.\n\nI\u0027d do something like:\n\n  if status.cur \u003d\u003d status.end:\n    disk \u003d get_disk\n    if disk is not None and disk.mirror is not None and disk.mirror.ready is not None:\n      # i.e. Use ready if it\u0027s present\n      if disk.mirror.ready \u003d\u003d \u0027abort\u0027:\n        # Belt and braces? Maybe raise an exception?\n\n      return disk.mirror.ready \u003d\u003d \u0027yes\u0027\n\n    # But, but, but...\n    # the docs say that ready may be unset if the copy is still in progress. How can we differentiate between this, which is the issue we were actually hitting which brought this to the fore, and when ready will *never* be present because libvirt/qemu are too old? I don\u0027t think this fixes the bug.\n\n    # ready is not present\n    return status.end !\u003d 0\n\n  return False","commit_id":"217627a92666f63c6f018afb1b864bfe2030dadb"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"4671ff912b5b6983ec7dda08dde548fcc32b82d6","unresolved":false,"context_lines":[{"line_number":728,"context_line":""},{"line_number":729,"context_line":"        # NOTE(lyarwood): If available use the mirror ready flag to determine"},{"line_number":730,"context_line":"        # if the block job is complete and ready to pivot."},{"line_number":731,"context_line":"        disk \u003d self._guest.get_disk(self._disk)"},{"line_number":732,"context_line":"        if disk and disk.mirror:"},{"line_number":733,"context_line":"            return disk.mirror.ready \u003d\u003d \u0027yes\u0027"},{"line_number":734,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a74a57a_e273e14b","line":731,"in_reply_to":"5a74a57a_2f306256","updated":"2016-11-23 12:14:39.000000000","message":"Thanks Matt, I take your point regarding fetching the domain XML too often, I\u0027ll add status.cur \u003d\u003d status.end as a check first. \n\n\u003e the docs say that ready may be unset if the copy is still \n\u003e in progress. How can we differentiate between this, which\n\u003e is the issue we were actually hitting which brought this \n\u003e to the fore, and when ready will *never* be present \n\u003e because libvirt/qemu are too old? I don\u0027t think this fixes \n\u003e the bug.\n\nSimply be checking if disk.mirror is not None. That\u0027s the reason for the additional config class around mirror btw, otherwise I would\u0027ve just added mirror_ready to the original disk object.","commit_id":"217627a92666f63c6f018afb1b864bfe2030dadb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"120c5a1e06266ac60caab06bc347f1e4c619b234","unresolved":false,"context_lines":[{"line_number":315,"context_line":"        # only the target dev of the device."},{"line_number":316,"context_line":"        node \u003d doc.find(\"./devices/disk/target[@dev\u003d\u0027%s\u0027]..\" % device)"},{"line_number":317,"context_line":"        if node is None:"},{"line_number":318,"context_line":"            node \u003d doc.find(\"./devices/disk/source[@file\u003d\u0027%s\u0027]..\" % device)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        if node is not None:"},{"line_number":321,"context_line":"            conf \u003d vconfig.LibvirtConfigGuestDisk()"}],"source_content_type":"text/x-python","patch_set":18,"id":"5a3905b3_6480f58b","line":318,"updated":"2017-01-24 16:45:56.000000000","message":"Yuk, but necessary. Happy there\u0027s a proper fix in the pipe.\n\nIdeally there would be a test for this, but I see 2 mitigating factors:\n\n1. AFAICT there\u0027s no test coverage for this function anyway :/\n2. You\u0027ve already got a patch in the pipe which would make the test obsolete.\n\nSo I\u0027d personally let this pass.","commit_id":"3810da160ec35e93c12f4b61e9c98a0df02079e1"}]}