)]}'
{"etc/nova/rootwrap.d/compute.filters":[{"author":{"_account_id":13431,"name":"Evgeny Antyshev","email":"eantyshev@gmail.com","username":"eantyshev"},"change_message_id":"0af288e4d2f1837900261f268657eb66fec39543","unresolved":false,"context_lines":[{"line_number":220,"context_line":""},{"line_number":221,"context_line":"# nova/virt/libvirt/imagebackend.py:"},{"line_number":222,"context_line":"ploop: RegExpFilter, ploop, root, ploop, restore-descriptor, .*"},{"line_number":223,"context_line":"prl_disk_tool: RegExpFilter, prl_disk_tool, root, prl_disk_tool, resize, --size, .*M$, --resize_partition, --hdd, .*"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"# nova/virt/libvirt/utils.py:"},{"line_number":226,"context_line":"ploop: RegExpFilter, ploop, root, ploop, init, -s, .*, -f, .*, -t, .*, .*"}],"source_content_type":"application/octet-stream","patch_set":23,"id":"7f515b1d_2dd60a51","side":"PARENT","line":223,"updated":"2017-09-26 14:22:56.000000000","message":"This breaks _copy_ploop_image() in nova/virt/libvirt/imagebackend.py\nand extend() in nova/privsep/libvirt.py\n\nPlease,","commit_id":"fa8d2294c9e17ff7a7dc89c900b493dd05c6e5b1"}],"nova/privsep/__init__.py":[{"author":{"_account_id":13431,"name":"Evgeny Antyshev","email":"eantyshev@gmail.com","username":"eantyshev"},"change_message_id":"df8d58c6e6a375b9660c64a594d02ed65476e656","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    cfg_section\u003d\u0027nova_sys_admin\u0027,"},{"line_number":38,"context_line":"    pypath\u003d__name__ + \u0027.sys_admin_pctxt\u0027,"},{"line_number":39,"context_line":"    capabilities\u003d[capabilities.CAP_SYS_ADMIN],"},{"line_number":40,"context_line":")"}],"source_content_type":"text/x-python","patch_set":22,"id":"9f5c4f37_b8d60e4f","line":40,"updated":"2017-09-21 15:08:51.000000000","message":"Excuse me, isn\u0027t this a re-definition of above?","commit_id":"03157056727881bdcefa2dbd30b06a6083509033"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"641b67aaa1662dfe05445ecfa22de2988b7ff48a","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    cfg_section\u003d\u0027nova_sys_admin\u0027,"},{"line_number":38,"context_line":"    pypath\u003d__name__ + \u0027.sys_admin_pctxt\u0027,"},{"line_number":39,"context_line":"    capabilities\u003d[capabilities.CAP_SYS_ADMIN],"},{"line_number":40,"context_line":")"}],"source_content_type":"text/x-python","patch_set":22,"id":"9f5c4f37_f1182176","line":40,"in_reply_to":"9f5c4f37_b8d60e4f","updated":"2017-09-21 16:12:19.000000000","message":"Aye, looks like a rebase mistake","commit_id":"03157056727881bdcefa2dbd30b06a6083509033"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"f2e1eb4887fa532effabecb1e382a091605d7baa","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    cfg_section\u003d\u0027nova_sys_admin\u0027,"},{"line_number":38,"context_line":"    pypath\u003d__name__ + \u0027.sys_admin_pctxt\u0027,"},{"line_number":39,"context_line":"    capabilities\u003d[capabilities.CAP_SYS_ADMIN],"},{"line_number":40,"context_line":")"}],"source_content_type":"text/x-python","patch_set":22,"id":"7f515b1d_5f2ed3a5","line":40,"in_reply_to":"9f5c4f37_f1182176","updated":"2017-09-22 23:15:35.000000000","message":"Looking at it now.","commit_id":"03157056727881bdcefa2dbd30b06a6083509033"}],"nova/privsep/libvirt.py":[{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"80843e8bc62d67ea805395b09b279834541c19ce","unresolved":false,"context_lines":[{"line_number":104,"context_line":"    # the libvirt driver, but it seems undesirable to me. It would be good to"},{"line_number":105,"context_line":"    # create the loop file with the right owner or group such that we don\u0027t"},{"line_number":106,"context_line":"    # need to have it world readable. I don\u0027t have access to a system to test"},{"line_number":107,"context_line":"    # this on however."},{"line_number":108,"context_line":"    st \u003d os.stat(disk_path)"},{"line_number":109,"context_line":"    os.chmod(disk_path,"},{"line_number":110,"context_line":"             st.st_mode | stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f287b81_5d29ce87","line":107,"updated":"2017-09-07 13:28:45.000000000","message":"ACK, I think we can leave that to the ploop folks to make it better later.","commit_id":"aa22a796e63777de158b8ef50835d90896e8e1cb"},{"author":{"_account_id":13431,"name":"Evgeny Antyshev","email":"eantyshev@gmail.com","username":"eantyshev"},"change_message_id":"df8d58c6e6a375b9660c64a594d02ed65476e656","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # this on however."},{"line_number":112,"context_line":"    st \u003d os.stat(disk_path)"},{"line_number":113,"context_line":"    os.chmod(disk_path,"},{"line_number":114,"context_line":"             st.st_mode | stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":22,"id":"9f5c4f37_fb46c04f","line":114,"updated":"2017-09-21 15:08:51.000000000","message":"This is not an equivalent of\n    utils.execute(\u0027chmod\u0027, \u0027-R\u0027, \u0027a+r\u0027, path,\n                  run_as_root\u003dTrue, check_exit_code\u003dTrue)\nas it runs under \u0027nova\u0027 user","commit_id":"03157056727881bdcefa2dbd30b06a6083509033"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"f2e1eb4887fa532effabecb1e382a091605d7baa","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # this on however."},{"line_number":112,"context_line":"    st \u003d os.stat(disk_path)"},{"line_number":113,"context_line":"    os.chmod(disk_path,"},{"line_number":114,"context_line":"             st.st_mode | stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":22,"id":"7f515b1d_9f0d6b40","line":114,"in_reply_to":"9f5c4f37_fb46c04f","updated":"2017-09-22 23:15:35.000000000","message":"That\u0027s incorrect. The privsep.sys_admin decorator above means that this code runs \"as root\" (not quite, but with escalated capabilities).","commit_id":"03157056727881bdcefa2dbd30b06a6083509033"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4f00ec8c7cd1014fb7584212fe75e17ce1f4b6f9","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # this on however."},{"line_number":112,"context_line":"    st \u003d os.stat(disk_path)"},{"line_number":113,"context_line":"    os.chmod(disk_path,"},{"line_number":114,"context_line":"             st.st_mode | stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":23,"id":"7f515b1d_43fa5c75","line":114,"range":{"start_line":114,"start_character":26,"end_line":114,"end_character":53},"updated":"2017-09-25 15:44:41.000000000","message":"Could be missing something, but is this necessary? Without it, wouldn\u0027t we simply be saying \"the current permissions (st.stmod) and (|) read-others (stat.S_IROTH)?","commit_id":"1d3e205f86484a7dd6e3f351f7fea213fca4f8af"},{"author":{"_account_id":13431,"name":"Evgeny Antyshev","email":"eantyshev@gmail.com","username":"eantyshev"},"change_message_id":"4c0b3f7914bc12fde2c5448e40f1db90d4f344a5","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # this on however."},{"line_number":112,"context_line":"    st \u003d os.stat(disk_path)"},{"line_number":113,"context_line":"    os.chmod(disk_path,"},{"line_number":114,"context_line":"             st.st_mode | stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":23,"id":"7f515b1d_acca036a","line":114,"range":{"start_line":114,"start_character":26,"end_line":114,"end_character":53},"in_reply_to":"7f515b1d_29c05e7b","updated":"2017-09-26 11:34:53.000000000","message":"Having this tested on Virtuozzo setup, I confirm that stat.S_IROTH is the only neccessary flag, others could be dropped.\nAddressing the above comment, the answer is no: \"ploop init\" could be run only under root, and the owner of ploop file is root initially. Therefore, changing mod is always needed","commit_id":"1d3e205f86484a7dd6e3f351f7fea213fca4f8af"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"2a2c4562f265ab1a9c83846b513e1e168b8e870d","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # this on however."},{"line_number":112,"context_line":"    st \u003d os.stat(disk_path)"},{"line_number":113,"context_line":"    os.chmod(disk_path,"},{"line_number":114,"context_line":"             st.st_mode | stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":23,"id":"7f515b1d_29c05e7b","line":114,"range":{"start_line":114,"start_character":26,"end_line":114,"end_character":53},"in_reply_to":"7f515b1d_43fa5c75","updated":"2017-09-25 17:50:21.000000000","message":"I think like mikal said, the existing chmod is a+r, which ensures all these bits. Without ploop authors to confirm a behavior change, I\u0027d rather not risk breaking them if this creates non readable files for some reason.","commit_id":"1d3e205f86484a7dd6e3f351f7fea213fca4f8af"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8721aa51d66f2533352e36cec92fca3f8d4df646","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # this on however."},{"line_number":112,"context_line":"    st \u003d os.stat(disk_path)"},{"line_number":113,"context_line":"    os.chmod(disk_path,"},{"line_number":114,"context_line":"             st.st_mode | stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":23,"id":"7f515b1d_5d4d5cc2","line":114,"range":{"start_line":114,"start_character":26,"end_line":114,"end_character":53},"in_reply_to":"7f515b1d_acca036a","updated":"2017-09-27 08:31:56.000000000","message":"I think you _may_ have misunderstood my question (or you\u0027ve understood it and I\u0027ve misunderstood your replies). What I\u0027m asking is why we do \u0027stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH\u0027 (which seems equivalent to \u0027chmod uga+r\u0027) instead of simply \u0027stat.S_IROTH\u0027 (Which seems equivalent to the original \u0027chmod a+r\u0027). Is there some cascading in the \u0027chmod\u0027 command going on that I\u0027m not aware of?\n\nIt\u0027s not a serious complaint but I would like to understand this.","commit_id":"1d3e205f86484a7dd6e3f351f7fea213fca4f8af"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4f00ec8c7cd1014fb7584212fe75e17ce1f4b6f9","unresolved":false,"context_lines":[{"line_number":150,"context_line":"    try:"},{"line_number":151,"context_line":"        with open(path, \u0027r\u0027) as f:"},{"line_number":152,"context_line":"            current_flags \u003d fcntl.fcntl(f.fileno(), fcntl.F_GETFL)"},{"line_number":153,"context_line":"            fcntl.fcntl(f.fileno(), fcntl.F_SETFL,"},{"line_number":154,"context_line":"                        current_flags | os.O_NONBLOCK)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            return f.read()"},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"7f515b1d_833bb4e9","line":154,"range":{"start_line":153,"start_character":12,"end_line":154,"end_character":54},"updated":"2017-09-25 15:44:41.000000000","message":"So you\u0027re setting the \u0027O_NONBLOCK\u0027 flag on this using fnctl instead of dd. Makes sense","commit_id":"1d3e205f86484a7dd6e3f351f7fea213fca4f8af"}],"nova/tests/unit/virt/libvirt/test_imagebackend.py":[{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"60ed5b7e75e670cccb7ffb5f17a7204cc5603dfd","unresolved":false,"context_lines":[{"line_number":1724,"context_line":"        image.create_image(fn, self.TEMPLATE_PATH, 2048, image_id\u003dNone)"},{"line_number":1725,"context_line":""},{"line_number":1726,"context_line":"        mock_copy.assert_called_once_with(self.TEMPLATE_PATH, img_path)"},{"line_number":1727,"context_line":"        mock_ploop_restore_descriptor.eassert_called_once_with(self.PATH,"},{"line_number":1728,"context_line":"                                                               img_path,"},{"line_number":1729,"context_line":"                                                               \"raw\")"},{"line_number":1730,"context_line":"        self.assertTrue(mock_sync.called)"}],"source_content_type":"text/x-python","patch_set":26,"id":"7f515b1d_7ca4a1a6","line":1727,"updated":"2017-09-27 03:08:35.000000000","message":"Not sure why this didn\u0027t fail in the gate, but surely \"eassert_called_once_with\" is not a thing?","commit_id":"ce48dd2eb322a7bac08e155cc127abf0a7da6c8d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b9cdfe2b8f726be97235ccd304f44fac3dfa4934","unresolved":false,"context_lines":[{"line_number":1724,"context_line":"        image.create_image(fn, self.TEMPLATE_PATH, 2048, image_id\u003dNone)"},{"line_number":1725,"context_line":""},{"line_number":1726,"context_line":"        mock_copy.assert_called_once_with(self.TEMPLATE_PATH, img_path)"},{"line_number":1727,"context_line":"        mock_ploop_restore_descriptor.eassert_called_once_with(self.PATH,"},{"line_number":1728,"context_line":"                                                               img_path,"},{"line_number":1729,"context_line":"                                                               \"raw\")"},{"line_number":1730,"context_line":"        self.assertTrue(mock_sync.called)"}],"source_content_type":"text/x-python","patch_set":26,"id":"7f515b1d_fd952834","line":1727,"in_reply_to":"7f515b1d_7ca4a1a6","updated":"2017-09-27 08:23:14.000000000","message":"Welcome to the wonderful world of mock, where it silently ignores things like this","commit_id":"ce48dd2eb322a7bac08e155cc127abf0a7da6c8d"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":13431,"name":"Evgeny Antyshev","email":"eantyshev@gmail.com","username":"eantyshev"},"change_message_id":"4dba6e922bf315054aa635b8b168af6df0d8ca42","unresolved":false,"context_lines":[{"line_number":1074,"context_line":"            libvirt_utils.copy_image(base, image_path)"},{"line_number":1075,"context_line":"            nova.privsep.libvirt.ploop_restore_descriptor(image_path,"},{"line_number":1076,"context_line":"                                                          target,"},{"line_number":1077,"context_line":"                                                          self.pcs_format)"},{"line_number":1078,"context_line":"            if size:"},{"line_number":1079,"context_line":"                self.resize_image(size)"},{"line_number":1080,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"7f515b1d_0791c94e","line":1077,"updated":"2017-09-27 12:38:55.000000000","message":"Would you please fix a minor flaw in my initial review: swap the first and the second arguments\n\nnova.privsep.libvirt.ploop_restore_descriptor(target, image_path, self.pcs_format)\n\nThe mistake wasn\u0027t caught by unit test as this line\u0027s check wasn\u0027t correct either:)","commit_id":"9b89ecaa0a8b8be5ac66ddc33a8a1c4933024dd5"}]}